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

Re: [XEN][RFC PATCH v2 07/12] xen/smmu: Add remove_device callback for smmu_iommu ops



Hi,

On 09/11/2021 08:02, Vikram Garhwal wrote:
Add remove_device callback for removing the device entry from smmu-master using
  following steps:
     1. Find if SMMU master exists for the device node.
     2. Remove the SMMU master

The commit title and message doesn't match the code.


Signed-off-by: Vikram Garhwal <fnu.vikram@xxxxxxxxxx>
---
  xen/drivers/passthrough/device_tree.c | 30 ++++++++++++++++++++++++++++++
  xen/include/xen/iommu.h               |  2 ++
  2 files changed, 32 insertions(+)

diff --git a/xen/drivers/passthrough/device_tree.c 
b/xen/drivers/passthrough/device_tree.c
index 98f2aa0..9d9eed8 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -127,6 +127,36 @@ int iommu_release_dt_devices(struct domain *d)
      return 0;
  }
+int iommu_remove_dt_device(struct dt_device_node *np)
+{
+    const struct iommu_ops *ops = iommu_get_ops();
+    struct device *dev = dt_to_dev(np);
+    int rc;
+
+    if ( !ops )
+        return -EOPNOTSUPP;
+
+    if ( iommu_dt_device_is_assigned(np) )
+        return -EPERM;

EPERM means the caller doesn't have the permission to request it. However, dom0 will have the permission to remote the device. The problem is the device is currently assigned to a domain. So it would be better to return EBUSY.

Also, most of the function wants to be protected with dtdevs_lock to prevent concurrent access to add/remove/assign/deassign.

I would create a version of iommu_dt_device_is_assigned() (maybe called iommu_dt_device_is_assigned_locked()) that would do the same but with the caller hold the lock.

+
+    /*
+     * The driver which supports generic IOMMU DT bindings must have
+     * these callback implemented.
+     */
+    if ( !ops->remove_device )
+        return -EOPNOTSUPP;
+
+    /*
+     * Remove master device from the IOMMU if latter is present and available.
+     */
+    rc = ops->remove_device(0, dev);
+
+    if ( rc == 0 )
+        iommu_fwspec_free(dev);
+
+    return rc;
+}
+
  int iommu_add_dt_device(struct dt_device_node *np)
  {
      const struct iommu_ops *ops = iommu_get_ops();
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 6b2cdff..c4d5d12 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -215,6 +215,8 @@ int iommu_release_dt_devices(struct domain *d);
   */
  int iommu_add_dt_device(struct dt_device_node *np);
+int iommu_remove_dt_device(struct dt_device_node *np);
+
  int iommu_do_dt_domctl(struct xen_domctl *, struct domain *,
                         XEN_GUEST_HANDLE_PARAM(xen_domctl_t));

Cheers,

--
Julien Grall



 


Rackspace

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