[PATCH] AMD IOMMU: Hanlde sibling device assignment correctly

[Wei Wang2 <wei.wang2@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

Devices on AMD IOMMU system share a same set of IO page tables if they
are indexed by a same device ID in device table. To prevent these
sibling devices from being assigned to different domains, a successful
device assignment requires all sibling devices are owned by  either
pciback or the target domain. This is a trial patch with minor
modifications in the common interface to handle this situation
correctly.  VTD code has also been slightly touched please let me know
if it breaks any VTD functionality. Thanks!

Wei

Signed-off-by: Wei Wang <wei.wang2@amd.com>
-- 
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy 


[-- Attachment #2: device_assignment.patch --]
[-- Type: text/x-patch, Size: 8754 bytes --]

diff -r e6f20d5ed5fe xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c	Tue May 06 11:05:00 2008 +0100
+++ b/xen/arch/x86/domctl.c	Tue May 06 16:01:15 2008 +0200
@@ -529,15 +529,23 @@ long arch_do_domctl(
     case XEN_DOMCTL_test_assign_device:
     {
         u8 bus, devfn;
+        struct domain *d;
 
         ret = -EINVAL;
         if ( !iommu_enabled )
             break;
+
+        if ( unlikely((d = get_domain_by_id(domctl->domain)) == NULL) )
+        {
+            gdprintk(XENLOG_ERR,
+                "XEN_DOMCTL_assign_device: get_domain_by_id() failed\n");
+            break;
+        }
 
         bus = (domctl->u.assign_device.machine_bdf >> 16) & 0xff;
         devfn = (domctl->u.assign_device.machine_bdf >> 8) & 0xff;
 
-        if ( device_assigned(bus, devfn) )
+        if ( device_assigned(d, bus, devfn) )
         {
             gdprintk(XENLOG_ERR, "XEN_DOMCTL_test_assign_device: "
                      "%x:%x:%x already assigned\n",
@@ -566,7 +574,7 @@ long arch_do_domctl(
         bus = (domctl->u.assign_device.machine_bdf >> 16) & 0xff;
         devfn = (domctl->u.assign_device.machine_bdf >> 8) & 0xff;
 
-        if ( device_assigned(bus, devfn) )
+        if ( device_assigned(d, bus, devfn) )
         {
             gdprintk(XENLOG_ERR, "XEN_DOMCTL_assign_device: "
                      "%x:%x:%x already assigned\n",
@@ -599,7 +607,7 @@ long arch_do_domctl(
         bus = (domctl->u.assign_device.machine_bdf >> 16) & 0xff;
         devfn = (domctl->u.assign_device.machine_bdf >> 8) & 0xff;
 
-        if ( !device_assigned(bus, devfn) )
+        if ( !device_assigned(d, bus, devfn) )
             break;
 
         deassign_device(d, bus, devfn);
diff -r e6f20d5ed5fe xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Tue May 06 11:05:00 2008 +0100
+++ b/xen/arch/x86/setup.c	Tue May 06 16:01:15 2008 +0200
@@ -38,6 +38,7 @@
 #include <asm/edd.h>
 #include <xsm/xsm.h>
 #include <asm/tboot.h>
+#include <xen/iommu.h>
 
 #if defined(CONFIG_X86_64)
 #define BOOTSTRAP_DIRECTMAP_END (1UL << 32) /* 4GB */
@@ -1041,6 +1042,8 @@ void __init __start_xen(unsigned long mb
                         cmdline) != 0)
         panic("Could not set up DOM0 guest OS\n");
 
+    iommu_setup_devices(dom0, cmdline);
+
     /* Scrub RAM that is still free and so may go to an unprivileged domain. */
     scrub_heap_pages();
 
diff -r e6f20d5ed5fe xen/drivers/passthrough/amd/pci_amd_iommu.c
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c	Tue May 06 11:05:00 2008 +0100
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c	Tue May 06 16:01:15 2008 +0200
@@ -292,9 +292,7 @@ static void amd_iommu_setup_domain_devic
 
 static void amd_iommu_setup_dom0_devices(struct domain *d)
 {
-    struct hvm_iommu *hd = domain_hvm_iommu(d);
     struct amd_iommu *iommu;
-    struct pci_dev *pdev;
     int bus, dev, func;
     u32 l;
     int bdf;
@@ -311,15 +309,10 @@ static void amd_iommu_setup_dom0_devices
                      (l == 0x0000ffff) || (l == 0xffff0000) )
                     continue;
 
-                pdev = xmalloc(struct pci_dev);
-                pdev->bus = bus;
-                pdev->devfn = PCI_DEVFN(dev, func);
-                list_add_tail(&pdev->list, &hd->pdev_list);
-
-                bdf = (bus << 8) | pdev->devfn;
+                bdf = (bus << 8) | PCI_DEVFN(dev, func);
                 /* supported device? */
                 iommu = (bdf < ivrs_bdf_entries) ?
-                    find_iommu_for_device(bus, pdev->devfn) : NULL;
+                    find_iommu_for_device(bus, PCI_DEVFN(dev, func)) : NULL;
 
                 if ( iommu )
                     amd_iommu_setup_domain_device(d, iommu, bdf);
@@ -555,6 +548,53 @@ static int amd_iommu_assign_device(struc
 
     pdev_flr(bus, devfn);
     return reassign_device(dom0, d, bus, devfn);
+}
+
+static int amd_iommu_device_assigned(struct domain *d, u8 bus, u8 devfn)
+{
+    struct pci_dev *pdev;
+    int i;
+    int bdf = ((int)bus << 8) | devfn;
+    int req_id = ivrs_mappings[bdf].dte_requestor_id;
+
+    for_each_pdev( dom0, pdev )
+        if ( (pdev->bus == bus ) && (pdev->devfn == devfn) )
+            goto siblings_check;
+
+    return 1;
+
+siblings_check:
+
+    for ( i = 0; i < ivrs_bdf_entries; i++ )
+    {
+        int bus = (i >> 8) & 0xff;
+        int devfn = i & 0xff;
+
+        if ( ivrs_mappings[i].dte_requestor_id != req_id )
+            continue;
+
+        if ( i == bdf )
+            continue;
+
+        /* Check if this sibling device has been assigned to d. */
+        for_each_pdev( d, pdev )
+            if ( (pdev->bus == bus ) && (pdev->devfn == devfn) )
+                goto check_next;
+
+        /* Check if this sibling device is still owned by pciback,
+        otherwise it has been assigned to another passthru domain.*/
+        for_each_pdev( dom0, pdev )
+            if ( (pdev->bus == bus ) && (pdev->devfn == devfn) )
+                goto check_next;
+
+        /* Found! */
+        return 1;
+
+check_next:
+        continue;
+   }
+    /* no sibling devices found */
+    return 0;
 }
 
 static void release_domain_devices(struct domain *d)
@@ -642,4 +682,5 @@ struct iommu_ops amd_iommu_ops = {
     .map_page = amd_iommu_map_page,
     .unmap_page = amd_iommu_unmap_page,
     .reassign_device = amd_iommu_return_device,
+    .test_assigned = amd_iommu_device_assigned,
 };
diff -r e6f20d5ed5fe xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Tue May 06 11:05:00 2008 +0100
+++ b/xen/drivers/passthrough/iommu.c	Tue May 06 16:01:15 2008 +0200
@@ -163,4 +163,54 @@ static int iommu_setup(void)
     printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
     return rc;
 }
+
+int device_assigned(struct domain* d, u8 bus, u8 devfn)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+
+    if ( !iommu_enabled || !hd->platform_ops )
+        return 0;
+
+    return hd->platform_ops->test_assigned(d, bus, devfn);
+}
+
+void iommu_setup_devices(struct domain *domain, char *cmdline)
+{
+    char *pos;
+    int dm, bus, dev, func, parsed;
+    struct pci_dev *pdev;
+    int err = 0;
+    struct hvm_iommu *hd = domain_hvm_iommu(domain);
+
+    if ( !iommu_enabled )
+        return;
+
+    if ( boot_cpu_data.x86_vendor != X86_VENDOR_AMD )
+        return;
+
+    pos = strstr(cmdline, "pciback.hide=");
+    pos += strlen("pciback.hide=");
+
+    do {
+        err = sscanf(pos, " (%x:%x:%x.%x) %n",
+                     &dm, &bus, &dev, &func, &parsed);
+        if ( err != 4 )
+        {
+            dm = 0;
+            err = sscanf(pos, " (%x:%x.%x) %n",
+                         &bus, &dev, &func, &parsed);
+            if ( err != 3 )
+                goto parse_error;
+
+            pdev = xmalloc(struct pci_dev);
+            pdev->bus = bus;
+            pdev->devfn = PCI_DEVFN(dev, func);
+            list_add_tail(&pdev->list, &hd->pdev_list);
+            pos += parsed;
+        }
+    } while ( parsed > 0 );
+
+parse_error:
+    return;
+}
 __initcall(iommu_setup);
diff -r e6f20d5ed5fe xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Tue May 06 11:05:00 2008 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Tue May 06 16:01:15 2008 +0200
@@ -1798,7 +1798,7 @@ int intel_vtd_setup(void)
  * If the device isn't owned by dom0, it means it already
  * has been assigned to other domain, or it's not exist.
  */
-int device_assigned(u8 bus, u8 devfn)
+static int intel_iommu_device_assigned(struct domain *d, u8 bus, u8 devfn)
 {
     struct pci_dev *pdev;
 
@@ -1924,6 +1924,7 @@ struct iommu_ops intel_iommu_ops = {
     .map_page = intel_iommu_map_page,
     .unmap_page = intel_iommu_unmap_page,
     .reassign_device = reassign_device_ownership,
+    .test_assigned = intel_iommu_device_assigned,
 };
 
 /*
diff -r e6f20d5ed5fe xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue May 06 11:05:00 2008 +0100
+++ b/xen/include/xen/iommu.h	Tue May 06 16:01:15 2008 +0200
@@ -53,9 +53,10 @@ struct iommu {
     struct intel_iommu *intel;
 };
 
+void iommu_setup_devices(struct domain *domain, char *cmdline);
 int iommu_domain_init(struct domain *d);
 void iommu_domain_destroy(struct domain *d);
-int device_assigned(u8 bus, u8 devfn);
+int device_assigned(struct domain*d, u8 bus, u8 devfn);
 int assign_device(struct domain *d, u8 bus, u8 devfn);
 void deassign_device(struct domain *d, u8 bus, u8 devfn);
 void reassign_device_ownership(struct domain *source,
@@ -94,6 +95,7 @@ struct iommu_ops {
     int (*unmap_page)(struct domain *d, unsigned long gfn);
     void (*reassign_device)(struct domain *s, struct domain *t,
                             u8 bus, u8 devfn);
+    int (*test_assigned)(struct domain *d, u8 bus, u8 devfn);
 };
 
 #endif /* _IOMMU_H_ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel