| Message ID | 12cfb59f-f7ca-d4df-eb7f-42348e357979@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
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@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 <m.szyprowski@samsung.com> > 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@samsung.com> >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@samsung.com> >--- > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----Original Message----- >From: Sricharan [mailto:sricharan@codeaurora.org] >Sent: Wednesday, October 12, 2016 11:55 AM >To: 'Marek Szyprowski' <m.szyprowski@samsung.com>; 'will.deacon@arm.com' <will.deacon@arm.com>; 'robin.murphy@arm.com' ><robin.murphy@arm.com>; 'joro@8bytes.org' <joro@8bytes.org>; 'iommu@lists.linux-foundation.org' <iommu@lists.linux- >foundation.org>; 'linux-arm-kernel@lists.infradead.org' <linux-arm-kernel@lists.infradead.org>; 'linux-arm-msm@vger.kernel.org' ><linux-arm-msm@vger.kernel.org>; 'laurent.pinchart@ideasonboard.com' <laurent.pinchart@ideasonboard.com>; >'tfiga@chromium.org' <tfiga@chromium.org>; 'srinivas.kandagatla@linaro.org' <srinivas.kandagatla@linaro.org> >Subject: RE: [PATCH V3 0/8] IOMMU probe deferral support > >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@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 <m.szyprowski@samsung.com> >> Hi Will,Robin,Joerg, I have tested the probe deferral for platform/pcie bus devices based on latest Generic bindings series merged [1], for pci/arm-smmu. It will good to know from you on whats the right way to take this forward ? Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Resending, missed out on the link last time. >-----Original Message----- >From: linux-arm-msm-owner@vger.kernel.org [mailto:linux-arm-msm-owner@vger.kernel.org] On Behalf Of Marek Szyprowski >Sent: Monday, October 10, 2016 6:07 PM >To: Sricharan R <sricharan@codeaurora.org>; will.deacon@arm.com; robin.murphy@arm.com; joro@8bytes.org; iommu@lists.linux- >foundation.org; linux-arm-kernel@lists.infradead.org; linux-arm-msm@vger.kernel.org; laurent.pinchart@ideasonboard.com; >tfiga@chromium.org; srinivas.kandagatla@linaro.org >Subject: Re: [PATCH V3 0/8] IOMMU probe deferral support > >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@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 <m.szyprowski@samsung.com> > Hi Will,Robin,Joerg, I have tested the probe deferral for platform/pcie bus devices based on latest Generic DT bindings series merged [1], for pci/arm-smmu. It will be good to know from you on whats the right way to take this forward ? [1] http://www.spinics.net/lists/devicetree/msg142943.html Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sricharan, On 2016-10-12 08:24, Sricharan wrote: > 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@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 <m.szyprowski@samsung.com> >> > 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 ? I assumed that IOMMU driver cannot be removed reliably, so all structures that it creates are permanent. I didn't use device_add()/device_remove() callbacks, because in current implementation device_add() is called too late (after dma-mapping glue triggers device_attach_iommu()). Maybe once your patchset is merged, I will move creation and management of the all IOMMU related structures to device_add/remove callbacks. Best regards
Hi Marek, >>>> 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@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 <m.szyprowski@samsung.com> >>> >> 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 ? > >I assumed that IOMMU driver cannot be removed reliably, so all >structures that it >creates are permanent. I didn't use device_add()/device_remove() >callbacks, because >in current implementation device_add() is called too late (after >dma-mapping glue >triggers device_attach_iommu()). > >Maybe once your patchset is merged, I will move creation and management >of the all >IOMMU related structures to device_add/remove callbacks. > ok understand it. Regards, Sricharan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }