[Xen-devel] [PATCH, RFC 1/7] IOMMU: adjust (re)assign operation parameters

["Jan Beulich" <JBeulich@xxxxxxxx>]

... to use a (struct pci_dev *, devfn) pair.

--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -332,34 +332,31 @@ void amd_iommu_disable_domain_device(str
         disable_ats_device(iommu->seg, bus, devfn);
 }
 
-static int reassign_device( struct domain *source, struct domain *target,
-                            u16 seg, u8 bus, u8 devfn)
+static int reassign_device(struct domain *source, struct domain *target,
+                           u8 devfn, struct pci_dev *pdev)
 {
-    struct pci_dev *pdev;
     struct amd_iommu *iommu;
     int bdf;
     struct hvm_iommu *t = domain_hvm_iommu(target);
 
-    ASSERT(spin_is_locked(&pcidevs_lock));
-    pdev = pci_get_pdev_by_domain(source, seg, bus, devfn);
-    if ( !pdev )
-        return -ENODEV;
-
-    bdf = PCI_BDF2(bus, devfn);
-    iommu = find_iommu_for_device(seg, bdf);
+    bdf = PCI_BDF2(pdev->bus, pdev->devfn);
+    iommu = find_iommu_for_device(pdev->seg, bdf);
     if ( !iommu )
     {
         AMD_IOMMU_DEBUG("Fail to find iommu."
                         " %04x:%02x:%x02.%x cannot be assigned to dom%d\n",
-                        seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                        pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                         target->domain_id);
         return -ENODEV;
     }
 
     amd_iommu_disable_domain_device(source, iommu, bdf);
 
-    list_move(&pdev->domain_list, &target->arch.pdev_list);
-    pdev->domain = target;
+    if ( devfn == pdev->devfn )
+    {
+        list_move(&pdev->domain_list, &target->arch.pdev_list);
+        pdev->domain = target;
+    }
 
     /* IO page tables might be destroyed after pci-detach the last device
      * In this case, we have to re-allocate root table for next pci-attach.*/
@@ -368,17 +365,18 @@ static int reassign_device( struct domai
 
     amd_iommu_setup_domain_device(target, iommu, bdf);
     AMD_IOMMU_DEBUG("Re-assign %04x:%02x:%02x.%u from dom%d to dom%d\n",
-                    seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                    pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                     source->domain_id, target->domain_id);
 
     return 0;
 }
 
-static int amd_iommu_assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
+static int amd_iommu_assign_device(struct domain *d, u8 devfn,
+                                   struct pci_dev *pdev)
 {
-    struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(seg);
-    int bdf = PCI_BDF2(bus, devfn);
-    int req_id = get_dma_requestor_id(seg, bdf);
+    struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(pdev->seg);
+    int bdf = PCI_BDF2(pdev->bus, devfn);
+    int req_id = get_dma_requestor_id(pdev->seg, bdf);
 
     if ( ivrs_mappings[req_id].unity_map_enable )
     {
@@ -390,7 +388,7 @@ static int amd_iommu_assign_device(struc
             ivrs_mappings[req_id].read_permission);
     }
 
-    return reassign_device(dom0, d, seg, bus, devfn);
+    return reassign_device(dom0, d, devfn, pdev);
 }
 
 static void deallocate_next_page_table(struct page_info* pg, int level)
@@ -451,12 +449,6 @@ static void amd_iommu_domain_destroy(str
     amd_iommu_flush_all_pages(d);
 }
 
-static int amd_iommu_return_device(
-    struct domain *s, struct domain *t, u16 seg, u8 bus, u8 devfn)
-{
-    return reassign_device(s, t, seg, bus, devfn);
-}
-
 static int amd_iommu_add_device(struct pci_dev *pdev)
 {
     struct amd_iommu *iommu;
@@ -593,7 +585,7 @@ const struct iommu_ops amd_iommu_ops = {
     .teardown = amd_iommu_domain_destroy,
     .map_page = amd_iommu_map_page,
     .unmap_page = amd_iommu_unmap_page,
-    .reassign_device = amd_iommu_return_device,
+    .reassign_device = reassign_device,
     .get_device_group_id = amd_iommu_group_id,
     .update_ire_from_apic = amd_iommu_ioapic_update_ire,
     .update_ire_from_msi = amd_iommu_msi_msg_update_ire,
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -229,11 +229,16 @@ static int assign_device(struct domain *
         return -EXDEV;
 
     spin_lock(&pcidevs_lock);
-    pdev = pci_get_pdev(seg, bus, devfn);
-    if ( pdev )
-        pdev->fault.count = 0;
+    pdev = pci_get_pdev_by_domain(dom0, seg, bus, devfn);
+    if ( !pdev )
+    {
+        rc = pci_get_pdev(seg, bus, devfn) ? -EBUSY : -ENODEV;
+        goto done;
+    }
+
+    pdev->fault.count = 0;
 
-    if ( (rc = hd->platform_ops->assign_device(d, seg, bus, devfn)) )
+    if ( (rc = hd->platform_ops->assign_device(d, devfn, pdev)) )
         goto done;
 
     if ( has_arch_pdevs(d) && !need_iommu(d) )
@@ -364,18 +369,11 @@ int deassign_device(struct domain *d, u1
         return -EINVAL;
 
     ASSERT(spin_is_locked(&pcidevs_lock));
-    pdev = pci_get_pdev(seg, bus, devfn);
+    pdev = pci_get_pdev_by_domain(d, seg, bus, devfn);
     if ( !pdev )
         return -ENODEV;
 
-    if ( pdev->domain != d )
-    {
-        dprintk(XENLOG_G_ERR,
-                "d%d: deassign a device not owned\n", d->domain_id);
-        return -EINVAL;
-    }
-
-    ret = hd->platform_ops->reassign_device(d, dom0, seg, bus, devfn);
+    ret = hd->platform_ops->reassign_device(d, dom0, devfn, pdev);
     if ( ret )
     {
         dprintk(XENLOG_G_ERR,
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1658,17 +1658,10 @@ out:
 static int reassign_device_ownership(
     struct domain *source,
     struct domain *target,
-    u16 seg, u8 bus, u8 devfn)
+    u8 devfn, struct pci_dev *pdev)
 {
-    struct pci_dev *pdev;
     int ret;
 
-    ASSERT(spin_is_locked(&pcidevs_lock));
-    pdev = pci_get_pdev_by_domain(source, seg, bus, devfn);
-
-    if (!pdev)
-        return -ENODEV;
-
     /*
      * Devices assigned to untrusted domains (here assumed to be any domU)
      * can attempt to send arbitrary LAPIC/MSI messages. We are unprotected
@@ -1677,16 +1670,19 @@ static int reassign_device_ownership(
     if ( (target != dom0) && !iommu_intremap )
         untrusted_msi = 1;
 
-    ret = domain_context_unmap(source, seg, bus, devfn);
+    ret = domain_context_unmap(source, pdev->seg, pdev->bus, devfn);
     if ( ret )
         return ret;
 
-    ret = domain_context_mapping(target, seg, bus, devfn);
+    ret = domain_context_mapping(target, pdev->seg, pdev->bus, devfn);
     if ( ret )
         return ret;
 
-    list_move(&pdev->domain_list, &target->arch.pdev_list);
-    pdev->domain = target;
+    if ( devfn == pdev->devfn )
+    {
+        list_move(&pdev->domain_list, &target->arch.pdev_list);
+        pdev->domain = target;
+    }
 
     return ret;
 }
@@ -2202,36 +2198,26 @@ int __init intel_vtd_setup(void)
 }
 
 static int intel_iommu_assign_device(
-    struct domain *d, u16 seg, u8 bus, u8 devfn)
+    struct domain *d, u8 devfn, struct pci_dev *pdev)
 {
     struct acpi_rmrr_unit *rmrr;
     int ret = 0, i;
-    struct pci_dev *pdev;
-    u16 bdf;
+    u16 bdf, seg;
+    u8 bus;
 
     if ( list_empty(&acpi_drhd_units) )
         return -ENODEV;
 
-    ASSERT(spin_is_locked(&pcidevs_lock));
-    pdev = pci_get_pdev(seg, bus, devfn);
-    if (!pdev)
-        return -ENODEV;
-
-    if (pdev->domain != dom0)
-    {
-        dprintk(XENLOG_ERR VTDPREFIX,
-                "IOMMU: assign a assigned device\n");
-       return -EBUSY;
-    }
-
-    ret = reassign_device_ownership(dom0, d, seg, bus, devfn);
+    ret = reassign_device_ownership(dom0, d, devfn, pdev);
     if ( ret )
         goto done;
 
     /* FIXME: Because USB RMRR conflicts with guest bios region,
      * ignore USB RMRR temporarily.
      */
-    if ( is_usb_device(seg, bus, devfn) )
+    seg = pdev->seg;
+    bus = pdev->bus;
+    if ( is_usb_device(seg, bus, pdev->devfn) )
     {
         ret = 0;
         goto done;
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -97,13 +97,13 @@ struct iommu_ops {
     int (*add_device)(struct pci_dev *pdev);
     int (*enable_device)(struct pci_dev *pdev);
     int (*remove_device)(struct pci_dev *pdev);
-    int (*assign_device)(struct domain *d, u16 seg, u8 bus, u8 devfn);
+    int (*assign_device)(struct domain *, u8 devfn, struct pci_dev *);
     void (*teardown)(struct domain *d);
     int (*map_page)(struct domain *d, unsigned long gfn, unsigned long mfn,
                     unsigned int flags);
     int (*unmap_page)(struct domain *d, unsigned long gfn);
     int (*reassign_device)(struct domain *s, struct domain *t,
-                          u16 seg, u8 bus, u8 devfn);
+                          u8 devfn, struct pci_dev *);
     int (*get_device_group_id)(u16 seg, u8 bus, u8 devfn);
     void (*update_ire_from_apic)(unsigned int apic, unsigned int reg, unsigned 
int value);
     void (*update_ire_from_msi)(struct msi_desc *msi_desc, struct msi_msg 
*msg);

Attachment: IOMMU-assign-params.patch
Description: Text document

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