ept: Fix bug in changeset 22526:7a5ee3800417 This fixes a bug in changeset 22526:7a5ee3800417, where the wrong value is read when deciding whether to flush the VTd tables. The effect is minor: in situations where the p2m entry is changed but the mfn is the same, the VTd tables will be unnecessarily flushed. old_entry is left untouched in the second case, since having a present old_entry will cause the an unnecessary check to be taken at the end of the function. Spotted-by: Zhang, Peng Fei Signed-off-by: George Dunlap diff -r f071d8e9f744 xen/arch/x86/mm/hap/p2m-ept.c --- a/xen/arch/x86/mm/hap/p2m-ept.c Tue Mar 08 10:23:52 2011 +0000 +++ b/xen/arch/x86/mm/hap/p2m-ept.c Wed Mar 09 16:58:51 2011 +0000 @@ -374,7 +374,9 @@ needs_sync = 0; /* If we're replacing a non-leaf entry with a leaf entry (1GiB or 2MiB), - * the intermediate tables will be freed below after the ept flush */ + * the intermediate tables will be freed below after the ept flush + * + * Read-then-write is OK because we hold the p2m lock. */ old_entry = *ept_entry; if ( mfn_valid(mfn_x(mfn)) || direct_mmio || p2m_is_paged(p2mt) || @@ -390,10 +392,10 @@ new_entry.access = p2ma; new_entry.rsvd2_snp = (iommu_enabled && iommu_snoop); - if ( new_entry.mfn == mfn_x(mfn) ) + new_entry.mfn = mfn_x(mfn); + + if ( old_entry.mfn == new_entry.mfn ) need_modify_vtd_table = 0; - else - new_entry.mfn = mfn_x(mfn); ept_p2m_type_to_flags(&new_entry, p2mt, p2ma); } @@ -438,10 +440,12 @@ new_entry.access = p2ma; new_entry.rsvd2_snp = (iommu_enabled && iommu_snoop); - if ( new_entry.mfn == mfn_x(mfn) ) + /* the caller should take care of the previous page */ + new_entry.mfn = mfn_x(mfn); + + /* Safe to read-then-write because we hold the p2m lock */ + if ( ept_entry->mfn == new_entry.mfn ) need_modify_vtd_table = 0; - else /* the caller should take care of the previous page */ - new_entry.mfn = mfn_x(mfn); ept_p2m_type_to_flags(&new_entry, p2mt, p2ma);