WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: 答复: [Xen-devel] Re: changeset 22526:7a5ee3800417

Pengfei,

You're asking why I didn't make any changes to the superpage spit in
that patch?

The purpose of that patch was to make accesses to the p2m table follow
these rules:
* Reads without the lock are done only once
* Old entries are replaced with completely new entries all at once

The rules are meant to make it so that if you read the p2m table without
holding the p2m lock, even if you're racing with someone who's changing
it, you'll get a "consistent" view.

In the superpage-split case, replacing the old superpage entry with a
pointer to a non-superpage entry already followed those rules; so it
didn't need to be changed.

 -George

On Wed, 2011-03-16 at 10:27 +0000, zpfalpc23 wrote:
> Hi George,
>       I have read you patch 22526 in xen-testing4.0 and xen-unstable 
> carefully. There is one thing confused me a lot. why the patch for 
> xen-testing4.0 did not handle the case In ept_set_entry where the superpage 
> should be splitted. Maybe the race can't happen in this scene.
>       Also When the race of updating p2m table happen, will the normal(be 
> changed by ept_get_entry) page be POD again? In this situation, what is it 
> for Xen and VM?
>       Thanks a lot for enlighten me.
> 
> ----Pengfei zhang
> 
> -----邮件原件-----
> 发件人: dunlapg@xxxxxxxxx [mailto:dunlapg@xxxxxxxxx] 代表 George Dunlap
> 发送时间: 2011年3月10日 17:43
> 收件人: George Dunlap
> 抄送: 张鹏飞; xen devel
> 主题: Re: [Xen-devel] Re: changeset 22526:7a5ee3800417
> 
> Oops, forgot to attach the patch...
> 
>  -George
> 
> On Thu, Mar 10, 2011 at 9:41 AM, George Dunlap <George.Dunlap@xxxxxxxxxxxxx> 
> wrote:
> > Peng fei,
> >
> > Can you (1) test this patch, and (2) make sure I've romanized your 
> > name properly?
> >
> > Thanks,
> >  -George
> >
> > On Wed, Mar 9, 2011 at 1:59 PM, George Dunlap 
> > <George.Dunlap@xxxxxxxxxxxxx> wrote:
> >> Peng fei,
> >>
> >> I just noticed this patch has a bug -- it doesn't set new_entry.mfn 
> >> if the new mfn is the same as the one already in ept_entry!  Standby 
> >> for a new one...
> >>
> >>  -George
> >>
> >> On Mon, Mar 7, 2011 at 4:41 PM, George Dunlap 
> >> <George.Dunlap@xxxxxxxxxxxxx> wrote:
> >>> Peng fei,
> >>>
> >>> Can you test the attached patch (to xen-unstable)?  I don't have 
> >>> EPT-enabled hardware handy...
> >>>
> >>>  -George
> >>>
> >>> On Mon, Mar 7, 2011 at 4:20 PM, George Dunlap <george.dunlap@xxxxxxxxxx> 
> >>> wrote:
> >>>> Hmm, yeah, that's obviously not right!  Let me take a look...
> >>>>
> >>>>  -George
> >>>>
> >>>> On Mon, 2011-03-07 at 09:55 +0000, 张鹏飞 wrote:
> >>>>> Hi,
> >>>>>     Recently, I did a research on the p2m and EPT,and did apply 
> >>>>> your
> >>>>> patch(22526) to the source code of mine. But there is one place 
> >>>>> confused me:
> >>>>>
> >>>>>
> >>>>>
> >>>>> ept_entry = table + index;
> >>>>> 1.91
> >>>>>     1.92 -        ept_entry->emt = epte_get_entry_emt(d, gfn, mfn, 
> >>>>> &ipat, direct_mmio);
> >>>>>     1.93 -        ept_entry->ipat = ipat;
> >>>>>     1.94 -        ept_entry->sp = i ? 1 : 0;
> >>>>>     1.95 -        ept_entry->avail1 = p2mt;
> >>>>>     1.96 -        ept_entry->avail2 = 0;
> >>>>>     1.97 +        new_entry.emt = epte_get_entry_emt(d, gfn, mfn, 
> >>>>> &ipat, direct_mmio);
> >>>>>     1.98 +        new_entry.ipat = ipat;
> >>>>>     1.99 +        new_entry.sp = i ? 1 : 0;
> >>>>>    1.100 +        new_entry.avail1 = p2mt;
> >>>>>    1.101 +        new_entry.avail2 = 0;
> >>>>>    1.102
> >>>>>    1.103 -        if ( ept_entry->mfn == mfn_x(mfn) )
> >>>>>    1.104 +        if ( new_entry.mfn == mfn_x(mfn) )
> >>>>>    1.105               need_modify_vtd_table = 0;
> >>>>>    1.106          else /* the caller should take care of the 
> >>>>> previous page */
> >>>>>    1.107 -            ept_entry->mfn = mfn_x(mfn);
> >>>>>    1.108 +            new_entry.mfn = mfn_x(mfn);
> >>>>>
> >>>>> I think, The new_entry.mfn is used without assigned any value. Maybe 
> >>>>> there was something else I did not think of.
> >>>>> Thank you for your kindness!
> >>>>>
> >>>>> http://xenbits.xen.org/xen-unstable.hg/rev/7a5ee3800417
> >>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> Xen-devel mailing list
> >>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
> >>>> http://lists.xensource.com/xen-devel
> >>>>
> >>>
> >>
> >
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel