-- Advanced Micro Devices GmbH Sitz: Dornach, Gemeinde Aschheim, Landkreis München Registergericht München, HRB Nr. 43632 WEEE-Reg-Nr: DE 12919551 Geschäftsführer: Alberto Bozzo, Andrew Bowd _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2011-Apr-04 10:59 UTC
Re: [Xen-devel] [RFC PATCH 1/3] AMD IOMMU: p2m table changes
> 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@amd.com> > > 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@lists.xensource.com > http://lists.xensource.com/xen-devel-- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei Wang2
2011-Apr-04 12:14 UTC
Re: [Xen-devel] [RFC PATCH 1/3] AMD IOMMU: p2m table changes
Tim, On Monday 04 April 2011 12:59:40 Tim Deegan wrote:> > 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.OK, I will add that. Thanks for point this out.> 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?AMD IOMMU hardware will generate IO page faults when guest try to access these pages with bit 52 - bit 58 in pte are non-zore. They are reserved bits for now. I could not clear present bits for the ptes since this might break other p2m functionalities but I should be no problem to clear read/write bits for those entries. Thanks, Wei> Cheers, > > Tim. > > > This patch is tested OK with both SPT and NPT > > guests. > > > > Signed-off-by: Wei Wang <wei.wang2@amd.com> > > > > 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@lists.xensource.com > > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel