Hi Robin,
On 2016-06-17 11:27, Robin Murphy wrote:
Hi Lorenzo,
I think this patch makes sense even independent of the rest of the
series, one nit inline notwithstanding.
Marek; I'm curious as to whether this could make the workaround in
722ec35f7 obsolete as well, or are all the drivers also bound
super-early in the setup you had there?
Yes, this will solve that problem too. I will also hide some possible
deferred probe issues, because the moment at which IOMMU is activated
will be postponed. The only drawback with this approach is the fact
that is drivers won't be allowed to do any dma-mapping operations on
devices, which they don't own. This should not be a big issue, but
this was the reason to setup IOMMU on device add instead of driver
bind.
While at it, please make sure that the case of failed client driver
probe will be handled properly. IOMMU might do some operations while
setting up and if the client driver fails to probe (for whatever
reason, might be a deferred probe too), those operation has to be
undone. However the current code of the driver core won't call any
notifier (like BUS_NOTIFY_UNBOUND_DRIVER or whatever else) in such
case.
Long time ago I used BUS_NOTIFY_BIND_DRIVER based approach for my
Exynos IOMMU patches and had to extend bus core with such patch:
https://patchwork.kernel.org/patch/4678181/ to properly cleanup
after failed client driver probe and avoid leaking resources. Please
read the discussion, because some changes were requested to it.
Best regards
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On 07/06/16 14:30, Lorenzo Pieralisi wrote:
Current bus notifier in ARM64 (__iommu_attach_notifier)
attempts to attach dma_ops to a device on BUS_NOTIFY_ADD_DEVICE
action notification.
This causes issues on ACPI based systems, where PCI devices
can be added before the IOMMUs the devices are attached to
had a chance to be probed, causing failures on attempts to
attach dma_ops in that the domain for the respective IOMMU
may not be set-up yet by the time the bus notifier is run.
Devices dma_ops do not require to be set-up till the matching
device drivers are probed. This means that instead of running
the notifier attaching dma_ops to devices (__iommu_attach_notifier)
on BUS_NOTIFY_ADD_DEVICE action, it can be run just before the
device driver is bound to the device in question (on action
BUS_NOTIFY_BIND_DRIVER) so that it is certain that its IOMMU
group and domain are set-up accordingly at the time the
notifier is triggered.
This patch changes the notifier action upon which dma_ops
are attached to devices and defer it to driver binding time,
so that IOMMU devices have a chance to be probed and to register
their bus notifiers before the dma_ops attach sequence for a
device is actually carried out.
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
Cc: Will Deacon <will.deacon@xxxxxxx>
Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: Robin Murphy <robin.murphy@xxxxxxx>
---
arch/arm64/mm/dma-mapping.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c566ec8..79b0882 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -848,7 +848,7 @@ static int __iommu_attach_notifier(struct
notifier_block *nb,
{
struct iommu_dma_notifier_data *master, *tmp;
- if (action != BUS_NOTIFY_ADD_DEVICE)
+ if (action != BUS_NOTIFY_BIND_DRIVER)
With this, you can also get rid of the priority setting and big fat
explanatory comment in register_iommu_dma_ops_notifier().
Robin.
return 0;
mutex_lock(&iommu_dma_notifier_lock);
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
