[PATCH v4 00/10] IOMMU probe deferral support

[Sricharan R]

This series calls the dma ops configuration for the devices
at a generic place so that it works for all busses.
The dma_configure_ops for a device is now called during
the device_attach callback just before the probe of the
bus/driver is called. Similarly dma_deconfigure is called during
device/driver_detach path.

pci_bus_add_devices (platform/amba)(_device_create/driver_register)
| |
pci_bus_add_device (device_add/driver_register)
| |
device_attach device_initial_probe
| |
__device_attach_driver __device_attach_driver
|
driver_probe_device
|
really_probe
|
dma_configure

Similarly on the device/driver_unregister path __device_release_driver is
called which inturn calls dma_deconfigure.

Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].

Tested with platform and pci devices for probe deferral
and reprobe on arm64 based platform. Added the patches [9,10],
drivers: acpi: Configure acpi devices dma operation at probe time
drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
for doing the dma ops configuration at probe time for acpi based platform
as well. Did not have a acpi based platform to test the changes and with
my still catching up knowledge on acpi/enumeration those two patches needs
to be reviewed/tested more.

Previous post of this series [5].

[V4]
* Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].

* Added the patches for moving the dma ops configuration of
acpi based devices to probe time as well.
[V3]
* Removed the patch to split dma_masks/dma_ops configuration
separately based on review comments that both masks and ops are
required only during the device probe time.

* Reworked the series based on Generic DT bindings series.

* Added call to iommu's remove_device in the cleanup path for arm and
arm64.

* Removed the notifier trick in arm64 to handle early device
registration.

* Added reset of dma_ops in cleanup path for arm based on comments.

* Fixed the pci_iommu_configure path and tested with PCI device as
well.

* Fixed a bug to return the correct iommu_ops from patch 7 [4] in
last post.

* Fixed few other cosmetic comments.

[V2]
* Updated the Initial post to call dma_configure/deconfigure from
generic code

* Added iommu add_device callback from of_iommu_configure path

[V1]
* Initial post from Laurent Pinchart [1]

[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
[2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
[3] https://lkml.org/lkml/2016/11/21/141
[4] https://www.mail-archive.com/***@lists.linux-foundation.org/msg13940.html
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460832.html

Laurent Pinchart (3):
of: dma: Move range size workaround to of_dma_get_range()
of: dma: Make of_dma_deconfigure() public
iommu: of: Handle IOMMU lookup failure with deferred probing or error

Robin Murphy (3):
iommu/of: Refactor of_iommu_configure() for error handling
iommu/of: Prepare for deferred IOMMU configuration
iommu/arm-smmu: Clean up early-probing workarounds

Sricharan R (4):
drivers: platform: Configure dma operations at probe time
arm64: dma-mapping: Remove the notifier trick to handle early setting
of dma_ops
drivers: acpi: Configure acpi devices dma operation at probe time
drivers: acpi: Handle IOMMU lookup failure with deferred probing or
error

arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
drivers/acpi/arm64/iort.c | 17 +++++-
drivers/acpi/glue.c | 6 --
drivers/acpi/scan.c | 7 ++-
drivers/base/dd.c | 10 ++++
drivers/base/dma-mapping.c | 32 +++++++++++
drivers/iommu/arm-smmu-v3.c | 35 +-----------
drivers/iommu/arm-smmu.c | 58 +++----------------
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 114 +++++++++++++++++++++++++++-----------
drivers/of/address.c | 20 ++++++-
drivers/of/device.c | 36 ++++++------
drivers/of/platform.c | 10 +---
drivers/pci/probe.c | 17 +++---
include/acpi/acpi_bus.h | 2 +-
include/linux/acpi.h | 7 ++-
include/linux/dma-mapping.h | 3 +
include/linux/of_device.h | 10 +++-
include/linux/pci.h | 5 ++
19 files changed, 238 insertions(+), 284 deletions(-)

[Sricharan R]

From: Robin Murphy <***@arm.com>

In preparation for some upcoming cleverness, rework the control flow in
of_iommu_configure() to minimise duplication and improve the propogation
of errors. It's also as good a time as any to switch over from the
now-just-a-compatibility-wrapper of_iommu_get_ops() to using the generic
IOMMU instance interface directly.

Signed-off-by: Robin Murphy <***@arm.com>
---
drivers/iommu/of_iommu.c | 83 +++++++++++++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 0f57ddc..ee49081 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -96,6 +96,28 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
}
EXPORT_SYMBOL_GPL(of_get_dma_window);

+static const struct iommu_ops
+*of_iommu_xlate(struct device *dev, struct of_phandle_args *iommu_spec)
+{
+ const struct iommu_ops *ops;
+ struct fwnode_handle *fwnode = &iommu_spec->np->fwnode;
+ int err;
+
+ ops = iommu_get_instance(fwnode);
+ if (!ops || !ops->of_xlate)
+ return NULL;
+
+ err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
+ if (err)
+ return ERR_PTR(err);
+
+ err = ops->of_xlate(dev, iommu_spec);
+ if (err)
+ return ERR_PTR(err);
+
+ return ops;
+}
+
static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
{
struct of_phandle_args *iommu_spec = data;
@@ -105,10 +127,11 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
}

static const struct iommu_ops
-*of_pci_iommu_configure(struct pci_dev *pdev, struct device_node *bridge_np)
+*of_pci_iommu_init(struct pci_dev *pdev, struct device_node *bridge_np)
{
const struct iommu_ops *ops;
struct of_phandle_args iommu_spec;
+ int err;

/*
* Start by tracing the RID alias down the PCI topology as
@@ -123,56 +146,56 @@ static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
* bus into the system beyond, and which IOMMU it ends up at.
*/
iommu_spec.np = NULL;
- if (of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
- "iommu-map-mask", &iommu_spec.np, iommu_spec.args))
- return NULL;
+ err = of_pci_map_rid(bridge_np, iommu_spec.args[0], "iommu-map",
+ "iommu-map-mask", &iommu_spec.np,
+ iommu_spec.args);
+ if (err)
+ return ERR_PTR(err);

- ops = of_iommu_get_ops(iommu_spec.np);
- if (!ops || !ops->of_xlate ||
- iommu_fwspec_init(&pdev->dev, &iommu_spec.np->fwnode, ops) ||
- ops->of_xlate(&pdev->dev, &iommu_spec))
- ops = NULL;
+ ops = of_iommu_xlate(&pdev->dev, &iommu_spec);

of_node_put(iommu_spec.np);
return ops;
}

-const struct iommu_ops *of_iommu_configure(struct device *dev,
- struct device_node *master_np)
+static const struct iommu_ops
+*of_platform_iommu_init(struct device *dev, struct device_node *np)
{
struct of_phandle_args iommu_spec;
- struct device_node *np;
const struct iommu_ops *ops = NULL;
int idx = 0;

- if (dev_is_pci(dev))
- return of_pci_iommu_configure(to_pci_dev(dev), master_np);
-
/*
* We don't currently walk up the tree looking for a parent IOMMU.
* See the `Notes:' section of
* Documentation/devicetree/bindings/iommu/iommu.txt
*/
- while (!of_parse_phandle_with_args(master_np, "iommus",
- "#iommu-cells", idx,
- &iommu_spec)) {
- np = iommu_spec.np;
- ops = of_iommu_get_ops(np);
-
- if (!ops || !ops->of_xlate ||
- iommu_fwspec_init(dev, &np->fwnode, ops) ||
- ops->of_xlate(dev, &iommu_spec))
- goto err_put_node;
-
- of_node_put(np);
+ while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells",
+ idx, &iommu_spec)) {
+ ops = of_iommu_xlate(dev, &iommu_spec);
+ of_node_put(iommu_spec.np);
idx++;
+ if (IS_ERR_OR_NULL(ops))
+ break;
}

return ops;
+}
+
+const struct iommu_ops *of_iommu_configure(struct device *dev,
+ struct device_node *master_np)
+{
+ const struct iommu_ops *ops;
+
+ if (!master_np)
+ return NULL;
+
+ if (dev_is_pci(dev))
+ ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
+ else
+ ops = of_platform_iommu_init(dev, master_np);

-err_put_node:
- of_node_put(np);
- return NULL;
+ return IS_ERR(ops) ? NULL : ops;
}

static int __init of_iommu_init(void)

[Sricharan R]

From: Robin Murphy <***@arm.com>

IOMMU configuration represents unchanging properties of the hardware,
and as such should only need happen once in a device's lifetime, but
the necessary interaction with the IOMMU device and driver complicates
exactly when that point should be.

Since the only reasonable tool available for handling the inter-device
dependency is probe deferral, we need to prepare of_iommu_configure()
to run later than it is currently called (i.e. at driver probe rather
than device creation), to handle being retried, and to tell whether a
not-yet present IOMMU should be waited for or skipped (by virtue of
having declared a built-in driver or not).

Signed-off-by: Robin Murphy <***@arm.com>
---
drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee49081..349bd1d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
int err;

ops = iommu_get_instance(fwnode);
- if (!ops || !ops->of_xlate)
+ if ((ops && !ops->of_xlate) ||
+ (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))
return NULL;

err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
if (err)
return ERR_PTR(err);
+ /*
+ * The otherwise-empty fwspec handily serves to indicate the specific
+ * IOMMU device we're waiting for, which will be useful if we ever get
+ * a proper probe-ordering dependency mechanism in future.
+ */
+ if (!ops)
+ return ERR_PTR(-EPROBE_DEFER);

err = ops->of_xlate(dev, iommu_spec);
if (err)
@@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
const struct iommu_ops *ops;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;

if (!master_np)
return NULL;

+ if (fwspec) {
+ if (fwspec->ops)
+ return fwspec->ops;
+
+ /* In the deferred case, start again from scratch */
+ iommu_fwspec_free(dev);
+ }
+
if (dev_is_pci(dev))
ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
else
ops = of_platform_iommu_init(dev, master_np);
+ /*
+ * If we have reason to believe the IOMMU driver missed the initial
+ * add_device callback for dev, replay it to get things in order.
+ */
+ if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+ dev->bus && !dev->iommu_group) {
+ int err = ops->add_device(dev);
+
+ if (err)
+ ops = ERR_PTR(err);
+ }

return IS_ERR(ops) ? NULL : ops;
}

[Lorenzo Pieralisi]

Sricharan, Robin,

I gave this series a go on ACPI and apart from an SMMU v3 fix-up
it seems to work, more thorough testing required though.

A key question below.

Post by Sricharan R
IOMMU configuration represents unchanging properties of the hardware,
and as such should only need happen once in a device's lifetime, but
the necessary interaction with the IOMMU device and driver complicates
exactly when that point should be.
Since the only reasonable tool available for handling the inter-device
dependency is probe deferral, we need to prepare of_iommu_configure()
to run later than it is currently called (i.e. at driver probe rather
than device creation), to handle being retried, and to tell whether a
not-yet present IOMMU should be waited for or skipped (by virtue of
having declared a built-in driver or not).
---
drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee49081..349bd1d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
int err;
ops = iommu_get_instance(fwnode);
- if (!ops || !ops->of_xlate)
+ if ((ops && !ops->of_xlate) ||
+ (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))

IIUC of_match_node() here is there to check there is a driver compiled
in for this device_node (aka compatible string in OF world), correct ?
If that's the case (and I think that's what Sricharan was referring to
in his ACPI query) I need to cook-up something on the ACPI side to
emulate the OF linker table behaviour (or anyway to detect a driver is
actually in the kernel), it is not that difficult but it is key to know,
I will give it some thought to make it as clean as possible.

Thanks,
Lorenzo

Post by Sricharan R
return NULL;
err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
if (err)
return ERR_PTR(err);
+ /*
+ * The otherwise-empty fwspec handily serves to indicate the specific
+ * IOMMU device we're waiting for, which will be useful if we ever get
+ * a proper probe-ordering dependency mechanism in future.
+ */
+ if (!ops)
+ return ERR_PTR(-EPROBE_DEFER);
err = ops->of_xlate(dev, iommu_spec);
if (err)
@@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
const struct iommu_ops *ops;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
if (!master_np)
return NULL;
+ if (fwspec) {
+ if (fwspec->ops)
+ return fwspec->ops;
+
+ /* In the deferred case, start again from scratch */
+ iommu_fwspec_free(dev);
+ }
+
if (dev_is_pci(dev))
ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
else
ops = of_platform_iommu_init(dev, master_np);
+ /*
+ * If we have reason to believe the IOMMU driver missed the initial
+ * add_device callback for dev, replay it to get things in order.
+ */
+ if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+ dev->bus && !dev->iommu_group) {
+ int err = ops->add_device(dev);
+
+ if (err)
+ ops = ERR_PTR(err);
+ }
return IS_ERR(ops) ? NULL : ops;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[Robin Murphy]

Post by Lorenzo Pieralisi
Sricharan, Robin,
I gave this series a go on ACPI and apart from an SMMU v3 fix-up
it seems to work, more thorough testing required though.
A key question below.

Post by Sricharan R
IOMMU configuration represents unchanging properties of the hardware,
and as such should only need happen once in a device's lifetime, but
the necessary interaction with the IOMMU device and driver complicates
exactly when that point should be.
Since the only reasonable tool available for handling the inter-device
dependency is probe deferral, we need to prepare of_iommu_configure()
to run later than it is currently called (i.e. at driver probe rather
than device creation), to handle being retried, and to tell whether a
not-yet present IOMMU should be waited for or skipped (by virtue of
having declared a built-in driver or not).
---
drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee49081..349bd1d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
int err;
ops = iommu_get_instance(fwnode);
- if (!ops || !ops->of_xlate)
+ if ((ops && !ops->of_xlate) ||
+ (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))

IIUC of_match_node() here is there to check there is a driver compiled
in for this device_node (aka compatible string in OF world), correct ?

Yes - specifically, it's checking the magic table for a matching
IOMMU_OF_DECLARE entry.

Post by Lorenzo Pieralisi
If that's the case (and I think that's what Sricharan was referring to
in his ACPI query) I need to cook-up something on the ACPI side to
emulate the OF linker table behaviour (or anyway to detect a driver is
actually in the kernel), it is not that difficult but it is key to know,
I will give it some thought to make it as clean as possible.

I didn't think this would be a concern for ACPI, since IORT works much
the same way the current of_iommu_init_fn/of_platform_device_create()
bodges in drivers so for DT. If you can only discover SMMUs from IORT,
then iort_init_platform_devices() will have already created every SMMU
that's going to exist before discovering other devices from wherever
they come from, thus you could never get into the situation of probing a
device without its SMMU being ready (if it's ever going to be). Is that
not right?

Robin.

Post by Lorenzo Pieralisi
Thanks,
Lorenzo

Post by Sricharan R
return NULL;
err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
if (err)
return ERR_PTR(err);
+ /*
+ * The otherwise-empty fwspec handily serves to indicate the specific
+ * IOMMU device we're waiting for, which will be useful if we ever get
+ * a proper probe-ordering dependency mechanism in future.
+ */
+ if (!ops)
+ return ERR_PTR(-EPROBE_DEFER);
err = ops->of_xlate(dev, iommu_spec);
if (err)
@@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
const struct iommu_ops *ops;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
if (!master_np)
return NULL;
+ if (fwspec) {
+ if (fwspec->ops)
+ return fwspec->ops;
+
+ /* In the deferred case, start again from scratch */
+ iommu_fwspec_free(dev);
+ }
+
if (dev_is_pci(dev))
ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
else
ops = of_platform_iommu_init(dev, master_np);
+ /*
+ * If we have reason to believe the IOMMU driver missed the initial
+ * add_device callback for dev, replay it to get things in order.
+ */
+ if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+ dev->bus && !dev->iommu_group) {
+ int err = ops->add_device(dev);
+
+ if (err)
+ ops = ERR_PTR(err);
+ }
return IS_ERR(ops) ? NULL : ops;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[Lorenzo Pieralisi]

Post by Robin Murphy

Post by Lorenzo Pieralisi
Sricharan, Robin,
I gave this series a go on ACPI and apart from an SMMU v3 fix-up
it seems to work, more thorough testing required though.
A key question below.

Post by Sricharan R
IOMMU configuration represents unchanging properties of the hardware,
and as such should only need happen once in a device's lifetime, but
the necessary interaction with the IOMMU device and driver complicates
exactly when that point should be.
Since the only reasonable tool available for handling the inter-device
dependency is probe deferral, we need to prepare of_iommu_configure()
to run later than it is currently called (i.e. at driver probe rather
than device creation), to handle being retried, and to tell whether a
not-yet present IOMMU should be waited for or skipped (by virtue of
having declared a built-in driver or not).
---
drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee49081..349bd1d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
int err;
ops = iommu_get_instance(fwnode);
- if (!ops || !ops->of_xlate)
+ if ((ops && !ops->of_xlate) ||
+ (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))

IIUC of_match_node() here is there to check there is a driver compiled
in for this device_node (aka compatible string in OF world), correct ?

Yes - specifically, it's checking the magic table for a matching
IOMMU_OF_DECLARE entry.

Post by Lorenzo Pieralisi
If that's the case (and I think that's what Sricharan was referring to
in his ACPI query) I need to cook-up something on the ACPI side to
emulate the OF linker table behaviour (or anyway to detect a driver is
actually in the kernel), it is not that difficult but it is key to know,
I will give it some thought to make it as clean as possible.

I didn't think this would be a concern for ACPI, since IORT works much
the same way the current of_iommu_init_fn/of_platform_device_create()
bodges in drivers so for DT. If you can only discover SMMUs from IORT,
then iort_init_platform_devices() will have already created every SMMU
that's going to exist before discovering other devices from wherever
they come from, thus you could never get into the situation of probing a
device without its SMMU being ready (if it's ever going to be). Is that
not right?

It is right, my point and question is: we are probing a device and we
have to know whether it is worth deferring its IOMMU DMA setup. On DT,
through of_match_node(&__iommu_of_table, iommu_device_node) we check at
once that:

1 - A device for the IOMMU exists

AND

2 - A driver for the IOMMU is compiled in the kernel

Is this correct ? As you said (1) is not a concern on ACPI IORT (because
we create the IOMMU device before _any_ other device so either the IOMMU
device is there or it will never be by the time master devices are
probed), but for (2) I need to slightly change how the IORT linker entry
work to make sure we can detect a driver is actually compiled in the
kernel, it is easy, I was just asking if my understanding was correct
and I think that was what Sricharan was referring to in his query.

I will put together a patch, again, it is a simple tweak.

Thanks !
Lorenzo

Post by Robin Murphy
Robin.

Post by Lorenzo Pieralisi
Thanks,
Lorenzo

Post by Sricharan R
return NULL;
err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops);
if (err)
return ERR_PTR(err);
+ /*
+ * The otherwise-empty fwspec handily serves to indicate the specific
+ * IOMMU device we're waiting for, which will be useful if we ever get
+ * a proper probe-ordering dependency mechanism in future.
+ */
+ if (!ops)
+ return ERR_PTR(-EPROBE_DEFER);
err = ops->of_xlate(dev, iommu_spec);
if (err)
@@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
struct device_node *master_np)
{
const struct iommu_ops *ops;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
if (!master_np)
return NULL;
+ if (fwspec) {
+ if (fwspec->ops)
+ return fwspec->ops;
+
+ /* In the deferred case, start again from scratch */
+ iommu_fwspec_free(dev);
+ }
+
if (dev_is_pci(dev))
ops = of_pci_iommu_init(to_pci_dev(dev), master_np);
else
ops = of_platform_iommu_init(dev, master_np);
+ /*
+ * If we have reason to believe the IOMMU driver missed the initial
+ * add_device callback for dev, replay it to get things in order.
+ */
+ if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+ dev->bus && !dev->iommu_group) {
+ int err = ops->add_device(dev);
+
+ if (err)
+ ops = ERR_PTR(err);
+ }
return IS_ERR(ops) ? NULL : ops;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[Sricharan]

Hi Robin,Lorenzo,

Post by Lorenzo Pieralisi

Post by Robin Murphy

Post by Lorenzo Pieralisi
Sricharan, Robin,
I gave this series a go on ACPI and apart from an SMMU v3 fix-up
it seems to work, more thorough testing required though.
A key question below.

Post by Sricharan R
IOMMU configuration represents unchanging properties of the hardware,
and as such should only need happen once in a device's lifetime, but
the necessary interaction with the IOMMU device and driver complicates
exactly when that point should be.
Since the only reasonable tool available for handling the inter-device
dependency is probe deferral, we need to prepare of_iommu_configure()
to run later than it is currently called (i.e. at driver probe rather
than device creation), to handle being retried, and to tell whether a
not-yet present IOMMU should be waited for or skipped (by virtue of
having declared a built-in driver or not).
---
drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index ee49081..349bd1d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
int err;
ops = iommu_get_instance(fwnode);
- if (!ops || !ops->of_xlate)
+ if ((ops && !ops->of_xlate) ||
+ (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np)))

IIUC of_match_node() here is there to check there is a driver compiled
in for this device_node (aka compatible string in OF world), correct ?

Yes - specifically, it's checking the magic table for a matching
IOMMU_OF_DECLARE entry.

Post by Lorenzo Pieralisi
If that's the case (and I think that's what Sricharan was referring to
in his ACPI query) I need to cook-up something on the ACPI side to
emulate the OF linker table behaviour (or anyway to detect a driver is
actually in the kernel), it is not that difficult but it is key to know,
I will give it some thought to make it as clean as possible.

I didn't think this would be a concern for ACPI, since IORT works much
the same way the current of_iommu_init_fn/of_platform_device_create()
bodges in drivers so for DT. If you can only discover SMMUs from IORT,
then iort_init_platform_devices() will have already created every SMMU
that's going to exist before discovering other devices from wherever
they come from, thus you could never get into the situation of probing a
device without its SMMU being ready (if it's ever going to be). Is that
not right?

It is right, my point and question is: we are probing a device and we
have to know whether it is worth deferring its IOMMU DMA setup. On DT,
through of_match_node(&__iommu_of_table, iommu_device_node) we check at
1 - A device for the IOMMU exists
AND
2 - A driver for the IOMMU is compiled in the kernel
Is this correct ? As you said (1) is not a concern on ACPI IORT (because
we create the IOMMU device before _any_ other device so either the IOMMU
device is there or it will never be by the time master devices are
probed), but for (2) I need to slightly change how the IORT linker entry
work to make sure we can detect a driver is actually compiled in the
kernel, it is easy, I was just asking if my understanding was correct
and I think that was what Sricharan was referring to in his query.

Yes right, this was what i was looking for in the ACPI case and putting this
in the iort_iommu_xlate was needed to return EPROBE_DEFER when the
driver is not yet been probed.

Regards,
Sricharan

[Sricharan R]

From: Laurent Pinchart <laurent.pinchart+***@ideasonboard.com>

Invalid dma-ranges values should be worked around when retrieving the
DMA range in of_dma_get_range(), not by all callers of the function.
This isn't much of a problem now that we have a single caller, but that
situation will change when moving DMA configuration to device probe
time.

Signed-off-by: Laurent Pinchart <laurent.pinchart+***@ideasonboard.com>
---
drivers/of/address.c | 20 ++++++++++++++++++--
drivers/of/device.c | 15 ---------------
2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903..6aeb816 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -819,8 +819,8 @@ void __iomem *of_io_request_and_map(struct device_node *np, int index,
* CPU addr (phys_addr_t) : pna cells
* size : nsize cells
*
- * It returns -ENODEV if "dma-ranges" property was not found
- * for this device in DT.
+ * Return 0 on success, -ENODEV if the "dma-ranges" property was not found for
+ * this device in DT, or -EINVAL if the CPU address or size is invalid.
*/
int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
{
@@ -880,6 +880,22 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
*dma_addr = dmaaddr;

*size = of_read_number(ranges + naddr + pna, nsize);
+ /*
+ * DT nodes sometimes incorrectly set the size as a mask. Work around
+ * those incorrect DT by computing the size as mask + 1.
+ */
+ if (*size & 1) {
+ pr_warn("%s: size 0x%llx for dma-range in node(%s) set as mask\n",
+ __func__, *size, np->full_name);
+ *size = *size + 1;
+ }
+
+ if (!*size) {
+ pr_err("%s: invalid size zero for dma-range in node(%s)\n",
+ __func__, np->full_name);
+ ret = -EINVAL;
+ goto out;
+ }

pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
*dma_addr, *paddr, *size);
diff --git a/drivers/of/device.c b/drivers/of/device.c
index fd5cfad..d9898d9 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -110,21 +110,6 @@ void of_dma_configure(struct device *dev, struct device_node *np)
size = dev->coherent_dma_mask + 1;
} else {
offset = PFN_DOWN(paddr - dma_addr);
-
- /*
- * Add a work around to treat the size as mask + 1 in case
- * it is defined in DT as a mask.
- */
- if (size & 1) {
- dev_warn(dev, "Invalid size 0x%llx for dma-range\n",
- size);
- size = size + 1;
- }
-
- if (!size) {
- dev_err(dev, "Adjusted size 0x%llx invalid\n", size);
- return;
- }
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
}

[Sricharan R]

From: Laurent Pinchart <laurent.pinchart+***@ideasonboard.com>

As part of moving DMA initializing to probe time the
of_dma_deconfigure() function will need to be called from different
source files. Make it public and move it to drivers/of/device.c where
the of_dma_configure() function is.

Signed-off-by: Laurent Pinchart <laurent.pinchart+***@ideasonboard.com>
---
drivers/of/device.c | 12 ++++++++++++
drivers/of/platform.c | 5 -----
include/linux/of_device.h | 3 +++
3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index d9898d9..1c843e2 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -136,6 +136,18 @@ void of_dma_configure(struct device *dev, struct device_node *np)
}
EXPORT_SYMBOL_GPL(of_dma_configure);

+/**
+ * of_dma_deconfigure - Clean up DMA configuration
+ * @dev: Device for which to clean up DMA configuration
+ *
+ * Clean up all configuration performed by of_dma_configure_ops() and free all
+ * resources that have been allocated.
+ */
+void of_dma_deconfigure(struct device *dev)
+{
+ arch_teardown_dma_ops(dev);
+}
+
int of_device_register(struct platform_device *pdev)
{
device_initialize(&pdev->dev);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index e4bf07d..fdb6f8e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -155,11 +155,6 @@ struct platform_device *of_device_alloc(struct device_node *np,
}
EXPORT_SYMBOL(of_device_alloc);

-static void of_dma_deconfigure(struct device *dev)
-{
- arch_teardown_dma_ops(dev);
-}
-
/**
* of_platform_device_create_pdata - Alloc, initialize and register an of_device
* @np: pointer to node to create device for
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index cc7dd687..d20a31a 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -56,6 +56,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
}

void of_dma_configure(struct device *dev, struct device_node *np);
+void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */

static inline int of_driver_match_device(struct device *dev,
@@ -100,6 +101,8 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
}
static inline void of_dma_configure(struct device *dev, struct device_node *np)
{}
+static inline void of_dma_deconfigure(struct device *dev)
+{}
#endif /* CONFIG_OF */

#endif /* _LINUX_OF_DEVICE_H */

[Sricharan R]

Configuring DMA ops at probe time will allow deferring device probe when
the IOMMU isn't available yet. The dma_configure for the device is now called
from the generic device_attach callback just before the bus/driver probe
is called. This way, configuring the DMA ops for the device would be called
at the same place for all bus_types, hence the deferred probing mechanism
should work for all buses as well.

pci_bus_add_devices (platform/amba)(_device_create/driver_register)
| |
pci_bus_add_device (device_add/driver_register)
| |
device_attach device_initial_probe
| |
__device_attach_driver __device_attach_driver
|
driver_probe_device
|
really_probe
|
dma_configure

Similarly on the device/driver_unregister path __device_release_driver is
called which inturn calls dma_deconfigure.

Signed-off-by: Sricharan R <***@codeaurora.org>
[rm: PCI hacks]
Signed-off-by: Robin Murphy <***@arm.com>
---
drivers/base/dd.c | 10 ++++++++++
drivers/base/dma-mapping.c | 20 ++++++++++++++++++++
drivers/of/platform.c | 5 +----
drivers/pci/probe.c | 15 +++++++--------
include/linux/dma-mapping.h | 3 +++
include/linux/pci.h | 5 +++++
6 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index d76cd97..6c0a885 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -19,6 +19,7 @@

#include <linux/device.h>
#include <linux/delay.h>
+#include <linux/dma-mapping.h>
#include <linux/module.h>
#include <linux/kthread.h>
#include <linux/wait.h>
@@ -351,6 +352,10 @@ static int really_probe(struct device *dev, struct device_driver *drv)
if (ret)
goto pinctrl_bind_failed;

+ ret = dma_configure(dev);
+ if (ret)
+ goto dma_failed;
+
if (driver_sysfs_add(dev)) {
printk(KERN_ERR "%s: driver_sysfs_add(%s) failed\n",
__func__, dev_name(dev));
@@ -412,6 +417,8 @@ static int really_probe(struct device *dev, struct device_driver *drv)
goto done;

probe_failed:
+ dma_deconfigure(dev);
+dma_failed:
if (dev->bus)
blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
@@ -796,6 +803,9 @@ static void __device_release_driver(struct device *dev)
dev->bus->remove(dev);
else if (drv->remove)
drv->remove(dev);
+
+ dma_deconfigure(dev);
+
devres_release_all(dev);
dev->driver = NULL;
dev_set_drvdata(dev, NULL);
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 8f8b68c..b2a5629 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -10,6 +10,7 @@
#include <linux/dma-mapping.h>
#include <linux/export.h>
#include <linux/gfp.h>
+#include <linux/of_device.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>

@@ -341,3 +342,22 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
vunmap(cpu_addr);
}
#endif
+
+/*
+ * Common configuration to enable DMA API use for a device
+ */
+#include <linux/pci.h>
+
+int dma_configure(struct device *dev)
+{
+ if (dev_is_pci(dev))
+ pci_dma_configure(dev);
+ else if (dev->of_node)
+ of_dma_configure(dev, dev->of_node);
+ return 0;
+}
+
+void dma_deconfigure(struct device *dev)
+{
+ of_dma_deconfigure(dev);
+}
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index fdb6f8e..b0a2f9e 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -22,6 +22,7 @@
#include <linux/slab.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
+#include <linux/of_iommu.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
@@ -183,11 +184,9 @@ static struct platform_device *of_platform_device_create_pdata(

dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
- of_dma_configure(&dev->dev, dev->dev.of_node);
of_msi_configure(&dev->dev, dev->dev.of_node);

if (of_device_add(dev) != 0) {
- of_dma_deconfigure(&dev->dev);
platform_device_put(dev);
goto err_clear_flag;
}
@@ -245,7 +244,6 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
dev_set_name(&dev->dev, "%s", bus_id);
else
of_device_make_bus_id(&dev->dev);
- of_dma_configure(&dev->dev, dev->dev.of_node);

/* Allow the HW Peripheral ID to be overridden */
prop = of_get_property(node, "arm,primecell-periphid", NULL);
@@ -539,7 +537,6 @@ static int of_platform_device_destroy(struct device *dev, void *data)
amba_device_unregister(to_amba_device(dev));
#endif

- of_dma_deconfigure(dev);
of_node_clear_flag(dev->of_node, OF_POPULATED);
of_node_clear_flag(dev->of_node, OF_POPULATED_BUS);
return 0;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c29e07a..04af770 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1719,26 +1719,26 @@ static void pci_set_msi_domain(struct pci_dev *dev)

/**
* pci_dma_configure - Setup DMA configuration
- * @dev: ptr to pci_dev struct of the PCI device
+ * @dev: ptr to device struct of the PCI device
*
* Function to update PCI devices's DMA configuration using the same
* info from the OF node or ACPI node of host bridge's parent (if any).
*/
-static void pci_dma_configure(struct pci_dev *dev)
+void pci_dma_configure(struct device *dev)
{
- struct device *bridge = pci_get_host_bridge_device(dev);
+ struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));

if (IS_ENABLED(CONFIG_OF) &&
- bridge->parent && bridge->parent->of_node) {
- of_dma_configure(&dev->dev, bridge->parent->of_node);
+ bridge->parent && bridge->parent->of_node) {
+ of_dma_configure(dev, bridge->parent->of_node);
} else if (has_acpi_companion(bridge)) {
struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
enum dev_dma_attr attr = acpi_get_dma_attr(adev);

if (attr == DEV_DMA_NOT_SUPPORTED)
- dev_warn(&dev->dev, "DMA not supported.\n");
+ dev_warn(dev, "DMA not supported.\n");
else
- acpi_dma_configure(&dev->dev, attr);
+ acpi_dma_configure(dev, attr);
}

pci_put_host_bridge_device(bridge);
@@ -1757,7 +1757,6 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
dev->dev.dma_mask = &dev->dma_mask;
dev->dev.dma_parms = &dev->dma_parms;
dev->dev.coherent_dma_mask = 0xffffffffull;
- pci_dma_configure(dev);

pci_set_dma_max_seg_size(dev, 65536);
pci_set_dma_seg_boundary(dev, 0xffffffff);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 08528af..6f3e6ca 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -702,6 +702,9 @@ void *dma_mark_declared_memory_occupied(struct device *dev,
}
#endif /* CONFIG_HAVE_GENERIC_DMA_COHERENT */

+int dma_configure(struct device *dev);
+void dma_deconfigure(struct device *dev);
+
/*
* Managed DMA API
*/
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0e49f70..d04f651 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -870,6 +870,8 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
#define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
#define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))

+void pci_dma_configure(struct device *dev);
+
/* Generic PCI functions exported to card drivers */

enum pci_lost_interrupt_reason {
@@ -1601,6 +1603,9 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
#define dev_is_pci(d) (false)
#define dev_is_pf(d) (false)
#define dev_num_vf(d) (0)
+
+static inline void pci_dma_configure(struct device *dev) { }
+
#endif /* CONFIG_PCI */

/* Include architecture-dependent settings and functions */

[Sricharan R]

From: Laurent Pinchart <laurent.pinchart+***@ideasonboard.com>

Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.

The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.

The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.

The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.

The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.

Signed-off-by: Laurent Pichart <laurent.pinchart+***@ideasonboard.com>
Signed-off-by: Sricharan R <***@codeaurora.org>
[rm: massive PCI hacks]
Signed-off-by: Robin Murphy <***@arm.com>
---
drivers/base/dma-mapping.c | 4 ++--
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 5 +++--
drivers/of/device.c | 9 +++++++--
drivers/pci/probe.c | 6 ++++--
include/linux/of_device.h | 9 ++++++---
include/linux/pci.h | 4 ++--
7 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index b2a5629..576fdfb 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
int dma_configure(struct device *dev)
{
if (dev_is_pci(dev))
- pci_dma_configure(dev);
+ return pci_dma_configure(dev);
else if (dev->of_node)
- of_dma_configure(dev, dev->of_node);
+ return of_dma_configure(dev, dev->of_node);
return 0;
}

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d2a7a46 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
base_pfn = max_t(unsigned long, 1, base >> order);
end_pfn = (base + size - 1) >> order;

+ dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain->geometry.aperture_end, *dev->dma_mask, dev->coherent_dma_mask);
/* Check the domain allows at least some access to the device... */
if (domain->geometry.force_aperture) {
if (base > domain->geometry.aperture_end ||
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 349bd1d..9529d6c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_iommu.h>
#include <linux/of_pci.h>
+#include <linux/pci.h>
#include <linux/slab.h>

static const struct of_device_id __iommu_of_table_sentinel
@@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
ops = ERR_PTR(err);
}

- return IS_ERR(ops) ? NULL : ops;
+ return ops;
}

static int __init of_iommu_init(void)
@@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
for_each_matching_node_and_match(np, matches, &match) {
const of_iommu_init_fn init_fn = match->data;

- if (init_fn(np))
+ if (init_fn && init_fn(np))
pr_err("Failed to initialise IOMMU %s\n",
of_node_full_name(np));
}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 1c843e2..d58595c 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+int of_dma_configure(struct device *dev, struct device_node *np)
{
u64 dma_addr, paddr, size;
int ret;
@@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask + 1;
+ size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
@@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
coherent ? " " : " not ");

iommu = of_iommu_configure(dev, np);
+ if (IS_ERR(iommu))
+ return PTR_ERR(iommu);
+
dev_dbg(dev, "device is%sbehind an iommu\n",
iommu ? " " : " not ");

arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(of_dma_configure);

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 04af770..6316cae 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev *dev)
* Function to update PCI devices's DMA configuration using the same
* info from the OF node or ACPI node of host bridge's parent (if any).
*/
-void pci_dma_configure(struct device *dev)
+int pci_dma_configure(struct device *dev)
{
struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));
+ int ret = 0;

if (IS_ENABLED(CONFIG_OF) &&
bridge->parent && bridge->parent->of_node) {
- of_dma_configure(dev, bridge->parent->of_node);
+ ret = of_dma_configure(dev, bridge->parent->of_node);
} else if (has_acpi_companion(bridge)) {
struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
enum dev_dma_attr attr = acpi_get_dma_attr(adev);
@@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
}

pci_put_host_bridge_device(bridge);
+ return ret;
}

void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index d20a31a..6dca65c 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
return of_node_get(cpu_dev->of_node);
}

-void of_dma_configure(struct device *dev, struct device_node *np);
+int of_dma_configure(struct device *dev, struct device_node *np);
void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */

@@ -99,8 +99,11 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
{
return NULL;
}
-static inline void of_dma_configure(struct device *dev, struct device_node *np)
-{}
+
+static inline int of_dma_configure(struct device *dev, struct device_node *np)
+{
+ return 0;
+}
static inline void of_dma_deconfigure(struct device *dev)
{}
#endif /* CONFIG_OF */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d04f651..989ca44 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
#define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
#define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))

-void pci_dma_configure(struct device *dev);
+int pci_dma_configure(struct device *dev);

/* Generic PCI functions exported to card drivers */

@@ -1604,7 +1604,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
#define dev_is_pf(d) (false)
#define dev_num_vf(d) (0)

-static inline void pci_dma_configure(struct device *dev) { }
+static inline int pci_dma_configure(struct device *dev) { return 0; }

#endif /* CONFIG_PCI */

[Marek Szyprowski]

Hi Sricharan and Robin,

Post by Sricharan R
Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.
The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.
The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.
The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.
[rm: massive PCI hacks]
---
drivers/base/dma-mapping.c | 4 ++--
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 5 +++--
drivers/of/device.c | 9 +++++++--
drivers/pci/probe.c | 6 ++++--
include/linux/of_device.h | 9 ++++++---
include/linux/pci.h | 4 ++--
7 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index b2a5629..576fdfb 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
int dma_configure(struct device *dev)
{
if (dev_is_pci(dev))
- pci_dma_configure(dev);
+ return pci_dma_configure(dev);
else if (dev->of_node)
- of_dma_configure(dev, dev->of_node);
+ return of_dma_configure(dev, dev->of_node);
return 0;
}
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d2a7a46 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
base_pfn = max_t(unsigned long, 1, base >> order);
end_pfn = (base + size - 1) >> order;
+ dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain->geometry.aperture_end,

This causes a NULL pointer dereference if caller passes NULL device pointer.
There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
Trivial to fix as it looks like a leftover from developement or
debugging stage.

Post by Sricharan R
*dev->dma_mask, dev->coherent_dma_mask);
/* Check the domain allows at least some access to the device... */
if (domain->geometry.force_aperture) {
if (base > domain->geometry.aperture_end ||
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 349bd1d..9529d6c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_iommu.h>
#include <linux/of_pci.h>
+#include <linux/pci.h>
#include <linux/slab.h>
static const struct of_device_id __iommu_of_table_sentinel
@@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
ops = ERR_PTR(err);
}
- return IS_ERR(ops) ? NULL : ops;
+ return ops;
}
static int __init of_iommu_init(void)
@@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
for_each_matching_node_and_match(np, matches, &match) {
const of_iommu_init_fn init_fn = match->data;
- if (init_fn(np))
+ if (init_fn && init_fn(np))
pr_err("Failed to initialise IOMMU %s\n",
of_node_full_name(np));
}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 1c843e2..d58595c 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+int of_dma_configure(struct device *dev, struct device_node *np)
{
u64 dma_addr, paddr, size;
int ret;
@@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask + 1;
+ size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
@@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
coherent ? " " : " not ");
iommu = of_iommu_configure(dev, np);
+ if (IS_ERR(iommu))
+ return PTR_ERR(iommu);
+
dev_dbg(dev, "device is%sbehind an iommu\n",
iommu ? " " : " not ");
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(of_dma_configure);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 04af770..6316cae 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev *dev)
* Function to update PCI devices's DMA configuration using the same
* info from the OF node or ACPI node of host bridge's parent (if any).
*/
-void pci_dma_configure(struct device *dev)
+int pci_dma_configure(struct device *dev)
{
struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));
+ int ret = 0;
if (IS_ENABLED(CONFIG_OF) &&
bridge->parent && bridge->parent->of_node) {
- of_dma_configure(dev, bridge->parent->of_node);
+ ret = of_dma_configure(dev, bridge->parent->of_node);
} else if (has_acpi_companion(bridge)) {
struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
enum dev_dma_attr attr = acpi_get_dma_attr(adev);
@@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
}
pci_put_host_bridge_device(bridge);
+ return ret;
}
void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index d20a31a..6dca65c 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
return of_node_get(cpu_dev->of_node);
}
-void of_dma_configure(struct device *dev, struct device_node *np);
+int of_dma_configure(struct device *dev, struct device_node *np);
void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */
@@ -99,8 +99,11 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
{
return NULL;
}
-static inline void of_dma_configure(struct device *dev, struct device_node *np)
-{}
+
+static inline int of_dma_configure(struct device *dev, struct device_node *np)
+{
+ return 0;
+}
static inline void of_dma_deconfigure(struct device *dev)
{}
#endif /* CONFIG_OF */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d04f651..989ca44 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
#define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
#define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
-void pci_dma_configure(struct device *dev);
+int pci_dma_configure(struct device *dev);
/* Generic PCI functions exported to card drivers */
@@ -1604,7 +1604,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
#define dev_is_pf(d) (false)
#define dev_num_vf(d) (0)
-static inline void pci_dma_configure(struct device *dev) { }
+static inline int pci_dma_configure(struct device *dev) { return 0; }
#endif /* CONFIG_PCI */

Best regards

[Sricharan]

Hi,

Post by Marek Szyprowski
Hi Sricharan and Robin,

Post by Sricharan R
Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.
The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.
The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.
The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.
[rm: massive PCI hacks]
---
drivers/base/dma-mapping.c | 4 ++--
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 5 +++--
drivers/of/device.c | 9 +++++++--
drivers/pci/probe.c | 6 ++++--
include/linux/of_device.h | 9 ++++++---
include/linux/pci.h | 4 ++--
7 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index b2a5629..576fdfb 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
int dma_configure(struct device *dev)
{
if (dev_is_pci(dev))
- pci_dma_configure(dev);
+ return pci_dma_configure(dev);
else if (dev->of_node)
- of_dma_configure(dev, dev->of_node);
+ return of_dma_configure(dev, dev->of_node);
return 0;
}
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d2a7a46 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
base_pfn = max_t(unsigned long, 1, base >> order);
end_pfn = (base + size - 1) >> order;
+ dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain-
geometry.aperture_end,

This causes a NULL pointer dereference if caller passes NULL device pointer.
There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
Trivial to fix as it looks like a leftover from developement or
debugging stage.

My bad, i should have removed it before posting. Noticed this, but missed to remove it.
Will do so.

Regards,
Sricharan

[Robin Murphy]

Post by Marek Szyprowski
Hi Sricharan and Robin,

Post by Sricharan R
Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.
The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.
The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.
The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.
Signed-off-by: Laurent Pichart
[rm: massive PCI hacks]
---
drivers/base/dma-mapping.c | 4 ++--
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 5 +++--
drivers/of/device.c | 9 +++++++--
drivers/pci/probe.c | 6 ++++--
include/linux/of_device.h | 9 ++++++---
include/linux/pci.h | 4 ++--
7 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index b2a5629..576fdfb 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t
size, unsigned long vm_flags)
int dma_configure(struct device *dev)
{
if (dev_is_pci(dev))
- pci_dma_configure(dev);
+ return pci_dma_configure(dev);
else if (dev->of_node)
- of_dma_configure(dev, dev->of_node);
+ return of_dma_configure(dev, dev->of_node);
return 0;
}
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d2a7a46 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
base_pfn = max_t(unsigned long, 1, base >> order);
end_pfn = (base + size - 1) >> order;
+ dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n",
base, size, domain->geometry.aperture_start,
domain->geometry.aperture_end,

This causes a NULL pointer dereference if caller passes NULL device pointer.
There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
Trivial to fix as it looks like a leftover from developement or
debugging stage.

Yes, this is some development crap which was never intended to go
upstream. Hence "massive PCI hacks" ;)

Other than the first two patches, the rest of the stuff from me here was
just an experiment which I'm not entirely convinced by the outcome of -
I don't particularly like the resulting fragmentation of having
pci_dma_configure() awkwardly floating around on its own in pci.c.

Robin.

Post by Marek Szyprowski

Post by Sricharan R
*dev->dma_mask, dev->coherent_dma_mask);
/* Check the domain allows at least some access to the device... */
if (domain->geometry.force_aperture) {
if (base > domain->geometry.aperture_end ||
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 349bd1d..9529d6c 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_iommu.h>
#include <linux/of_pci.h>
+#include <linux/pci.h>
#include <linux/slab.h>
static const struct of_device_id __iommu_of_table_sentinel
@@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
ops = ERR_PTR(err);
}
- return IS_ERR(ops) ? NULL : ops;
+ return ops;
}
static int __init of_iommu_init(void)
@@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
for_each_matching_node_and_match(np, matches, &match) {
const of_iommu_init_fn init_fn = match->data;
- if (init_fn(np))
+ if (init_fn && init_fn(np))
pr_err("Failed to initialise IOMMU %s\n",
of_node_full_name(np));
}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 1c843e2..d58595c 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
* can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
* to fix up DMA configuration.
*/
-void of_dma_configure(struct device *dev, struct device_node *np)
+int of_dma_configure(struct device *dev, struct device_node *np)
{
u64 dma_addr, paddr, size;
int ret;
@@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
if (ret < 0) {
dma_addr = offset = 0;
- size = dev->coherent_dma_mask + 1;
+ size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
} else {
offset = PFN_DOWN(paddr - dma_addr);
dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
@@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
coherent ? " " : " not ");
iommu = of_iommu_configure(dev, np);
+ if (IS_ERR(iommu))
+ return PTR_ERR(iommu);
+
dev_dbg(dev, "device is%sbehind an iommu\n",
iommu ? " " : " not ");
arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(of_dma_configure);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 04af770..6316cae 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev *dev)
* Function to update PCI devices's DMA configuration using the same
* info from the OF node or ACPI node of host bridge's parent (if any).
*/
-void pci_dma_configure(struct device *dev)
+int pci_dma_configure(struct device *dev)
{
struct device *bridge =
pci_get_host_bridge_device(to_pci_dev(dev));
+ int ret = 0;
if (IS_ENABLED(CONFIG_OF) &&
bridge->parent && bridge->parent->of_node) {
- of_dma_configure(dev, bridge->parent->of_node);
+ ret = of_dma_configure(dev, bridge->parent->of_node);
} else if (has_acpi_companion(bridge)) {
struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
enum dev_dma_attr attr = acpi_get_dma_attr(adev);
@@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
}
pci_put_host_bridge_device(bridge);
+ return ret;
}
void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index d20a31a..6dca65c 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -55,7 +55,7 @@ static inline struct device_node
*of_cpu_device_node_get(int cpu)
return of_node_get(cpu_dev->of_node);
}
-void of_dma_configure(struct device *dev, struct device_node *np);
+int of_dma_configure(struct device *dev, struct device_node *np);
void of_dma_deconfigure(struct device *dev);
#else /* CONFIG_OF */
@@ -99,8 +99,11 @@ static inline struct device_node
*of_cpu_device_node_get(int cpu)
{
return NULL;
}
-static inline void of_dma_configure(struct device *dev, struct device_node *np)
-{}
+
+static inline int of_dma_configure(struct device *dev, struct device_node *np)
+{
+ return 0;
+}
static inline void of_dma_deconfigure(struct device *dev)
{}
#endif /* CONFIG_OF */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d04f651..989ca44 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
#define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
#define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
-void pci_dma_configure(struct device *dev);
+int pci_dma_configure(struct device *dev);
/* Generic PCI functions exported to card drivers */
@@ -1604,7 +1604,7 @@ static inline struct pci_dev
*pci_get_bus_and_slot(unsigned int bus,
#define dev_is_pf(d) (false)
#define dev_num_vf(d) (0)
-static inline void pci_dma_configure(struct device *dev) { }
+static inline int pci_dma_configure(struct device *dev) { return 0; }
#endif /* CONFIG_PCI */

Best regards

[Sricharan]

Hi Robin,

Post by Robin Murphy

Post by Marek Szyprowski
Hi Sricharan and Robin,

Post by Sricharan R
Failures to look up an IOMMU when parsing the DT iommus property need to
be handled separately from the .of_xlate() failures to support deferred
probing.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.
The first case occurs when the device tree describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.
The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.
The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.
Signed-off-by: Laurent Pichart
[rm: massive PCI hacks]
---
drivers/base/dma-mapping.c | 4 ++--
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 5 +++--
drivers/of/device.c | 9 +++++++--
drivers/pci/probe.c | 6 ++++--
include/linux/of_device.h | 9 ++++++---
include/linux/pci.h | 4 ++--
7 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index b2a5629..576fdfb 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t
size, unsigned long vm_flags)
int dma_configure(struct device *dev)
{
if (dev_is_pci(dev))
- pci_dma_configure(dev);
+ return pci_dma_configure(dev);
else if (dev->of_node)
- of_dma_configure(dev, dev->of_node);
+ return of_dma_configure(dev, dev->of_node);
return 0;
}
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d2a7a46 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain
*domain, dma_addr_t base,
base_pfn = max_t(unsigned long, 1, base >> order);
end_pfn = (base + size - 1) >> order;
+ dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n",
base, size, domain->geometry.aperture_start,
domain->geometry.aperture_end,

This causes a NULL pointer dereference if caller passes NULL device pointer.
There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
Trivial to fix as it looks like a leftover from developement or
debugging stage.

Yes, this is some development crap which was never intended to go
upstream. Hence "massive PCI hacks" ;)
Other than the first two patches, the rest of the stuff from me here was
just an experiment which I'm not entirely convinced by the outcome of -
I don't particularly like the resulting fragmentation of having
pci_dma_configure() awkwardly floating around on its own in pci.c.

Ha, sorry looks like that i have misunderstood that then. I thought that
the V3 changes that i had for pci was hacky and the reworked patches
in your latest branch were correct. So do you suggest that the
pci_dma_configure gets removed and the pci related changes gets handled
in of_iommu_configure /acpi_dma_configure itself ?

Regards,
Sricharan

[Sricharan R]

With arch_setup_dma_ops now being called late during device's probe after the
device's iommu is probed, the notifier trick required to handle the early
setup of dma_ops before the iommu group gets created is not required.
So removing the notifier's here.

Signed-off-by: Sricharan R <***@codeaurora.org>
[rm: clean up even more]
Signed-off-by: Robin Murphy <***@arm.com>
---
arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
1 file changed, 12 insertions(+), 120 deletions(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index aa6c8f8..401f79a 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -800,140 +800,32 @@ static void __iommu_unmap_sg_attrs(struct device *dev,
.mapping_error = iommu_dma_mapping_error,
};

-/*
- * TODO: Right now __iommu_setup_dma_ops() gets called too early to do
- * everything it needs to - the device is only partially created and the
- * IOMMU driver hasn't seen it yet, so it can't have a group. Thus we
- * need this delayed attachment dance. Once IOMMU probe ordering is sorted
- * to move the arch_setup_dma_ops() call later, all the notifier bits below
- * become unnecessary, and will go away.
- */
-struct iommu_dma_notifier_data {
- struct list_head list;
- struct device *dev;
- const struct iommu_ops *ops;
- u64 dma_base;
- u64 size;
-};
-static LIST_HEAD(iommu_dma_masters);
-static DEFINE_MUTEX(iommu_dma_notifier_lock);
-
-static bool do_iommu_attach(struct device *dev, const struct iommu_ops *ops,
- u64 dma_base, u64 size)
-{
- struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
- /*
- * If the IOMMU driver has the DMA domain support that we require,
- * then the IOMMU core will have already configured a group for this
- * device, and allocated the default domain for that group.
- */
- if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
- pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
- dev_name(dev));
- return false;
- }
-
- dev->archdata.dma_ops = &iommu_dma_ops;
- return true;
-}
-
-static void queue_iommu_attach(struct device *dev, const struct iommu_ops *ops,
- u64 dma_base, u64 size)
-{
- struct iommu_dma_notifier_data *iommudata;
-
- iommudata = kzalloc(sizeof(*iommudata), GFP_KERNEL);
- if (!iommudata)
- return;
-
- iommudata->dev = dev;
- iommudata->ops = ops;
- iommudata->dma_base = dma_base;
- iommudata->size = size;
-
- mutex_lock(&iommu_dma_notifier_lock);
- list_add(&iommudata->list, &iommu_dma_masters);
- mutex_unlock(&iommu_dma_notifier_lock);
-}
-
-static int __iommu_attach_notifier(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct iommu_dma_notifier_data *master, *tmp;
-
- if (action != BUS_NOTIFY_BIND_DRIVER)
- return 0;
-
- mutex_lock(&iommu_dma_notifier_lock);
- list_for_each_entry_safe(master, tmp, &iommu_dma_masters, list) {
- if (data == master->dev && do_iommu_attach(master->dev,
- master->ops, master->dma_base, master->size)) {
- list_del(&master->list);
- kfree(master);
- break;
- }
- }
- mutex_unlock(&iommu_dma_notifier_lock);
- return 0;
-}
-
-static int __init register_iommu_dma_ops_notifier(struct bus_type *bus)
-{
- struct notifier_block *nb = kzalloc(sizeof(*nb), GFP_KERNEL);
- int ret;
-
- if (!nb)
- return -ENOMEM;
-
- nb->notifier_call = __iommu_attach_notifier;
-
- ret = bus_register_notifier(bus, nb);
- if (ret) {
- pr_warn("Failed to register DMA domain notifier; IOMMU DMA ops unavailable on bus '%s'\n",
- bus->name);
- kfree(nb);
- }
- return ret;
-}
-
static int __init __iommu_dma_init(void)
{
- int ret;
-
- ret = iommu_dma_init();
- if (!ret)
- ret = register_iommu_dma_ops_notifier(&platform_bus_type);
- if (!ret)
- ret = register_iommu_dma_ops_notifier(&amba_bustype);
-#ifdef CONFIG_PCI
- if (!ret)
- ret = register_iommu_dma_ops_notifier(&pci_bus_type);
-#endif
- return ret;
+ return iommu_dma_init();
}
arch_initcall(__iommu_dma_init);

static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
const struct iommu_ops *ops)
{
- struct iommu_group *group;
+ struct iommu_domain *domain;

if (!ops)
return;
+
/*
- * TODO: As a concession to the future, we're ready to handle being
- * called both early and late (i.e. after bus_add_device). Once all
- * the platform bus code is reworked to call us late and the notifier
- * junk above goes away, move the body of do_iommu_attach here.
+ * The IOMMU core code allocates the default DMA domain, which the
+ * underlying IOMMU driver needs to support via the dma-iommu layer.
*/
- group = iommu_group_get(dev);
- if (group) {
- do_iommu_attach(dev, ops, dma_base, size);
- iommu_group_put(group);
- } else {
- queue_iommu_attach(dev, ops, dma_base, size);
+ domain = iommu_get_domain_for_dev(dev);
+ if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
+ pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
+ dev_name(dev));
+ return;
}
+
+ dev->archdata.dma_ops = &iommu_dma_ops;
}

void arch_teardown_dma_ops(struct device *dev)

[Sricharan R]

From: Robin Murphy <***@arm.com>

Now that the appropriate ordering is enforced via profe-deferral of
masters in core code, rip it all out and bask in the simplicity.

Signed-off-by: Robin Murphy <***@arm.com>
[sricharan: Rebased on top of ACPI IORT SMMU series]
Signed-off-by: Sricharan R <***@codeaurora.org>
---
[v4] Removed arm_smmu_acpi_init function and let the
module_platform_driver to take care of the driver
registration.

drivers/iommu/arm-smmu-v3.c | 35 ++--------------------------
drivers/iommu/arm-smmu.c | 56 ++++++++-------------------------------------
2 files changed, 11 insertions(+), 80 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d22c428..257a6a3 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2729,40 +2729,9 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
.probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
};
+module_platform_driver(arm_smmu_driver);

-static int __init arm_smmu_init(void)
-{
- static bool registered;
- int ret = 0;
-
- if (!registered) {
- ret = platform_driver_register(&arm_smmu_driver);
- registered = !ret;
- }
- return ret;
-}
-
-static void __exit arm_smmu_exit(void)
-{
- return platform_driver_unregister(&arm_smmu_driver);
-}
-
-subsys_initcall(arm_smmu_init);
-module_exit(arm_smmu_exit);
-
-static int __init arm_smmu_of_init(struct device_node *np)
-{
- int ret = arm_smmu_init();
-
- if (ret)
- return ret;
-
- if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root))
- return -ENODEV;
-
- return 0;
-}
-IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", arm_smmu_of_init);
+IOMMU_OF_DECLARE(arm_smmuv3, "arm,smmu-v3", NULL);

#ifdef CONFIG_ACPI
static int __init acpi_smmu_v3_init(struct acpi_table_header *table)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8e66b16..eaa8f44 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2127,57 +2127,19 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
.probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
};
-
-static int __init arm_smmu_init(void)
-{
- static bool registered;
- int ret = 0;
-
- if (!registered) {
- ret = platform_driver_register(&arm_smmu_driver);
- registered = !ret;
- }
- return ret;
-}
-
-static void __exit arm_smmu_exit(void)
-{
- return platform_driver_unregister(&arm_smmu_driver);
-}
-
-subsys_initcall(arm_smmu_init);
-module_exit(arm_smmu_exit);
-
-static int __init arm_smmu_of_init(struct device_node *np)
-{
- int ret = arm_smmu_init();
-
- if (ret)
- return ret;
-
- if (!of_platform_device_create(np, NULL, platform_bus_type.dev_root))
- return -ENODEV;
-
- return 0;
-}
-IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", arm_smmu_of_init);
-IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", arm_smmu_of_init);
-IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", arm_smmu_of_init);
+module_platform_driver(arm_smmu_driver);

#ifdef CONFIG_ACPI
-static int __init arm_smmu_acpi_init(struct acpi_table_header *table)
-{
- if (iort_node_match(ACPI_IORT_NODE_SMMU))
- return arm_smmu_init();
-
- return 0;
-}
-IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, arm_smmu_acpi_init);
+IORT_ACPI_DECLARE(arm_smmu, ACPI_SIG_IORT, NULL);
#endif

+IOMMU_OF_DECLARE(arm_smmuv1, "arm,smmu-v1", NULL);
+IOMMU_OF_DECLARE(arm_smmuv2, "arm,smmu-v2", NULL);
+IOMMU_OF_DECLARE(arm_mmu400, "arm,mmu-400", NULL);
+IOMMU_OF_DECLARE(arm_mmu401, "arm,mmu-401", NULL);
+IOMMU_OF_DECLARE(arm_mmu500, "arm,mmu-500", NULL);
+IOMMU_OF_DECLARE(cavium_smmuv2, "cavium,smmu-v2", NULL);
+
MODULE_DESCRIPTION("IOMMU API for ARM architected SMMU implementations");
MODULE_AUTHOR("Will Deacon <***@arm.com>");
MODULE_LICENSE("GPL v2");

[Sricharan R]

With all the DT based devices getting their dma ops configured
during probe time to have the right iommu setup, let us do the
same for acpi based devices as well.

Configuring DMA ops at probe time will allow deferring device probe when
the IOMMU isn't available yet. The dma_configure for the device is now called
from the generic device_attach callback just before the bus/driver probe
is called. This way, configuring the DMA ops for the device would be called
at the same place for all bus_types, hence the deferred probing mechanism
should work for all buses as well.

Signed-off-by: Sricharan R <***@codeaurora.org>
---
drivers/acpi/glue.c | 6 ------
drivers/base/dma-mapping.c | 12 ++++++++++++
2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index f8d6564..458445f 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -168,7 +168,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
struct list_head *physnode_list;
unsigned int node_id;
int retval = -EINVAL;
- enum dev_dma_attr attr;

if (has_acpi_companion(dev)) {
if (acpi_dev) {
@@ -225,10 +224,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
if (!has_acpi_companion(dev))
ACPI_COMPANION_SET(dev, acpi_dev);

- attr = acpi_get_dma_attr(acpi_dev);
- if (attr != DEV_DMA_NOT_SUPPORTED)
- acpi_dma_configure(dev, attr);
-
acpi_physnode_link_name(physical_node_name, node_id);
retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
physical_node_name);
@@ -250,7 +245,6 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
return 0;

err:
- acpi_dma_deconfigure(dev);
ACPI_COMPANION_SET(dev, NULL);
put_device(dev);
put_device(&acpi_dev->dev);
diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 576fdfb..2ec4dae 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -347,17 +347,29 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
* Common configuration to enable DMA API use for a device
*/
#include <linux/pci.h>
+#include <linux/acpi.h>

int dma_configure(struct device *dev)
{
+ struct acpi_device *adev;
+ enum dev_dma_attr attr;
+
if (dev_is_pci(dev))
return pci_dma_configure(dev);
else if (dev->of_node)
return of_dma_configure(dev, dev->of_node);
+ else if (has_acpi_companion(dev)) {
+ adev = to_acpi_device_node(dev->fwnode);
+ attr = acpi_get_dma_attr(adev);
+ if (attr != DEV_DMA_NOT_SUPPORTED)
+ return acpi_dma_configure(dev, attr);
+ }
+
return 0;
}

void dma_deconfigure(struct device *dev)
{
of_dma_deconfigure(dev);
+ acpi_dma_deconfigure(dev);
}

[Sricharan R]

This is an equivalent to the DT's handling of the iommu master's probe
with deferred probing when the corrsponding iommu is not probed yet.
The lack of a registered IOMMU can be caused by the lack of a driver for
the IOMMU, the IOMMU device probe not having been performed yet, having
been deferred, or having failed.

The first case occurs when the firmware describes the bus master and
IOMMU topology correctly but no device driver exists for the IOMMU yet
or the device driver has not been compiled in. Return NULL, the caller
will configure the device without an IOMMU.

The second and third cases are handled by deferring the probe of the bus
master device which will eventually get reprobed after the IOMMU.

The last case is currently handled by deferring the probe of the bus
master device as well. A mechanism to either configure the bus master
device without an IOMMU or to fail the bus master device probe depending
on whether the IOMMU is optional or mandatory would be a good
enhancement.

Signed-off-by: Sricharan R <***@codeaurora.org>
---
drivers/acpi/arm64/iort.c | 17 ++++++++++++++++-
drivers/acpi/scan.c | 7 +++++--
drivers/pci/probe.c | 2 +-
include/acpi/acpi_bus.h | 2 +-
include/linux/acpi.h | 7 +++++--
5 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 47bace8..18ad4d9 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -537,8 +537,9 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev,
return NULL;

ops = iommu_get_instance(iort_fwnode);
+ /* This means that the iommu driver is still not probed */
if (!ops)
- return NULL;
+ return ERR_PTR(-EPROBE_DEFER);

ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops);
}
@@ -590,12 +591,26 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev)

while (parent) {
ops = iort_iommu_xlate(dev, parent, streamid);
+ if (IS_ERR_OR_NULL(ops))
+ return ops;

parent = iort_node_get_id(node, &streamid,
IORT_IOMMU_TYPE, i++);
}
}

+ /*
+ * If we have reason to believe the IOMMU driver missed the initial
+ * add_device callback for dev, replay it to get things in order.
+ */
+ if (!IS_ERR_OR_NULL(ops) && ops->add_device &&
+ dev->bus && !dev->iommu_group) {
+ int err = ops->add_device(dev);
+
+ if (err)
+ ops = ERR_PTR(err);
+ }
+
return ops;
}

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 80698d3..c80e51e 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1376,7 +1376,7 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
* @dev: The pointer to the device
* @attr: device dma attributes
*/
-void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
+int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
{
const struct iommu_ops *iommu;

@@ -1395,13 +1395,16 @@ void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
dev->dma_mask = &dev->coherent_dma_mask;

iommu = iort_iommu_configure(dev);
-
+ if (IS_ERR(iommu))
+ return PTR_ERR(iommu);
/*
* Assume dma valid range starts at 0 and covers the whole
* coherent_dma_mask.
*/
arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
attr == DEV_DMA_COHERENT);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(acpi_dma_configure);

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6316cae..e32c7e5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1739,7 +1739,7 @@ int pci_dma_configure(struct device *dev)
if (attr == DEV_DMA_NOT_SUPPORTED)
dev_warn(dev, "DMA not supported.\n");
else
- acpi_dma_configure(dev, attr);
+ ret = acpi_dma_configure(dev, attr);
}

pci_put_host_bridge_device(bridge);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 4242c31..9aa762f 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -573,7 +573,7 @@ struct acpi_pci_root {

bool acpi_dma_supported(struct acpi_device *adev);
enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
-void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
+int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
void acpi_dma_deconfigure(struct device *dev);

struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 8d15fc5..0e8d33f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -765,8 +765,11 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
return DEV_DMA_NOT_SUPPORTED;
}

-static inline void acpi_dma_configure(struct device *dev,
- enum dev_dma_attr attr) { }
+static inline int acpi_dma_configure(struct device *dev,
+ enum dev_dma_attr attr)
+{
+ return 0;
+}

static inline void acpi_dma_deconfigure(struct device *dev) { }

[Marek Szyprowski]

Hi All,

Post by Sricharan R
This series calls the dma ops configuration for the devices
at a generic place so that it works for all busses.
The dma_configure_ops for a device is now called during
the device_attach callback just before the probe of the
bus/driver is called. Similarly dma_deconfigure is called during
device/driver_detach path.
pci_bus_add_devices (platform/amba)(_device_create/driver_register)
| |
pci_bus_add_device (device_add/driver_register)
| |
device_attach device_initial_probe
| |
__device_attach_driver __device_attach_driver
|
driver_probe_device
|
really_probe
|
dma_configure
Similarly on the device/driver_unregister path __device_release_driver is
called which inturn calls dma_deconfigure.
Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].
Tested with platform and pci devices for probe deferral
and reprobe on arm64 based platform. Added the patches [9,10],
drivers: acpi: Configure acpi devices dma operation at probe time
drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
for doing the dma ops configuration at probe time for acpi based platform
as well. Did not have a acpi based platform to test the changes and with
my still catching up knowledge on acpi/enumeration those two patches needs
to be reviewed/tested more.

I've checked this patchset on my development boards: Odroid U3
(ARM/Exynos4412),
XU3 (ARM/Exynos5422) and TM2 (ARM64/Exynos5433) and my queued patches
for Exynos
IOMMU driver [1]. Besides the issue with an excessive dev_info, which
causes NULL
pointer dereference (reported in reply to patch #6), it works fine on
all those
platforms! Please add (again) my:

Tested-by: Marek Szyprowski <***@samsung.com>

[1] https://www.spinics.net/lists/linux-samsung-soc/msg56354.html

Post by Sricharan R
Previous post of this series [5].
[V4]
* Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].
* Added the patches for moving the dma ops configuration of
acpi based devices to probe time as well.
[V3]
* Removed the patch to split dma_masks/dma_ops configuration
separately based on review comments that both masks and ops are
required only during the device probe time.
* Reworked the series based on Generic DT bindings series.
* Added call to iommu's remove_device in the cleanup path for arm and
arm64.
* Removed the notifier trick in arm64 to handle early device
registration.
* Added reset of dma_ops in cleanup path for arm based on comments.
* Fixed the pci_iommu_configure path and tested with PCI device as
well.
* Fixed a bug to return the correct iommu_ops from patch 7 [4] in
last post.
* Fixed few other cosmetic comments.
[V2]
* Updated the Initial post to call dma_configure/deconfigure from
generic code
* Added iommu add_device callback from of_iommu_configure path
[V1]
* Initial post from Laurent Pinchart [1]
[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
[2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
[3] https://lkml.org/lkml/2016/11/21/141
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460832.html
of: dma: Move range size workaround to of_dma_get_range()
of: dma: Make of_dma_deconfigure() public
iommu: of: Handle IOMMU lookup failure with deferred probing or error
iommu/of: Refactor of_iommu_configure() for error handling
iommu/of: Prepare for deferred IOMMU configuration
iommu/arm-smmu: Clean up early-probing workarounds
drivers: platform: Configure dma operations at probe time
arm64: dma-mapping: Remove the notifier trick to handle early setting
of dma_ops
drivers: acpi: Configure acpi devices dma operation at probe time
drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
drivers/acpi/arm64/iort.c | 17 +++++-
drivers/acpi/glue.c | 6 --
drivers/acpi/scan.c | 7 ++-
drivers/base/dd.c | 10 ++++
drivers/base/dma-mapping.c | 32 +++++++++++
drivers/iommu/arm-smmu-v3.c | 35 +-----------
drivers/iommu/arm-smmu.c | 58 +++----------------
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 114 +++++++++++++++++++++++++++-----------
drivers/of/address.c | 20 ++++++-
drivers/of/device.c | 36 ++++++------
drivers/of/platform.c | 10 +---
drivers/pci/probe.c | 17 +++---
include/acpi/acpi_bus.h | 2 +-
include/linux/acpi.h | 7 ++-
include/linux/dma-mapping.h | 3 +
include/linux/of_device.h | 10 +++-
include/linux/pci.h | 5 ++
19 files changed, 238 insertions(+), 284 deletions(-)

Best regards

[Sricharan]

Hi Marek,

Post by Marek Szyprowski

Post by Sricharan R
This series calls the dma ops configuration for the devices
at a generic place so that it works for all busses.
The dma_configure_ops for a device is now called during
the device_attach callback just before the probe of the
bus/driver is called. Similarly dma_deconfigure is called during
device/driver_detach path.
pci_bus_add_devices (platform/amba)(_device_create/driver_register)
| |
pci_bus_add_device (device_add/driver_register)
| |
device_attach device_initial_probe
| |
__device_attach_driver __device_attach_driver
|
driver_probe_device
|
really_probe
|
dma_configure
Similarly on the device/driver_unregister path __device_release_driver is
called which inturn calls dma_deconfigure.
Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].
Tested with platform and pci devices for probe deferral
and reprobe on arm64 based platform. Added the patches [9,10],
drivers: acpi: Configure acpi devices dma operation at probe time
drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
for doing the dma ops configuration at probe time for acpi based platform
as well. Did not have a acpi based platform to test the changes and with
my still catching up knowledge on acpi/enumeration those two patches needs
to be reviewed/tested more.

I've checked this patchset on my development boards: Odroid U3
(ARM/Exynos4412),
XU3 (ARM/Exynos5422) and TM2 (ARM64/Exynos5433) and my queued patches
for Exynos
IOMMU driver [1]. Besides the issue with an excessive dev_info, which
causes NULL
pointer dereference (reported in reply to patch #6), it works fine on
all those
[1] https://www.spinics.net/lists/linux-samsung-soc/msg56354.html

Post by Sricharan R
Previous post of this series [5].

Thanks for the testing every time.

Regards,
Sricharan

Post by Marek Szyprowski

Post by Sricharan R
[V4]
* Took the reworked patches [2] from Robin's branch and
rebased on top of Lorenzo's ACPI IORT ARM support series [3].
* Added the patches for moving the dma ops configuration of
acpi based devices to probe time as well.
[V3]
* Removed the patch to split dma_masks/dma_ops configuration
separately based on review comments that both masks and ops are
required only during the device probe time.
* Reworked the series based on Generic DT bindings series.
* Added call to iommu's remove_device in the cleanup path for arm and
arm64.
* Removed the notifier trick in arm64 to handle early device
registration.
* Added reset of dma_ops in cleanup path for arm based on comments.
* Fixed the pci_iommu_configure path and tested with PCI device as
well.
* Fixed a bug to return the correct iommu_ops from patch 7 [4] in
last post.
* Fixed few other cosmetic comments.
[V2]
* Updated the Initial post to call dma_configure/deconfigure from
generic code
* Added iommu add_device callback from of_iommu_configure path
[V1]
* Initial post from Laurent Pinchart [1]
[1] http://lists.linuxfoundation.org/pipermail/iommu/2015-May/013016.html
[2] http://www.linux-arm.org/git?p=linux-rm.git;a=shortlog;h=refs/heads/iommu/defer
[3] https://lkml.org/lkml/2016/11/21/141
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/460832.html
of: dma: Move range size workaround to of_dma_get_range()
of: dma: Make of_dma_deconfigure() public
iommu: of: Handle IOMMU lookup failure with deferred probing or error
iommu/of: Refactor of_iommu_configure() for error handling
iommu/of: Prepare for deferred IOMMU configuration
iommu/arm-smmu: Clean up early-probing workarounds
drivers: platform: Configure dma operations at probe time
arm64: dma-mapping: Remove the notifier trick to handle early setting
of dma_ops
drivers: acpi: Configure acpi devices dma operation at probe time
drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
arch/arm64/mm/dma-mapping.c | 132 ++++----------------------------------------
drivers/acpi/arm64/iort.c | 17 +++++-
drivers/acpi/glue.c | 6 --
drivers/acpi/scan.c | 7 ++-
drivers/base/dd.c | 10 ++++
drivers/base/dma-mapping.c | 32 +++++++++++
drivers/iommu/arm-smmu-v3.c | 35 +-----------
drivers/iommu/arm-smmu.c | 58 +++----------------
drivers/iommu/dma-iommu.c | 1 +
drivers/iommu/of_iommu.c | 114 +++++++++++++++++++++++++++-----------
drivers/of/address.c | 20 ++++++-
drivers/of/device.c | 36 ++++++------
drivers/of/platform.c | 10 +---
drivers/pci/probe.c | 17 +++---
include/acpi/acpi_bus.h | 2 +-
include/linux/acpi.h | 7 ++-
include/linux/dma-mapping.h | 3 +
include/linux/of_device.h | 10 +++-
include/linux/pci.h | 5 ++
19 files changed, 238 insertions(+), 284 deletions(-)

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
linux-arm-kernel mailing list
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel