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

On Mon, 2011-03-07 at 16:48 +0000, Tim Deegan wrote:
> Hi, 
> 
> At 16:41 +0000 on 07 Mar (1299516062), George Dunlap wrote:
> > diff -r 809ea782d2e6 -r 7ddd021a2079 xen/arch/x86/mm/hap/p2m-ept.c
> > --- a/xen/arch/x86/mm/hap/p2m-ept.c Mon Mar 07 16:33:45 2011 +0000
> > +++ b/xen/arch/x86/mm/hap/p2m-ept.c Mon Mar 07 16:41:02 2011 +0000
> > @@ -390,7 +390,7 @@
> >              new_entry.access = p2ma;
> >              new_entry.rsvd2_snp = (iommu_enabled && iommu_snoop);
> >  
> > -            if ( new_entry.mfn == mfn_x(mfn) )
> > +            if ( ept_entry->mfn == mfn_x(mfn) )
> 
> Better to use old_entry.mfn, in the spirit of the original cset
> ("access-once semantics")?  

I started to do that, but the one below didn't have an old_entry
already.  

> In fact, I suspect that to be safe, you need
> to do an atomic RMW instead of just an atomic set, and then decide
> whether the VT-d tables will need to be synced.

Are we not holding the p2m lock when writing entries?

Reading should be safe here if we are holding the p2m lock.  When
holding the lock, we only need write-once semantics (to avoid racing the
read-once semantics of someone reading without the lock).

 -George

> 
> Cheers,
> 
> Tim. 
> 
> >                  need_modify_vtd_table = 0;
> >              else
> >                  new_entry.mfn = mfn_x(mfn);
> > @@ -438,7 +438,7 @@
> >          new_entry.access = p2ma;
> >          new_entry.rsvd2_snp = (iommu_enabled && iommu_snoop);
> >  
> > -        if ( new_entry.mfn == mfn_x(mfn) )
> > +        if ( ept_entry->mfn == mfn_x(mfn) )
> >               need_modify_vtd_table = 0;
> >          else /* the caller should take care of the previous page */
> >              new_entry.mfn = mfn_x(mfn);
> 
> > _______________________________________________
> > 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