[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [RFC PATCH 0/3] AMD IOMMU: Share p2m table with iommu



Yang is correct.  I was confused by what appears to be QEMU VGA or xl related 
issues in older changesets.

The common code that caused problem is the following.

typedef enum {
-    p2m_invalid = 0,            /* Nothing mapped here */
-    p2m_ram_rw = 1,             /* Normal read/write guest RAM */
+    p2m_ram_rw = 0,             /* Normal read/write guest RAM */
+    p2m_invalid = 1,            /* Nothing mapped here */

With the above change, guest with device direct assignment fails to boot.  QEMU 
VGA displays some weird color patterns.  We also see the following error 
messages during first guest boot:

....
(XEN) memory.c:196:d0 Bad page free for domain 1
(XEN) mm.c:2137:d0 Error pfn 0: rd=ffff8301c94c6000, od=ffff8301d8039000, caf=80
00000000000001, taf=7400000000000001
(XEN) memory.c:196:d0 Bad page free for domain 1
(XEN) mm.c:2137:d0 Error pfn 0: rd=ffff8301c94c6000, od=ffff8301d8039000, caf=80
00000000000001, taf=7400000000000001
(XEN) memory.c:196:d0 Bad page free for domain 1
(XEN) mm.c:2137:d0 Error pfn 0: rd=ffff8301c94c6000, od=ffff8301d8039000, caf=80
00000000000001, taf=7400000000000001
...

If I back out the above change, the latest changeset works again so this is the 
only change that causes the problem.  I remember we had to do a lot of special 
treatment of p2m_invlid quite a bit during initial enabling of vt-d device 
passthrough.

Allen

-----Original Message-----
From: Zhang, Yang Z 
Sent: Monday, May 16, 2011 1:36 AM
To: Tim Deegan
Cc: Wei Wang; xen-devel@xxxxxxxxxxxxxxxxxxx; Kay, Allen M
Subject: RE: [Xen-devel] [RFC PATCH 0/3] AMD IOMMU: Share p2m table with iommu

> What's the failure mode?  Or better, what change in the common code are
> you objecting to?
The following patch cause the vt-d fail to work. I suspect that the change is 
not appropriate for intel platform.
Add allen to CC list. Maybe he can give a more authoritative answer.

diff -r 51d89366c859 -r 78145a98915c xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c     Mon Apr 18 15:12:04 2011 +0100
+++ b/xen/arch/x86/mm/p2m.c     Mon Apr 18 17:24:21 2011 +0100
@@ -80,7 +80,12 @@ unsigned long p2m_type_to_flags(p2m_type  {
     unsigned long flags;
 #ifdef __x86_64__
-    flags = (unsigned long)(t & 0x3fff) << 9;
+    /*
+     * AMD IOMMU: When we share p2m table with iommu, bit 9 - bit 11 will be
+     * used for iommu hardware to encode next io page level. Bit 59 - bit 62
+     * are used for iommu flags, We could not use these bits to store p2m 
types.
+     */
+    flags = (unsigned long)(t & 0x7f) << 12;
 #else
     flags = (t & 0x7UL) << 9;
 #endif
@@ -1826,6 +1831,9 @@ static mfn_t p2m_gfn_to_mfn_current(stru
             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 51d89366c859 -r 78145a98915c xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h Mon Apr 18 15:12:04 2011 +0100
+++ b/xen/include/asm-x86/p2m.h Mon Apr 18 17:24:21 2011 +0100
@@ -63,9 +63,15 @@
  * Further expansions of the type system will only be supported on
  * 64-bit Xen.
  */
+
+/*
+ * AMD IOMMU: When we share p2m table with iommu, bit 52 -bit 58 in pte
+ * cannot be non-zero, otherwise, hardware generates io page faults 
+when
+ * device access those pages. Therefore, p2m_ram_rw has to be defined as 0.
+ */
 typedef enum {
-    p2m_invalid = 0,            /* Nothing mapped here */
-    p2m_ram_rw = 1,             /* Normal read/write guest RAM */
+    p2m_ram_rw = 0,             /* Normal read/write guest RAM */
+    p2m_invalid = 1,            /* Nothing mapped here */
     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 */
@@ -375,7 +381,13 @@ static inline p2m_type_t p2m_flags_to_ty  {
     /* Type is stored in the "available" bits */  #ifdef __x86_64__
-    return (flags >> 9) & 0x3fff;
+    /*
+     * AMD IOMMU: When we share p2m table with iommu, bit 9 - bit 11 will be
+     * used for iommu hardware to encode next io page level. Bit 59 - bit 62
+     * are used for iommu flags, We could not use these bits to store p2m 
types.
+     */
+
+    return (flags >> 12) & 0x7f;
 #else
     return (flags >> 9) & 0x7;
 #endif

> 
> BTW, this is not the patch set that was applied; when I applied the
> revised patches (about a month ago) I CC'd the Intel IOMMU maintainer.
> 
> Tim.
> 
> > best regards
> > yang
> >
> > > -----Original Message-----
> > > From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
> > > [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Wei Wang
> > > Sent: Friday, March 25, 2011 6:32 PM
> > > To: xen-devel@xxxxxxxxxxxxxxxxxxx
> > > Subject: [Xen-devel] [RFC PATCH 0/3] AMD IOMMU: Share p2m table with
> > > iommu
> > >
> > > Hi,
> > > This patch set implements p2m table sharing for amd iommu. Please
> > > comment it.
> > > Thanks,
> > > Wei
> > >
> > > --
> > > 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@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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.