Message ID | 20210423163234.3651547-3-thierry.reding@gmail.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | iommu: Support identity mappings of reserved-memory regions | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success |
23.04.2021 19:32, Thierry Reding пишет: > +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list) > +{ > + struct of_phandle_iterator it; > + int err; > + > + of_for_each_phandle(&it, err, dev->of_node, "memory-region", "#memory-region-cells", 0) { > + struct iommu_resv_region *region; > + struct of_phandle_args args; > + struct resource res; > + > + args.args_count = of_phandle_iterator_args(&it, args.args, MAX_PHANDLE_ARGS); > + > + err = of_address_to_resource(it.node, 0, &res); > + if (err < 0) { > + dev_err(dev, "failed to parse memory region %pOF: %d\n", > + it.node, err); > + continue; > + } > + > + if (args.args_count > 0) { > + /* > + * Active memory regions are expected to be accessed by hardware during > + * boot and must therefore have an identity mapping created prior to the > + * driver taking control of the hardware. This ensures that non-quiescent > + * hardware doesn't cause IOMMU faults during boot. > + */ > + if (args.args[0] & MEMORY_REGION_IDENTITY_MAPPING) { > + region = iommu_alloc_resv_region(res.start, resource_size(&res), > + IOMMU_READ | IOMMU_WRITE, > + IOMMU_RESV_DIRECT_RELAXABLE); > + if (!region) > + continue; > + > + list_add_tail(®ion->list, list); > + } > + } > + } > +} > +EXPORT_SYMBOL(of_iommu_get_resv_regions); Any reason why this is not EXPORT_SYMBOL_GPL? I'm curious what is the logic behind the OF symbols in general since it looks like half of them are GPL.
On Fri, Jul 2, 2021 at 8:05 AM Dmitry Osipenko <digetx@gmail.com> wrote: > > 23.04.2021 19:32, Thierry Reding пишет: > > +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list) > > +{ > > + struct of_phandle_iterator it; > > + int err; > > + > > + of_for_each_phandle(&it, err, dev->of_node, "memory-region", "#memory-region-cells", 0) { > > + struct iommu_resv_region *region; > > + struct of_phandle_args args; > > + struct resource res; > > + > > + args.args_count = of_phandle_iterator_args(&it, args.args, MAX_PHANDLE_ARGS); > > + > > + err = of_address_to_resource(it.node, 0, &res); > > + if (err < 0) { > > + dev_err(dev, "failed to parse memory region %pOF: %d\n", > > + it.node, err); > > + continue; > > + } > > + > > + if (args.args_count > 0) { > > + /* > > + * Active memory regions are expected to be accessed by hardware during > > + * boot and must therefore have an identity mapping created prior to the > > + * driver taking control of the hardware. This ensures that non-quiescent > > + * hardware doesn't cause IOMMU faults during boot. > > + */ > > + if (args.args[0] & MEMORY_REGION_IDENTITY_MAPPING) { > > + region = iommu_alloc_resv_region(res.start, resource_size(&res), > > + IOMMU_READ | IOMMU_WRITE, > > + IOMMU_RESV_DIRECT_RELAXABLE); > > + if (!region) > > + continue; > > + > > + list_add_tail(®ion->list, list); > > + } > > + } > > + } > > +} > > +EXPORT_SYMBOL(of_iommu_get_resv_regions); > > Any reason why this is not EXPORT_SYMBOL_GPL? I'm curious what is the > logic behind the OF symbols in general since it looks like half of them > are GPL. Generally, new ones are _GPL. Old ones probably predate _GPL. This one is up to the IOMMU maintainers. Rob
16.07.2021 17:41, Rob Herring пишет: > On Fri, Jul 2, 2021 at 8:05 AM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 23.04.2021 19:32, Thierry Reding пишет: >>> +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list) >>> +{ >>> + struct of_phandle_iterator it; >>> + int err; >>> + >>> + of_for_each_phandle(&it, err, dev->of_node, "memory-region", "#memory-region-cells", 0) { >>> + struct iommu_resv_region *region; >>> + struct of_phandle_args args; >>> + struct resource res; >>> + >>> + args.args_count = of_phandle_iterator_args(&it, args.args, MAX_PHANDLE_ARGS); >>> + >>> + err = of_address_to_resource(it.node, 0, &res); >>> + if (err < 0) { >>> + dev_err(dev, "failed to parse memory region %pOF: %d\n", >>> + it.node, err); >>> + continue; >>> + } >>> + >>> + if (args.args_count > 0) { >>> + /* >>> + * Active memory regions are expected to be accessed by hardware during >>> + * boot and must therefore have an identity mapping created prior to the >>> + * driver taking control of the hardware. This ensures that non-quiescent >>> + * hardware doesn't cause IOMMU faults during boot. >>> + */ >>> + if (args.args[0] & MEMORY_REGION_IDENTITY_MAPPING) { >>> + region = iommu_alloc_resv_region(res.start, resource_size(&res), >>> + IOMMU_READ | IOMMU_WRITE, >>> + IOMMU_RESV_DIRECT_RELAXABLE); >>> + if (!region) >>> + continue; >>> + >>> + list_add_tail(®ion->list, list); >>> + } >>> + } >>> + } >>> +} >>> +EXPORT_SYMBOL(of_iommu_get_resv_regions); >> >> Any reason why this is not EXPORT_SYMBOL_GPL? I'm curious what is the >> logic behind the OF symbols in general since it looks like half of them >> are GPL. > > Generally, new ones are _GPL. Old ones probably predate _GPL. > > This one is up to the IOMMU maintainers. Thank you.
On Sat, Jul 17, 2021 at 02:07:12PM +0300, Dmitry Osipenko wrote: > 16.07.2021 17:41, Rob Herring пишет: > > On Fri, Jul 2, 2021 at 8:05 AM Dmitry Osipenko <digetx@gmail.com> wrote: > >> > >> 23.04.2021 19:32, Thierry Reding пишет: > >>> +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list) > >>> +{ > >>> + struct of_phandle_iterator it; > >>> + int err; > >>> + > >>> + of_for_each_phandle(&it, err, dev->of_node, "memory-region", "#memory-region-cells", 0) { > >>> + struct iommu_resv_region *region; > >>> + struct of_phandle_args args; > >>> + struct resource res; > >>> + > >>> + args.args_count = of_phandle_iterator_args(&it, args.args, MAX_PHANDLE_ARGS); > >>> + > >>> + err = of_address_to_resource(it.node, 0, &res); > >>> + if (err < 0) { > >>> + dev_err(dev, "failed to parse memory region %pOF: %d\n", > >>> + it.node, err); > >>> + continue; > >>> + } > >>> + > >>> + if (args.args_count > 0) { > >>> + /* > >>> + * Active memory regions are expected to be accessed by hardware during > >>> + * boot and must therefore have an identity mapping created prior to the > >>> + * driver taking control of the hardware. This ensures that non-quiescent > >>> + * hardware doesn't cause IOMMU faults during boot. > >>> + */ > >>> + if (args.args[0] & MEMORY_REGION_IDENTITY_MAPPING) { > >>> + region = iommu_alloc_resv_region(res.start, resource_size(&res), > >>> + IOMMU_READ | IOMMU_WRITE, > >>> + IOMMU_RESV_DIRECT_RELAXABLE); > >>> + if (!region) > >>> + continue; > >>> + > >>> + list_add_tail(®ion->list, list); > >>> + } > >>> + } > >>> + } > >>> +} > >>> +EXPORT_SYMBOL(of_iommu_get_resv_regions); > >> > >> Any reason why this is not EXPORT_SYMBOL_GPL? I'm curious what is the > >> logic behind the OF symbols in general since it looks like half of them > >> are GPL. > > > > Generally, new ones are _GPL. Old ones probably predate _GPL. > > > > This one is up to the IOMMU maintainers. > > Thank you. I prefer EXPORT_SYMBOL_GPL(). That's aligned with the symbols exported by iommu.c, with the *single* exception of generic_iommu_put_resv_regions(), which I think should be changed to _GPL() as well. Will
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index a9d2df001149..321ebd5fdaba 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -11,12 +11,15 @@ #include <linux/module.h> #include <linux/msi.h> #include <linux/of.h> +#include <linux/of_address.h> #include <linux/of_iommu.h> #include <linux/of_pci.h> #include <linux/pci.h> #include <linux/slab.h> #include <linux/fsl/mc.h> +#include <dt-bindings/reserved-memory.h> + #define NO_IOMMU 1 /** @@ -240,3 +243,54 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, return ops; } + +/** + * of_iommu_get_resv_regions - reserved region driver helper for device tree + * @dev: device for which to get reserved regions + * @list: reserved region list + * + * IOMMU drivers can use this to implement their .get_resv_regions() callback + * for memory regions attached to a device tree node. See the reserved-memory + * device tree bindings on how to use these: + * + * Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt + */ +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list) +{ + struct of_phandle_iterator it; + int err; + + of_for_each_phandle(&it, err, dev->of_node, "memory-region", "#memory-region-cells", 0) { + struct iommu_resv_region *region; + struct of_phandle_args args; + struct resource res; + + args.args_count = of_phandle_iterator_args(&it, args.args, MAX_PHANDLE_ARGS); + + err = of_address_to_resource(it.node, 0, &res); + if (err < 0) { + dev_err(dev, "failed to parse memory region %pOF: %d\n", + it.node, err); + continue; + } + + if (args.args_count > 0) { + /* + * Active memory regions are expected to be accessed by hardware during + * boot and must therefore have an identity mapping created prior to the + * driver taking control of the hardware. This ensures that non-quiescent + * hardware doesn't cause IOMMU faults during boot. + */ + if (args.args[0] & MEMORY_REGION_IDENTITY_MAPPING) { + region = iommu_alloc_resv_region(res.start, resource_size(&res), + IOMMU_READ | IOMMU_WRITE, + IOMMU_RESV_DIRECT_RELAXABLE); + if (!region) + continue; + + list_add_tail(®ion->list, list); + } + } + } +} +EXPORT_SYMBOL(of_iommu_get_resv_regions); diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h index 16f4b3e87f20..8412437acaac 100644 --- a/include/linux/of_iommu.h +++ b/include/linux/of_iommu.h @@ -16,6 +16,9 @@ extern const struct iommu_ops *of_iommu_configure(struct device *dev, struct device_node *master_np, const u32 *id); +extern void of_iommu_get_resv_regions(struct device *dev, + struct list_head *list); + #else static inline int of_get_dma_window(struct device_node *dn, const char *prefix, @@ -32,6 +35,11 @@ static inline const struct iommu_ops *of_iommu_configure(struct device *dev, return NULL; } +static inline void of_iommu_get_resv_regions(struct device *dev, + struct list_head *list) +{ +} + #endif /* CONFIG_OF_IOMMU */ #endif /* __OF_IOMMU_H */