| Message ID | 20221117185424.2359687-1-thierry.reding@gmail.com |
|---|---|
| Headers | show |
| Series | iommu: Support mappings/reservations in reserved-memory regions | expand |
On Thu, 17 Nov 2022 19:54:21 +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > This function is similar to of_translate_dma_address() but also reads a > length in addition to an address from a device tree property. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > drivers/of/address.c | 41 ++++++++++++++++++++++++++++++++++++++ > include/linux/of_address.h | 2 ++ > 2 files changed, 43 insertions(+) > Reviewed-by: Rob Herring <robh@kernel.org>
On Thu, Nov 17, 2022 at 07:54:23PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > This is an implementation that IOMMU drivers can use to obtain reserved > memory regions from a device tree node. It uses the reserved-memory DT > bindings to find the regions associated with a given device. If these > regions are marked accordingly, identity mappings will be created for > them in the IOMMU domain that the devices will be attached to. > > Cc: Frank Rowand <frowand.list@gmail.com> > Cc: devicetree@vger.kernel.org > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > Changes in v10: > - extract more code into the new iommu_resv_region_get_type() function > - rename variables for physical and I/O virtual addresses for clarity > - default to IOMMU_RESV_DIRECT instead of IOMMU_RESV_DIRECT_RELAXABLE > - use newly introduced of_translate_dma_region() > > Changes in v9: > - address review comments by Robin Murphy: > - warn about non-direct mappings since they are not supported yet > - cleanup code to require less indentation > - narrow scope of variables > > Changes in v8: > - cleanup set-but-unused variables > > Changes in v6: > - remove reference to now unused dt-bindings/reserved-memory.h include > > Changes in v5: > - update for new "iommu-addresses" device tree bindings > > Changes in v4: > - fix build failure on !CONFIG_OF_ADDRESS > > Changes in v3: > - change "active" property to identity mapping flag that is part of the > memory region specifier (as defined by #memory-region-cells) to allow > per-reference flags to be used > > Changes in v2: > - use "active" property to determine whether direct mappings are needed > > drivers/iommu/of_iommu.c | 94 ++++++++++++++++++++++++++++++++++++++++ > include/linux/of_iommu.h | 8 ++++ > 2 files changed, 102 insertions(+) Hi Robin, can you take a look at this to see if all your concerns have now been addressed? Thanks, Thierry > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index 5696314ae69e..fa7c63a4abbf 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -11,6 +11,7 @@ > #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> > @@ -172,3 +173,96 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, > > return ops; > } > + > +static enum iommu_resv_type iommu_resv_region_get_type(struct device *dev, struct resource *phys, > + phys_addr_t start, size_t length) > +{ > + phys_addr_t end = start + length - 1; > + > + /* > + * IOMMU regions without an associated physical region cannot be > + * mapped and are simply reservations. > + */ > + if (phys->start >= phys->end) > + return IOMMU_RESV_RESERVED; > + > + /* may be IOMMU_RESV_DIRECT_RELAXABLE for certain cases */ > + if (start == phys->start && end == phys->end) > + return IOMMU_RESV_DIRECT; > + > + dev_warn(dev, "treating non-direct mapping [%pr] -> [%pap-%pap] as reservation\n", &phys, > + &start, &end); > + return IOMMU_RESV_RESERVED; > +} > + > +/** > + * 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) > +{ > +#if IS_ENABLED(CONFIG_OF_ADDRESS) > + struct of_phandle_iterator it; > + int err; > + > + of_for_each_phandle(&it, err, dev->of_node, "memory-region", NULL, 0) { > + const __be32 *maps, *end; > + struct resource phys; > + int size; > + > + memset(&phys, 0, sizeof(phys)); > + > + /* > + * The "reg" property is optional and can be omitted by reserved-memory regions > + * that represent reservations in the IOVA space, which are regions that should > + * not be mapped. > + */ > + if (of_find_property(it.node, "reg", NULL)) { > + err = of_address_to_resource(it.node, 0, &phys); > + if (err < 0) { > + dev_err(dev, "failed to parse memory region %pOF: %d\n", > + it.node, err); > + continue; > + } > + } > + > + maps = of_get_property(it.node, "iommu-addresses", &size); > + if (!maps) > + continue; > + > + end = maps + size / sizeof(__be32); > + > + while (maps < end) { > + struct device_node *np; > + u32 phandle; > + > + phandle = be32_to_cpup(maps++); > + np = of_find_node_by_phandle(phandle); > + > + if (np == dev->of_node) { > + int prot = IOMMU_READ | IOMMU_WRITE; > + struct iommu_resv_region *region; > + enum iommu_resv_type type; > + phys_addr_t iova; > + size_t length; > + > + maps = of_translate_dma_region(np, maps, &iova, &length); > + type = iommu_resv_region_get_type(dev, &phys, iova, length); > + > + region = iommu_alloc_resv_region(iova, length, prot, type, > + GFP_KERNEL); > + if (region) > + list_add_tail(®ion->list, list); > + } > + } > + } > +#endif > +} > +EXPORT_SYMBOL(of_iommu_get_resv_regions); > diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h > index 55c1eb300a86..9a5e6b410dd2 100644 > --- a/include/linux/of_iommu.h > +++ b/include/linux/of_iommu.h > @@ -12,6 +12,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 const struct iommu_ops *of_iommu_configure(struct device *dev, > @@ -21,6 +24,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 */ > -- > 2.38.1 >
On Thu, Nov 17, 2022 at 07:54:20PM +0100, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > Hi, > > This version is a minor update to the previous v11, which can be found > here: > > https://lore.kernel.org/all/20221111161806.630527-1-thierry.reding@gmail.com/ > > The only change here is that the #dma-{address,size}-cells is dropped. > It turns out to be much simpler to just update #{address,size}-cells to > what they should be rather than add extra complexity for the DMA work- > around. There's a minor update to the DT binding so that it can now > properly validate cases where we have both reg and iommu-addresses > properties. > > An example is included in the DT bindings, but here is an extract of > what I've used to test this: > > reserved-memory { > #address-cells = <2>; > #size-cells = <2>; > ranges; > > /* > * Creates an identity mapping for the framebuffer that > * the firmware has setup to scan out a bootsplash from. > */ > fb: framebuffer@92cb2000 { > reg = <0x0 0x92cb2000 0x0 0x00800000>; > iommu-addresses = <&dc0 0x0 0x92cb2000 0x0 0x00800000>; > }; > > /* > * Creates a reservation in the IOVA space to prevent > * any buffers from being mapped to that region. Note > * that on Tegra the range is actually quite different > * from this, but it would conflict with the display > * driver that I tested this against, so this is just > * a dummy region for testing. > */ > adsp: reservation-adsp { > iommu-addresses = <&dc0 0x0 0x90000000 0x0 0x00010000>; > }; > }; > > host1x@50000000 { > dc@54200000 { > memory-region = <&fb>, <&adsp>; > }; > }; > > This is abbreviated a little to focus on the essentials. Note also that > the ADSP reservation is not actually used on this device and the driver > for this doesn't exist yet, but I wanted to include this variant for > testing, because we'll want to use these bindings for the reservation > use-case as well at some point. > > I've also been able to make use of this binding and the IOMMU code in > conjunction with the simple-framebuffer driver to hand over a display > configuration set up by UEFI to the Linux kernel. > > Janne has confirmed[0] this to be suitable for indirect mappings as > well, though these patches don't implement that feature yet. Potential > extensions to this have been discussed but are not yet included at this > time to not further complicate things. > > Thierry > > [0]: https://lore.kernel.org/all/20220909144504.GA4024@jannau.net/ > > Thierry Reding (4): > of: Introduce of_translate_dma_region() > dt-bindings: reserved-memory: Document iommu-addresses > iommu: Implement of_iommu_get_resv_regions() > iommu: dma: Use of_iommu_get_resv_regions() > > .../reserved-memory/reserved-memory.yaml | 89 +++++++++++++++++- > drivers/iommu/dma-iommu.c | 3 + > drivers/iommu/of_iommu.c | 94 +++++++++++++++++++ > drivers/of/address.c | 41 ++++++++ > include/linux/of_address.h | 2 + > include/linux/of_iommu.h | 8 ++ > 6 files changed, 233 insertions(+), 4 deletions(-) Joerg, Rob, Is there anything left to do on the series? It'd be great to get some feedback from Robin on patch 3 since he had some concerns about how the reservation type was getting determined. All those should now be addressed and I think overall this is ready to go. Rob, you've given a Reviewed-by on all the DT-related parts, does that mean you're okay with this going through Joerg's tree? Joerg, other than a Reviewed-by from Robin on patch 3, anything else you'd like to see before you pick this up? Thierry
From: Thierry Reding <treding@nvidia.com> Hi, This version is a minor update to the previous v11, which can be found here: https://lore.kernel.org/all/20221111161806.630527-1-thierry.reding@gmail.com/ The only change here is that the #dma-{address,size}-cells is dropped. It turns out to be much simpler to just update #{address,size}-cells to what they should be rather than add extra complexity for the DMA work- around. There's a minor update to the DT binding so that it can now properly validate cases where we have both reg and iommu-addresses properties. An example is included in the DT bindings, but here is an extract of what I've used to test this: reserved-memory { #address-cells = <2>; #size-cells = <2>; ranges; /* * Creates an identity mapping for the framebuffer that * the firmware has setup to scan out a bootsplash from. */ fb: framebuffer@92cb2000 { reg = <0x0 0x92cb2000 0x0 0x00800000>; iommu-addresses = <&dc0 0x0 0x92cb2000 0x0 0x00800000>; }; /* * Creates a reservation in the IOVA space to prevent * any buffers from being mapped to that region. Note * that on Tegra the range is actually quite different * from this, but it would conflict with the display * driver that I tested this against, so this is just * a dummy region for testing. */ adsp: reservation-adsp { iommu-addresses = <&dc0 0x0 0x90000000 0x0 0x00010000>; }; }; host1x@50000000 { dc@54200000 { memory-region = <&fb>, <&adsp>; }; }; This is abbreviated a little to focus on the essentials. Note also that the ADSP reservation is not actually used on this device and the driver for this doesn't exist yet, but I wanted to include this variant for testing, because we'll want to use these bindings for the reservation use-case as well at some point. I've also been able to make use of this binding and the IOMMU code in conjunction with the simple-framebuffer driver to hand over a display configuration set up by UEFI to the Linux kernel. Janne has confirmed[0] this to be suitable for indirect mappings as well, though these patches don't implement that feature yet. Potential extensions to this have been discussed but are not yet included at this time to not further complicate things. Thierry [0]: https://lore.kernel.org/all/20220909144504.GA4024@jannau.net/ Thierry Reding (4): of: Introduce of_translate_dma_region() dt-bindings: reserved-memory: Document iommu-addresses iommu: Implement of_iommu_get_resv_regions() iommu: dma: Use of_iommu_get_resv_regions() .../reserved-memory/reserved-memory.yaml | 89 +++++++++++++++++- drivers/iommu/dma-iommu.c | 3 + drivers/iommu/of_iommu.c | 94 +++++++++++++++++++ drivers/of/address.c | 41 ++++++++ include/linux/of_address.h | 2 + include/linux/of_iommu.h | 8 ++ 6 files changed, 233 insertions(+), 4 deletions(-)