* [PATCH V4 1/4] iommu: of: Fix check for returning EPROBE_DEFER @ 2017-05-18 14:54 Sricharan R 2017-05-18 14:54 ` [PATCH V4 2/4] iommu: of: Ignore all errors except EPROBE_DEFER Sricharan R [not found] ` <1495119257-26724-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 0 siblings, 2 replies; 9+ messages in thread From: Sricharan R @ 2017-05-18 14:54 UTC (permalink / raw) To: robin.murphy-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, joro-zLv9SwRftAIdnm+yROfE0A, lorenzo.pieralisi-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pci-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, okaya-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8, rjw-LthD3rsA81gm4RdzfppkhA, lenb-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, arnd-r2nGTMty4D4, linux-arch-u79uwXL29TY76Z2rM5mHXA, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, j.neuschaefer-hi6Y0CQ0nG0, geert-Td1EMuHUCqxL1ZNQvxDV9g, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w Now with IOMMU probe deferral, we return -EPROBE_DEFER for masters that are connected to an IOMMU which is not probed yet, but going to get probed, so that we can attach the correct dma_ops. So while trying to defer the probe of the master, check if the of_iommu node that it is connected to is marked in DT as 'status=disabled', then the IOMMU is never is going to get probed. So simply return NULL and let the master work without an IOMMU. Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred probing or error") Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Reported-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> Tested-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> Tested-by: Magnus Damn <magnus.damn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> --- drivers/iommu/of_iommu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 9f44ee8..e6e9bec 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -118,6 +118,7 @@ static bool of_iommu_driver_present(struct device_node *np) ops = iommu_ops_from_fwnode(fwnode); if ((ops && !ops->of_xlate) || + !of_device_is_available(iommu_spec->np) || (!ops && !of_iommu_driver_present(iommu_spec->np))) return NULL; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V4 2/4] iommu: of: Ignore all errors except EPROBE_DEFER 2017-05-18 14:54 [PATCH V4 1/4] iommu: of: Fix check for returning EPROBE_DEFER Sricharan R @ 2017-05-18 14:54 ` Sricharan R [not found] ` <1495119257-26724-2-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> [not found] ` <1495119257-26724-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 1 sibling, 1 reply; 9+ messages in thread From: Sricharan R @ 2017-05-18 14:54 UTC (permalink / raw) To: robin.murphy, will.deacon, joro, lorenzo.pieralisi, iommu, linux-arm-kernel, linux-arm-msm, m.szyprowski, bhelgaas, linux-pci, linux-acpi, tn, hanjun.guo, okaya, robh+dt, frowand.list, devicetree, linux-kernel, sudeep.holla, rjw, lenb, catalin.marinas, arnd, linux-arch, laurent.pinchart, j.neuschaefer, geert, magnus.damm Cc: sricharan While deferring the probe of IOMMU masters, xlate and add_device callbacks called from of_iommu_configure can pass back error values like -ENODEV, which means the IOMMU cannot be connected with that master for real reasons. Before the IOMMU probe deferral, all such errors were ignored. Now all those errors are propagated back, killing the master's probe for such errors. Instead ignore all the errors except EPROBE_DEFER, which is the only one of concern and let the master work without IOMMU, thus restoring the old behavior. Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred probing or error") Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Tested-by: Magnus Damn <magnus.damn@gmail.com> Signed-off-by: Sricharan R <sricharan@codeaurora.org> --- [V4] Reworded commit log and changed dev_info to dev_dbg drivers/iommu/of_iommu.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index e6e9bec..19779b8 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -237,6 +237,12 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, ops = ERR_PTR(err); } + /* Ignore all other errors apart from EPROBE_DEFER */ + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); + ops = NULL; + } + return ops; } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 9+ messages in thread
[parent not found: <1495119257-26724-2-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH V4 2/4] iommu: of: Ignore all errors except EPROBE_DEFER [not found] ` <1495119257-26724-2-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2017-05-18 14:56 ` Laurent Pinchart 0 siblings, 0 replies; 9+ messages in thread From: Laurent Pinchart @ 2017-05-18 14:56 UTC (permalink / raw) To: Sricharan R Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, okaya-sgV2jX0FEOL9JmXXK+q4OQ, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, linux-arch-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, linux-acpi-u79uwXL29TY76Z2rM5mHXA, geert-Td1EMuHUCqxL1ZNQvxDV9g, linux-pci-u79uwXL29TY76Z2rM5mHXA, lenb-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, j.neuschaefer-hi6Y0CQ0nG0, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, rjw-LthD3rsA81gm4RdzfppkhA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 Hi Sricharan, Thank you for the patch. On Thursday 18 May 2017 20:24:15 Sricharan R wrote: > While deferring the probe of IOMMU masters, xlate and > add_device callbacks called from of_iommu_configure > can pass back error values like -ENODEV, which means > the IOMMU cannot be connected with that master for real > reasons. Before the IOMMU probe deferral, all such errors > were ignored. Now all those errors are propagated back, > killing the master's probe for such errors. Instead ignore > all the errors except EPROBE_DEFER, which is the only one > of concern and let the master work without IOMMU, thus > restoring the old behavior. > > Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred > probing or error") Reported-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org> > Tested-by: Magnus Damn <magnus.damn-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> > --- > [V4] Reworded commit log and changed dev_info to dev_dbg > > drivers/iommu/of_iommu.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index e6e9bec..19779b8 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -237,6 +237,12 @@ const struct iommu_ops *of_iommu_configure(struct > device *dev, ops = ERR_PTR(err); > } > > + /* Ignore all other errors apart from EPROBE_DEFER */ > + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { > + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); > + ops = NULL; > + } > + > return ops; > } -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <1495119257-26724-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* [PATCH V4 3/4] ACPI/IORT: Ignore all errors except EPROBE_DEFER [not found] ` <1495119257-26724-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2017-05-18 14:54 ` Sricharan R 2017-05-22 10:37 ` Lorenzo Pieralisi 2017-05-18 14:54 ` [PATCH V4 4/4] ARM: dma-mapping: Don't tear third-party mappings Sricharan R 1 sibling, 1 reply; 9+ messages in thread From: Sricharan R @ 2017-05-18 14:54 UTC (permalink / raw) To: robin.murphy-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, joro-zLv9SwRftAIdnm+yROfE0A, lorenzo.pieralisi-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pci-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, okaya-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8, rjw-LthD3rsA81gm4RdzfppkhA, lenb-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, arnd-r2nGTMty4D4, linux-arch-u79uwXL29TY76Z2rM5mHXA, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, j.neuschaefer-hi6Y0CQ0nG0, geert-Td1EMuHUCqxL1ZNQvxDV9g, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w While deferring the probe of IOMMU masters, xlate and add_device callbacks called from iort_iommu_configure can pass back error values like -ENODEV, which means the IOMMU cannot be connected with that master for real reasons. Before the IOMMU probe deferral, all such errors were ignored. Now all those errors are propagated back, killing the master's probe for such errors. Instead ignore all the errors except EPROBE_DEFER, which is the only one of concern and let the master work without IOMMU, thus restoring the old behavior. Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> --- [V4] Added this patch newly. drivers/acpi/arm64/iort.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index c5fecf9..16e101f 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -782,6 +782,12 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) if (err) ops = ERR_PTR(err); + /* Ignore all other errors apart from EPROBE_DEFER */ + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); + ops = NULL; + } + return ops; } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V4 3/4] ACPI/IORT: Ignore all errors except EPROBE_DEFER 2017-05-18 14:54 ` [PATCH V4 3/4] ACPI/IORT: " Sricharan R @ 2017-05-22 10:37 ` Lorenzo Pieralisi 2017-05-22 11:05 ` Sricharan R 0 siblings, 1 reply; 9+ messages in thread From: Lorenzo Pieralisi @ 2017-05-22 10:37 UTC (permalink / raw) To: Sricharan R Cc: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel, linux-arm-msm, m.szyprowski, bhelgaas, linux-pci, linux-acpi, tn, hanjun.guo, okaya, robh+dt, frowand.list, devicetree, linux-kernel, sudeep.holla, rjw, lenb, catalin.marinas, arnd, linux-arch, laurent.pinchart, j.neuschaefer, geert, magnus.damm Hi Sricharan, On Thu, May 18, 2017 at 08:24:16PM +0530, Sricharan R wrote: > While deferring the probe of IOMMU masters, xlate and > add_device callbacks called from iort_iommu_configure > can pass back error values like -ENODEV, which means > the IOMMU cannot be connected with that master for real > reasons. Before the IOMMU probe deferral, all such errors > were ignored. Now all those errors are propagated back, > killing the master's probe for such errors. Instead ignore > all the errors except EPROBE_DEFER, which is the only one > of concern and let the master work without IOMMU, thus > restoring the old behavior. > > Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > --- > [V4] Added this patch newly. > > drivers/acpi/arm64/iort.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index c5fecf9..16e101f 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -782,6 +782,12 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) > if (err) > ops = ERR_PTR(err); > > + /* Ignore all other errors apart from EPROBE_DEFER */ > + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { > + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); > + ops = NULL; > + } > + > return ops; > } > It is getting a bit convoluted. Is not it better to propagate the error back and let acpi_dma_configure() make a decision accordingly ? Something like: -- >8 -- diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index e39ec7b..3a10d757 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1371,8 +1371,8 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) iort_set_dma_mask(dev); iommu = iort_iommu_configure(dev); - if (IS_ERR(iommu)) - return PTR_ERR(iommu); + if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER) + return -EPROBE_DEFER; size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1); /* ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V4 3/4] ACPI/IORT: Ignore all errors except EPROBE_DEFER 2017-05-22 10:37 ` Lorenzo Pieralisi @ 2017-05-22 11:05 ` Sricharan R [not found] ` <ffb088d6-e40f-7a7d-dafb-e52dc86704fc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Sricharan R @ 2017-05-22 11:05 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, okaya-sgV2jX0FEOL9JmXXK+q4OQ, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, linux-arch-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, linux-acpi-u79uwXL29TY76Z2rM5mHXA, geert-Td1EMuHUCqxL1ZNQvxDV9g, linux-pci-u79uwXL29TY76Z2rM5mHXA, lenb-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, j.neuschaefer-hi6Y0CQ0nG0, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, rjw-LthD3rsA81gm4RdzfppkhA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 Hi Lorenzo, On 5/22/2017 4:07 PM, Lorenzo Pieralisi wrote: > Hi Sricharan, > > On Thu, May 18, 2017 at 08:24:16PM +0530, Sricharan R wrote: >> While deferring the probe of IOMMU masters, xlate and >> add_device callbacks called from iort_iommu_configure >> can pass back error values like -ENODEV, which means >> the IOMMU cannot be connected with that master for real >> reasons. Before the IOMMU probe deferral, all such errors >> were ignored. Now all those errors are propagated back, >> killing the master's probe for such errors. Instead ignore >> all the errors except EPROBE_DEFER, which is the only one >> of concern and let the master work without IOMMU, thus >> restoring the old behavior. >> >> Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") >> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> >> --- >> [V4] Added this patch newly. >> >> drivers/acpi/arm64/iort.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >> index c5fecf9..16e101f 100644 >> --- a/drivers/acpi/arm64/iort.c >> +++ b/drivers/acpi/arm64/iort.c >> @@ -782,6 +782,12 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) >> if (err) >> ops = ERR_PTR(err); >> >> + /* Ignore all other errors apart from EPROBE_DEFER */ >> + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { >> + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); >> + ops = NULL; >> + } >> + >> return ops; >> } >> > > It is getting a bit convoluted. Is not it better to propagate the > error back and let acpi_dma_configure() make a decision accordingly ? > ok, I was trying to keep it in same way as DT, where of_dma_configure (here acpi_dma_configure) calls of_iommu_configure(here iort_iommu_configure) which ends up doing the check. So will have to be changed in both places for symmetry. > Something like: > > -- >8 -- > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index e39ec7b..3a10d757 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1371,8 +1371,8 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) > iort_set_dma_mask(dev); > > iommu = iort_iommu_configure(dev); > - if (IS_ERR(iommu)) > - return PTR_ERR(iommu); > + if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER) > + return -EPROBE_DEFER; In case of other errors apart from EPROBE_DEFER, iommu = NULL should be set, before passing it on to arch_setup_dma_ops. Regards, Sricharan > > size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1); > /* > -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <ffb088d6-e40f-7a7d-dafb-e52dc86704fc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH V4 3/4] ACPI/IORT: Ignore all errors except EPROBE_DEFER [not found] ` <ffb088d6-e40f-7a7d-dafb-e52dc86704fc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2017-05-22 11:31 ` Lorenzo Pieralisi 2017-05-22 13:02 ` Sricharan R 0 siblings, 1 reply; 9+ messages in thread From: Lorenzo Pieralisi @ 2017-05-22 11:31 UTC (permalink / raw) To: Sricharan R Cc: robin.murphy-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, joro-zLv9SwRftAIdnm+yROfE0A, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pci-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, okaya-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8, rjw-LthD3rsA81gm4RdzfppkhA, lenb-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, arnd-r2nGTMty4D4, linux-arch-u79uwXL29TY76Z2rM5mHXA, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, j.neuschaefer-hi6Y0CQ0nG0, geert-Td1EMuHUCqxL1ZNQvxDV9g, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w On Mon, May 22, 2017 at 04:35:43PM +0530, Sricharan R wrote: > Hi Lorenzo, > > On 5/22/2017 4:07 PM, Lorenzo Pieralisi wrote: > > Hi Sricharan, > > > > On Thu, May 18, 2017 at 08:24:16PM +0530, Sricharan R wrote: > >> While deferring the probe of IOMMU masters, xlate and > >> add_device callbacks called from iort_iommu_configure > >> can pass back error values like -ENODEV, which means > >> the IOMMU cannot be connected with that master for real > >> reasons. Before the IOMMU probe deferral, all such errors > >> were ignored. Now all those errors are propagated back, > >> killing the master's probe for such errors. Instead ignore > >> all the errors except EPROBE_DEFER, which is the only one > >> of concern and let the master work without IOMMU, thus > >> restoring the old behavior. > >> > >> Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") > >> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > >> --- > >> [V4] Added this patch newly. > >> > >> drivers/acpi/arm64/iort.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > >> index c5fecf9..16e101f 100644 > >> --- a/drivers/acpi/arm64/iort.c > >> +++ b/drivers/acpi/arm64/iort.c > >> @@ -782,6 +782,12 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) > >> if (err) > >> ops = ERR_PTR(err); > >> > >> + /* Ignore all other errors apart from EPROBE_DEFER */ > >> + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { > >> + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); > >> + ops = NULL; > >> + } > >> + > >> return ops; > >> } > >> > > > > It is getting a bit convoluted. Is not it better to propagate the > > error back and let acpi_dma_configure() make a decision accordingly ? > > > > ok, I was trying to keep it in same way as DT, where of_dma_configure > (here acpi_dma_configure) calls of_iommu_configure(here iort_iommu_configure) > which ends up doing the check. So will have to be changed in both places > for symmetry. > > > Something like: > > > > -- >8 -- > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > index e39ec7b..3a10d757 100644 > > --- a/drivers/acpi/scan.c > > +++ b/drivers/acpi/scan.c > > @@ -1371,8 +1371,8 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) > > iort_set_dma_mask(dev); > > > > iommu = iort_iommu_configure(dev); > > - if (IS_ERR(iommu)) > > - return PTR_ERR(iommu); > > + if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER) > > + return -EPROBE_DEFER; > > In case of other errors apart from EPROBE_DEFER, > iommu = NULL should be set, before passing it on to > arch_setup_dma_ops. Ah yes, sorry, that makes it uglier :( Point is, it does not make sense to me to have of/acpi_dma_configure() check a return code with (IS_ERR()) whilst we know the only error code you are allowed to handle is -EPROBE_DEFER, while at it it is better to make the -EPROBE_DEFER check explicit (otherwise it may seem that of/acpi_dma_configure() can handle an error code that is different from -EPROBE_DEFER - but they don't), no big deal either way though. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V4 3/4] ACPI/IORT: Ignore all errors except EPROBE_DEFER 2017-05-22 11:31 ` Lorenzo Pieralisi @ 2017-05-22 13:02 ` Sricharan R 0 siblings, 0 replies; 9+ messages in thread From: Sricharan R @ 2017-05-22 13:02 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, okaya-sgV2jX0FEOL9JmXXK+q4OQ, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, linux-arch-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, linux-acpi-u79uwXL29TY76Z2rM5mHXA, geert-Td1EMuHUCqxL1ZNQvxDV9g, linux-pci-u79uwXL29TY76Z2rM5mHXA, lenb-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, j.neuschaefer-hi6Y0CQ0nG0, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, rjw-LthD3rsA81gm4RdzfppkhA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, sudeep.holla-5wv7dgnIgG8 Hi Lorenzo, On 5/22/2017 5:01 PM, Lorenzo Pieralisi wrote: > On Mon, May 22, 2017 at 04:35:43PM +0530, Sricharan R wrote: >> Hi Lorenzo, >> >> On 5/22/2017 4:07 PM, Lorenzo Pieralisi wrote: >>> Hi Sricharan, >>> >>> On Thu, May 18, 2017 at 08:24:16PM +0530, Sricharan R wrote: >>>> While deferring the probe of IOMMU masters, xlate and >>>> add_device callbacks called from iort_iommu_configure >>>> can pass back error values like -ENODEV, which means >>>> the IOMMU cannot be connected with that master for real >>>> reasons. Before the IOMMU probe deferral, all such errors >>>> were ignored. Now all those errors are propagated back, >>>> killing the master's probe for such errors. Instead ignore >>>> all the errors except EPROBE_DEFER, which is the only one >>>> of concern and let the master work without IOMMU, thus >>>> restoring the old behavior. >>>> >>>> Fixes: 5a1bb638d567 ("drivers: acpi: Handle IOMMU lookup failure with deferred probing or error") >>>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> >>>> --- >>>> [V4] Added this patch newly. >>>> >>>> drivers/acpi/arm64/iort.c | 6 ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>>> index c5fecf9..16e101f 100644 >>>> --- a/drivers/acpi/arm64/iort.c >>>> +++ b/drivers/acpi/arm64/iort.c >>>> @@ -782,6 +782,12 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) >>>> if (err) >>>> ops = ERR_PTR(err); >>>> >>>> + /* Ignore all other errors apart from EPROBE_DEFER */ >>>> + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { >>>> + dev_dbg(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); >>>> + ops = NULL; >>>> + } >>>> + >>>> return ops; >>>> } >>>> >>> >>> It is getting a bit convoluted. Is not it better to propagate the >>> error back and let acpi_dma_configure() make a decision accordingly ? >>> >> >> ok, I was trying to keep it in same way as DT, where of_dma_configure >> (here acpi_dma_configure) calls of_iommu_configure(here iort_iommu_configure) >> which ends up doing the check. So will have to be changed in both places >> for symmetry. >> >>> Something like: >>> >>> -- >8 -- >>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >>> index e39ec7b..3a10d757 100644 >>> --- a/drivers/acpi/scan.c >>> +++ b/drivers/acpi/scan.c >>> @@ -1371,8 +1371,8 @@ int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) >>> iort_set_dma_mask(dev); >>> >>> iommu = iort_iommu_configure(dev); >>> - if (IS_ERR(iommu)) >>> - return PTR_ERR(iommu); >>> + if (IS_ERR(iommu) && PTR_ERR(iommu) == -EPROBE_DEFER) >>> + return -EPROBE_DEFER; >> >> In case of other errors apart from EPROBE_DEFER, >> iommu = NULL should be set, before passing it on to >> arch_setup_dma_ops. > > Ah yes, sorry, that makes it uglier :( > > Point is, it does not make sense to me to have of/acpi_dma_configure() > check a return code with (IS_ERR()) whilst we know the only error > code you are allowed to handle is -EPROBE_DEFER, while at it it is > better to make the -EPROBE_DEFER check explicit (otherwise it may > seem that of/acpi_dma_configure() can handle an error code that > is different from -EPROBE_DEFER - but they don't), no big deal either > way though. > ok, in which case, the above check that you have shown to be added additionally to the original patch. Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V4 4/4] ARM: dma-mapping: Don't tear third-party mappings [not found] ` <1495119257-26724-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-05-18 14:54 ` [PATCH V4 3/4] ACPI/IORT: " Sricharan R @ 2017-05-18 14:54 ` Sricharan R 1 sibling, 0 replies; 9+ messages in thread From: Sricharan R @ 2017-05-18 14:54 UTC (permalink / raw) To: robin.murphy-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, joro-zLv9SwRftAIdnm+yROfE0A, lorenzo.pieralisi-5wv7dgnIgG8, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pci-u79uwXL29TY76Z2rM5mHXA, linux-acpi-u79uwXL29TY76Z2rM5mHXA, tn-nYOzD4b6Jr9Wk0Htik3J/w, hanjun.guo-QSEj5FYQhm4dnm+yROfE0A, okaya-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, frowand.list-Re5JQEeQqe8AvxtiuMwx3w, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8, rjw-LthD3rsA81gm4RdzfppkhA, lenb-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, arnd-r2nGTMty4D4, linux-arch-u79uwXL29TY76Z2rM5mHXA, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, j.neuschaefer-hi6Y0CQ0nG0, geert-Td1EMuHUCqxL1ZNQvxDV9g, magnus.damm-Re5JQEeQqe8AvxtiuMwx3w From: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> arch_setup_dma_ops() is used in device probe code paths to create an IOMMU mapping and attach it to the device. The function assumes that the device is attached to a device-specific IOMMU instance (or at least a device-specific TLB in a shared IOMMU instance) and thus creates a separate mapping for every device. On several systems (Renesas R-Car Gen2 being one of them), that assumption is not true, and IOMMU mappings must be shared between multiple devices. In those cases the IOMMU driver knows better than the generic ARM dma-mapping layer and attaches mapping to devices manually with arm_iommu_attach_device(), which sets the DMA ops for the device. The arch_setup_dma_ops() function takes this into account and bails out immediately if the device already has DMA ops assigned. However, the corresponding arch_teardown_dma_ops() function, called from driver unbind code paths (including probe deferral), will tear the mapping down regardless of who created it. When the device is reprobed arch_setup_dma_ops() will be called again but won't perform any operation as the DMA ops will still be set. We need to reset the DMA ops in arch_teardown_dma_ops() to fix this. However, we can't do so unconditionally, as then a new mapping would be created by arch_setup_dma_ops() when the device is reprobed, regardless of whether the device needs to share a mapping or not. We must thus keep track of whether arch_setup_dma_ops() created the mapping, and only in that case tear it down in arch_teardown_dma_ops(). Keep track of that information in the dev_archdata structure. As the structure is embedded in all instances of struct device let's not grow it, but turn the existing dma_coherent bool field into a bitfield that can be used for other purposes. Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") Reviewed-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> --- arch/arm/include/asm/device.h | 3 ++- arch/arm/mm/dma-mapping.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h index 36ec9c8..3234fe9 100644 --- a/arch/arm/include/asm/device.h +++ b/arch/arm/include/asm/device.h @@ -19,7 +19,8 @@ struct dev_archdata { #ifdef CONFIG_XEN const struct dma_map_ops *dev_dma_ops; #endif - bool dma_coherent; + unsigned int dma_coherent:1; + unsigned int dma_ops_setup:1; }; struct omap_device; diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index c742dfd..b48998f 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -2430,9 +2430,13 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, dev->dma_ops = xen_dma_ops; } #endif + dev->archdata.dma_ops_setup = true; } void arch_teardown_dma_ops(struct device *dev) { + if (!dev->archdata.dma_ops_setup) + return; + arm_teardown_iommu_dma_ops(dev); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-05-22 13:02 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-18 14:54 [PATCH V4 1/4] iommu: of: Fix check for returning EPROBE_DEFER Sricharan R 2017-05-18 14:54 ` [PATCH V4 2/4] iommu: of: Ignore all errors except EPROBE_DEFER Sricharan R [not found] ` <1495119257-26724-2-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-05-18 14:56 ` Laurent Pinchart [not found] ` <1495119257-26724-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-05-18 14:54 ` [PATCH V4 3/4] ACPI/IORT: " Sricharan R 2017-05-22 10:37 ` Lorenzo Pieralisi 2017-05-22 11:05 ` Sricharan R [not found] ` <ffb088d6-e40f-7a7d-dafb-e52dc86704fc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2017-05-22 11:31 ` Lorenzo Pieralisi 2017-05-22 13:02 ` Sricharan R 2017-05-18 14:54 ` [PATCH V4 4/4] ARM: dma-mapping: Don't tear third-party mappings Sricharan R
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).