Re: [Xen-devel] [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m

["Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>]

> At 12:12 -0700 on 14 Mar (1331727126), Andres Lagar-Cavilla wrote:
>> > Hmmm.  If we need to have this, we should probably have it in the main
>> > l*_from_pfn and l*_get_pfn code rather than needing to scatter it
>> around
>> > in the callers.  And we need a check in the e820 map to make sure we
>> > don't ever use MFN 0xffffffff (once CPUs start supporting it).
>> >
>> > The alternative would be to add another type so we don't have to store
>> > INVALID_MFN in p2m_ram_paging_in entries.
>>
>> A lot of callers store INVALID_MFN into p2m entries
>> (clear_mmio_p2m_entry,
>> p2m_remove_page, more). For all of them, as well as for paging eviction,
>> what matters is the type stored, not the mfn.
>>
>> That is why retrieving the INVALID_MFN is not the problem. The ept code
>> itself clips the INVALID_MFN (typecast to bitfield in ept_entry union)
>> and
>> everything seems to work fine when returning the clipped INVALID_MFN: no
>> one actually cares about that mfn.
>>
>> Because of that, I believe it's neither necessary to unclip on the
>> extraction path, nor to take any special precautions in the e820.
>
> Righto.  In that case, I'd be happy with just clipping MFNs and not
> trying to unpack them.  But I think it should happen in the main
> pte-building macros, not scattered around the p2m code.  It should just
> be a matter of using PADDR_MASK in the right place.

Something along these lines? (RFC, not tested yet)
Andres

# HG changeset patch
# Parent e490b3ca615d0141c1d97a89c1880dc8dc8f0783
Clip mfn to allowable width when building a PTE.

Otherwise, INVALID_MFN tramples over high order bits used for additional
flags.

Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>

diff -r e490b3ca615d xen/include/asm-x86/page.h
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -107,13 +107,17 @@

 /* Construct a pte from a pfn and access flags. */
 #define l1e_from_pfn(pfn, flags)   \
-    ((l1_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |
put_pte_flags(flags) })
+    ((l1_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) << 
  \
+                        PAGE_SHIFT) | put_pte_flags(flags) })
 #define l2e_from_pfn(pfn, flags)   \
-    ((l2_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |
put_pte_flags(flags) })
+    ((l2_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) << 
  \
+                        PAGE_SHIFT) | put_pte_flags(flags) })
 #define l3e_from_pfn(pfn, flags)   \
-    ((l3_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |
put_pte_flags(flags) })
+    ((l3_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) << 
  \
+                        PAGE_SHIFT) | put_pte_flags(flags) })
 #define l4e_from_pfn(pfn, flags)   \
-    ((l4_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |
put_pte_flags(flags) })
+    ((l4_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) << 
  \
+                        PAGE_SHIFT) | put_pte_flags(flags) })

 /* Construct a pte from a physical address and access flags. */
 #ifndef __ASSEMBLY__

>
> Cheers,
>
> Tim.
>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel