Re: çå: [Xen-devel] Re: changeset 22526:7a5ee3800417

[George Dunlap <george.dunlap@xxxxxxxxxx>]

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