[PATCH 06/10] iommu: of: Handle IOMMU lookup failure with deferred probing or error
Marek Szyprowski
m.szyprowski at samsung.com
Tue Nov 29 23:54:55 PST 2016
Hi Sricharan and Robin,
On 2016-11-30 01:22, Sricharan R wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
>
> Failures to look up an IOMMU when parsing the DT iommus property need to
> be handled separately from the .of_xlate() failures to support deferred
> probing.
>
> The lack of a registered IOMMU can be caused by the lack of a driver for
> the IOMMU, the IOMMU device probe not having been performed yet, having
> been deferred, or having failed.
>
> The first case occurs when the device tree describes the bus master and
> IOMMU topology correctly but no device driver exists for the IOMMU yet
> or the device driver has not been compiled in. Return NULL, the caller
> will configure the device without an IOMMU.
>
> The second and third cases are handled by deferring the probe of the bus
> master device which will eventually get reprobed after the IOMMU.
>
> The last case is currently handled by deferring the probe of the bus
> master device as well. A mechanism to either configure the bus master
> device without an IOMMU or to fail the bus master device probe depending
> on whether the IOMMU is optional or mandatory would be a good
> enhancement.
>
> Signed-off-by: Laurent Pichart <laurent.pinchart+renesas at ideasonboard.com>
> Signed-off-by: Sricharan R <sricharan at codeaurora.org>
> [rm: massive PCI hacks]
> Signed-off-by: Robin Murphy <robin.murphy at arm.com>
> ---
> drivers/base/dma-mapping.c | 4 ++--
> drivers/iommu/dma-iommu.c | 1 +
> drivers/iommu/of_iommu.c | 5 +++--
> drivers/of/device.c | 9 +++++++--
> drivers/pci/probe.c | 6 ++++--
> include/linux/of_device.h | 9 ++++++---
> include/linux/pci.h | 4 ++--
> 7 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index b2a5629..576fdfb 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -351,9 +351,9 @@ void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags)
> int dma_configure(struct device *dev)
> {
> if (dev_is_pci(dev))
> - pci_dma_configure(dev);
> + return pci_dma_configure(dev);
> else if (dev->of_node)
> - of_dma_configure(dev, dev->of_node);
> + return of_dma_configure(dev, dev->of_node);
> return 0;
> }
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index c5ab866..d2a7a46 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -148,6 +148,7 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
> base_pfn = max_t(unsigned long, 1, base >> order);
> end_pfn = (base + size - 1) >> order;
>
> + dev_info(dev, "0x%llx 0x%llx, 0x%llx 0x%llx, 0x%llx 0x%llx\n", base, size, domain->geometry.aperture_start, domain->geometry.aperture_end,
This causes a NULL pointer dereference if caller passes NULL device pointer.
There is such caller in drivers/gpu/drm/exynos/exynos_drm_iommu.h.
Trivial to fix as it looks like a leftover from developement or
debugging stage.
> *dev->dma_mask, dev->coherent_dma_mask);
> /* Check the domain allows at least some access to the device... */
> if (domain->geometry.force_aperture) {
> if (base > domain->geometry.aperture_end ||
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 349bd1d..9529d6c 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -23,6 +23,7 @@
> #include <linux/of.h>
> #include <linux/of_iommu.h>
> #include <linux/of_pci.h>
> +#include <linux/pci.h>
> #include <linux/slab.h>
>
> static const struct of_device_id __iommu_of_table_sentinel
> @@ -223,7 +224,7 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> ops = ERR_PTR(err);
> }
>
> - return IS_ERR(ops) ? NULL : ops;
> + return ops;
> }
>
> static int __init of_iommu_init(void)
> @@ -234,7 +235,7 @@ static int __init of_iommu_init(void)
> for_each_matching_node_and_match(np, matches, &match) {
> const of_iommu_init_fn init_fn = match->data;
>
> - if (init_fn(np))
> + if (init_fn && init_fn(np))
> pr_err("Failed to initialise IOMMU %s\n",
> of_node_full_name(np));
> }
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 1c843e2..d58595c 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -82,7 +82,7 @@ int of_device_add(struct platform_device *ofdev)
> * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
> * to fix up DMA configuration.
> */
> -void of_dma_configure(struct device *dev, struct device_node *np)
> +int of_dma_configure(struct device *dev, struct device_node *np)
> {
> u64 dma_addr, paddr, size;
> int ret;
> @@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
> ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
> if (ret < 0) {
> dma_addr = offset = 0;
> - size = dev->coherent_dma_mask + 1;
> + size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
> } else {
> offset = PFN_DOWN(paddr - dma_addr);
> dev_dbg(dev, "dma_pfn_offset(%#08lx)\n", offset);
> @@ -129,10 +129,15 @@ void of_dma_configure(struct device *dev, struct device_node *np)
> coherent ? " " : " not ");
>
> iommu = of_iommu_configure(dev, np);
> + if (IS_ERR(iommu))
> + return PTR_ERR(iommu);
> +
> dev_dbg(dev, "device is%sbehind an iommu\n",
> iommu ? " " : " not ");
>
> arch_setup_dma_ops(dev, dma_addr, size, iommu, coherent);
> +
> + return 0;
> }
> EXPORT_SYMBOL_GPL(of_dma_configure);
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 04af770..6316cae 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1724,13 +1724,14 @@ static void pci_set_msi_domain(struct pci_dev *dev)
> * Function to update PCI devices's DMA configuration using the same
> * info from the OF node or ACPI node of host bridge's parent (if any).
> */
> -void pci_dma_configure(struct device *dev)
> +int pci_dma_configure(struct device *dev)
> {
> struct device *bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> + int ret = 0;
>
> if (IS_ENABLED(CONFIG_OF) &&
> bridge->parent && bridge->parent->of_node) {
> - of_dma_configure(dev, bridge->parent->of_node);
> + ret = of_dma_configure(dev, bridge->parent->of_node);
> } else if (has_acpi_companion(bridge)) {
> struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
> enum dev_dma_attr attr = acpi_get_dma_attr(adev);
> @@ -1742,6 +1743,7 @@ void pci_dma_configure(struct device *dev)
> }
>
> pci_put_host_bridge_device(bridge);
> + return ret;
> }
>
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
> index d20a31a..6dca65c 100644
> --- a/include/linux/of_device.h
> +++ b/include/linux/of_device.h
> @@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
> return of_node_get(cpu_dev->of_node);
> }
>
> -void of_dma_configure(struct device *dev, struct device_node *np);
> +int of_dma_configure(struct device *dev, struct device_node *np);
> void of_dma_deconfigure(struct device *dev);
> #else /* CONFIG_OF */
>
> @@ -99,8 +99,11 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
> {
> return NULL;
> }
> -static inline void of_dma_configure(struct device *dev, struct device_node *np)
> -{}
> +
> +static inline int of_dma_configure(struct device *dev, struct device_node *np)
> +{
> + return 0;
> +}
> static inline void of_dma_deconfigure(struct device *dev)
> {}
> #endif /* CONFIG_OF */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d04f651..989ca44 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -870,7 +870,7 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> #define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
> #define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
>
> -void pci_dma_configure(struct device *dev);
> +int pci_dma_configure(struct device *dev);
>
> /* Generic PCI functions exported to card drivers */
>
> @@ -1604,7 +1604,7 @@ static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
> #define dev_is_pf(d) (false)
> #define dev_num_vf(d) (0)
>
> -static inline void pci_dma_configure(struct device *dev) { }
> +static inline int pci_dma_configure(struct device *dev) { return 0; }
>
> #endif /* CONFIG_PCI */
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
More information about the linux-arm-kernel
mailing list