Re: [Xen-devel] [PATCH 2 of 3] Clip mfn to allowable width when building a PTE

[Tim Deegan <tim@xxxxxxx>]

At 09:18 +0000 on 22 Mar (1332407899), Jan Beulich wrote:
>  >>> On 21.03.12 at 20:22, Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx> 
> wrote:
> > xen/include/asm-x86/page.h |  12 ++++++++----
> >  1 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > 
> > Otherwise, INVALID_MFN tramples over high order bits used for additional 
> > flags.
> 
> But is passing INVALID_PFN into these macros valid/sensible in the first
> place?

The p2m code uses pte layout even for entries that don't have the
_PAGE_PRESENT bit set.  We can:
 - mask out in these macros, making everything safe;
 - make new macros just for p2m code;
 - rewrite p2m callers not to use INVALID_MFN; or
 - have the p2m code explicitly replace INVALID_MFN with some other
   value when callers specify it.

I'm happy to do it this way but I guess maybe it slows down paths that
don't need the mask.

Tim.

> > Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Tim Deegan <tim@xxxxxxx>
> > 
> > diff -r 642c0e6a01c2 -r e325da663345 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) << PAGE_SHIFT) & PADDR_MASK)    \
> > +                        | 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) << PAGE_SHIFT) & PADDR_MASK)    \
> > +                        | 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) << PAGE_SHIFT) & PADDR_MASK)    \
> > +                        | 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) << PAGE_SHIFT) & PADDR_MASK)    \
> > +                        | put_pte_flags(flags) })
> >  
> >  /* Construct a pte from a physical address and access flags. */
> >  #ifndef __ASSEMBLY__
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx 
> > http://lists.xen.org/xen-devel 
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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