Re: [Xen-devel] [RFC PATCH 1/3] AMD IOMMU: p2m table changes

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

> AMD IOMMU hardware uses bit 9 - bit 11 to encode lower page levels. 
> Therefore, 
> p2m type bits has to be shifted from bit 9 to bit 12 in p2m flags.

OK, but you need to add a comment explaining it, so the next person
doesn't try to reuse those three bits. 

> Also, bit 52
> to bit 60 cannot be non-zero for iommu pte. So, I have to swap the definition 
> of 
> p2m_ram_rw with p2m_invalid.

That _definitely_ needs a comment explianing why it's so. 

Also, what's the failure mode if the guest tries to access other types
of memory via the IOMMU?  In the current code the entries will not be
present in the IOMMU table; after this change they'll be present but
malformed.  Is that equivalent?

Cheers,

Tim.

> This patch is tested OK with both SPT and NPT
> guests.
> 
> Signed-off-by: Wei Wang <wei.wang2@xxxxxxx>
> 
> diff -r 12f7c7ac7f19 -r 6827fd35cf58 xen/arch/x86/mm/p2m.c
> --- a/xen/arch/x86/mm/p2m.c   Fri Mar 18 17:15:52 2011 +0000
> +++ b/xen/arch/x86/mm/p2m.c   Thu Mar 24 16:18:28 2011 +0100
> @@ -79,7 +79,7 @@
>  {
>      unsigned long flags;
>  #ifdef __x86_64__
> -    flags = (unsigned long)(t & 0x3fff) << 9;
> +    flags = (unsigned long)(t & 0x3fff) << 12;
>  #else
>      flags = (t & 0x7UL) << 9;
>  #endif
> @@ -1760,6 +1760,9 @@
>              p2mt = p2m_flags_to_type(l1e_get_flags(l1e));
>              ASSERT(l1e_get_pfn(l1e) != INVALID_MFN || !p2m_is_ram(p2mt));
>  
> +            if ( l1e.l1 == 0 )
> +                p2mt = p2m_invalid;
> +
>              if ( p2m_flags_to_type(l1e_get_flags(l1e))
>                   == p2m_populate_on_demand )
>              {
> diff -r 12f7c7ac7f19 -r 6827fd35cf58 xen/include/asm-x86/p2m.h
> --- a/xen/include/asm-x86/p2m.h       Fri Mar 18 17:15:52 2011 +0000
> +++ b/xen/include/asm-x86/p2m.h       Thu Mar 24 16:18:28 2011 +0100
> @@ -64,8 +64,8 @@
>   * 64-bit Xen.
>   */
>  typedef enum {
> -    p2m_invalid = 0,            /* Nothing mapped here */
> -    p2m_ram_rw = 1,             /* Normal read/write guest RAM */
> +    p2m_invalid = 1,            /* Nothing mapped here */
> +    p2m_ram_rw = 0,             /* Normal read/write guest RAM */
>      p2m_ram_logdirty = 2,       /* Temporarily read-only for log-dirty */
>      p2m_ram_ro = 3,             /* Read-only; writes are silently dropped */
>      p2m_mmio_dm = 4,            /* Reads and write go to the device model */
> @@ -312,7 +312,7 @@
>  {
>      /* Type is stored in the "available" bits */
>  #ifdef __x86_64__
> -    return (flags >> 9) & 0x3fff;
> +    return (flags >> 12) & 0x3fff;
>  #else
>      return (flags >> 9) & 0x7;
>  #endif

> _______________________________________________
> 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