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

[PATCH v6 5/9] xen/arm: smmuv2: Add PCI devices support for SMMUv2


  • To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Date: Thu, 9 Nov 2023 13:27:08 -0500
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=MfQJ30MMlF1LcmA2vSAup2UpciA3BnHADAP/CNrNni8=; b=KnsUwQiXEpgoGJ+DLa7M+tQpBHJ3k164twUlRo5eHEbqSqfICpCvSXIEYh11RoqtrMuvzRJ8RF5xoI/b34lteAX/vKfwGi0mfLYhcfa7+MUUjgb6NeAPQgZaKtkKM9oDq45JTYqVG3s9qvokd1UC6UlrNFWLSZL6VzMya6BPEbg/M8sNenotif9IgPAtcWW9/IwkjRWaxyaSXmI/hI7TKtgnP69gTQXA2682L3oy37Tc1CD89aoucWkJU/0Y0dYG6H7vNEyp3X9D2tj6Eo6Xuw/Vf0YoMxL2IRIdezBjFfVFfOfY0DsTqQZgcDGirs4mYNyMQyCKQz5/XGOy8KYG8A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=d6ZMefSRfGE9az7ESJBVT9GiK9Lgjwf3ggPPSbT7XJzE7CXut4VS1rCiVPewvdRMawTuAiNnbCzfZa0zyAwnFM3t3O6EIrwwOaCEJ6GKs/rXTL8+1hbc5zC7gCKsuP9jxzapDItI5h0Sgdbnz1UZ3vLeRwkHozjVBc1X8rljC5lSd/POdilqon5byWViczXUbRzIFvFM8NThfLmMJK5Hs4U7M6yHVYXYjFGQ5tslnyw134jiHImN9UKeLcdGJIOysXvJXgA20k1mvojQAsnit5EJFOsyRja7/brK4lLxcbHz06oyXt0Q3STOqcwfDlAy3DxDlUL3qaRJrqQ3y32Naw==
  • Cc: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, "Michal Orzel" <michal.orzel@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>, Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
  • Delivery-date: Thu, 09 Nov 2023 18:29:30 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>
---
v5->v6:
* check for hardware_domain == NULL (dom0less test case)
* locking: assign pdev->domain before list_add()

v4->v5:
* assign device to pdev->domain (usually dom0) by default in add_device() hook
* deassign from hwdom
* rebase on top of ("dynamic node programming using overlay dtbo") series
* remove TODO in comment about device prints
* add TODO regarding locking
* fixup after dropping ("xen/arm: Move is_protected flag to struct device")

v3->v4:
* add new device_is_protected check in add_device hook to match SMMUv3 and
  IPMMU-VMSA drivers

v2->v3:
* invoke iommu_add_pci_sideband_ids() from add_device hook

v1->v2:
* ignore add_device/assign_device/reassign_device calls for phantom functions
  (i.e. devfn != pdev->devfn)

downstream->v1:
* wrap unused function in #ifdef 0
* remove the remove_device() stub since it was submitted separately to the list
  [XEN][PATCH v6 12/19] xen/smmu: Add remove_device callback for smmu_iommu ops
  https://lists.xenproject.org/archives/html/xen-devel/2023-05/msg00204.html
* arm_smmu_(de)assign_dev: return error instead of crashing system
* update condition in arm_smmu_reassign_dev
* style fixup
* add && !is_hardware_domain(d) into condition in arm_smmu_assign_dev()

(cherry picked from commit 0c11a7f65f044c26d87d1e27ac6283ef1f9cfb7a from
 the downstream branch spider-master from
 https://github.com/xen-troops/xen.git)
---
 xen/drivers/passthrough/arm/smmu.c | 199 ++++++++++++++++++++++++-----
 1 file changed, 169 insertions(+), 30 deletions(-)

diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index 11fc1d22ef0a..24d1c0353025 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -131,11 +131,21 @@ enum irqreturn {
 
 typedef enum irqreturn irqreturn_t;
 
-/* Device logger functions
- * TODO: Handle PCI
- */
-#define dev_print(dev, lvl, fmt, ...)                                          
\
-        printk(lvl "smmu: %s: " fmt, dt_node_full_name(dev_to_dt(dev)), ## 
__VA_ARGS__)
+/* Device logger functions */
+#ifndef CONFIG_HAS_PCI
+#define dev_print(dev, lvl, fmt, ...)    \
+    printk(lvl "smmu: %s: " fmt, dev_name(dev), ## __VA_ARGS__)
+#else
+#define dev_print(dev, lvl, fmt, ...) ({                                \
+    if ( !dev_is_pci((dev)) )                                           \
+        printk(lvl "smmu: %s: " fmt, dev_name((dev)), ## __VA_ARGS__);  \
+    else                                                                \
+    {                                                                   \
+        struct pci_dev *pdev = dev_to_pci((dev));                       \
+        printk(lvl "smmu: %pp: " fmt, &pdev->sbdf, ## __VA_ARGS__);     \
+    }                                                                   \
+})
+#endif
 
 #define dev_dbg(dev, fmt, ...) dev_print(dev, XENLOG_DEBUG, fmt, ## 
__VA_ARGS__)
 #define dev_notice(dev, fmt, ...) dev_print(dev, XENLOG_INFO, fmt, ## 
__VA_ARGS__)
@@ -187,6 +197,7 @@ static void __iomem *devm_ioremap_resource(struct device 
*dev,
  * Xen: PCI functions
  * TODO: It should be implemented when PCI will be supported
  */
+#if 0 /* unused */
 #define to_pci_dev(dev)        (NULL)
 static inline int pci_for_each_dma_alias(struct pci_dev *pdev,
                                         int (*fn) (struct pci_dev *pdev,
@@ -196,6 +207,7 @@ static inline int pci_for_each_dma_alias(struct pci_dev 
*pdev,
        BUG();
        return 0;
 }
+#endif
 
 /* Xen: misc */
 #define PHYS_MASK_SHIFT                PADDR_BITS
@@ -631,7 +643,7 @@ struct arm_smmu_master_cfg {
        for (i = 0; idx = cfg->smendx[i], i < num; ++i)
 
 struct arm_smmu_master {
-       struct device_node              *of_node;
+       struct device                   *dev;
        struct rb_node                  node;
        struct arm_smmu_master_cfg      cfg;
 };
@@ -723,7 +735,7 @@ arm_smmu_get_fwspec(struct arm_smmu_master_cfg *cfg)
 {
        struct arm_smmu_master *master = container_of(cfg,
                                                              struct 
arm_smmu_master, cfg);
-       return dev_iommu_fwspec_get(&master->of_node->dev);
+       return dev_iommu_fwspec_get(master->dev);
 }
 
 static void parse_driver_options(struct arm_smmu_device *smmu)
@@ -756,7 +768,7 @@ static struct device_node *dev_get_dev_node(struct device 
*dev)
 }
 
 static struct arm_smmu_master *find_smmu_master(struct arm_smmu_device *smmu,
-                                               struct device_node *dev_node)
+                                               struct device *dev)
 {
        struct rb_node *node = smmu->masters.rb_node;
 
@@ -765,9 +777,9 @@ static struct arm_smmu_master *find_smmu_master(struct 
arm_smmu_device *smmu,
 
                master = container_of(node, struct arm_smmu_master, node);
 
-               if (dev_node < master->of_node)
+               if (dev < master->dev)
                        node = node->rb_left;
-               else if (dev_node > master->of_node)
+               else if (dev > master->dev)
                        node = node->rb_right;
                else
                        return master;
@@ -802,9 +814,9 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
                        = container_of(*new, struct arm_smmu_master, node);
 
                parent = *new;
-               if (master->of_node < this->of_node)
+               if (master->dev < this->dev)
                        new = &((*new)->rb_left);
-               else if (master->of_node > this->of_node)
+               else if (master->dev > this->dev)
                        new = &((*new)->rb_right);
                else
                        return -EEXIST;
@@ -836,28 +848,37 @@ static int arm_smmu_dt_add_device_legacy(struct 
arm_smmu_device *smmu,
        struct arm_smmu_master *master;
        struct device_node *dev_node = dev_get_dev_node(dev);
 
-       master = find_smmu_master(smmu, dev_node);
+       master = find_smmu_master(smmu, dev);
        if (master) {
                dev_err(dev,
                        "rejecting multiple registrations for master device 
%s\n",
-                       dev_node->name);
+                       dev_node ? dev_node->name : "");
                return -EBUSY;
        }
 
        master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL);
        if (!master)
                return -ENOMEM;
-       master->of_node = dev_node;
+       master->dev = dev;
 
-       /* Xen: Let Xen know that the device is protected by an SMMU */
-       dt_device_set_protected(dev_node);
+       if ( !dev_is_pci(dev) )
+       {
+               if ( dt_device_is_protected(dev_node) )
+               {
+                       dev_err(dev, "Already added to SMMU\n");
+                       return -EEXIST;
+               }
+
+               /* Xen: Let Xen know that the device is protected by an SMMU */
+               dt_device_set_protected(dev_node);
+       }
 
        for (i = 0; i < fwspec->num_ids; ++i) {
                if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
                     (fwspec->ids[i] >= smmu->num_mapping_groups)) {
                        dev_err(dev,
                                "stream ID for master device %s greater than 
maximum allowed (%d)\n",
-                               dev_node->name, smmu->num_mapping_groups);
+                               dev_node ? dev_node->name : "", 
smmu->num_mapping_groups);
                        return -ERANGE;
                }
                master->cfg.smendx[i] = INVALID_SMENDX;
@@ -872,7 +893,7 @@ static int arm_smmu_dt_remove_device_legacy(struct 
arm_smmu_device *smmu,
        struct device_node *dev_node = dev_get_dev_node(dev);
        int ret;
 
-       master = find_smmu_master(smmu, dev_node);
+       master = find_smmu_master(smmu, dev);
        if (master == NULL) {
                dev_err(dev,
                        "No registrations found for master device %s\n",
@@ -884,8 +905,9 @@ static int arm_smmu_dt_remove_device_legacy(struct 
arm_smmu_device *smmu,
        if (ret)
                return ret;
 
-       /* Protected by dt_host_lock and dtdevs_lock as caller holds these 
locks. */
-       dev_node->is_protected = false;
+       if ( !dev_is_pci(dev) )
+               /* Protected by dt_host_lock and dtdevs_lock as caller holds 
these locks. */
+               dev_node->is_protected = false;
 
        kfree(master);
        return 0;
@@ -914,6 +936,12 @@ static int register_smmu_master(struct arm_smmu_device 
*smmu,
                                             fwspec);
 }
 
+/* Forward declaration */
+static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
+                              struct device *dev, u32 flag);
+static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn,
+                                struct device *dev);
+
 /*
  * The driver which supports generic IOMMU DT bindings must have this
  * callback implemented.
@@ -938,6 +966,22 @@ static int arm_smmu_dt_add_device_generic(u8 devfn, struct 
device *dev)
 {
        struct arm_smmu_device *smmu;
        struct iommu_fwspec *fwspec;
+       int ret;
+
+#ifdef CONFIG_HAS_PCI
+       if ( dev_is_pci(dev) )
+       {
+               struct pci_dev *pdev = dev_to_pci(dev);
+               int ret;
+
+               if ( devfn != pdev->devfn )
+                       return 0;
+
+               ret = iommu_add_pci_sideband_ids(pdev);
+               if ( ret < 0 )
+                       iommu_fwspec_free(dev);
+       }
+#endif
 
        fwspec = dev_iommu_fwspec_get(dev);
        if (fwspec == NULL)
@@ -947,7 +991,24 @@ static int arm_smmu_dt_add_device_generic(u8 devfn, struct 
device *dev)
        if (smmu == NULL)
                return -ENXIO;
 
-       return arm_smmu_dt_add_device_legacy(smmu, dev, fwspec);
+       ret = arm_smmu_dt_add_device_legacy(smmu, dev, fwspec);
+       if ( ret )
+               return ret;
+
+#ifdef CONFIG_HAS_PCI
+       if ( dev_is_pci(dev) )
+       {
+               struct pci_dev *pdev = dev_to_pci(dev);
+
+               /*
+                * During PHYSDEVOP_pci_device_add, Xen does not assign the
+                * device, so we must do it here.
+                */
+               ret = arm_smmu_assign_dev(pdev->domain, devfn, dev, 0);
+       }
+#endif
+
+       return ret;
 }
 
 static int arm_smmu_dt_xlate_generic(struct device *dev,
@@ -970,11 +1031,10 @@ static struct arm_smmu_device 
*find_smmu_for_device(struct device *dev)
 {
        struct arm_smmu_device *smmu;
        struct arm_smmu_master *master = NULL;
-       struct device_node *dev_node = dev_get_dev_node(dev);
 
        spin_lock(&arm_smmu_devices_lock);
        list_for_each_entry(smmu, &arm_smmu_devices, list) {
-               master = find_smmu_master(smmu, dev_node);
+               master = find_smmu_master(smmu, dev);
                if (master)
                        break;
        }
@@ -2066,6 +2126,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
 }
 #endif
 
+#if 0 /* Not used */
 static int __arm_smmu_get_pci_sid(struct pci_dev *pdev, u16 alias, void *data)
 {
        *((u16 *)data) = alias;
@@ -2076,6 +2137,7 @@ static void __arm_smmu_release_pci_iommudata(void *data)
 {
        kfree(data);
 }
+#endif
 
 static int arm_smmu_add_device(struct device *dev)
 {
@@ -2083,12 +2145,13 @@ static int arm_smmu_add_device(struct device *dev)
        struct arm_smmu_master_cfg *cfg;
        struct iommu_group *group;
        void (*releasefn)(void *data) = NULL;
-       int ret;
 
        smmu = find_smmu_for_device(dev);
        if (!smmu)
                return -ENODEV;
 
+       /* There is no need to distinguish here, thanks to PCI-IOMMU DT 
bindings */
+#if 0
        if (dev_is_pci(dev)) {
                struct pci_dev *pdev = to_pci_dev(dev);
                struct iommu_fwspec *fwspec;
@@ -2113,10 +2176,12 @@ static int arm_smmu_add_device(struct device *dev)
                                       &fwspec->ids[0]);
                releasefn = __arm_smmu_release_pci_iommudata;
                cfg->smmu = smmu;
-       } else {
+       } else
+#endif
+       {
                struct arm_smmu_master *master;
 
-               master = find_smmu_master(smmu, dev->of_node);
+               master = find_smmu_master(smmu, dev);
                if (!master) {
                        return -ENODEV;
                }
@@ -2784,6 +2849,61 @@ static int arm_smmu_assign_dev(struct domain *d, u8 
devfn,
                        return -ENOMEM;
        }
 
+#ifdef CONFIG_HAS_PCI
+       if ( dev_is_pci(dev) && !is_hardware_domain(d) )
+       {
+               struct pci_dev *pdev = dev_to_pci(dev);
+
+               printk(XENLOG_INFO "Assigning device %04x:%02x:%02x.%u to 
dom%d\n",
+                      pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                      d->domain_id);
+
+               if ( devfn != pdev->devfn || pdev->domain == d )
+                       return 0;
+
+               ASSERT(pcidevs_locked());
+
+               /* TODO: acquire pci_lock */
+#if 0
+               write_lock(&pdev->domain->pci_lock);
+#endif
+               list_del(&pdev->domain_list);
+#if 0
+               write_unlock(&pdev->domain->pci_lock);
+#endif
+
+               pdev->domain = d;
+
+#if 0
+               write_lock(&d->pci_lock);
+#endif
+               list_add(&pdev->domain_list, &d->pdev_list);
+#if 0
+               write_unlock(&d->pci_lock);
+#endif
+
+               if ( hardware_domain )
+               {
+                       domain = dev_iommu_domain(dev);
+
+                       /*
+                        * Xen may not deassign the device from hwdom before
+                        * assigning it elsewhere.
+                        */
+                       if ( domain && 
is_hardware_domain(domain->priv->cfg.domain) )
+                       {
+                               ret = arm_smmu_deassign_dev(hardware_domain, 
devfn, dev);
+                               if ( ret )
+                                       return ret;
+                       }
+               }
+
+               /* dom_io is used as a sentinel for quarantined devices */
+               if ( d == dom_io )
+                       return 0;
+       }
+#endif
+
        if (!dev_iommu_group(dev)) {
                ret = arm_smmu_add_device(dev);
                if (ret)
@@ -2833,11 +2953,30 @@ out:
        return ret;
 }
 
-static int arm_smmu_deassign_dev(struct domain *d, struct device *dev)
+static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn,
+                                struct device *dev)
 {
        struct iommu_domain *domain = dev_iommu_domain(dev);
        struct arm_smmu_xen_domain *xen_domain;
 
+#ifdef CONFIG_HAS_PCI
+       if ( dev_is_pci(dev) )
+       {
+               struct pci_dev *pdev = dev_to_pci(dev);
+
+               printk(XENLOG_INFO "Deassigning device %04x:%02x:%02x.%u from 
dom%d\n",
+                      pdev->seg, pdev->bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
+                      d->domain_id);
+
+               if ( devfn != pdev->devfn )
+                       return 0;
+
+               /* dom_io is used as a sentinel for quarantined devices */
+               if ( d == dom_io )
+                       return 0;
+       }
+#endif
+
        xen_domain = dom_iommu(d)->arch.priv;
 
        if (!domain || domain->priv->cfg.domain != d) {
@@ -2865,13 +3004,13 @@ static int arm_smmu_reassign_dev(struct domain *s, 
struct domain *t,
        int ret = 0;
 
        /* Don't allow remapping on other domain than hwdom */
-       if ( t && !is_hardware_domain(t) )
+       if ( t && !is_hardware_domain(t) && t != dom_io )
                return -EPERM;
 
        if (t == s)
                return 0;
 
-       ret = arm_smmu_deassign_dev(s, dev);
+       ret = arm_smmu_deassign_dev(s, devfn, dev);
        if (ret)
                return ret;
 
-- 
2.42.0




 


Rackspace

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