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

["Sricharan" <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

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
>