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

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

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")?  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.

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


-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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