From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sricharan" Subject: RE: [PATCH V3 0/8] IOMMU probe deferral support Date: Wed, 12 Oct 2016 11:54:57 +0530 Message-ID: <000601d22451$5eb46fc0$1c1d4f40$@codeaurora.org> References: <1475600632-21289-1-git-send-email-sricharan@codeaurora.org> <12cfb59f-f7ca-d4df-eb7f-42348e357979@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <12cfb59f-f7ca-d4df-eb7f-42348e357979-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> Content-Language: en-us List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: 'Marek Szyprowski' , will.deacon-5wv7dgnIgG8@public.gmane.org, robin.murphy-5wv7dgnIgG8@public.gmane.org, joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org, tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org List-Id: linux-arm-msm@vger.kernel.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 > 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 >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 >--- > 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: sricharan@codeaurora.org (Sricharan) Date: Wed, 12 Oct 2016 11:54:57 +0530 Subject: [PATCH V3 0/8] IOMMU probe deferral support In-Reply-To: <12cfb59f-f7ca-d4df-eb7f-42348e357979@samsung.com> References: <1475600632-21289-1-git-send-email-sricharan@codeaurora.org> <12cfb59f-f7ca-d4df-eb7f-42348e357979@samsung.com> Message-ID: <000601d22451$5eb46fc0$1c1d4f40$@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.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 at arm.com [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 > 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 >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 >--- > 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 >