I think the below fix is correct, though as I''m far from comfortable with this code I''d like some comments first... ''val'' is the unmodified pte value written by the guest. We need to use nl1e instead, which has done the needed PAGE_GLOBAL etc. modifications. cheers, john # HG changeset patch # User john.levon@sun.com # Date 1192664022 25200 # Node ID d2eed7e51be31880f04033d1241861602d3628bb # Parent a6b52e05bcd026a7d378fff130b90d6eda62209a diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -3204,7 +3204,7 @@ static int ptwr_emulated_update( if ( shadow_mode_enabled(d) ) shadow_lock(d); ol1e = l1e_from_intpte(old); - if ( cmpxchg((intpte_t *)pl1e, old, val) != old ) + if ( cmpxchg((intpte_t *)pl1e, old, l1e_get_intpte(nl1e)) != old ) { if ( shadow_mode_enabled(d) ) shadow_unlock(d); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
You could be right, although what version of Xen is that code snippet from? Xen 3.1 and Xen unstable both look quite different. -- Keir On 18/10/07 02:11, "John Levon" <levon@movementarian.org> wrote:> > I think the below fix is correct, though as I''m far from comfortable > with this code I''d like some comments first... > > ''val'' is the unmodified pte value written by the guest. We need to use > nl1e instead, which has done the needed PAGE_GLOBAL etc. modifications. > > cheers, > john > > # HG changeset patch > # User john.levon@sun.com > # Date 1192664022 25200 > # Node ID d2eed7e51be31880f04033d1241861602d3628bb > # Parent a6b52e05bcd026a7d378fff130b90d6eda62209a > > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -3204,7 +3204,7 @@ static int ptwr_emulated_update( > if ( shadow_mode_enabled(d) ) > shadow_lock(d); > ol1e = l1e_from_intpte(old); > - if ( cmpxchg((intpte_t *)pl1e, old, val) != old ) > + if ( cmpxchg((intpte_t *)pl1e, old, l1e_get_intpte(nl1e)) != old ) > { > if ( shadow_mode_enabled(d) ) > shadow_unlock(d); > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Thu, Oct 18, 2007 at 08:05:48AM +0100, Keir Fraser wrote:> You could be right, although what version of Xen is that code snippet from? > Xen 3.1 and Xen unstable both look quite different.3.0.4. I just checked xen-unstable (sorry) - it has the same bug: 3377 adjust_guest_l1e(nl1e, d); 3378 ... 3382 if ( do_cmpxchg ) 3383 { ... 3388 okay = paging_cmpxchg_guest_entry(v, &l1e_get_intpte(*pl1e), 3389 &t, val, _mfn(mfn)); "val" not "nl1e" I suspect this is hurting performance for us. cheers, john PS slightly related note - is _PAGE_GUEST_KERNEL part of the ABI? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 18/10/07 11:23, "John Levon" <levon@movementarian.org> wrote:> 3.0.4. I just checked xen-unstable (sorry) - it has the same bug:Yes, now fixed.> PS slightly related note - is _PAGE_GUEST_KERNEL part of the ABI?Yes, rather unavoidably! Certainly we don''t intend to randomly change which pte bit we use for this purpose. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel