With the rest of the API internals converted, it's time to finally
tackle probe_device and how we bootstrap the per-device ops association
to begin with. This ends up being disappointingly straightforward, since
fwspec users are already doing it in order to find their of_xlate
callback, and it works out that we can easily do the equivalent for
other drivers too. Then shuffle the remaining awareness of iommu_ops
into the couple of core headers that still need it, and breathe a sigh
of relief...
Ding dong the bus ops are gone!
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Christoph Hellwig <hch@lst.de>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 27 ++++++++++++++++-----------
include/acpi/acpi_bus.h | 2 ++
include/linux/device.h | 1 -
include/linux/device/bus.h | 5 -----
include/linux/dma-map-ops.h | 1 +
5 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1a31d94adff5..8997b8f2e79f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -218,13 +218,6 @@ int iommu_device_register(struct iommu_device *iommu,
/* We need to be able to take module references appropriately */
if (WARN_ON(is_module_address((unsigned long)ops) && !ops->owner))
return -EINVAL;
- /*
- * Temporarily enforce global restriction to a single driver. This was
- * already the de-facto behaviour, since any possible combination of
- * existing drivers would compete for at least the PCI or platform bus.
- */
- if (iommu_buses[0]->iommu_ops && iommu_buses[0]->iommu_ops != ops)
- return -EBUSY;
iommu->ops = ops;
if (hwdev)
@@ -234,10 +227,8 @@ int iommu_device_register(struct iommu_device *iommu,
list_add_tail(&iommu->list, &iommu_device_list);
spin_unlock(&iommu_device_lock);
- for (int i = 0; i < ARRAY_SIZE(iommu_buses) && !err; i++) {
- iommu_buses[i]->iommu_ops = ops;
+ for (int i = 0; i < ARRAY_SIZE(iommu_buses) && !err; i++)
err = bus_iommu_probe(iommu_buses[i]);
- }
if (err)
iommu_device_unregister(iommu);
return err;
@@ -303,12 +294,26 @@ static u32 dev_iommu_get_max_pasids(struct device *dev)
static int __iommu_probe_device(struct device *dev, struct list_head *group_list)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
+ const struct iommu_ops *ops;
struct iommu_device *iommu_dev;
+ struct iommu_fwspec *fwspec;
struct iommu_group *group;
static DEFINE_MUTEX(iommu_probe_device_lock);
int ret;
+ /*
+ * For FDT-based systems and ACPI IORT/VIOT, drivers register IOMMU
+ * instances with non-NULL fwnodes, and client devices should have been
+ * identified with a fwspec by this point. For Intel/AMD/s390/PAMU we
+ * can assume a single active driver with global ops, and so grab those
+ * from any registered instance, cheekily co-opting the same mechanism.
+ */
+ fwspec = dev_iommu_fwspec_get(dev);
+ if (fwspec && fwspec->ops)
+ ops = fwspec->ops;
+ else
+ ops = iommu_ops_from_fwnode(NULL);
+
if (!ops)
return -ENODEV;
/*
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index cd3b75e08ec3..067dde9291c9 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -614,6 +614,8 @@ struct acpi_pci_root {
/* helper */
+struct iommu_ops;
+
bool acpi_dma_supported(const struct acpi_device *adev);
enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
int acpi_iommu_fwspec_init(struct device *dev, u32 id,
diff --git a/include/linux/device.h b/include/linux/device.h
index 44e3acae7b36..f7a7ecafedd3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -41,7 +41,6 @@ struct class;
struct subsys_private;
struct device_node;
struct fwnode_handle;
-struct iommu_ops;
struct iommu_group;
struct dev_pin_info;
struct dev_iommu;
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index d8b29ccd07e5..4ece3470112f 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -63,9 +63,6 @@ struct fwnode_handle;
* this bus.
* @pm: Power management operations of this bus, callback the specific
* device driver's pm-ops.
- * @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU
- * driver implementations to a bus and allow the driver to do
- * bus-specific setup
* @p: The private data of the driver core, only the driver core can
* touch this.
* @lock_key: Lock class key for use by the lock validator
@@ -109,8 +106,6 @@ struct bus_type {
const struct dev_pm_ops *pm;
- const struct iommu_ops *iommu_ops;
-
struct subsys_private *p;
struct lock_class_key lock_key;
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index d678afeb8a13..e8ebf0bf611b 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -10,6 +10,7 @@
#include <linux/pgtable.h>
struct cma;
+struct iommu_ops;
/*
* Values for struct dma_map_ops.flags:
--
2.36.1.dirtyOn Thu, Jan 19, 2023 at 07:18:24PM +0000, Robin Murphy wrote: > With the rest of the API internals converted, it's time to finally > tackle probe_device and how we bootstrap the per-device ops association > to begin with. This ends up being disappointingly straightforward, since > fwspec users are already doing it in order to find their of_xlate > callback, and it works out that we can easily do the equivalent for > other drivers too. Then shuffle the remaining awareness of iommu_ops > into the couple of core headers that still need it, and breathe a sigh > of relief... > > Ding dong the bus ops are gone! > > CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > CC: Christoph Hellwig <hch@lst.de> > Signed-off-by: Robin Murphy <robin.murphy@arm.com> Nice work! Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On 2023/1/20 3:18, Robin Murphy wrote: > + /* > + * For FDT-based systems and ACPI IORT/VIOT, drivers register IOMMU > + * instances with non-NULL fwnodes, and client devices should have been > + * identified with a fwspec by this point. For Intel/AMD/s390/PAMU we > + * can assume a single active driver with global ops, and so grab those > + * from any registered instance, cheekily co-opting the same mechanism. > + */ > + fwspec = dev_iommu_fwspec_get(dev); > + if (fwspec && fwspec->ops) > + ops = fwspec->ops; > + else > + ops = iommu_ops_from_fwnode(NULL); I'm imagining if Intel/AMD/s390 drivers need to give up global ops. Is there any way to allow them to make such conversion? I am just thinking about whether this is a hard limitation for these drivers. Best regards, baolu
On 2023-01-20 00:27, Baolu Lu wrote: > On 2023/1/20 3:18, Robin Murphy wrote: >> + /* >> + * For FDT-based systems and ACPI IORT/VIOT, drivers register IOMMU >> + * instances with non-NULL fwnodes, and client devices should >> have been >> + * identified with a fwspec by this point. For >> Intel/AMD/s390/PAMU we >> + * can assume a single active driver with global ops, and so grab >> those >> + * from any registered instance, cheekily co-opting the same >> mechanism. >> + */ >> + fwspec = dev_iommu_fwspec_get(dev); >> + if (fwspec && fwspec->ops) >> + ops = fwspec->ops; >> + else >> + ops = iommu_ops_from_fwnode(NULL); > > I'm imagining if Intel/AMD/s390 drivers need to give up global ops. > Is there any way to allow them to make such conversion? I am just > thinking about whether this is a hard limitation for these drivers. Yes, they could perhaps bodge into the existing fwnode mechanism, or we could make bigger changes to adapt and generalise the whole instance-registration-token-lookup concept, or if the driver can resolve the correct instance for a device internally, then it could suffice to just have all its device ops share a single common .probe_device implementation that does the right thing. The comment is merely noting the fact that we can get away without having to worry about those changes just yet, since all the drivers *are* currently still built around the hard constraint of a single set of device ops per bus. Thanks, Robin.
On 2023/1/20 20:31, Robin Murphy wrote: > On 2023-01-20 00:27, Baolu Lu wrote: >> On 2023/1/20 3:18, Robin Murphy wrote: >>> + /* >>> + * For FDT-based systems and ACPI IORT/VIOT, drivers register IOMMU >>> + * instances with non-NULL fwnodes, and client devices should >>> have been >>> + * identified with a fwspec by this point. For >>> Intel/AMD/s390/PAMU we >>> + * can assume a single active driver with global ops, and so >>> grab those >>> + * from any registered instance, cheekily co-opting the same >>> mechanism. >>> + */ >>> + fwspec = dev_iommu_fwspec_get(dev); >>> + if (fwspec && fwspec->ops) >>> + ops = fwspec->ops; >>> + else >>> + ops = iommu_ops_from_fwnode(NULL); >> >> I'm imagining if Intel/AMD/s390 drivers need to give up global ops. >> Is there any way to allow them to make such conversion? I am just >> thinking about whether this is a hard limitation for these drivers. > > Yes, they could perhaps bodge into the existing fwnode mechanism, or we > could make bigger changes to adapt and generalise the whole > instance-registration-token-lookup concept, or if the driver can resolve > the correct instance for a device internally, then it could suffice to > just have all its device ops share a single common .probe_device > implementation that does the right thing. Yes. Sharing a common .probe_device entry and let the IOMMU driver connect the device and its iommu ops is a feasible solution. Thanks! > > The comment is merely noting the fact that we can get away without > having to worry about those changes just yet, since all the drivers > *are* currently still built around the hard constraint of a single set > of device ops per bus. Yes. Fair enough. -- Best regards, baolu
© 2016 - 2025 Red Hat, Inc.