RE: [PATCH V3 0/8] IOMMU probe deferral support

Sricharan
2016-10-12T06:24:57Z

Hi Marek,

>Hi Sricharan,
>
>
>On 2016-10-04 19:03, Sricharan R wrote:
>> Initial post from Laurent Pinchart[1]. This is
>> 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.
>>
>> If the ACPI bus code follows the same, we can add acpi_dma_configure
>> at the same place as of_dma_configure.
>>
>> This series is based on the recently merged Generic DT bindings for
>> PCI IOMMUs and ARM SMMU from Robin Murphy robin.murphy-5wv7dgnIgG8@public.gmane.org [2]
>>
>> This time tested this with platform and pci device for probe deferral
>> and reprobe on arm64 based platform. There is an issue on the cleanup
>> path for arm64 though, where there is WARN_ON if the dma_ops is reset while
>> device is attached to an domain in arch_teardown_dma_ops.
>> But with iommu_groups created from the iommu driver, the device is always
>> attached to a domain/default_domain. So so the WARN has to be removed/handled
>> probably.
>
>Thanks for continuing work on this feature! Your can add my:
>
>Tested-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
Thanks for testing this. So the for the below fix, the remove_device callback
gets called on the dma_ops cleanup path, so would it be easy to remove the
data for the device there ?

Regards,
Sricharan

>It works fine with Exynos SYSMMU driver, although a patch is needed to fix
>infinite loop due to list corruption (same element is added twice if master
>device fails with deferred probe):
>
>From: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>Date: Mon, 10 Oct 2016 14:22:42 +0200
>Subject: [PATCH] iommu/exynos: ensure that sysmmu is added only once to its
> master
>
>Since adding IOMMU deferred probing support, of_xlate() callback might
>be called more than once for given master device (for example it happens
>when masters device driver fails with EPROBE_DEFER), so ensure that
>SYSMMU controller is added to its master device (owner) only once.
>
>Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>---
> drivers/iommu/exynos-iommu.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>index 30808e91b775..1525a86eb829 100644
>--- a/drivers/iommu/exynos-iommu.c
>+++ b/drivers/iommu/exynos-iommu.c
>@@ -1253,7 +1253,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
> {
> struct exynos_iommu_owner *owner = dev->archdata.iommu;
> struct platform_device *sysmmu = of_find_device_by_node(spec->np);
>- struct sysmmu_drvdata *data;
>+ struct sysmmu_drvdata *data, *entry;
>
> if (!sysmmu)
> return -ENODEV;
>@@ -1271,6 +1271,10 @@ static int exynos_iommu_of_xlate(struct device *dev,
> dev->archdata.iommu = owner;
> }
>
>+ list_for_each_entry(entry, &owner->controllers, owner_node)
>+ if (entry == data)
>+ return 0;
>+
> list_add_tail(&data->owner_node, &owner->controllers);
> return 0;
> }
>--
>1.9.1
>