[PATCH v7] dt-bindings: reserved-memory: Document iommu-addresses
From: Thierry Reding
This adds the "iommu-addresses" property to reserved-memory nodes, which
allow describing the interaction of memory regions with IOMMUs. Two use-
cases are supported:
1. Static mappings can be described by pairing the "iommu-addresses"
property with a "reg" property. This is mostly useful for adopting
firmware-allocated buffers via identity mappings. One common use-
case where this is required is if early firmware or bootloaders
have set up a bootsplash framebuffer that a display controller is
actively scanning out from during the operating system boot
process.
2. If an "iommu-addresses" property exists without a "reg" property,
the reserved-memory node describes an IOVA reservation. Such memory
regions are excluded from the IOVA space available to operating
system drivers and can be used for regions that must not be used to
map arbitrary buffers.
Each mapping or reservation is tied to a specific device via a phandle
to the device's device tree node. This allows a reserved-memory region
to be reused across multiple devices.
Signed-off-by: Thierry Reding
---
Changes in v7:
- keep reserved-memory.txt to avoid broken references
Changes in v6:
- add device phandle to iommu-addresses property in examples
- remove now unused dt-bindings/reserved-memory.h header
---
.../reserved-memory/reserved-memory.yaml | 62 +++
1 file changed, 62 insertions(+)
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
index 7a0744052ff6..8b885ee82ff4 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
@@ -52,6 +52,30 @@ properties:
Address and Length pairs. Specifies regions of memory that are
acceptable to allocate from.
+ iommu-addresses:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+description: >
+ A list of phandle and specifier pairs that describe static IO virtual
+ address space mappings and carveouts associated with a given reserved
+ memory region. The phandle in the first cell refers to the device for
+ which the mapping or carveout is to be created.
+
+ The specifier consists of an address/size pair and denotes the IO
+ virtual address range of the region for the given device. The exact
+ format depends on the values of the "#address-cells" and "#size-cells"
+ properties of the device referenced via the phandle.
+
+ When used in combination with a "reg" property, an IOVA mapping is to
+ be established for this memory region. One example where this can be
+ useful is to create an identity mapping for physical memory that the
+ firmware has configured some hardware to access (such as a bootsplash
+ framebuffer).
+
+ If no "reg" property is specified, the "iommu-addresses" property
+ defines carveout regions in the IOVA space for the given device. This
+ can be useful if a certain memory region should not be mapped through
+ the IOMMU.
+
no-map:
type: boolean
description: >
@@ -97,4 +121,42 @@ oneOf:
additionalProperties: true
+examples:
+ - |
+/ {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ reserved-memory {
+ranges;
+
+adsp_resv: reservation-adsp {
+ /*
+ * Restrict IOVA mappings for ADSP buffers to the 512 MiB region
+ * from 0x4000 - 0x5fff. Anything outside is reserved by
+ * the ADSP for I/O memory and private memory allocations.
+ */
+ iommu-addresses = < 0x0 0x 0x00 0x4000>,
+< 0x0 0x6000 0xff 0xa000>;
+};
+
+fb: framebuffer@9000 {
+ reg = <0x0 0x9000 0x0 0x0080>;
+ iommu-addresses = < 0x0 0x9000 0x0 0x0080>;
+};
+ };
+
+ bus@0 {
+adsp: adsp@299 {
+ reg = <0x0 0x299 0x0 0x2000>;
+ memory-region = <_resv>;
+};
+
+dc0: display@1520 {
+ reg = <0x0 0x1520 0x0 0x1>;
+ memory-region = <>;
+};
+ };
+};
+
...
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 5/5] iommu/tegra-smmu: Support managed domains
From: Navneet Kumar
Allow creating identity and DMA API compatible IOMMU domains. When
creating a DMA API compatible domain, make sure to also create the
required cookie.
Signed-off-by: Navneet Kumar
Signed-off-by: Thierry Reding
---
Changes in v5:
- remove DMA cookie initialization that's now no longer needed
drivers/iommu/tegra-smmu.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 93879c40056c..f8b2b863c111 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -277,7 +278,9 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
- if (type != IOMMU_DOMAIN_UNMANAGED)
+ if (type != IOMMU_DOMAIN_UNMANAGED &&
+ type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_IDENTITY)
return NULL;
as = kzalloc(sizeof(*as), GFP_KERNEL);
@@ -287,25 +290,16 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->attr = SMMU_PD_READABLE | SMMU_PD_WRITABLE | SMMU_PD_NONSECURE;
as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
- if (!as->pd) {
- kfree(as);
- return NULL;
- }
+ if (!as->pd)
+ goto free_as;
as->count = kcalloc(SMMU_NUM_PDE, sizeof(u32), GFP_KERNEL);
- if (!as->count) {
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->count)
+ goto free_pd_range;
as->pts = kcalloc(SMMU_NUM_PDE, sizeof(*as->pts), GFP_KERNEL);
- if (!as->pts) {
- kfree(as->count);
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->pts)
+ goto free_pts;
spin_lock_init(>lock);
@@ -315,6 +309,15 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->domain.geometry.force_aperture = true;
return >domain;
+
+free_pts:
+ kfree(as->pts);
+free_pd_range:
+ __free_page(as->pd);
+free_as:
+ kfree(as);
+
+ return NULL;
}
static void tegra_smmu_domain_free(struct iommu_domain *domain)
@@ -1009,7 +1012,7 @@ static const struct iommu_ops tegra_smmu_ops = {
.probe_device = tegra_smmu_probe_device,
.release_device = tegra_smmu_release_device,
.device_group = tegra_smmu_device_group,
- .get_resv_regions = of_iommu_get_resv_regions,
+ .get_resv_regions = iommu_dma_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.of_xlate = tegra_smmu_of_xlate,
.pgsize_bitmap = SZ_4K,
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 4/5] iommu/tegra-smmu: Add support for reserved regions
From: Thierry Reding
The Tegra DRM driver currently uses the IOMMU API explicitly. This means
that it has fine-grained control over when exactly the translation
through the IOMMU is enabled. This currently happens after the driver
probes, so the driver is in a DMA quiesced state when the IOMMU
translation is enabled.
During the transition of the Tegra DRM driver to use the DMA API instead
of the IOMMU API explicitly, it was observed that on certain platforms
the display controllers were still actively fetching from memory. When a
DMA IOMMU domain is created as part of the DMA/IOMMU API setup during
boot, the IOMMU translation for the display controllers can be enabled a
significant amount of time before the driver has had a chance to reset
the hardware into a sane state. This causes the SMMU to detect faults on
the addresses that the display controller is trying to fetch.
To avoid this, and as a byproduct paving the way for seamless transition
of display from the bootloader to the kernel, add support for reserved
regions in the Tegra SMMU driver. This is implemented using the standard
reserved memory device tree bindings, which let us describe regions of
memory which the kernel is forbidden from using for regular allocations.
The Tegra SMMU driver will parse the nodes associated with each device
via the "memory-region" property and return reserved regions that the
IOMMU core will then create direct mappings for prior to attaching the
IOMMU domains to the devices. This ensures that a 1:1 mapping is in
place when IOMMU translation starts and prevents the SMMU from detecting
any faults.
Signed-off-by: Thierry Reding
---
drivers/iommu/tegra-smmu.c | 47 --
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2f2b12033618..93879c40056c 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -471,6 +472,7 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu,
tegra_smmu_free_asid(smmu, as->id);
dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD, DMA_TO_DEVICE);
+ as->pd_dma = 0;
as->smmu = NULL;
@@ -534,6 +536,38 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as,
unsigned long iova,
struct tegra_smmu *smmu = as->smmu;
u32 *pd = page_address(as->pd);
unsigned long offset = pd_index * sizeof(*pd);
+ bool unmap = false;
+
+ /*
+* XXX Move this outside of this function. Perhaps add a struct
+* iommu_domain parameter to ->{get,put}_resv_regions() so that
+* the mapping can be done there.
+*
+* The problem here is that as->smmu is only known once we attach
+* the domain to a device (because then we look up the right SMMU
+* instance via the dev->archdata.iommu pointer). When the direct
+* mappings are created for reserved regions, the domain has not
+* been attached to a device yet, so we don't know. We currently
+* fix that up in ->apply_resv_regions() because that is the first
+* time where we have access to a struct device that will be used
+* with the IOMMU domain. However, that's asymmetric and doesn't
+* take care of the page directory mapping either, so we need to
+* come up with something better.
+*/
+ if (WARN_ON_ONCE(as->pd_dma == 0)) {
+ as->pd_dma = dma_map_page(smmu->dev, as->pd, 0, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(smmu->dev, as->pd_dma))
+ return;
+
+ if (!smmu_dma_addr_valid(smmu, as->pd_dma)) {
+ dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ return;
+ }
+
+ unmap = true;
+ }
/* Set the page directory entry first */
pd[pd_index] = value;
@@ -546,6 +580,12 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as,
unsigned long iova,
smmu_flush_ptc(smmu, as->pd_dma, offset);
smmu_flush_tlb_section(smmu, as->id, iova);
smmu_flush(smmu);
+
+ if (unmap) {
+ dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ as->pd_dma = 0;
+ }
}
static u32 *tegra_smmu_pte_offset(struct page *pt_page, unsigned long iova)
@@ -846,7 +886,6 @@ static struct iommu_device *tegra_smmu_probe_device(struct
device *dev)
smmu = tegra_smmu_find(args.np);
if (smmu) {
err = tegra_smmu_configure(smmu, dev, );
-
if (err < 0) {
[PATCH v6 2/5] iommu: Implement of_iommu_get_resv_regions()
From: Thierry Reding
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
Cc: devicet...@vger.kernel.org
Reviewed-by: Rob Herring
Signed-off-by: Thierry Reding
---
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 | 88
include/linux/of_iommu.h | 8
2 files changed, 96 insertions(+)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 41f4eb005219..c62da41516eb 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -172,3 +173,90 @@ 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)
+{
+#if IS_ENABLED(CONFIG_OF_ADDRESS)
+ struct of_phandle_iterator it;
+ int err;
+
+ of_for_each_phandle(, err, dev->of_node, "memory-region", NULL, 0) {
+ struct iommu_resv_region *region;
+ struct resource res;
+ const __be32 *maps;
+ int size;
+
+ memset(, 0, sizeof(res));
+
+ /*
+* 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, );
+ 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", );
+ if (maps) {
+ const __be32 *end = maps + size / sizeof(__be32);
+ struct device_node *np;
+ unsigned int index = 0;
+ u32 phandle;
+ int na, ns;
+
+ while (maps < end) {
+ phys_addr_t start, end;
+ size_t length;
+
+ phandle = be32_to_cpup(maps++);
+ np = of_find_node_by_phandle(phandle);
+ na = of_n_addr_cells(np);
+ ns = of_n_size_cells(np);
+
+ start = of_translate_dma_address(np, maps);
+ length = of_read_number(maps + na, ns);
+ end = start + length - 1;
+
+ if (np == dev->of_node) {
+ int prot = IOMMU_READ | IOMMU_WRITE;
+ enum iommu_resv_type type;
+
+ /*
+* IOMMU regions without an associated
physical region
+* cannot be mapped and are simply
reservations.
+*/
+ if (res.end > res.start)
+ type =
IOMMU_RESV_DIRECT_RELAXABLE;
+ else
+ type = IOMMU_RESV_RESERVED;
+
+ region = iommu_alloc_resv_region(start,
length, prot, type);
+
[PATCH v6 3/5] iommu: dma: Use of_iommu_get_resv_regions()
From: Thierry Reding For device tree nodes, use the standard of_iommu_get_resv_regions() implementation to obtain the reserved memory regions associated with a device. Cc: Rob Herring Cc: Frank Rowand Cc: devicet...@vger.kernel.org Signed-off-by: Thierry Reding --- drivers/iommu/dma-iommu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 1910f4f1612b..84fad59bc789 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -389,6 +390,8 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) iort_iommu_msi_get_resv_regions(dev, list); + if (dev->of_node) + of_iommu_get_resv_regions(dev, list); } EXPORT_SYMBOL(iommu_dma_get_resv_regions); -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 1/5] dt-bindings: reserved-memory: Document iommu-addresses
From: Thierry Reding
This adds the "iommu-addresses" property to reserved-memory nodes, which
allow describing the interaction of memory regions with IOMMUs. Two use-
cases are supported:
1. Static mappings can be described by pairing the "iommu-addresses"
property with a "reg" property. This is mostly useful for adopting
firmware-allocated buffers via identity mappings. One common use-
case where this is required is if early firmware or bootloaders
have set up a bootsplash framebuffer that a display controller is
actively scanning out from during the operating system boot
process.
2. If an "iommu-addresses" property exists without a "reg" property,
the reserved-memory node describes an IOVA reservation. Such memory
regions are excluded from the IOVA space available to operating
system drivers and can be used for regions that must not be used to
map arbitrary buffers.
Each mapping or reservation is tied to a specific device via a phandle
to the device's device tree node. This allows a reserved-memory region
to be reused across multiple devices.
Signed-off-by: Thierry Reding
---
Changes in v6:
- add device phandle to iommu-addresses property in examples
- remove now unused dt-bindings/reserved-memory.h header
.../reserved-memory/reserved-memory.txt | 1 -
.../reserved-memory/reserved-memory.yaml | 62 +++
2 files changed, 62 insertions(+), 1 deletion(-)
delete mode 100644
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
deleted file mode 100644
index 1810701a8509..
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ /dev/null
@@ -1 +0,0 @@
-This file has been moved to reserved-memory.yaml.
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
index 7a0744052ff6..8b885ee82ff4 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
@@ -52,6 +52,30 @@ properties:
Address and Length pairs. Specifies regions of memory that are
acceptable to allocate from.
+ iommu-addresses:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+description: >
+ A list of phandle and specifier pairs that describe static IO virtual
+ address space mappings and carveouts associated with a given reserved
+ memory region. The phandle in the first cell refers to the device for
+ which the mapping or carveout is to be created.
+
+ The specifier consists of an address/size pair and denotes the IO
+ virtual address range of the region for the given device. The exact
+ format depends on the values of the "#address-cells" and "#size-cells"
+ properties of the device referenced via the phandle.
+
+ When used in combination with a "reg" property, an IOVA mapping is to
+ be established for this memory region. One example where this can be
+ useful is to create an identity mapping for physical memory that the
+ firmware has configured some hardware to access (such as a bootsplash
+ framebuffer).
+
+ If no "reg" property is specified, the "iommu-addresses" property
+ defines carveout regions in the IOVA space for the given device. This
+ can be useful if a certain memory region should not be mapped through
+ the IOMMU.
+
no-map:
type: boolean
description: >
@@ -97,4 +121,42 @@ oneOf:
additionalProperties: true
+examples:
+ - |
+/ {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ reserved-memory {
+ranges;
+
+adsp_resv: reservation-adsp {
+ /*
+ * Restrict IOVA mappings for ADSP buffers to the 512 MiB region
+ * from 0x4000 - 0x5fff. Anything outside is reserved by
+ * the ADSP for I/O memory and private memory allocations.
+ */
+ iommu-addresses = < 0x0 0x 0x00 0x4000>,
+< 0x0 0x6000 0xff 0xa000>;
+};
+
+fb: framebuffer@9000 {
+ reg = <0x0 0x9000 0x0 0x0080>;
+ iommu-addresses = < 0x0 0x9000 0x0 0x0080>;
+};
+ };
+
+ bus@0 {
+adsp: adsp@299 {
+ reg = <0x0 0x299 0x0 0x2000>;
+ memory-region = <_resv>;
+};
+
+dc0: display@1520 {
+ reg = <0x0 0x1520 0x0 0x1>;
+ memory-region = <>;
+};
+ };
+};
+
...
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v6 0/5] iommu: Support mappings/reservations in reserved-memory regions
From: Thierry Reding
Hi,
This version has several fixes over the previous v5, which can be found
here:
https://lore.kernel.org/all/20220512190052.1152377-1-thierry.red...@gmail.com/
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 0x0080>;
iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
};
/*
* 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 = < 0x0 0x9000 0x0 0x0001>;
};
};
host1x@5000 {
dc@5420 {
memory-region = <>, <>;
};
};
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.
Adding Alyssa and Janne who have in the past tried to make these
bindings work on Apple M1. Also adding Sameer from the Tegra audio team
to look at the ADSP reservation and double-check that this is suitable
for our needs.
Thierry
Navneet Kumar (1):
iommu/tegra-smmu: Support managed domains
Thierry Reding (4):
dt-bindings: reserved-memory: Document iommu-addresses
iommu: Implement of_iommu_get_resv_regions()
iommu: dma: Use of_iommu_get_resv_regions()
iommu/tegra-smmu: Add support for reserved regions
.../reserved-memory/reserved-memory.txt | 1 -
.../reserved-memory/reserved-memory.yaml | 62 +
drivers/iommu/dma-iommu.c | 3 +
drivers/iommu/of_iommu.c | 88 +++
drivers/iommu/tegra-smmu.c| 82 +
include/linux/of_iommu.h | 8 ++
6 files changed, 225 insertions(+), 19 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions
On Sun, May 15, 2022 at 12:35:44PM +0200, Janne Grunau wrote:
> Hej,
>
> I'm working on the display controller for Apple silicon SoCs and will
> add some comments with support for it in mind.
>
> added as...@lists.linux.dev to CC for the Apple silicon related aspects
>
> On 2022-05-12 21:00:47 +0200, Thierry Reding wrote:
> >
> > this is another attempt at solving the problem of passing IOMMU
> > configuration via device tree. It has significantly evolved since the
> > last attempt, based on the discussion that followed. The discussion can
> > be found here:
> >
> >
> > https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/
> >
> > Rather than using a memory-region specifier, this new version introduces
> > a new "iommu-addresses" property for the reserved-memory regions
> > themselves.
>
> If experimented with both proposed bindings for dcp and I think this
> binding is easer to understand and to work with.
>
> > These are used to describe either a static mapping or
> > reservation that should be created for a given device. If both "reg" and
> > "iommu-addresses" properties are given, a mapping will be created
> > (typically this would be an identity mapping)
>
> dcp on Apple silicon will not use identity mappings. The IOMMU supports
> identity mapping but the pre-configured mappings setup by Apple's system
> firmware will not work with identity mapping. It maps multiple regions
> which are incompatible with a linear identity mapping. In addition the
> embbeded aarch64 micro controllers used in the display subsystem appears
> to use a heap mapped at low IOVA space starting at 0.
>
> > whereas if only an "iommu-addresses" property is specified, a
> > reservation for the specified range will be installed.
> >
> > 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 0x0080>;
> > iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
> > };
>
> The binding supports mapping the same region to multiple devices. The
> code supports that and it will be used on Apple silicon. Not necessary
> to extend and complicate the example for I wanted to mention it
> explicitly.
>
> >
> > /*
> > * 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 = < 0x0 0x9000 0x0 0x0001>;
> > };
> > };
> >
> > host1x@5000 {
> > dc@5420 {
> > memory-region = <>, <>;
> > };
> > };
> >
> > 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.
> >
> > Adding Alyssa and Janne who have in the past tried to make these
> > bindings work on Apple M1. Also adding Sameer from the Tegra audio team
> > to look at the ADSP reservation and double-check that this is suitable
> > for our needs.
>
> The binding itself is sufficient for the needs of the display subsystem
> on Apple silicon. The device tree parsing code for reserved regions is
> of limited use in it's current form. We will have either to extend or
> duplicate it to retrieve the non-identity mappings. That's our problem
> to solve.
I had looked at it a bit to see if I could easily implement that, but
the direct mapping support in the IOMMU subsystem currently only
supports either reservatio
Re: [PATCH v5 1/5] dt-bindings: reserved-memory: Document iommu-addresses
On Sun, May 15, 2022 at 12:45:54PM +0200, Janne Grunau wrote:
> On 2022-05-12 21:00:48 +0200, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > This adds the "iommu-addresses" property to reserved-memory nodes, which
> > allow describing the interaction of memory regions with IOMMUs. Two use-
> > cases are supported:
> >
> > 1. Static mappings can be described by pairing the "iommu-addresses"
> > property with a "reg" property. This is mostly useful for adopting
> > firmware-allocated buffers via identity mappings. One common use-
> > case where this is required is if early firmware or bootloaders
> > have set up a bootsplash framebuffer that a display controller is
> > actively scanning out from during the operating system boot
> > process.
> >
> > 2. If an "iommu-addresses" property exists without a "reg" property,
> > the reserved-memory node describes an IOVA reservation. Such memory
> > regions are excluded from the IOVA space available to operating
> > system drivers and can be used for regions that must not be used to
> > map arbitrary buffers.
> >
> > Each mapping or reservation is tied to a specific device via a phandle
> > to the device's device tree node. This allows a reserved-memory region
> > to be reused across multiple devices.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > .../reserved-memory/reserved-memory.txt | 1 -
> > .../reserved-memory/reserved-memory.yaml | 62 +++
> > include/dt-bindings/reserved-memory.h | 8 +++
> > 3 files changed, 70 insertions(+), 1 deletion(-)
> > delete mode 100644
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > create mode 100644 include/dt-bindings/reserved-memory.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > deleted file mode 100644
> > index 1810701a8509..
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +++ /dev/null
> > @@ -1 +0,0 @@
> > -This file has been moved to reserved-memory.yaml.
> > diff --git
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > index 7a0744052ff6..3a769aa66e1c 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > @@ -52,6 +52,30 @@ properties:
> >Address and Length pairs. Specifies regions of memory that are
> >acceptable to allocate from.
> >
> > + iommu-addresses:
> > +$ref: /schemas/types.yaml#/definitions/phandle-array
> > +description: >
> > + A list of phandle and specifier pairs that describe static IO virtual
> > + address space mappings and carveouts associated with a given reserved
> > + memory region. The phandle in the first cell refers to the device for
> > + which the mapping or carveout is to be created.
> > +
> > + The specifier consists of an address/size pair and denotes the IO
> > + virtual address range of the region for the given device. The exact
> > + format depends on the values of the "#address-cells" and
> > "#size-cells"
> > + properties of the device referenced via the phandle.
> > +
> > + When used in combination with a "reg" property, an IOVA mapping is to
> > + be established for this memory region. One example where this can be
> > + useful is to create an identity mapping for physical memory that the
> > + firmware has configured some hardware to access (such as a bootsplash
> > + framebuffer).
> > +
> > + If no "reg" property is specified, the "iommu-addresses" property
> > + defines carveout regions in the IOVA space for the given device. This
> > + can be useful if a certain memory region should not be mapped through
> > + the IOMMU.
> > +
> >no-map:
> > type: boolean
> > description: >
> > @@ -97,4 +121,42 @@ oneOf:
> >
> > additionalProperties: true
> >
> > +examples:
> > + - |
> > +reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
Re: [PATCH v5 2/5] iommu: Implement of_iommu_get_resv_regions()
On Sun, May 15, 2022 at 01:10:38PM +0200, Janne Grunau wrote:
> On 2022-05-12 21:00:49 +0200, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > 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.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > 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 | 90
> > include/linux/of_iommu.h | 8
> > 2 files changed, 98 insertions(+)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 5696314ae69e..9e341b5e307f 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -11,12 +11,15 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > #include
> > #include
> >
> > +#include
> > +
> > #define NO_IOMMU 1
> >
> > static int of_iommu_xlate(struct device *dev,
> > @@ -172,3 +175,90 @@ 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)
> > +{
> > +#if IS_ENABLED(CONFIG_OF_ADDRESS)
> > + struct of_phandle_iterator it;
> > + int err;
> > +
> > + of_for_each_phandle(, err, dev->of_node, "memory-region", NULL, 0) {
> > + struct iommu_resv_region *region;
> > + struct resource res;
> > + const __be32 *maps;
> > + int size;
>
> Adding 'if (!of_device_is_available(it.node)) continue;' here would help
> backwards compatibility. My plan was to add the reserved regions with
> "iommu-addresses" with all zero adresses and sizes with status =
> "disabled" to the devicetree. A bootloader update is required to fill
> those.
Yes, good point. My plan was originally to have the bootloader/firmware
generate these nodes in their entirety, but yeah, prepopulating them and
having firmware just fill in updated values and setting status = "okay"
seems reasonable to me.
> > +
> > + memset(, 0, sizeof(res));
> > +
> > + /*
> > +* 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, );
> > + 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", );
> > + if (maps) {
> > + const __be32 *end = maps + size / sizeof(__be32);
> > + struct device_node *np;
> > + u
Re: [PATCH v5 5/9] iommu/arm-smmu: Attach to host1x context device bus
On Mon, May 16, 2022 at 02:20:18PM +0300, Mikko Perttunen wrote:
> On 5/16/22 13:44, Robin Murphy wrote:
> > On 2022-05-16 11:13, Mikko Perttunen wrote:
> > > On 5/16/22 13:07, Will Deacon wrote:
> > > > On Mon, May 16, 2022 at 11:52:54AM +0300, cyn...@kapsi.fi wrote:
> > > > > From: Mikko Perttunen
> > > > >
> > > > > Set itself as the IOMMU for the host1x context device bus, containing
> > > > > "dummy" devices used for Host1x context isolation.
> > > > >
> > > > > Signed-off-by: Mikko Perttunen
> > > > > ---
> > > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +
> > > > > 1 file changed, 13 insertions(+)
> > > > >
> > > > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > > > b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > > > index 568cce590ccc..9ff54eaecf81 100644
> > > > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > > > > @@ -39,6 +39,7 @@
> > > > > #include
> > > > > #include
> > > > > +#include
> > > > > #include "arm-smmu.h"
> > > > > @@ -2053,8 +2054,20 @@ static int arm_smmu_bus_init(struct
> > > > > iommu_ops *ops)
> > > > > goto err_reset_pci_ops;
> > > > > }
> > > > > #endif
> > > > > +#ifdef CONFIG_TEGRA_HOST1X_CONTEXT_BUS
> > > > > + if (!iommu_present(_context_device_bus_type)) {
> > > > > + err = bus_set_iommu(_context_device_bus_type, ops);
> > > > > + if (err)
> > > > > + goto err_reset_fsl_mc_ops;
> > > > > + }
> > > > > +#endif
> > > > > +
> > > > > return 0;
> > > > > +err_reset_fsl_mc_ops: __maybe_unused;
> > > > > +#ifdef CONFIG_FSL_MC_BUS
> > > > > + bus_set_iommu(_mc_bus_type, NULL);
> > > > > +#endif
> > > >
> > > > bus_set_iommu() is going away:
> > > >
> > > > https://lore.kernel.org/r/cover.1650890638.git.robin.mur...@arm.com
> > > >
> > > > Will
> > >
> > > Thanks for the heads-up. Robin had pointed out that this work was
> > > ongoing but I hadn't seen the patches yet. I'll look into it.
> >
> > Although that *is* currently blocked on the mystery intel-iommu problem
> > that I can't reproduce... If this series is ready to land right now for
> > 5.19 then in principle that might be the easiest option overall.
> > Hopefully at least patch #2 could sneak in so that the compile-time
> > dependencies are ready for me to roll up host1x into the next rebase of
> > "iommu: Always register bus notifiers".
> >
> > Cheers,
> > Robin.
>
> My guess is that the series as a whole is not ready to land in the 5.19
> timeframe, but #2 could be possible.
>
> Thierry, any opinion?
Dave and Daniel typically want new material to be in by -rc6 and I've
already sent the PR for this cycle. I can ask them if they'd take
another one, though, if it make things simpler for the next cycle.
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 5/5] iommu/tegra-smmu: Support managed domains
From: Navneet Kumar
Allow creating identity and DMA API compatible IOMMU domains. When
creating a DMA API compatible domain, make sure to also create the
required cookie.
Signed-off-by: Navneet Kumar
Signed-off-by: Thierry Reding
---
Changes in v5:
- remove DMA cookie initialization that's now no longer needed
drivers/iommu/tegra-smmu.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 93879c40056c..f8b2b863c111 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -277,7 +278,9 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
- if (type != IOMMU_DOMAIN_UNMANAGED)
+ if (type != IOMMU_DOMAIN_UNMANAGED &&
+ type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_IDENTITY)
return NULL;
as = kzalloc(sizeof(*as), GFP_KERNEL);
@@ -287,25 +290,16 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->attr = SMMU_PD_READABLE | SMMU_PD_WRITABLE | SMMU_PD_NONSECURE;
as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
- if (!as->pd) {
- kfree(as);
- return NULL;
- }
+ if (!as->pd)
+ goto free_as;
as->count = kcalloc(SMMU_NUM_PDE, sizeof(u32), GFP_KERNEL);
- if (!as->count) {
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->count)
+ goto free_pd_range;
as->pts = kcalloc(SMMU_NUM_PDE, sizeof(*as->pts), GFP_KERNEL);
- if (!as->pts) {
- kfree(as->count);
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->pts)
+ goto free_pts;
spin_lock_init(>lock);
@@ -315,6 +309,15 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->domain.geometry.force_aperture = true;
return >domain;
+
+free_pts:
+ kfree(as->pts);
+free_pd_range:
+ __free_page(as->pd);
+free_as:
+ kfree(as);
+
+ return NULL;
}
static void tegra_smmu_domain_free(struct iommu_domain *domain)
@@ -1009,7 +1012,7 @@ static const struct iommu_ops tegra_smmu_ops = {
.probe_device = tegra_smmu_probe_device,
.release_device = tegra_smmu_release_device,
.device_group = tegra_smmu_device_group,
- .get_resv_regions = of_iommu_get_resv_regions,
+ .get_resv_regions = iommu_dma_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.of_xlate = tegra_smmu_of_xlate,
.pgsize_bitmap = SZ_4K,
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 4/5] iommu/tegra-smmu: Add support for reserved regions
From: Thierry Reding
The Tegra DRM driver currently uses the IOMMU API explicitly. This means
that it has fine-grained control over when exactly the translation
through the IOMMU is enabled. This currently happens after the driver
probes, so the driver is in a DMA quiesced state when the IOMMU
translation is enabled.
During the transition of the Tegra DRM driver to use the DMA API instead
of the IOMMU API explicitly, it was observed that on certain platforms
the display controllers were still actively fetching from memory. When a
DMA IOMMU domain is created as part of the DMA/IOMMU API setup during
boot, the IOMMU translation for the display controllers can be enabled a
significant amount of time before the driver has had a chance to reset
the hardware into a sane state. This causes the SMMU to detect faults on
the addresses that the display controller is trying to fetch.
To avoid this, and as a byproduct paving the way for seamless transition
of display from the bootloader to the kernel, add support for reserved
regions in the Tegra SMMU driver. This is implemented using the standard
reserved memory device tree bindings, which let us describe regions of
memory which the kernel is forbidden from using for regular allocations.
The Tegra SMMU driver will parse the nodes associated with each device
via the "memory-region" property and return reserved regions that the
IOMMU core will then create direct mappings for prior to attaching the
IOMMU domains to the devices. This ensures that a 1:1 mapping is in
place when IOMMU translation starts and prevents the SMMU from detecting
any faults.
Signed-off-by: Thierry Reding
---
drivers/iommu/tegra-smmu.c | 47 --
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 2f2b12033618..93879c40056c 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -471,6 +472,7 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu *smmu,
tegra_smmu_free_asid(smmu, as->id);
dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD, DMA_TO_DEVICE);
+ as->pd_dma = 0;
as->smmu = NULL;
@@ -534,6 +536,38 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as,
unsigned long iova,
struct tegra_smmu *smmu = as->smmu;
u32 *pd = page_address(as->pd);
unsigned long offset = pd_index * sizeof(*pd);
+ bool unmap = false;
+
+ /*
+* XXX Move this outside of this function. Perhaps add a struct
+* iommu_domain parameter to ->{get,put}_resv_regions() so that
+* the mapping can be done there.
+*
+* The problem here is that as->smmu is only known once we attach
+* the domain to a device (because then we look up the right SMMU
+* instance via the dev->archdata.iommu pointer). When the direct
+* mappings are created for reserved regions, the domain has not
+* been attached to a device yet, so we don't know. We currently
+* fix that up in ->apply_resv_regions() because that is the first
+* time where we have access to a struct device that will be used
+* with the IOMMU domain. However, that's asymmetric and doesn't
+* take care of the page directory mapping either, so we need to
+* come up with something better.
+*/
+ if (WARN_ON_ONCE(as->pd_dma == 0)) {
+ as->pd_dma = dma_map_page(smmu->dev, as->pd, 0, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(smmu->dev, as->pd_dma))
+ return;
+
+ if (!smmu_dma_addr_valid(smmu, as->pd_dma)) {
+ dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ return;
+ }
+
+ unmap = true;
+ }
/* Set the page directory entry first */
pd[pd_index] = value;
@@ -546,6 +580,12 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as,
unsigned long iova,
smmu_flush_ptc(smmu, as->pd_dma, offset);
smmu_flush_tlb_section(smmu, as->id, iova);
smmu_flush(smmu);
+
+ if (unmap) {
+ dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ as->pd_dma = 0;
+ }
}
static u32 *tegra_smmu_pte_offset(struct page *pt_page, unsigned long iova)
@@ -846,7 +886,6 @@ static struct iommu_device *tegra_smmu_probe_device(struct
device *dev)
smmu = tegra_smmu_find(args.np);
if (smmu) {
err = tegra_smmu_configure(smmu, dev, );
-
if (err < 0) {
[PATCH v5 3/5] iommu: dma: Use of_iommu_get_resv_regions()
From: Thierry Reding For device tree nodes, use the standard of_iommu_get_resv_regions() implementation to obtain the reserved memory regions associated with a device. Signed-off-by: Thierry Reding --- drivers/iommu/dma-iommu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 1ca85d37eeab..3a40ae7a450b 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -386,6 +387,8 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) iort_iommu_msi_get_resv_regions(dev, list); + if (dev->of_node) + of_iommu_get_resv_regions(dev, list); } EXPORT_SYMBOL(iommu_dma_get_resv_regions); -- 2.36.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v5 2/5] iommu: Implement of_iommu_get_resv_regions()
From: Thierry Reding
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.
Signed-off-by: Thierry Reding
---
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 | 90
include/linux/of_iommu.h | 8
2 files changed, 98 insertions(+)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5696314ae69e..9e341b5e307f 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -11,12 +11,15 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
+
#define NO_IOMMU 1
static int of_iommu_xlate(struct device *dev,
@@ -172,3 +175,90 @@ 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)
+{
+#if IS_ENABLED(CONFIG_OF_ADDRESS)
+ struct of_phandle_iterator it;
+ int err;
+
+ of_for_each_phandle(, err, dev->of_node, "memory-region", NULL, 0) {
+ struct iommu_resv_region *region;
+ struct resource res;
+ const __be32 *maps;
+ int size;
+
+ memset(, 0, sizeof(res));
+
+ /*
+* 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, );
+ 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", );
+ if (maps) {
+ const __be32 *end = maps + size / sizeof(__be32);
+ struct device_node *np;
+ unsigned int index = 0;
+ u32 phandle;
+ int na, ns;
+
+ while (maps < end) {
+ phys_addr_t start, end;
+ size_t length;
+
+ phandle = be32_to_cpup(maps++);
+ np = of_find_node_by_phandle(phandle);
+ na = of_n_addr_cells(np);
+ ns = of_n_size_cells(np);
+
+ start = of_translate_dma_address(np, maps);
+ length = of_read_number(maps + na, ns);
+ end = start + length - 1;
+
+ if (np == dev->of_node) {
+ int prot = IOMMU_READ | IOMMU_WRITE;
+ enum iommu_resv_type type;
+
+ /*
+* IOMMU regions without an associated
physical region
+* cannot be mapped and are simply
reservations.
+*/
+ if (res.end > res.start)
+ type =
IOMMU_RESV_DIRECT_RELAXABLE;
+ else
+ type = IOMMU_RESV_RESERVED;
+
+ region = iommu_alloc_resv_region(start,
length, prot, type);
+
[PATCH v5 1/5] dt-bindings: reserved-memory: Document iommu-addresses
From: Thierry Reding
This adds the "iommu-addresses" property to reserved-memory nodes, which
allow describing the interaction of memory regions with IOMMUs. Two use-
cases are supported:
1. Static mappings can be described by pairing the "iommu-addresses"
property with a "reg" property. This is mostly useful for adopting
firmware-allocated buffers via identity mappings. One common use-
case where this is required is if early firmware or bootloaders
have set up a bootsplash framebuffer that a display controller is
actively scanning out from during the operating system boot
process.
2. If an "iommu-addresses" property exists without a "reg" property,
the reserved-memory node describes an IOVA reservation. Such memory
regions are excluded from the IOVA space available to operating
system drivers and can be used for regions that must not be used to
map arbitrary buffers.
Each mapping or reservation is tied to a specific device via a phandle
to the device's device tree node. This allows a reserved-memory region
to be reused across multiple devices.
Signed-off-by: Thierry Reding
---
.../reserved-memory/reserved-memory.txt | 1 -
.../reserved-memory/reserved-memory.yaml | 62 +++
include/dt-bindings/reserved-memory.h | 8 +++
3 files changed, 70 insertions(+), 1 deletion(-)
delete mode 100644
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
create mode 100644 include/dt-bindings/reserved-memory.h
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
deleted file mode 100644
index 1810701a8509..
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ /dev/null
@@ -1 +0,0 @@
-This file has been moved to reserved-memory.yaml.
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
index 7a0744052ff6..3a769aa66e1c 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
@@ -52,6 +52,30 @@ properties:
Address and Length pairs. Specifies regions of memory that are
acceptable to allocate from.
+ iommu-addresses:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+description: >
+ A list of phandle and specifier pairs that describe static IO virtual
+ address space mappings and carveouts associated with a given reserved
+ memory region. The phandle in the first cell refers to the device for
+ which the mapping or carveout is to be created.
+
+ The specifier consists of an address/size pair and denotes the IO
+ virtual address range of the region for the given device. The exact
+ format depends on the values of the "#address-cells" and "#size-cells"
+ properties of the device referenced via the phandle.
+
+ When used in combination with a "reg" property, an IOVA mapping is to
+ be established for this memory region. One example where this can be
+ useful is to create an identity mapping for physical memory that the
+ firmware has configured some hardware to access (such as a bootsplash
+ framebuffer).
+
+ If no "reg" property is specified, the "iommu-addresses" property
+ defines carveout regions in the IOVA space for the given device. This
+ can be useful if a certain memory region should not be mapped through
+ the IOMMU.
+
no-map:
type: boolean
description: >
@@ -97,4 +121,42 @@ oneOf:
additionalProperties: true
+examples:
+ - |
+reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ adsp: reservation-adsp {
+/*
+ * Restrict IOVA mappings for ADSP buffers to the 512 MiB region
+ * from 0x4000 - 0x5fff. Anything outside is reserved by
+ * the ADSP for I/O memory and private memory allocations.
+ */
+iommu-addresses = <0x0 0x 0x00 0x4000>,
+ <0x0 0x6000 0xff 0xa000>;
+ };
+
+ fb: framebuffer@9000 {
+reg = <0x0 0x9000 0x0 0x0080>;
+iommu-addresses = < 0x0 0x9000 0x0 0x0080>;
+ };
+};
+
+bus@0 {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ adsp@299 {
+reg = <0x0 0x299 0x0 0x2000>;
+memory-region = <>;
+ };
+
+ display@1520 {
+reg = <0x0 0x1520 0x0 0x1>;
+memory-region = <>;
+ };
+};
+
...
diff --git a/include/dt-bindings/reserved-memory.h
b/include/dt-bindings/r
[PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions
From: Thierry Reding
Hi,
this is another attempt at solving the problem of passing IOMMU
configuration via device tree. It has significantly evolved since the
last attempt, based on the discussion that followed. The discussion can
be found here:
https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/
Rather than using a memory-region specifier, this new version introduces
a new "iommu-addresses" property for the reserved-memory regions
themselves. These are used to describe either a static mapping or
reservation that should be created for a given device. If both "reg" and
"iommu-addresses" properties are given, a mapping will be created
(typically this would be an identity mapping) whereas if only an
"iommu-addresses" property is specified, a reservation for the specified
range will be installed.
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 0x0080>;
iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
};
/*
* 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 = < 0x0 0x9000 0x0 0x0001>;
};
};
host1x@5000 {
dc@5420 {
memory-region = <>, <>;
};
};
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.
Adding Alyssa and Janne who have in the past tried to make these
bindings work on Apple M1. Also adding Sameer from the Tegra audio team
to look at the ADSP reservation and double-check that this is suitable
for our needs.
Thierry
Navneet Kumar (1):
iommu/tegra-smmu: Support managed domains
Thierry Reding (4):
dt-bindings: reserved-memory: Document iommu-addresses
iommu: Implement of_iommu_get_resv_regions()
iommu: dma: Use of_iommu_get_resv_regions()
iommu/tegra-smmu: Add support for reserved regions
.../reserved-memory/reserved-memory.txt | 1 -
.../reserved-memory/reserved-memory.yaml | 62 +
drivers/iommu/dma-iommu.c | 3 +
drivers/iommu/of_iommu.c | 90 +++
drivers/iommu/tegra-smmu.c| 82 +
include/dt-bindings/reserved-memory.h | 8 ++
include/linux/of_iommu.h | 8 ++
7 files changed, 235 insertions(+), 19 deletions(-)
delete mode 100644
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
create mode 100644 include/dt-bindings/reserved-memory.h
--
2.36.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/3] iommu/arm-smmu: Support Tegra234 SMMU
On Thu, May 05, 2022 at 03:53:08PM +0100, Will Deacon wrote: > On Thu, May 05, 2022 at 04:15:29PM +0200, Thierry Reding wrote: > > On Fri, Apr 29, 2022 at 10:22:40AM +0200, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > Hi Joerg, > > > > > > this is essentially a resend of v2 with a Acked-by:s from Robin and Will > > > added. These have been on the list for quite a while now, but apparently > > > there was a misunderstanding, so neither you nor Will picked this up. > > > > > > Since Will acked these, I think it's probably best for you to pick these > > > up directly. If not, let me know and I'll work with Will to merge via > > > the ARM SMMU tree. > > > > > > Thanks, > > > Thierry > > > > > > Thierry Reding (3): > > > dt-bindings: arm-smmu: Document nvidia,memory-controller property > > > dt-bindings: arm-smmu: Add compatible for Tegra234 SOC > > > iommu/arm-smmu: Support Tegra234 SMMU > > > > > > .../devicetree/bindings/iommu/arm,smmu.yaml | 23 +-- > > > drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 ++- > > > 2 files changed, 23 insertions(+), 3 deletions(-) > > > > Joerg, > > > > anything left to do on this from your perspective, or can this go into > > v5.19? > > I'll pick them up in the Arm SMMU queue, as there are some other SMMU > patches kicking around and we may as well keep them all together. Sounds good, thanks! Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/3] iommu/arm-smmu: Support Tegra234 SMMU
On Fri, Apr 29, 2022 at 10:22:40AM +0200, Thierry Reding wrote: > From: Thierry Reding > > Hi Joerg, > > this is essentially a resend of v2 with a Acked-by:s from Robin and Will > added. These have been on the list for quite a while now, but apparently > there was a misunderstanding, so neither you nor Will picked this up. > > Since Will acked these, I think it's probably best for you to pick these > up directly. If not, let me know and I'll work with Will to merge via > the ARM SMMU tree. > > Thanks, > Thierry > > Thierry Reding (3): > dt-bindings: arm-smmu: Document nvidia,memory-controller property > dt-bindings: arm-smmu: Add compatible for Tegra234 SOC > iommu/arm-smmu: Support Tegra234 SMMU > > .../devicetree/bindings/iommu/arm,smmu.yaml | 23 +-- > drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 ++- > 2 files changed, 23 insertions(+), 3 deletions(-) Joerg, anything left to do on this from your perspective, or can this go into v5.19? Thanks, Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 3/3] iommu/arm-smmu: Support Tegra234 SMMU
From: Thierry Reding Allow the NVIDIA-specific ARM SMMU implementation to bind to the SMMU instances found on Tegra234. Acked-by: Robin Murphy Acked-by: Will Deacon Signed-off-by: Thierry Reding --- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 2c25cce38060..658f3cc83278 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -211,7 +211,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) if (of_property_read_bool(np, "calxeda,smmu-secure-config-access")) smmu->impl = _impl; - if (of_device_is_compatible(np, "nvidia,tegra194-smmu") || + if (of_device_is_compatible(np, "nvidia,tegra234-smmu") || + of_device_is_compatible(np, "nvidia,tegra194-smmu") || of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 2/3] dt-bindings: arm-smmu: Add compatible for Tegra234 SOC
From: Thierry Reding The NVIDIA Tegra234 SoC comes with one single-instance ARM SMMU used by isochronous memory clients and two dual-instance ARM SMMUs used by non- isochronous memory clients. Reviewed-by: Rob Herring Acked-by: Will Deacon Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 44606ad5aa39..590cc8dc8323 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -62,8 +62,9 @@ properties: for improved performance. items: - enum: - - nvidia,tegra194-smmu - nvidia,tegra186-smmu + - nvidia,tegra194-smmu + - nvidia,tegra234-smmu - const: nvidia,smmu-500 - items: - const: arm,mmu-500 @@ -183,8 +184,9 @@ allOf: compatible: contains: enum: - - nvidia,tegra194-smmu - nvidia,tegra186-smmu + - nvidia,tegra194-smmu + - nvidia,tegra234-smmu then: properties: reg: -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/3] dt-bindings: arm-smmu: Document nvidia, memory-controller property
From: Thierry Reding On NVIDIA SoC's the ARM SMMU needs to interact with the memory controller in order to map memory clients to the corresponding stream IDs. Document how the nvidia,memory-controller property can be used to achieve this. Note that this is a backwards-incompatible change that is, however, necessary to ensure correctness. Without the new property, most of the devices would still work but it is not guaranteed that all will. Reviewed-by: Rob Herring Acked-by: Will Deacon Signed-off-by: Thierry Reding --- Changes in v2: - clarify why the new nvidia,memory-controller property is required .../devicetree/bindings/iommu/arm,smmu.yaml | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index da5381c8ee11..44606ad5aa39 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -157,6 +157,17 @@ properties: power-domains: maxItems: 1 + nvidia,memory-controller: +description: | + A phandle to the memory controller on NVIDIA Tegra186 and later SoCs. + The memory controller needs to be programmed with a mapping of memory + client IDs to ARM SMMU stream IDs. + + If this property is absent, the mapping programmed by early firmware + will be used and it is not guaranteed that IOMMU translations will be + enabled for any given device. +$ref: /schemas/types.yaml#/definitions/phandle + required: - compatible - reg @@ -179,6 +190,12 @@ allOf: reg: minItems: 1 maxItems: 2 + + # The reference to the memory controller is required to ensure that the + # memory client to stream ID mapping can be done synchronously with the + # IOMMU attachment. + required: +- nvidia,memory-controller else: properties: reg: -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/3] iommu/arm-smmu: Support Tegra234 SMMU
From: Thierry Reding Hi Joerg, this is essentially a resend of v2 with a Acked-by:s from Robin and Will added. These have been on the list for quite a while now, but apparently there was a misunderstanding, so neither you nor Will picked this up. Since Will acked these, I think it's probably best for you to pick these up directly. If not, let me know and I'll work with Will to merge via the ARM SMMU tree. Thanks, Thierry Thierry Reding (3): dt-bindings: arm-smmu: Document nvidia,memory-controller property dt-bindings: arm-smmu: Add compatible for Tegra234 SOC iommu/arm-smmu: Support Tegra234 SMMU .../devicetree/bindings/iommu/arm,smmu.yaml | 23 +-- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 ++- 2 files changed, 23 insertions(+), 3 deletions(-) -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Fri, Feb 11, 2022 at 12:15:44AM +0100, Janne Grunau wrote: > On 2022-02-09 17:31:16 +0100, Thierry Reding wrote: > > On Sun, Feb 06, 2022 at 11:27:00PM +0100, Janne Grunau wrote: > > > On 2021-09-15 17:19:39 +0200, Thierry Reding wrote: > > > > On Tue, Sep 07, 2021 at 07:44:44PM +0200, Thierry Reding wrote: > > > > > On Tue, Sep 07, 2021 at 10:33:24AM -0500, Rob Herring wrote: > > > > > > On Fri, Sep 3, 2021 at 10:36 AM Thierry Reding > > > > > > wrote: > > > > > > > > > > > > > > On Fri, Sep 03, 2021 at 09:36:33AM -0500, Rob Herring wrote: > > > > > > > > On Fri, Sep 3, 2021 at 8:52 AM Thierry Reding > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote: > > > > > > > > > > > > > > > > > > > > Couldn't we keep this all in /reserved-memory? Just add an > > > > > > > > > > iova > > > > > > > > > > version of reg. Perhaps abuse 'assigned-address' for this > > > > > > > > > > purpose. The > > > > > > > > > > issue I see would be handling reserved iova areas without a > > > > > > > > > > physical > > > > > > > > > > area. That can be handled with just a iova and no reg. We > > > > > > > > > > already have > > > > > > > > > > a no reg case. > > > > > > > > > > > > > > > > > > I had thought about that initially. One thing I'm worried > > > > > > > > > about is that > > > > > > > > > every child node in /reserved-memory will effectively cause > > > > > > > > > the memory > > > > > > > > > that it described to be reserved. But we don't want that for > > > > > > > > > regions > > > > > > > > > that are "virtual only" (i.e. IOMMU reservations). > > > > > > > > > > > > > > > > By virtual only, you mean no physical mapping, just a region of > > > > > > > > virtual space, right? For that we'd have no 'reg' and therefore > > > > > > > > no > > > > > > > > (physical) reservation by the OS. It's similar to non-static > > > > > > > > regions. > > > > > > > > You need a specific handler for them. We'd probably want a > > > > > > > > compatible > > > > > > > > as well for these virtual reservations. > > > > > > > > > > > > > > Yeah, these would be purely used for reserving regions in the > > > > > > > IOVA so > > > > > > > that they won't be used by the IOVA allocator. Typically these > > > > > > > would be > > > > > > > used for cases where those addresses have some special meaning. > > > > > > > > > > > > > > Do we want something like: > > > > > > > > > > > > > > compatible = "iommu-reserved"; > > > > > > > > > > > > > > for these? Or would that need to be: > > > > > > > > > > > > > > compatible = "linux,iommu-reserved"; > > > > > > > > > > > > > > ? There seems to be a mix of vendor-prefix vs. non-vendor-prefix > > > > > > > compatible strings in the reserved-memory DT bindings directory. > > > > > > > > > > > > I would not use 'linux,' here. > > > > > > > > > > > > > > > > > > > > On the other hand, do we actually need the compatible string? > > > > > > > Because we > > > > > > > don't really want to associate much extra information with this > > > > > > > like we > > > > > > > do for example with "shared-dma-pool". The logic to handle this > > > > > > > would > > > > > > > all be within the IOMMU framework. All we really need is for the > > > > > > > standard reservation code to skip nodes that don't have a reg > > > > > > > pr
Re: [Patch v1] iommu: arm-smmu: Use arm-smmu-nvidia impl for Tegra234
On Tue, Mar 29, 2022 at 10:14:36AM +0530, Ashish Mhetre wrote: > Tegra234 has 2 pairs of ARM MMU-500 instances. Each pair is used > together and should be programmed identically. > Add compatible string of Tegra234 iommu nodes in arm_smmu_impl_init() > so that arm-smmu-nvidia implementation will be used for programming > these SMMU instances. > > Signed-off-by: Ashish Mhetre > --- > drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) I already sent out this patch a couple of months ago, though I realize that it still hasn't been applied: http://patchwork.ozlabs.org/project/linux-tegra/list/?series=276030 Joerg, any chance we can still get that series into v5.18? I've already applied patch 4 given that Rob had acked the DT bindings changes. I know it's a bit late, but this has been on the list for a couple of months and has Rob's Reviewed-by on the bindings and Will's Acked-by on the ARM SMMU driver patches. If it's too late for v5.18, is there anything else you're waiting for so that this can go into v5.19? Thanks, Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: arm-smmu: Document nvidia,memory-controller property
On Wed, Feb 16, 2022 at 02:25:20PM +0100, Thierry Reding wrote: > On Thu, Dec 09, 2021 at 05:35:57PM +0100, Thierry Reding wrote: > > From: Thierry Reding > > > > On NVIDIA SoC's the ARM SMMU needs to interact with the memory > > controller in order to map memory clients to the corresponding stream > > IDs. Document how the nvidia,memory-controller property can be used to > > achieve this. > > > > Note that this is a backwards-incompatible change that is, however, > > necessary to ensure correctness. Without the new property, most of the > > devices would still work but it is not guaranteed that all will. > > > > Signed-off-by: Thierry Reding > > --- > > Changes in v2: > > - clarify why the new nvidia,memory-controller property is required > > > > .../devicetree/bindings/iommu/arm,smmu.yaml | 17 + > > 1 file changed, 17 insertions(+) > > Hi Joerg, > > can you pick up patches 1-3 of this series? DT bindings have been > reviewed by Rob and Will acked the ARM SMMU change. I can take the > device tree changes (patch 4) through the Tegra tree. Will, Robin, Joerg, I haven't seen this show up in linux-next yet but was hoping to see this go in for v5.18. Anything I can do to help this move along? Thanks, Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Fix missing put_device() call in tegra_smmu_find
On Fri, Jan 07, 2022 at 08:09:11AM +, Miaoqian Lin wrote:
> The reference taken by 'of_find_device_by_node()' must be released when
> not needed anymore.
> Add the corresponding 'put_device()' in the error handling path.
>
> Fixes: 765a9d1d02b2 ("iommu/tegra-smmu: Fix mc errors on tegra124-nyan")
> Signed-off-by: Miaoqian Lin
> ---
> drivers/iommu/tegra-smmu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index e900e3c46903..2561ce8a2ce8 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -808,8 +808,10 @@ static struct tegra_smmu *tegra_smmu_find(struct
> device_node *np)
> return NULL;
>
> mc = platform_get_drvdata(pdev);
> - if (!mc)
> + if (!mc) {
> + put_device(>dev);
> return NULL;
> + }
>
> return mc->smmu;
> }
Sorry for the late reply, looks correct. We probably also need a similar
call in ->release_device(). I also wonder if we should be returning an
-EPROBE_DEFER here, which is technically the correct thing to do, though
in practice that will likely never happen because these pointers are set
during an arch_initcall, so should always be available by the time a
driver tries to attach to an IOMMU.
Acked-by: Thierry Reding
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: arm-smmu: Document nvidia,memory-controller property
On Thu, Dec 09, 2021 at 05:35:57PM +0100, Thierry Reding wrote: > From: Thierry Reding > > On NVIDIA SoC's the ARM SMMU needs to interact with the memory > controller in order to map memory clients to the corresponding stream > IDs. Document how the nvidia,memory-controller property can be used to > achieve this. > > Note that this is a backwards-incompatible change that is, however, > necessary to ensure correctness. Without the new property, most of the > devices would still work but it is not guaranteed that all will. > > Signed-off-by: Thierry Reding > --- > Changes in v2: > - clarify why the new nvidia,memory-controller property is required > > .../devicetree/bindings/iommu/arm,smmu.yaml | 17 + > 1 file changed, 17 insertions(+) Hi Joerg, can you pick up patches 1-3 of this series? DT bindings have been reviewed by Rob and Will acked the ARM SMMU change. I can take the device tree changes (patch 4) through the Tegra tree. Thanks, Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Sun, Feb 06, 2022 at 11:27:00PM +0100, Janne Grunau wrote: > On 2021-09-15 17:19:39 +0200, Thierry Reding wrote: > > On Tue, Sep 07, 2021 at 07:44:44PM +0200, Thierry Reding wrote: > > > On Tue, Sep 07, 2021 at 10:33:24AM -0500, Rob Herring wrote: > > > > On Fri, Sep 3, 2021 at 10:36 AM Thierry Reding > > > > wrote: > > > > > > > > > > On Fri, Sep 03, 2021 at 09:36:33AM -0500, Rob Herring wrote: > > > > > > On Fri, Sep 3, 2021 at 8:52 AM Thierry Reding > > > > > > wrote: > > > > > > > > > > > > > > On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote: > > > > > > > > > > > > > > > > Couldn't we keep this all in /reserved-memory? Just add an iova > > > > > > > > version of reg. Perhaps abuse 'assigned-address' for this > > > > > > > > purpose. The > > > > > > > > issue I see would be handling reserved iova areas without a > > > > > > > > physical > > > > > > > > area. That can be handled with just a iova and no reg. We > > > > > > > > already have > > > > > > > > a no reg case. > > > > > > > > > > > > > > I had thought about that initially. One thing I'm worried about > > > > > > > is that > > > > > > > every child node in /reserved-memory will effectively cause the > > > > > > > memory > > > > > > > that it described to be reserved. But we don't want that for > > > > > > > regions > > > > > > > that are "virtual only" (i.e. IOMMU reservations). > > > > > > > > > > > > By virtual only, you mean no physical mapping, just a region of > > > > > > virtual space, right? For that we'd have no 'reg' and therefore no > > > > > > (physical) reservation by the OS. It's similar to non-static > > > > > > regions. > > > > > > You need a specific handler for them. We'd probably want a > > > > > > compatible > > > > > > as well for these virtual reservations. > > > > > > > > > > Yeah, these would be purely used for reserving regions in the IOVA so > > > > > that they won't be used by the IOVA allocator. Typically these would > > > > > be > > > > > used for cases where those addresses have some special meaning. > > > > > > > > > > Do we want something like: > > > > > > > > > > compatible = "iommu-reserved"; > > > > > > > > > > for these? Or would that need to be: > > > > > > > > > > compatible = "linux,iommu-reserved"; > > > > > > > > > > ? There seems to be a mix of vendor-prefix vs. non-vendor-prefix > > > > > compatible strings in the reserved-memory DT bindings directory. > > > > > > > > I would not use 'linux,' here. > > > > > > > > > > > > > > On the other hand, do we actually need the compatible string? Because > > > > > we > > > > > don't really want to associate much extra information with this like > > > > > we > > > > > do for example with "shared-dma-pool". The logic to handle this would > > > > > all be within the IOMMU framework. All we really need is for the > > > > > standard reservation code to skip nodes that don't have a reg property > > > > > so we don't reserve memory for "virtual-only" allocations. > > > > > > > > It doesn't hurt to have one and I can imagine we might want to iterate > > > > over all the nodes. It's slightly easier and more common to iterate > > > > over compatible nodes rather than nodes with some property. > > > > > > > > > > Are these being global in DT going to be a problem? Presumably we > > > > > > have > > > > > > a virtual space per IOMMU. We'd know which IOMMU based on a device's > > > > > > 'iommus' and 'memory-region' properties, but within /reserved-memory > > > > > > we wouldn't be able to distinguish overlapping addresses from > > > > > > separate > > > > > > address spaces. Or we could have 2 different IOVA
Re: [PATCH v2 03/11] drm/tegra: vic: Fix DMA API misuse
On Fri, Dec 10, 2021 at 05:54:44PM +, Robin Murphy wrote:
> Upon failure, dma_alloc_coherent() returns NULL. If that does happen,
> passing some uninitialised stack contents to dma_mapping_error() - which
> belongs to a different API in the first place - has precious little
> chance of detecting it.
>
> Also include the correct header, because the fragile transitive
> inclusion currently providing it is going to break soon.
>
> Fixes: 20e7dce255e9 ("drm/tegra: Remove memory allocation from Falcon
> library")
> CC: Thierry Reding
> CC: Mikko Perttunen
> CC: dri-de...@lists.freedesktop.org
> Signed-off-by: Robin Murphy
>
> ---
>
> It also doesn't appear to handle failure of the tegra_drm_alloc() path
> either, but that's a loose thread I have no desire to pull on... ;)
>
> v2: Resend as part of the series, originally posted separately here:
>
> https://lore.kernel.org/dri-devel/2703882439344010e33bf21ecd63cf9e5e6dc00d.1637781007.git.robin.mur...@arm.com/
>
> drivers/gpu/drm/tegra/vic.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
Applied, thanks. I've also fixed up the missing failure handling for
tegra_drm_alloc(), which was actually quite trivial to do.
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 02/11] gpu: host1x: Add missing DMA API include
On Fri, Dec 10, 2021 at 05:54:43PM +, Robin Murphy wrote: > Host1x seems to be relying on picking up dma-mapping.h transitively from > iova.h, which has no reason to include it in the first place. Fix the > former issue before we totally break things by fixing the latter one. > > CC: Thierry Reding > CC: Mikko Perttunen > CC: dri-de...@lists.freedesktop.org > Signed-off-by: Robin Murphy > --- > > v2: No change > > drivers/gpu/host1x/bus.c | 1 + > 1 file changed, 1 insertion(+) Applied, thanks. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 4/4] arm64: tegra: Add Tegra234 IOMMUs
From: Thierry Reding
The NVIDIA Tegra234 SoC comes with one single-instance ARM SMMU used by
isochronous memory clients and two dual-instance ARM SMMUs used by non-
isochronous memory clients.
Add the corresponding device tree nodes and hook up existing memory
clients (SDHCI and BPMP).
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 426 +++
1 file changed, 426 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 3df2217fd826..7acae44a09b8 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -160,6 +160,7 @@ mmc@346 {
interconnects = < TEGRA234_MEMORY_CLIENT_SDMMCRAB
>,
< TEGRA234_MEMORY_CLIENT_SDMMCWAB
>;
interconnect-names = "dma-mem", "write";
+ iommus = <_niso1 TEGRA234_SID_SDMMC4>;
nvidia,pad-autocal-pull-up-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-down-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-up-offset-1v8-timeout = <0x0a>;
@@ -198,6 +199,148 @@ hsp_top0: hsp@3c0 {
#mbox-cells = <2>;
};
+ smmu_niso1: iommu@800 {
+ compatible = "nvidia,tegra234-smmu", "nvidia,smmu-500";
+ reg = <0x800 0x100>,
[PATCH v2 3/4] iommu/arm-smmu: Support Tegra234 SMMU
From: Thierry Reding Allow the NVIDIA-specific ARM SMMU implementation to bind to the SMMU instances found on Tegra234. Signed-off-by: Thierry Reding --- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 2c25cce38060..658f3cc83278 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -211,7 +211,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) if (of_property_read_bool(np, "calxeda,smmu-secure-config-access")) smmu->impl = _impl; - if (of_device_is_compatible(np, "nvidia,tegra194-smmu") || + if (of_device_is_compatible(np, "nvidia,tegra234-smmu") || + of_device_is_compatible(np, "nvidia,tegra194-smmu") || of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); -- 2.34.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/4] dt-bindings: arm-smmu: Add compatible for Tegra234 SOC
From: Thierry Reding The NVIDIA Tegra234 SoC comes with one single-instance ARM SMMU used by isochronous memory clients and two dual-instance ARM SMMUs used by non- isochronous memory clients. Reviewed-by: Rob Herring Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index e239157eb30c..7fd0522bcd84 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -60,8 +60,9 @@ properties: for improved performance. items: - enum: - - nvidia,tegra194-smmu - nvidia,tegra186-smmu + - nvidia,tegra194-smmu + - nvidia,tegra234-smmu - const: nvidia,smmu-500 - items: - const: arm,mmu-500 @@ -181,8 +182,9 @@ allOf: compatible: contains: enum: - - nvidia,tegra194-smmu - nvidia,tegra186-smmu + - nvidia,tegra194-smmu + - nvidia,tegra234-smmu then: properties: reg: -- 2.34.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/4] dt-bindings: arm-smmu: Document nvidia, memory-controller property
From: Thierry Reding On NVIDIA SoC's the ARM SMMU needs to interact with the memory controller in order to map memory clients to the corresponding stream IDs. Document how the nvidia,memory-controller property can be used to achieve this. Note that this is a backwards-incompatible change that is, however, necessary to ensure correctness. Without the new property, most of the devices would still work but it is not guaranteed that all will. Signed-off-by: Thierry Reding --- Changes in v2: - clarify why the new nvidia,memory-controller property is required .../devicetree/bindings/iommu/arm,smmu.yaml | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index f66a3effba73..e239157eb30c 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -155,6 +155,17 @@ properties: power-domains: maxItems: 1 + nvidia,memory-controller: +description: | + A phandle to the memory controller on NVIDIA Tegra186 and later SoCs. + The memory controller needs to be programmed with a mapping of memory + client IDs to ARM SMMU stream IDs. + + If this property is absent, the mapping programmed by early firmware + will be used and it is not guaranteed that IOMMU translations will be + enabled for any given device. +$ref: /schemas/types.yaml#/definitions/phandle + required: - compatible - reg @@ -177,6 +188,12 @@ allOf: reg: minItems: 1 maxItems: 2 + + # The reference to the memory controller is required to ensure that the + # memory client to stream ID mapping can be done synchronously with the + # IOMMU attachment. + required: +- nvidia,memory-controller else: properties: reg: -- 2.34.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/4] iommu/arm-smmu: Support Tegra234 SMMU
On Fri, Nov 12, 2021 at 01:43:54PM +, Robin Murphy wrote: > On 12/11/2021 1:12 pm, Thierry Reding wrote: > > From: Thierry Reding > > > > Hi, > > > > this series of patches adds and enables support for the ARM SMMU > > instances found on the new Tegra234 SoC. This is mostly similar to what > > can be found on Tegra194 except that there are a few more instances to > > meet increased bandwidth needs. > > > > In addition to adding support for the new Tegra234 compatible string, > > this also adds a missing description for the nvidia,memory-controller > > property to the ARM SMMU device tree binding. > > Besides a nitpick about the inconsistent enum ordering in patch #2, Heh, yeah, I'll go fix that up so it's consistent in both the top-level compatible property description and the conditionals. > > Acked-by: Robin Murphy > > for patches #1-3. Thanks! Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/4] arm64: tegra: Add Tegra234 IOMMUs
From: Thierry Reding
The NVIDIA Tegra234 SoC comes with one single-instance ARM SMMU used by
isochronous memory clients and two dual-instance ARM SMMUs used by non-
isochronous memory clients.
Add the corresponding device tree nodes and hook up existing memory
clients (SDHCI and BPMP).
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 426 +++
1 file changed, 426 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 07ab9137f681..104e5fdd5f8a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -160,6 +160,7 @@ mmc@346 {
interconnects = < TEGRA234_MEMORY_CLIENT_SDMMCRAB
>,
< TEGRA234_MEMORY_CLIENT_SDMMCWAB
>;
interconnect-names = "dma-mem", "write";
+ iommus = <_niso1 TEGRA234_SID_SDMMC4>;
nvidia,pad-autocal-pull-up-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-down-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-up-offset-1v8-timeout = <0x0a>;
@@ -198,6 +199,148 @@ hsp_top0: hsp@3c0 {
#mbox-cells = <2>;
};
+ smmu_niso1: iommu@800 {
+ compatible = "nvidia,tegra234-smmu", "nvidia,smmu-500";
+ reg = <0x800 0x100>,
[PATCH 1/4] dt-bindings: arm-smmu: Document nvidia, memory-controller property
From: Thierry Reding On NVIDIA SoC's the ARM SMMU needs to interact with the memory controller in order to map memory clients to the corresponding stream IDs. Document how the nvidia,memory-controller property can be used to achieve this. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index f66a3effba73..cf32a7955475 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -155,6 +155,12 @@ properties: power-domains: maxItems: 1 + nvidia,memory-controller: +description: A phandle to the memory controller on NVIDIA Tegra186 + and later SoCs. The memory controller needs to be programmed with + a mapping of memory client IDs to ARM SMMU stream IDs. +$ref: /schemas/types.yaml#/definitions/phandle + required: - compatible - reg @@ -177,6 +183,9 @@ allOf: reg: minItems: 1 maxItems: 2 + + required: +- nvidia,memory-controller else: properties: reg: -- 2.33.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/4] iommu/arm-smmu: Support Tegra234 SMMU
From: Thierry Reding Allow the NVIDIA-specific ARM SMMU implementation to bind to the SMMU instances found on Tegra234. Signed-off-by: Thierry Reding --- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 2c25cce38060..658f3cc83278 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -211,7 +211,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) if (of_property_read_bool(np, "calxeda,smmu-secure-config-access")) smmu->impl = _impl; - if (of_device_is_compatible(np, "nvidia,tegra194-smmu") || + if (of_device_is_compatible(np, "nvidia,tegra234-smmu") || + of_device_is_compatible(np, "nvidia,tegra194-smmu") || of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); -- 2.33.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/4] dt-bindings: arm-smmu: Add compatible for Tegra234 SOC
From: Thierry Reding The NVIDIA Tegra234 SoC comes with one single-instance ARM SMMU used by isochronous memory clients and two dual-instance ARM SMMUs used by non- isochronous memory clients. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index cf32a7955475..21d293a2dadd 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -60,6 +60,7 @@ properties: for improved performance. items: - enum: + - nvidia,tegra234-smmu - nvidia,tegra194-smmu - nvidia,tegra186-smmu - const: nvidia,smmu-500 @@ -176,8 +177,9 @@ allOf: compatible: contains: enum: - - nvidia,tegra194-smmu - nvidia,tegra186-smmu + - nvidia,tegra194-smmu + - nvidia,tegra234-smmu then: properties: reg: -- 2.33.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/4] iommu/arm-smmu: Support Tegra234 SMMU
From: Thierry Reding Hi, this series of patches adds and enables support for the ARM SMMU instances found on the new Tegra234 SoC. This is mostly similar to what can be found on Tegra194 except that there are a few more instances to meet increased bandwidth needs. In addition to adding support for the new Tegra234 compatible string, this also adds a missing description for the nvidia,memory-controller property to the ARM SMMU device tree binding. I plan on picking up patch 4 into the Tegra tree because it has a dependency (for the stream ID definitions) on a separate patch series for the memory controller that I sent out earlier. Thanks, Thierry Thierry Reding (4): dt-bindings: arm-smmu: Document nvidia,memory-controller property dt-bindings: arm-smmu: Add compatible for Tegra234 SOC iommu/arm-smmu: Support Tegra234 SMMU arm64: tegra: Add Tegra234 IOMMUs .../devicetree/bindings/iommu/arm,smmu.yaml | 13 +- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 426 ++ drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 +- 3 files changed, 440 insertions(+), 2 deletions(-) -- 2.33.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/tegra-smmu: Use devm_bitmap_zalloc when applicable
On Sun, Sep 26, 2021 at 03:07:18PM +0200, Christophe JAILLET wrote: > 'smmu->asids' is a bitmap. So use 'devm_kzalloc()' to simplify code, > improve the semantic of the code and avoid some open-coded arithmetic in > allocator arguments. > > Signed-off-by: Christophe JAILLET > --- > drivers/iommu/tegra-smmu.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 6/6] iommu/tegra-smmu: Add pagetable mappings to debugfs
On Mon, Sep 13, 2021 at 06:38:58PM -0700, Nicolin Chen wrote:
> This patch dumps all active mapping entries from pagetable
> to a debugfs directory named "mappings".
>
> Attaching an example:
>
> SWGROUP: hc
> as->id: 0
> as->attr: R|W|N
> as->pd_dma: 0x80c03000
> {
> [index: 1023] 0xf0080c3e (count: 2)
> {
> PTE RANGE | ATTR | PHYS | IOVA
> | SIZE
> [#1022, #1023] | 0x5 | 0x00010bbf1000 |
> 0xe000 | 0x2000
> }
> }
> Total PDE count: 1
> Total PTE count: 2
>
> Signed-off-by: Nicolin Chen
> ---
> drivers/iommu/tegra-smmu.c | 145 +
> 1 file changed, 145 insertions(+)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 68c34a4a0ecc..aac977e181f6 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -46,6 +46,7 @@ struct tegra_smmu {
> struct list_head list;
>
> struct dentry *debugfs;
> + struct dentry *debugfs_mappings;
>
> struct iommu_device iommu; /* IOMMU Core code handle */
> };
> @@ -153,6 +154,9 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu,
> unsigned long offset)
>
> #define SMMU_PDE_ATTR(SMMU_PDE_READABLE | SMMU_PDE_WRITABLE
> | \
>SMMU_PDE_NONSECURE)
> +#define SMMU_PTE_ATTR(SMMU_PTE_READABLE | SMMU_PTE_WRITABLE
> | \
> + SMMU_PTE_NONSECURE)
> +#define SMMU_PTE_ATTR_SHIFT 29
>
> static unsigned int iova_pd_index(unsigned long iova)
> {
> @@ -164,6 +168,12 @@ static unsigned int iova_pt_index(unsigned long iova)
> return (iova >> SMMU_PTE_SHIFT) & (SMMU_NUM_PTE - 1);
> }
>
> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int
> pt_index)
> +{
> + return ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT |
> +((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT;
> +}
> +
> static bool smmu_dma_addr_valid(struct tegra_smmu *smmu, dma_addr_t addr)
> {
> addr >>= 12;
> @@ -496,6 +506,8 @@ static void tegra_smmu_as_unprepare(struct tegra_smmu
> *smmu,
> mutex_unlock(>lock);
> }
>
> +static const struct file_operations tegra_smmu_debugfs_mappings_fops;
Could the implementation be moved up here to avoid the forward
declaration?
> +
> static void tegra_smmu_attach_as(struct tegra_smmu *smmu,
>struct tegra_smmu_as *as,
>unsigned int swgroup)
> @@ -517,6 +529,12 @@ static void tegra_smmu_attach_as(struct tegra_smmu *smmu,
> dev_warn(smmu->dev,
>"overwriting group->as for swgroup: %s\n",
> swgrp->name);
> group->as = as;
> +
> + if (smmu->debugfs_mappings)
> + debugfs_create_file(group->swgrp->name, 0444,
> + smmu->debugfs_mappings, group,
> + _smmu_debugfs_mappings_fops);
> +
> break;
> }
>
> @@ -541,6 +559,12 @@ static void tegra_smmu_detach_as(struct tegra_smmu *smmu,
> if (group->swgrp != swgrp)
> continue;
> group->as = NULL;
> +
> + if (smmu->debugfs_mappings) {
> + d = debugfs_lookup(group->swgrp->name,
> smmu->debugfs_mappings);
> + debugfs_remove(d);
> + }
> +
> break;
> }
>
> @@ -1124,6 +1148,125 @@ static int tegra_smmu_clients_show(struct seq_file
> *s, void *data)
>
> DEFINE_SHOW_ATTRIBUTE(tegra_smmu_clients);
>
> +static int tegra_smmu_debugfs_mappings_show(struct seq_file *s, void *data)
> +{
> + struct tegra_smmu_group *group = s->private;
> + const struct tegra_smmu_swgroup *swgrp;
> + struct tegra_smmu_as *as;
> + struct tegra_smmu *smmu;
> + unsigned int pd_index;
> + unsigned int pt_index;
> + unsigned long flags;
> + u64 pte_count = 0;
> + u32 pde_count = 0;
> + u32 *pd, val;
> +
> + if (!group || !group->as || !group->swgrp)
> + return 0;
> +
> + swgrp = group->swgrp;
> + smmu = group->smmu;
> + as = group->as;
> +
> + mutex_lock(>lock);
> +
> + val = smmu_readl(smmu, swgrp->reg) & SMMU_ASID_ENABLE;
> + if (!val)
> + goto unlock;
> +
> + pd = page_address(as->pd);
> + if (!pd)
> + goto unlock;
> +
> + seq_printf(s, "\nSWGROUP: %s\n", swgrp->name);
> + seq_printf(s, "as->id: %d\nas->attr: %c|%c|%s\nas->pd_dma: %pad\n",
> as->id,
> +as->attr & SMMU_PD_READABLE ? 'R' : '-',
> +as->attr & SMMU_PD_WRITABLE ? 'W' : '-',
> +as->attr & SMMU_PD_NONSECURE ? "NS" : "S",
> +>pd_dma);
> + seq_puts(s, "{\n");
Maybe this can be more compact by putting the name, ID,
Re: [PATCH v6 5/6] iommu/tegra-smmu: Attach as pointer to tegra_smmu_group
On Mon, Sep 13, 2021 at 06:38:57PM -0700, Nicolin Chen wrote: > This could ease driver to access corresponding as pointer > when having tegra_smmu_group pointer only, which can help > new mappings debugfs nodes. > > Also moving tegra_smmu_find_group_soc() upward, for using > it in new tegra_smmu_attach_as(); and it's better to have > all tegra_smmu_find_* functions together. > > Signed-off-by: Nicolin Chen > --- > drivers/iommu/tegra-smmu.c | 94 +++--- > 1 file changed, 78 insertions(+), 16 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 4/6] iommu/tegra-smmu: Use swgrp pointer instead of swgroup id
On Mon, Sep 13, 2021 at 06:38:56PM -0700, Nicolin Chen wrote: > This patch changes in struct tegra_smmu_group to use swgrp > pointer instead of swgroup, as a preparational change for > the "mappings" debugfs feature. > > Signed-off-by: Nicolin Chen > --- > drivers/iommu/tegra-smmu.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) Seems fine: Acked-by: Thierry Reding signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 3/6] iommu/tegra-smmu: Rename struct tegra_smmu_swgroup *group to *swgrp
On Mon, Sep 13, 2021 at 06:38:55PM -0700, Nicolin Chen wrote:
> There are both tegra_smmu_swgroup and tegra_smmu_group structs
> using "group" for their pointer instances. This gets confusing
> to read the driver sometimes.
>
> So this patch renames "group" of struct tegra_smmu_swgroup to
> "swgrp" as a cleanup. Also renames its "find" function.
>
> Note that we already have "swgroup" being used for an unsigned
> int type variable that is inside struct tegra_smmu_swgroup, so
> it's not able to use "swgroup" but only something like "swgrp".
>
> Signed-off-by: Nicolin Chen
> ---
> drivers/iommu/tegra-smmu.c | 34 +-
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index a32ed347e25d..0f3883045ffa 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -334,35 +334,35 @@ static void tegra_smmu_domain_free(struct iommu_domain
> *domain)
> }
>
> static const struct tegra_smmu_swgroup *
> -tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup)
> +tegra_smmu_find_swgrp(struct tegra_smmu *smmu, unsigned int swgroup)
This makes things inconsistent now. The tegra_smmu_find_swgroup() name
indicates that we're looking for some "swgroup" entity within an "smmu"
object. The entity that we're looking for is a struct tegra_smmu_swgroup
so I think it makes sense to use that full name in the function name.
> {
> - const struct tegra_smmu_swgroup *group = NULL;
> + const struct tegra_smmu_swgroup *swgrp = NULL;
I don't think the existing naming is confusing. The variable name
"group" is consistently used for tegra_smmu_swgroup structures and there
are no cases where we would confuse them with struct tegra_smmu_group
instances.
However, I don't feel strongly about it, so I'm fine with changing the
variable names to "swgrp" if you think that makes things less confusing.
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 2/6] iommu/tegra-smmu: Rename struct tegra_smmu_group_soc *soc to *group_soc
On Mon, Sep 13, 2021 at 06:38:54PM -0700, Nicolin Chen wrote:
> There are both tegra_smmu_soc and tegra_smmu_group_soc using "soc"
> for their pointer instances. This patch renames the one of struct
> tegra_smmu_group_soc from "soc" to "group_soc" to distinguish it.
>
> Signed-off-by: Nicolin Chen
> ---
> drivers/iommu/tegra-smmu.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
I think the context makes it clear which one this is. The "soc" field in
struct tegra_smmu_group clearly refers to the group SoC data, whereas
the "soc" field in struct tegra_smmu refers to the SMMU SoC data.
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 6ebae635d3aa..a32ed347e25d 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -22,7 +22,7 @@
> struct tegra_smmu_group {
> struct list_head list;
> struct tegra_smmu *smmu;
> - const struct tegra_smmu_group_soc *soc;
> + const struct tegra_smmu_group_soc *group_soc;
> struct iommu_group *grp;
> unsigned int swgroup;
> };
> @@ -870,7 +870,7 @@ static struct iommu_device
> *tegra_smmu_probe_device(struct device *dev)
> static void tegra_smmu_release_device(struct device *dev) {}
>
> static const struct tegra_smmu_group_soc *
> -tegra_smmu_find_group(struct tegra_smmu *smmu, unsigned int swgroup)
> +tegra_smmu_find_group_soc(struct tegra_smmu *smmu, unsigned int swgroup)
This one might be okay to disambiguate, but even here I think this isn't
really necessary. It's already clear from the return value what's being
returned.
> {
> unsigned int i, j;
>
> @@ -896,19 +896,20 @@ static struct iommu_group
> *tegra_smmu_device_group(struct device *dev)
> {
> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> struct tegra_smmu *smmu = dev_iommu_priv_get(dev);
> - const struct tegra_smmu_group_soc *soc;
> + const struct tegra_smmu_group_soc *group_soc;
> unsigned int swgroup = fwspec->ids[0];
> struct tegra_smmu_group *group;
> struct iommu_group *grp;
>
> /* Find group_soc associating with swgroup */
> - soc = tegra_smmu_find_group(smmu, swgroup);
> + group_soc = tegra_smmu_find_group_soc(smmu, swgroup);
>
> mutex_lock(>lock);
>
> /* Find existing iommu_group associating with swgroup or group_soc */
> list_for_each_entry(group, >groups, list)
> - if ((group->swgroup == swgroup) || (soc && group->soc == soc)) {
> + if ((group->swgroup == swgroup) ||
> + (group_soc && group->group_soc == group_soc)) {
> grp = iommu_group_ref_get(group->grp);
> mutex_unlock(>lock);
> return grp;
> @@ -921,9 +922,9 @@ static struct iommu_group *tegra_smmu_device_group(struct
> device *dev)
> }
>
> INIT_LIST_HEAD(>list);
> + group->group_soc = group_soc;
> group->swgroup = swgroup;
> group->smmu = smmu;
> - group->soc = soc;
As another example, it's pretty evident that group->soc refers to the
group SoC data rather than the SMMU SoC data. The latter can be obtained
from group->smmu->soc, which again is enough context to make it clear
what this is.
So I don't think this makes things any clearer. It only makes the names
more redundant and awkward to write.
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 1/6] iommu/tegra-smmu: Rename struct iommu_group *group to *grp
On Mon, Sep 13, 2021 at 06:38:53PM -0700, Nicolin Chen wrote: > There are a few structs using "group" for their pointer instances. > This gets confusing sometimes. The instance of struct iommu_group > is used in local function with an alias "grp", which can separate > it from others. > > So this patch simply renames "group" to "grp" as a cleanup. > > Signed-off-by: Nicolin Chen > --- > drivers/iommu/tegra-smmu.c | 16 ---- > 1 file changed, 8 insertions(+), 8 deletions(-) Acked-by: Thierry Reding signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/5] iommu: Support identity mappings of reserved-memory regions
On Sun, Oct 03, 2021 at 04:09:56AM +0300, Dmitry Osipenko wrote: > 23.04.2021 19:32, Thierry Reding пишет: > > I've made corresponding changes in the proprietary bootloader, added a > > compatibility shim in U-Boot (which forwards information created by the > > proprietary bootloader to the kernel) and the attached patches to test > > this on Jetson TX1, Jetson TX2 and Jetson AGX Xavier. > > Could you please tell what downstream kernel does for coping with the > identity mappings in conjunction with the original proprietary bootloader? > > If there is some other method of passing mappings to kernel, could it be > supported by upstream? Putting burden on users to upgrade bootloader > feels a bit inconvenient. It depends on the chip generation. As far as I know there have been several iterations. The earliest was to pass this information via a command-line option, but more recent versions use device tree to pass this information in a similar way as described here. However, these use non-standard DT bindings, so I don't think we can just implement them as-is. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Tue, Sep 07, 2021 at 07:44:44PM +0200, Thierry Reding wrote: > On Tue, Sep 07, 2021 at 10:33:24AM -0500, Rob Herring wrote: > > On Fri, Sep 3, 2021 at 10:36 AM Thierry Reding > > wrote: > > > > > > On Fri, Sep 03, 2021 at 09:36:33AM -0500, Rob Herring wrote: > > > > On Fri, Sep 3, 2021 at 8:52 AM Thierry Reding > > > > wrote: > > > > > > > > > > On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote: > > > > > > On Wed, Sep 1, 2021 at 9:13 AM Thierry Reding > > > > > > wrote: > > > > > > > > > > > > > > On Fri, Jul 02, 2021 at 05:16:25PM +0300, Dmitry Osipenko wrote: > > > > > > > > 01.07.2021 21:14, Thierry Reding пишет: > > > > > > > > > On Tue, Jun 08, 2021 at 06:51:40PM +0200, Thierry Reding > > > > > > > > > wrote: > > > > > > > > >> On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding > > > > > > > > >> wrote: > > > > > > > > >>> On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote: > > > > > > > > >>>> On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding > > > > > > > > >>>> wrote: > > > > > > > > >>>>> From: Thierry Reding > > > > > > > > >>>>> > > > > > > > > >>>>> Reserved memory region phandle references can be > > > > > > > > >>>>> accompanied by a > > > > > > > > >>>>> specifier that provides additional information about how > > > > > > > > >>>>> that specific > > > > > > > > >>>>> reference should be treated. > > > > > > > > >>>>> > > > > > > > > >>>>> One use-case is to mark a memory region as needing an > > > > > > > > >>>>> identity mapping > > > > > > > > >>>>> in the system's IOMMU for the device that references the > > > > > > > > >>>>> region. This is > > > > > > > > >>>>> needed for example when the bootloader has set up > > > > > > > > >>>>> hardware (such as a > > > > > > > > >>>>> display controller) to actively access a memory region > > > > > > > > >>>>> (e.g. a boot > > > > > > > > >>>>> splash screen framebuffer) during boot. The operating > > > > > > > > >>>>> system can use the > > > > > > > > >>>>> identity mapping flag from the specifier to make sure an > > > > > > > > >>>>> IOMMU identity > > > > > > > > >>>>> mapping is set up for the framebuffer before IOMMU > > > > > > > > >>>>> translations are > > > > > > > > >>>>> enabled for the display controller. > > > > > > > > >>>>> > > > > > > > > >>>>> Signed-off-by: Thierry Reding > > > > > > > > >>>>> --- > > > > > > > > >>>>> .../reserved-memory/reserved-memory.txt | 21 > > > > > > > > >>>>> +++ > > > > > > > > >>>>> include/dt-bindings/reserved-memory.h | 8 > > > > > > > > >>>>> +++ > > > > > > > > >>>>> 2 files changed, 29 insertions(+) > > > > > > > > >>>>> create mode 100644 include/dt-bindings/reserved-memory.h > > > > > > > > >>>> > > > > > > > > >>>> Sorry for being slow on this. I have 2 concerns. > > > > > > > > >>>> > > > > > > > > >>>> First, this creates an ABI issue. A DT with cells in > > > > > > > > >>>> 'memory-region' > > > > > > > > >>>> will not be understood by an existing OS. I'm less > > > > > > > > >>>> concerned about this > > >
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Tue, Sep 07, 2021 at 10:33:24AM -0500, Rob Herring wrote: > On Fri, Sep 3, 2021 at 10:36 AM Thierry Reding > wrote: > > > > On Fri, Sep 03, 2021 at 09:36:33AM -0500, Rob Herring wrote: > > > On Fri, Sep 3, 2021 at 8:52 AM Thierry Reding > > > wrote: > > > > > > > > On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote: > > > > > On Wed, Sep 1, 2021 at 9:13 AM Thierry Reding > > > > > wrote: > > > > > > > > > > > > On Fri, Jul 02, 2021 at 05:16:25PM +0300, Dmitry Osipenko wrote: > > > > > > > 01.07.2021 21:14, Thierry Reding пишет: > > > > > > > > On Tue, Jun 08, 2021 at 06:51:40PM +0200, Thierry Reding wrote: > > > > > > > >> On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding wrote: > > > > > > > >>> On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote: > > > > > > > >>>> On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding > > > > > > > >>>> wrote: > > > > > > > >>>>> From: Thierry Reding > > > > > > > >>>>> > > > > > > > >>>>> Reserved memory region phandle references can be > > > > > > > >>>>> accompanied by a > > > > > > > >>>>> specifier that provides additional information about how > > > > > > > >>>>> that specific > > > > > > > >>>>> reference should be treated. > > > > > > > >>>>> > > > > > > > >>>>> One use-case is to mark a memory region as needing an > > > > > > > >>>>> identity mapping > > > > > > > >>>>> in the system's IOMMU for the device that references the > > > > > > > >>>>> region. This is > > > > > > > >>>>> needed for example when the bootloader has set up hardware > > > > > > > >>>>> (such as a > > > > > > > >>>>> display controller) to actively access a memory region > > > > > > > >>>>> (e.g. a boot > > > > > > > >>>>> splash screen framebuffer) during boot. The operating > > > > > > > >>>>> system can use the > > > > > > > >>>>> identity mapping flag from the specifier to make sure an > > > > > > > >>>>> IOMMU identity > > > > > > > >>>>> mapping is set up for the framebuffer before IOMMU > > > > > > > >>>>> translations are > > > > > > > >>>>> enabled for the display controller. > > > > > > > >>>>> > > > > > > > >>>>> Signed-off-by: Thierry Reding > > > > > > > >>>>> --- > > > > > > > >>>>> .../reserved-memory/reserved-memory.txt | 21 > > > > > > > >>>>> +++ > > > > > > > >>>>> include/dt-bindings/reserved-memory.h | 8 +++ > > > > > > > >>>>> 2 files changed, 29 insertions(+) > > > > > > > >>>>> create mode 100644 include/dt-bindings/reserved-memory.h > > > > > > > >>>> > > > > > > > >>>> Sorry for being slow on this. I have 2 concerns. > > > > > > > >>>> > > > > > > > >>>> First, this creates an ABI issue. A DT with cells in > > > > > > > >>>> 'memory-region' > > > > > > > >>>> will not be understood by an existing OS. I'm less concerned > > > > > > > >>>> about this > > > > > > > >>>> if we address that with a stable fix. (Though I'm pretty > > > > > > > >>>> sure we've > > > > > > > >>>> naively added #?-cells in the past ignoring this issue.) > > > > > > > >>> > > > > > > > >>> A while ago I had proposed adding memory-region*s* as an > > > > > > > >>> alternative > > > >
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Fri, Sep 03, 2021 at 09:36:33AM -0500, Rob Herring wrote: > On Fri, Sep 3, 2021 at 8:52 AM Thierry Reding > wrote: > > > > On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote: > > > On Wed, Sep 1, 2021 at 9:13 AM Thierry Reding > > > wrote: > > > > > > > > On Fri, Jul 02, 2021 at 05:16:25PM +0300, Dmitry Osipenko wrote: > > > > > 01.07.2021 21:14, Thierry Reding пишет: > > > > > > On Tue, Jun 08, 2021 at 06:51:40PM +0200, Thierry Reding wrote: > > > > > >> On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding wrote: > > > > > >>> On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote: > > > > > >>>> On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote: > > > > > >>>>> From: Thierry Reding > > > > > >>>>> > > > > > >>>>> Reserved memory region phandle references can be accompanied by > > > > > >>>>> a > > > > > >>>>> specifier that provides additional information about how that > > > > > >>>>> specific > > > > > >>>>> reference should be treated. > > > > > >>>>> > > > > > >>>>> One use-case is to mark a memory region as needing an identity > > > > > >>>>> mapping > > > > > >>>>> in the system's IOMMU for the device that references the > > > > > >>>>> region. This is > > > > > >>>>> needed for example when the bootloader has set up hardware > > > > > >>>>> (such as a > > > > > >>>>> display controller) to actively access a memory region (e.g. a > > > > > >>>>> boot > > > > > >>>>> splash screen framebuffer) during boot. The operating system > > > > > >>>>> can use the > > > > > >>>>> identity mapping flag from the specifier to make sure an IOMMU > > > > > >>>>> identity > > > > > >>>>> mapping is set up for the framebuffer before IOMMU translations > > > > > >>>>> are > > > > > >>>>> enabled for the display controller. > > > > > >>>>> > > > > > >>>>> Signed-off-by: Thierry Reding > > > > > >>>>> --- > > > > > >>>>> .../reserved-memory/reserved-memory.txt | 21 > > > > > >>>>> +++ > > > > > >>>>> include/dt-bindings/reserved-memory.h | 8 +++ > > > > > >>>>> 2 files changed, 29 insertions(+) > > > > > >>>>> create mode 100644 include/dt-bindings/reserved-memory.h > > > > > >>>> > > > > > >>>> Sorry for being slow on this. I have 2 concerns. > > > > > >>>> > > > > > >>>> First, this creates an ABI issue. A DT with cells in > > > > > >>>> 'memory-region' > > > > > >>>> will not be understood by an existing OS. I'm less concerned > > > > > >>>> about this > > > > > >>>> if we address that with a stable fix. (Though I'm pretty sure > > > > > >>>> we've > > > > > >>>> naively added #?-cells in the past ignoring this issue.) > > > > > >>> > > > > > >>> A while ago I had proposed adding memory-region*s* as an > > > > > >>> alternative > > > > > >>> name for memory-region to make the naming more consistent with > > > > > >>> other > > > > > >>> types of properties (think clocks, resets, gpios, ...). If we > > > > > >>> added > > > > > >>> that, we could easily differentiate between the "legacy" cases > > > > > >>> where > > > > > >>> no #memory-region-cells was allowed and the new cases where it > > > > > >>> was. > > > > > >>> > > > > > >>>> Second, it could be the bootloader setting up the reserved > > > > > >>>> region. If a > > &g
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Fri, Sep 03, 2021 at 08:20:55AM -0500, Rob Herring wrote: > On Wed, Sep 1, 2021 at 9:13 AM Thierry Reding > wrote: > > > > On Fri, Jul 02, 2021 at 05:16:25PM +0300, Dmitry Osipenko wrote: > > > 01.07.2021 21:14, Thierry Reding пишет: > > > > On Tue, Jun 08, 2021 at 06:51:40PM +0200, Thierry Reding wrote: > > > >> On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding wrote: > > > >>> On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote: > > > >>>> On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote: > > > >>>>> From: Thierry Reding > > > >>>>> > > > >>>>> Reserved memory region phandle references can be accompanied by a > > > >>>>> specifier that provides additional information about how that > > > >>>>> specific > > > >>>>> reference should be treated. > > > >>>>> > > > >>>>> One use-case is to mark a memory region as needing an identity > > > >>>>> mapping > > > >>>>> in the system's IOMMU for the device that references the region. > > > >>>>> This is > > > >>>>> needed for example when the bootloader has set up hardware (such as > > > >>>>> a > > > >>>>> display controller) to actively access a memory region (e.g. a boot > > > >>>>> splash screen framebuffer) during boot. The operating system can > > > >>>>> use the > > > >>>>> identity mapping flag from the specifier to make sure an IOMMU > > > >>>>> identity > > > >>>>> mapping is set up for the framebuffer before IOMMU translations are > > > >>>>> enabled for the display controller. > > > >>>>> > > > >>>>> Signed-off-by: Thierry Reding > > > >>>>> --- > > > >>>>> .../reserved-memory/reserved-memory.txt | 21 > > > >>>>> +++ > > > >>>>> include/dt-bindings/reserved-memory.h | 8 +++ > > > >>>>> 2 files changed, 29 insertions(+) > > > >>>>> create mode 100644 include/dt-bindings/reserved-memory.h > > > >>>> > > > >>>> Sorry for being slow on this. I have 2 concerns. > > > >>>> > > > >>>> First, this creates an ABI issue. A DT with cells in 'memory-region' > > > >>>> will not be understood by an existing OS. I'm less concerned about > > > >>>> this > > > >>>> if we address that with a stable fix. (Though I'm pretty sure we've > > > >>>> naively added #?-cells in the past ignoring this issue.) > > > >>> > > > >>> A while ago I had proposed adding memory-region*s* as an alternative > > > >>> name for memory-region to make the naming more consistent with other > > > >>> types of properties (think clocks, resets, gpios, ...). If we added > > > >>> that, we could easily differentiate between the "legacy" cases where > > > >>> no #memory-region-cells was allowed and the new cases where it was. > > > >>> > > > >>>> Second, it could be the bootloader setting up the reserved region. > > > >>>> If a > > > >>>> node already has 'memory-region', then adding more regions is more > > > >>>> complicated compared to adding new properties. And defining what each > > > >>>> memory-region entry is or how many in schemas is impossible. > > > >>> > > > >>> It's true that updating the property gets a bit complicated, but it's > > > >>> not exactly rocket science. We really just need to splice the array. I > > > >>> have a working implemention for this in U-Boot. > > > >>> > > > >>> For what it's worth, we could run into the same issue with any new > > > >>> property that we add. Even if we renamed this to iommu-memory-region, > > > >>> it's still possible that a bootloader may have to update this property > > > >>> if it already exists (it could be hard-coded in DT, or it could have > > > >>> been added by some earlier bootloader or firmware). > > > >&g
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Fri, Jul 02, 2021 at 05:16:25PM +0300, Dmitry Osipenko wrote: > 01.07.2021 21:14, Thierry Reding пишет: > > On Tue, Jun 08, 2021 at 06:51:40PM +0200, Thierry Reding wrote: > >> On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding wrote: > >>> On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote: > >>>> On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote: > >>>>> From: Thierry Reding > >>>>> > >>>>> Reserved memory region phandle references can be accompanied by a > >>>>> specifier that provides additional information about how that specific > >>>>> reference should be treated. > >>>>> > >>>>> One use-case is to mark a memory region as needing an identity mapping > >>>>> in the system's IOMMU for the device that references the region. This is > >>>>> needed for example when the bootloader has set up hardware (such as a > >>>>> display controller) to actively access a memory region (e.g. a boot > >>>>> splash screen framebuffer) during boot. The operating system can use the > >>>>> identity mapping flag from the specifier to make sure an IOMMU identity > >>>>> mapping is set up for the framebuffer before IOMMU translations are > >>>>> enabled for the display controller. > >>>>> > >>>>> Signed-off-by: Thierry Reding > >>>>> --- > >>>>> .../reserved-memory/reserved-memory.txt | 21 +++ > >>>>> include/dt-bindings/reserved-memory.h | 8 +++ > >>>>> 2 files changed, 29 insertions(+) > >>>>> create mode 100644 include/dt-bindings/reserved-memory.h > >>>> > >>>> Sorry for being slow on this. I have 2 concerns. > >>>> > >>>> First, this creates an ABI issue. A DT with cells in 'memory-region' > >>>> will not be understood by an existing OS. I'm less concerned about this > >>>> if we address that with a stable fix. (Though I'm pretty sure we've > >>>> naively added #?-cells in the past ignoring this issue.) > >>> > >>> A while ago I had proposed adding memory-region*s* as an alternative > >>> name for memory-region to make the naming more consistent with other > >>> types of properties (think clocks, resets, gpios, ...). If we added > >>> that, we could easily differentiate between the "legacy" cases where > >>> no #memory-region-cells was allowed and the new cases where it was. > >>> > >>>> Second, it could be the bootloader setting up the reserved region. If a > >>>> node already has 'memory-region', then adding more regions is more > >>>> complicated compared to adding new properties. And defining what each > >>>> memory-region entry is or how many in schemas is impossible. > >>> > >>> It's true that updating the property gets a bit complicated, but it's > >>> not exactly rocket science. We really just need to splice the array. I > >>> have a working implemention for this in U-Boot. > >>> > >>> For what it's worth, we could run into the same issue with any new > >>> property that we add. Even if we renamed this to iommu-memory-region, > >>> it's still possible that a bootloader may have to update this property > >>> if it already exists (it could be hard-coded in DT, or it could have > >>> been added by some earlier bootloader or firmware). > >>> > >>>> Both could be addressed with a new property. Perhaps something like > >>>> 'iommu-memory-region = <>;'. I think the 'iommu' prefix is > >>>> appropriate given this is entirely because of the IOMMU being in the > >>>> mix. I might feel differently if we had other uses for cells, but I > >>>> don't really see it in this case. > >>> > >>> I'm afraid that down the road we'll end up with other cases and then we > >>> might proliferate a number of *-memory-region properties with varying > >>> prefixes. > >>> > >>> I am aware of one other case where we might need something like this: on > >>> some Tegra SoCs we have audio processors that will access memory buffers > >>> using a DMA engine. These processors are booted from early firmware > >>> using firmware from system memory. In order to avoid trashi
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Tue, Jun 08, 2021 at 06:51:40PM +0200, Thierry Reding wrote:
> On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding wrote:
> > On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote:
> > > On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote:
> > > > From: Thierry Reding
> > > >
> > > > Reserved memory region phandle references can be accompanied by a
> > > > specifier that provides additional information about how that specific
> > > > reference should be treated.
> > > >
> > > > One use-case is to mark a memory region as needing an identity mapping
> > > > in the system's IOMMU for the device that references the region. This is
> > > > needed for example when the bootloader has set up hardware (such as a
> > > > display controller) to actively access a memory region (e.g. a boot
> > > > splash screen framebuffer) during boot. The operating system can use the
> > > > identity mapping flag from the specifier to make sure an IOMMU identity
> > > > mapping is set up for the framebuffer before IOMMU translations are
> > > > enabled for the display controller.
> > > >
> > > > Signed-off-by: Thierry Reding
> > > > ---
> > > > .../reserved-memory/reserved-memory.txt | 21 +++
> > > > include/dt-bindings/reserved-memory.h | 8 +++
> > > > 2 files changed, 29 insertions(+)
> > > > create mode 100644 include/dt-bindings/reserved-memory.h
> > >
> > > Sorry for being slow on this. I have 2 concerns.
> > >
> > > First, this creates an ABI issue. A DT with cells in 'memory-region'
> > > will not be understood by an existing OS. I'm less concerned about this
> > > if we address that with a stable fix. (Though I'm pretty sure we've
> > > naively added #?-cells in the past ignoring this issue.)
> >
> > A while ago I had proposed adding memory-region*s* as an alternative
> > name for memory-region to make the naming more consistent with other
> > types of properties (think clocks, resets, gpios, ...). If we added
> > that, we could easily differentiate between the "legacy" cases where
> > no #memory-region-cells was allowed and the new cases where it was.
> >
> > > Second, it could be the bootloader setting up the reserved region. If a
> > > node already has 'memory-region', then adding more regions is more
> > > complicated compared to adding new properties. And defining what each
> > > memory-region entry is or how many in schemas is impossible.
> >
> > It's true that updating the property gets a bit complicated, but it's
> > not exactly rocket science. We really just need to splice the array. I
> > have a working implemention for this in U-Boot.
> >
> > For what it's worth, we could run into the same issue with any new
> > property that we add. Even if we renamed this to iommu-memory-region,
> > it's still possible that a bootloader may have to update this property
> > if it already exists (it could be hard-coded in DT, or it could have
> > been added by some earlier bootloader or firmware).
> >
> > > Both could be addressed with a new property. Perhaps something like
> > > 'iommu-memory-region = <>;'. I think the 'iommu' prefix is
> > > appropriate given this is entirely because of the IOMMU being in the
> > > mix. I might feel differently if we had other uses for cells, but I
> > > don't really see it in this case.
> >
> > I'm afraid that down the road we'll end up with other cases and then we
> > might proliferate a number of *-memory-region properties with varying
> > prefixes.
> >
> > I am aware of one other case where we might need something like this: on
> > some Tegra SoCs we have audio processors that will access memory buffers
> > using a DMA engine. These processors are booted from early firmware
> > using firmware from system memory. In order to avoid trashing the
> > firmware, we need to reserve memory. We can do this using reserved
> > memory nodes. However, the audio DMA engine also uses the SMMU, so we
> > need to make sure that the firmware memory is marked as reserved within
> > the SMMU. This is similar to the identity mapping case, but not exactly
> > the same. Instead of creating a 1:1 mapping, we just want that IOVA
> > region to be reserved (i.e. IOMMU_RESV_RESERVED instead of
> > IOMMU_RESV_DIRECT{,_RELAXABLE}).
> >
> > That would also fall into the IOMMU domain, but
Re: [PATCH rdma-next v1 1/2] lib/scatterlist: Fix wrong update of orig_nents
On Wed, Jun 30, 2021 at 01:12:26PM +0200, Marek Szyprowski wrote:
> Hi Leon,
>
> On 29.06.2021 10:40, Leon Romanovsky wrote:
> > From: Maor Gottlieb
> >
> > orig_nents should represent the number of entries with pages,
> > but __sg_alloc_table_from_pages sets orig_nents as the number of
> > total entries in the table. This is wrong when the API is used for
> > dynamic allocation where not all the table entries are mapped with
> > pages. It wasn't observed until now, since RDMA umem who uses this
> > API in the dynamic form doesn't use orig_nents implicit or explicit
> > by the scatterlist APIs.
> >
> > Fix it by:
> > 1. Set orig_nents as number of entries with pages also in
> > __sg_alloc_table_from_pages.
> > 2. Add a new field total_nents to reflect the total number of entries
> > in the table. This is required for the release flow (sg_free_table).
> > This filed should be used internally only by scatterlist.
> >
> > Fixes: 07da1223ec93 ("lib/scatterlist: Add support in dynamic allocation of
> > SG table from pages")
> > Signed-off-by: Maor Gottlieb
> > Signed-off-by: Leon Romanovsky
>
> This patch landed in linux-next 20210630 as commit a52724456928
> ("lib/scatterlist: Fix wrong update of orig_nents"). It causes serious
> regression in DMA-IOMMU integration, which can be observed for example
> on ARM Juno board during boot:
>
> Unable to handle kernel paging request at virtual address 00376f42a6e40454
> Mem abort info:
> ESR = 0x9604
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x0004
> CM = 0, WnR = 0
> [00376f42a6e40454] address between user and kernel address ranges
> Internal error: Oops: 9604 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.13.0-next-20210630+ #3585
> Hardware name: ARM Juno development board (r1) (DT)
> pstate: 8005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> pc : __sg_free_table+0x60/0xa0
> lr : __sg_free_table+0x7c/0xa0
> ..
> Call trace:
> __sg_free_table+0x60/0xa0
> sg_free_table+0x1c/0x28
> iommu_dma_alloc+0xc8/0x388
> dma_alloc_attrs+0xcc/0xf0
> dmam_alloc_attrs+0x68/0xb8
> sil24_port_start+0x60/0xe0
> ata_host_start.part.32+0xbc/0x208
> ata_host_activate+0x64/0x150
> sil24_init_one+0x1e8/0x268
> local_pci_probe+0x3c/0xa0
> pci_device_probe+0x128/0x1c8
> really_probe+0x138/0x2d0
> __driver_probe_device+0x78/0xd8
> driver_probe_device+0x40/0x110
> __driver_attach+0xcc/0x118
> bus_for_each_dev+0x68/0xc8
> driver_attach+0x20/0x28
> bus_add_driver+0x168/0x1f8
> driver_register+0x60/0x110
> __pci_register_driver+0x5c/0x68
> sil24_pci_driver_init+0x20/0x28
> do_one_initcall+0x84/0x450
> kernel_init_freeable+0x31c/0x38c
> kernel_init+0x20/0x120
> ret_from_fork+0x10/0x18
> Code: d37be885 6b01007f 5284 54a2 (f8656813)
> ---[ end trace 4ba4f0c9c48711a1 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
>
> It looks that some changes to the scatterlist structures are missing
> outside of the lib/scatterlist.c.
>
> For now I would suggest to revert this change.
I see a very similar crash on Tegra during the HDA driver's probe.
Leon, let me know if you need a tester for a replacement patch.
Thanks,
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/9] dt-bindings: arm-smmu: Add Tegra186 compatible string
On Mon, Jun 21, 2021 at 04:54:18PM +0100, Will Deacon wrote:
> On Mon, Jun 21, 2021 at 04:11:55PM +0200, Thierry Reding wrote:
> > On Mon, Jun 21, 2021 at 08:46:54AM +0200, Krzysztof Kozlowski wrote:
> > > On 18/06/2021 21:47, Rob Herring wrote:
> > > > On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding
> > > > wrote:
> > > >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > >> b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > >> index 9d27aa5111d4..1181b590db71 100644
> > > >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > > >> @@ -54,8 +54,14 @@ properties:
> > > >>- const: arm,mmu-500
> > > >>- description: NVIDIA SoCs that program two ARM MMU-500s
> > > >> identically
> > > >> items:
> > > >> + - description: NVIDIA SoCs that require memory controller
> > > >> interaction
> > > >
> > > > This is not valid jsonschema:
> > > >
> > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > > > properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one
> > > > must be fixed:
> > > > None is not of type 'object', 'boolean'
> > > > None is not of type 'array'
> > > > from schema $id: http://json-schema.org/draft-07/schema#
> > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one
> > > > must be fixed:
> > > > None is not of type 'object'
> > > > None is not of type 'array'
> > > > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
> > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > > > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one
> > > > must be fixed:
> > > > None is not of type 'object'
> > > > None is not of type 'array'
> > > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> > > > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > > > properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one
> > > > must be fixed:
> > > > [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const':
> > > > 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of
> > > > type 'object'
> > > > {'const': 'nvidia,tegra194-smmu'} is not of type 'string'
> > > > {'const': 'nvidia,tegra186-smmu'} is not of type 'string'
> > > > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> > > >
> > > >
> > > > This was not reviewed nor tested since the DT list was not Cc'ed.
> > >
> > > Ugh, I see now weird empty item on a list... and not only DT list was
> > > skipped - Thierry did not Cc you either.
> >
> > This seemed like a too trivial addition to waste Rob's time on, so I
> > didn't add him (or the DT list for that matter) on Cc. The ARM SMMU
> > maintainers had reviewed this, which seemed like it was enough for what
> > the DT bindings change was doing.
>
> Hmm, I didn't review it. I find the yaml stuff unreadable so I usually
> wait for the DT folks to ack bindings changes before I queue them in the
> SMMU tree.
Alright... in the future I'll make sure to always Cc DT folks, even for
trivial stuff like this.
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/9] dt-bindings: arm-smmu: Add Tegra186 compatible string
On Mon, Jun 21, 2021 at 08:46:54AM +0200, Krzysztof Kozlowski wrote:
> On 18/06/2021 21:47, Rob Herring wrote:
> > On Thu, Jun 3, 2021 at 10:49 AM Thierry Reding
> > wrote:
> >>
> >> From: Thierry Reding
> >>
> >> The ARM SMMU instantiations found on Tegra186 and later need inter-
> >> operation with the memory controller in order to correctly program
> >> stream ID overrides.
> >>
> >> Furthermore, on Tegra194 multiple instances of the SMMU can gang up
> >> to achieve higher throughput. In order to do this, they have to be
> >> programmed identically so that the memory controller can interleave
> >> memory accesses between them.
> >>
> >> Add the Tegra186 compatible string to make sure the interoperation
> >> with the memory controller can be enabled on that SoC generation.
> >>
> >> Signed-off-by: Thierry Reding
> >> ---
> >> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +--
> >> 1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> index 9d27aa5111d4..1181b590db71 100644
> >> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> >> @@ -54,8 +54,14 @@ properties:
> >>- const: arm,mmu-500
> >>- description: NVIDIA SoCs that program two ARM MMU-500s identically
> >> items:
> >> + - description: NVIDIA SoCs that require memory controller
> >> interaction
> >
> > This is not valid jsonschema:
> >
> > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > properties:compatible:oneOf:4:items: 'anyOf' conditional failed, one
> > must be fixed:
> > None is not of type 'object', 'boolean'
> > None is not of type 'array'
> > from schema $id: http://json-schema.org/draft-07/schema#
> > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one
> > must be fixed:
> > None is not of type 'object'
> > None is not of type 'array'
> > from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
> > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > properties:compatible:oneOf:4:items: 'oneOf' conditional failed, one
> > must be fixed:
> > None is not of type 'object'
> > None is not of type 'array'
> > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> > /builds/robherring/linux-dt/Documentation/devicetree/bindings/iommu/arm,smmu.yaml:
> > properties:compatible:oneOf:5:items: 'oneOf' conditional failed, one
> > must be fixed:
> > [{'enum': [{'const': 'nvidia,tegra194-smmu'}, {'const':
> > 'nvidia,tegra186-smmu'}]}, {'const': 'nvidia,smmu-500'}] is not of
> > type 'object'
> > {'const': 'nvidia,tegra194-smmu'} is not of type 'string'
> > {'const': 'nvidia,tegra186-smmu'} is not of type 'string'
> > from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> >
> >
> > This was not reviewed nor tested since the DT list was not Cc'ed.
>
> Ugh, I see now weird empty item on a list... and not only DT list was
> skipped - Thierry did not Cc you either.
This seemed like a too trivial addition to waste Rob's time on, so I
didn't add him (or the DT list for that matter) on Cc. The ARM SMMU
maintainers had reviewed this, which seemed like it was enough for what
the DT bindings change was doing.
In any case, I clearly should've checked the DT binding check output
more carefully. It's rather messy for Tegra because there's quite a few
that we haven't converted yet. I'll have to resume my effort to convert
the remaining ones and fixup the device trees so that we can actually
run the DT binding and DTB validation checks more usefully.
> My bad, I did not check that patch thoroughly before applying.
>
> Thierry, please Cc folks mentioned by get_maintainer.pl. Either sent a
> fix or a revert, if fix needs more time.
I've sent out a follow-up fix that removes the two bogus lines. It looks
like that was the result of a bad conflict resolution on my part.
> Additionally, why the patch changes reg to "minItems: 1" for
> nvidia,tegra194-smmu?
This is because originally the Tegra194 SMMU nodes were supposed to only
represent a "dual" instance. However, on Tegra194 there are three SMMU
instances in total, with the third in
[PATCH] dt-bindings: arm-smmu: Fix json-schema syntax
From: Thierry Reding
Commit 4287861dca9d ("dt-bindings: arm-smmu: Add Tegra186 compatible
string") introduced a jsonschema syntax error as a result of a rebase
gone wrong. Fix it.
Fixes: 4287861dca9d ("dt-bindings: arm-smmu: Add Tegra186 compatible string")
Reported-by: Rob Herring
Signed-off-by: Thierry Reding
---
Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 1181b590db71..03f2b2d4db30 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -52,16 +52,14 @@ properties:
items:
- const: marvell,ap806-smmu-500
- const: arm,mmu-500
- - description: NVIDIA SoCs that program two ARM MMU-500s identically
-items:
- description: NVIDIA SoCs that require memory controller interaction
and may program multiple ARM MMU-500s identically with the memory
controller interleaving translations between multiple instances
for improved performance.
items:
- enum:
- - const: nvidia,tegra194-smmu
- - const: nvidia,tegra186-smmu
+ - nvidia,tegra194-smmu
+ - nvidia,tegra186-smmu
- const: nvidia,smmu-500
- items:
- const: arm,mmu-500
--
2.32.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/9] arm64: tegra: Prevent early SMMU faults
On Fri, Jun 11, 2021 at 08:48:00AM +0200, Krzysztof Kozlowski wrote: > On Thu, 3 Jun 2021 18:46:23 +0200, Thierry Reding wrote: > > this is a set of patches that is the result of earlier discussions > > regarding early identity mappings that are needed to avoid SMMU faults > > during early boot. > > > > The goal here is to avoid early identity mappings altogether and instead > > postpone the need for the identity mappings to when devices are attached > > to the SMMU. This works by making the SMMU driver coordinate with the > > memory controller driver on when to start enforcing SMMU translations. > > This makes Tegra behave in a more standard way and pushes the code to > > deal with the Tegra-specific programming into the NVIDIA SMMU > > implementation. > > > > [...] > > Applied, thanks! > > [1/9] memory: tegra: Implement SID override programming > (no commit info) > [2/9] dt-bindings: arm-smmu: Add Tegra186 compatible string > commit: 4287861dca9d77490ee50de42aa3ada92da86c9d > > [3/9] - skipped > > [4/9] iommu/arm-smmu: tegra: Detect number of instances at runtime > commit: 7ecbf253f8d64c08de28d16a66e3abbe873f6c9f > [5/9] iommu/arm-smmu: tegra: Implement SID override programming > commit: 8eb68595475ac5fcaaa3718a173283df48cb4ef1 > [6/9] iommu/arm-smmu: Use Tegra implementation on Tegra186 > commit: 2c1bc371268862a991a6498e1dddc8971b9076b8 I've applied patches 7-9 to the Tegra tree. Thanks Krzysztof and Will for your help in getting this over the finish line! Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] memory: Tegra memory controller for v5.14
On Thu, Jun 10, 2021 at 04:23:56PM +0200, Krzysztof Kozlowski wrote: > On 10/06/2021 11:19, Thierry Reding wrote: > > On Tue, Jun 08, 2021 at 05:48:51PM +0100, Will Deacon wrote: > >> On Tue, Jun 08, 2021 at 04:38:48PM +0200, Thierry Reding wrote: > >>> On Tue, Jun 08, 2021 at 01:01:29PM +0100, Will Deacon wrote: > >>>> On Mon, Jun 07, 2021 at 10:49:10AM +0200, Krzysztof Kozlowski wrote: > >>>>> This is the pull for you to base further SMMU aptches (prevent early > >>>>> SMMU > >>>>> faults). > >>>> > >>>> This is a tonne of code for me to pull into the SMMU tree given that I > >>>> only > >>>> want one patch! > >>>> > >>>> Thierry, if I just stick: > >>>> > >>>> https://lore.kernel.org/r/20210603164632.1000458-4-thierry.red...@gmail.com > >>>> > >>>> on its own branch, can you stitch together whatever you need? > >>> > >>> I'm not sure I understand what you're proposing. For reference, here's > >>> the set of patches that I sent out: > >>> > >>> 1. memory: tegra: Implement SID override programming > >>> 2. dt-bindings: arm-smmu: Add Tegra186 compatible string > >>> 3. iommu/arm-smmu: Implement ->probe_finalize() > >>> 4. iommu/arm-smmu: tegra: Detect number of instances at runtime > >>> 5. iommu/arm-smmu: tegra: Implement SID override programming > >>> 6. iommu/arm-smmu: Use Tegra implementation on Tegra186 > >>> 7. arm64: tegra: Use correct compatible string for Tegra186 SMMU > >>> 8. arm64: tegra: Hook up memory controller to SMMU on Tegra186 > >>> 9. arm64: tegra: Enable SMMU support on Tegra194 > >>> > >>> Krzysztof already picked up patch 1 and I was assuming that you'd pick > >>> up 2-6 because they are to the ARM SMMU driver. However, if you're > >>> primarily interested in just patch 3, which is more "core" ARM SMMU than > >>> the rest, which are Tegra-specific, then I suppose what we could do is > >>> for you to give an Acked-by on the rest (2, 4-6) and then Krzysztof or I > >>> can pick them up and take them via ARM SoC, based on the stable branch > >>> from your tree that only has patch 3. > >> > >> I think you previously said that patch 5 depends on patch 1, so I can't > >> take 2-6 without also pulling in the memory controller queue. > >> > >>> Patch 6 touches arm-smmu-impl.c, though it's a two-line change that > >>> touches only the Tegra-specific matching bit in arm_smmu_impl_init(), so > >>> the likelihood of that conflicting with anything else is fairly small. > >>> > >>> Is that what you were proposing? > >> > >> I can queue as much or as little of 2-6 as you like, but I would like to > >> avoid pulling in the memory controller queue into the arm smmu tree. But > >> yes, whichever of those I take, I can put them on a separate branch so > >> that you're not blocked for the later patches. > >> > >> You have a better handle on the dependencies, so please tell me what works > >> for you. I just want to make sure that at least patch 3 lands in my tree, > >> so we don't get late conflicts with other driver changes. > > > > Yes, if you could pick up patch 3 and send out a link with the stable > > branch, I think Krzysztof or I could pull in that branch and pick up the > > remaining patches. It'd be good if you could also ack the remaining SMMU > > patches so that ARM SoC knows that they've been sanctioned. > > > > Krzysztof: would you be okay with picking up patches 2 and 4-6 on top of > > your memory branch for v5.14? > > You mean the iommu patches? Yes, I can take them and later explain to > Arnd/Olof why they come through me. Okay, great. Will, can you provide that stable branch? Or would you prefer if I prepared it and sent you a pull request? We're kind of running out of time, since for ARM SoC the cut-off point for new material is usually -rc6 and that's coming up pretty fast. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/io-pgtable-arm: Optimize partial walk flush for large scatter-gather list
On Thu, Jun 10, 2021 at 12:33:56PM +0100, Robin Murphy wrote:
> On 2021-06-10 10:36, Sai Prakash Ranjan wrote:
> > Hi Robin,
> >
> > On 2021-06-10 14:38, Robin Murphy wrote:
> > > On 2021-06-10 06:24, Sai Prakash Ranjan wrote:
> > > > Hi Robin,
> > > >
> > > > On 2021-06-10 00:14, Robin Murphy wrote:
> > > > > On 2021-06-09 15:53, Sai Prakash Ranjan wrote:
> > > > > > Currently for iommu_unmap() of large scatter-gather list
> > > > > > with page size
> > > > > > elements, the majority of time is spent in flushing of
> > > > > > partial walks in
> > > > > > __arm_lpae_unmap() which is a VA based TLB invalidation (TLBIVA for
> > > > > > arm-smmu).
> > > > > >
> > > > > > For example: to unmap a 32MB scatter-gather list with
> > > > > > page size elements
> > > > > > (8192 entries), there are 16->2MB buffer unmaps based on
> > > > > > the pgsize (2MB
> > > > > > for 4K granule) and each of 2MB will further result in
> > > > > > 512 TLBIVAs (2MB/4K)
> > > > > > resulting in a total of 8192 TLBIVAs (512*16) for
> > > > > > 16->2MB causing a huge
> > > > > > overhead.
> > > > > >
> > > > > > So instead use io_pgtable_tlb_flush_all() to invalidate
> > > > > > the entire context
> > > > > > if size (pgsize) is greater than the granule size (4K,
> > > > > > 16K, 64K). For this
> > > > > > example of 32MB scatter-gather list unmap, this results
> > > > > > in just 16 ASID
> > > > > > based TLB invalidations or tlb_flush_all() callback
> > > > > > (TLBIASID in case of
> > > > > > arm-smmu) as opposed to 8192 TLBIVAs thereby increasing
> > > > > > the performance of
> > > > > > unmaps drastically.
> > > > > >
> > > > > > Condition (size > granule size) is chosen for
> > > > > > io_pgtable_tlb_flush_all()
> > > > > > because for any granule with supported pgsizes, we will
> > > > > > have at least 512
> > > > > > TLB invalidations for which tlb_flush_all() is already
> > > > > > recommended. For
> > > > > > example, take 4K granule with 2MB pgsize, this will
> > > > > > result in 512 TLBIVA
> > > > > > in partial walk flush.
> > > > > >
> > > > > > Test on QTI SM8150 SoC for 10 iterations of iommu_{map_sg}/unmap:
> > > > > > (average over 10 iterations)
> > > > > >
> > > > > > Before this optimization:
> > > > > >
> > > > > > size iommu_map_sg iommu_unmap
> > > > > > 4K 2.067 us 1.854 us
> > > > > > 64K 9.598 us 8.802 us
> > > > > > 1M 148.890 us 130.718 us
> > > > > > 2M 305.864 us 67.291 us
> > > > > > 12M 1793.604 us 390.838 us
> > > > > > 16M 2386.848 us 518.187 us
> > > > > > 24M 3563.296 us 775.989 us
> > > > > > 32M 4747.171 us 1033.364 us
> > > > > >
> > > > > > After this optimization:
> > > > > >
> > > > > > size iommu_map_sg iommu_unmap
> > > > > > 4K 1.723 us 1.765 us
> > > > > > 64K 9.880 us 8.869 us
> > > > > > 1M 155.364 us 135.223 us
> > > > > > 2M 303.906 us 5.385 us
> > > > > > 12M 1786.557 us 21.250 us
> > > > > > 16M 2391.890 us 27.437 us
> > > > > > 24M 3570.895 us 39.937 us
> > > > > > 32M 4755.234 us 51.797 us
> > > > > >
> > > > > > This is further reduced once the map/unmap_pages()
> > > > > > support gets in which
> > > > > > will result in just 1 tlb_flush_all() as opposed to 16
> > > > > > tlb_flush_all().
> > > > > >
> > > > > > Signed-off-by: Sai Prakash Ranjan
> > > > > > ---
> > > > > > drivers/iommu/io-pgtable-arm.c | 7 +--
> > > > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/iommu/io-pgtable-arm.c
> > > > > > b/drivers/iommu/io-pgtable-arm.c
> > > > > > index 87def58e79b5..c3cb9add3179 100644
> > > > > > --- a/drivers/iommu/io-pgtable-arm.c
> > > > > > +++ b/drivers/iommu/io-pgtable-arm.c
> > > > > > @@ -589,8 +589,11 @@ static size_t
> > > > > > __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
> > > > > > if (!iopte_leaf(pte, lvl, iop->fmt)) {
> > > > > > /* Also flush any partial walks */
> > > > > > - io_pgtable_tlb_flush_walk(iop, iova, size,
> > > > > > - ARM_LPAE_GRANULE(data));
> > > > > > + if (size > ARM_LPAE_GRANULE(data))
> > > > > > + io_pgtable_tlb_flush_all(iop);
> > > > > > + else
> > > > >
> > > > > Erm, when will the above condition ever not be true? ;)
> > > > >
> > > >
> > > > Ah right, silly me :)
> > > >
> > > > > Taking a step back, though, what about the impact to drivers other
> > > > > than SMMUv2?
> > > >
> > > > Other drivers would be msm_iommu.c, qcom_iommu.c which does the same
> > > > thing as arm-smmu-v2 (page based invalidations), then there is
> > > > ipmmu-vmsa.c
> > > > which does
Re: [GIT PULL] memory: Tegra memory controller for v5.14
On Tue, Jun 08, 2021 at 05:48:51PM +0100, Will Deacon wrote: > On Tue, Jun 08, 2021 at 04:38:48PM +0200, Thierry Reding wrote: > > On Tue, Jun 08, 2021 at 01:01:29PM +0100, Will Deacon wrote: > > > On Mon, Jun 07, 2021 at 10:49:10AM +0200, Krzysztof Kozlowski wrote: > > > > This is the pull for you to base further SMMU aptches (prevent early > > > > SMMU > > > > faults). > > > > > > This is a tonne of code for me to pull into the SMMU tree given that I > > > only > > > want one patch! > > > > > > Thierry, if I just stick: > > > > > > https://lore.kernel.org/r/20210603164632.1000458-4-thierry.red...@gmail.com > > > > > > on its own branch, can you stitch together whatever you need? > > > > I'm not sure I understand what you're proposing. For reference, here's > > the set of patches that I sent out: > > > > 1. memory: tegra: Implement SID override programming > > 2. dt-bindings: arm-smmu: Add Tegra186 compatible string > > 3. iommu/arm-smmu: Implement ->probe_finalize() > > 4. iommu/arm-smmu: tegra: Detect number of instances at runtime > > 5. iommu/arm-smmu: tegra: Implement SID override programming > > 6. iommu/arm-smmu: Use Tegra implementation on Tegra186 > > 7. arm64: tegra: Use correct compatible string for Tegra186 SMMU > > 8. arm64: tegra: Hook up memory controller to SMMU on Tegra186 > > 9. arm64: tegra: Enable SMMU support on Tegra194 > > > > Krzysztof already picked up patch 1 and I was assuming that you'd pick > > up 2-6 because they are to the ARM SMMU driver. However, if you're > > primarily interested in just patch 3, which is more "core" ARM SMMU than > > the rest, which are Tegra-specific, then I suppose what we could do is > > for you to give an Acked-by on the rest (2, 4-6) and then Krzysztof or I > > can pick them up and take them via ARM SoC, based on the stable branch > > from your tree that only has patch 3. > > I think you previously said that patch 5 depends on patch 1, so I can't > take 2-6 without also pulling in the memory controller queue. > > > Patch 6 touches arm-smmu-impl.c, though it's a two-line change that > > touches only the Tegra-specific matching bit in arm_smmu_impl_init(), so > > the likelihood of that conflicting with anything else is fairly small. > > > > Is that what you were proposing? > > I can queue as much or as little of 2-6 as you like, but I would like to > avoid pulling in the memory controller queue into the arm smmu tree. But > yes, whichever of those I take, I can put them on a separate branch so > that you're not blocked for the later patches. > > You have a better handle on the dependencies, so please tell me what works > for you. I just want to make sure that at least patch 3 lands in my tree, > so we don't get late conflicts with other driver changes. Yes, if you could pick up patch 3 and send out a link with the stable branch, I think Krzysztof or I could pull in that branch and pick up the remaining patches. It'd be good if you could also ack the remaining SMMU patches so that ARM SoC knows that they've been sanctioned. Krzysztof: would you be okay with picking up patches 2 and 4-6 on top of your memory branch for v5.14? Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Fri, May 28, 2021 at 06:54:55PM +0200, Thierry Reding wrote:
> On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote:
> > On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding
> > >
> > > Reserved memory region phandle references can be accompanied by a
> > > specifier that provides additional information about how that specific
> > > reference should be treated.
> > >
> > > One use-case is to mark a memory region as needing an identity mapping
> > > in the system's IOMMU for the device that references the region. This is
> > > needed for example when the bootloader has set up hardware (such as a
> > > display controller) to actively access a memory region (e.g. a boot
> > > splash screen framebuffer) during boot. The operating system can use the
> > > identity mapping flag from the specifier to make sure an IOMMU identity
> > > mapping is set up for the framebuffer before IOMMU translations are
> > > enabled for the display controller.
> > >
> > > Signed-off-by: Thierry Reding
> > > ---
> > > .../reserved-memory/reserved-memory.txt | 21 +++
> > > include/dt-bindings/reserved-memory.h | 8 +++
> > > 2 files changed, 29 insertions(+)
> > > create mode 100644 include/dt-bindings/reserved-memory.h
> >
> > Sorry for being slow on this. I have 2 concerns.
> >
> > First, this creates an ABI issue. A DT with cells in 'memory-region'
> > will not be understood by an existing OS. I'm less concerned about this
> > if we address that with a stable fix. (Though I'm pretty sure we've
> > naively added #?-cells in the past ignoring this issue.)
>
> A while ago I had proposed adding memory-region*s* as an alternative
> name for memory-region to make the naming more consistent with other
> types of properties (think clocks, resets, gpios, ...). If we added
> that, we could easily differentiate between the "legacy" cases where
> no #memory-region-cells was allowed and the new cases where it was.
>
> > Second, it could be the bootloader setting up the reserved region. If a
> > node already has 'memory-region', then adding more regions is more
> > complicated compared to adding new properties. And defining what each
> > memory-region entry is or how many in schemas is impossible.
>
> It's true that updating the property gets a bit complicated, but it's
> not exactly rocket science. We really just need to splice the array. I
> have a working implemention for this in U-Boot.
>
> For what it's worth, we could run into the same issue with any new
> property that we add. Even if we renamed this to iommu-memory-region,
> it's still possible that a bootloader may have to update this property
> if it already exists (it could be hard-coded in DT, or it could have
> been added by some earlier bootloader or firmware).
>
> > Both could be addressed with a new property. Perhaps something like
> > 'iommu-memory-region = <>;'. I think the 'iommu' prefix is
> > appropriate given this is entirely because of the IOMMU being in the
> > mix. I might feel differently if we had other uses for cells, but I
> > don't really see it in this case.
>
> I'm afraid that down the road we'll end up with other cases and then we
> might proliferate a number of *-memory-region properties with varying
> prefixes.
>
> I am aware of one other case where we might need something like this: on
> some Tegra SoCs we have audio processors that will access memory buffers
> using a DMA engine. These processors are booted from early firmware
> using firmware from system memory. In order to avoid trashing the
> firmware, we need to reserve memory. We can do this using reserved
> memory nodes. However, the audio DMA engine also uses the SMMU, so we
> need to make sure that the firmware memory is marked as reserved within
> the SMMU. This is similar to the identity mapping case, but not exactly
> the same. Instead of creating a 1:1 mapping, we just want that IOVA
> region to be reserved (i.e. IOMMU_RESV_RESERVED instead of
> IOMMU_RESV_DIRECT{,_RELAXABLE}).
>
> That would also fall into the IOMMU domain, but we can't reuse the
> iommu-memory-region property for that because then we don't have enough
> information to decide which type of reservation we need.
>
> We could obviously make iommu-memory-region take a specifier, but we
> could just as well use memory-regions in that case since we have
> something more generic anyway.
>
> With the #memory-region-cells proposal, we can easily extend the cell in
> the specifier
Re: [GIT PULL] memory: Tegra memory controller for v5.14
On Tue, Jun 08, 2021 at 01:01:29PM +0100, Will Deacon wrote: > Hi Krzysztof, Thierry, > > On Mon, Jun 07, 2021 at 10:49:10AM +0200, Krzysztof Kozlowski wrote: > > Hi Thierry and Will, > > > > This is the pull for you to base further SMMU aptches (prevent early SMMU > > faults). > > This is a tonne of code for me to pull into the SMMU tree given that I only > want one patch! > > Thierry, if I just stick: > > https://lore.kernel.org/r/20210603164632.1000458-4-thierry.red...@gmail.com > > on its own branch, can you stitch together whatever you need? I'm not sure I understand what you're proposing. For reference, here's the set of patches that I sent out: 1. memory: tegra: Implement SID override programming 2. dt-bindings: arm-smmu: Add Tegra186 compatible string 3. iommu/arm-smmu: Implement ->probe_finalize() 4. iommu/arm-smmu: tegra: Detect number of instances at runtime 5. iommu/arm-smmu: tegra: Implement SID override programming 6. iommu/arm-smmu: Use Tegra implementation on Tegra186 7. arm64: tegra: Use correct compatible string for Tegra186 SMMU 8. arm64: tegra: Hook up memory controller to SMMU on Tegra186 9. arm64: tegra: Enable SMMU support on Tegra194 Krzysztof already picked up patch 1 and I was assuming that you'd pick up 2-6 because they are to the ARM SMMU driver. However, if you're primarily interested in just patch 3, which is more "core" ARM SMMU than the rest, which are Tegra-specific, then I suppose what we could do is for you to give an Acked-by on the rest (2, 4-6) and then Krzysztof or I can pick them up and take them via ARM SoC, based on the stable branch from your tree that only has patch 3. Patch 6 touches arm-smmu-impl.c, though it's a two-line change that touches only the Tegra-specific matching bit in arm_smmu_impl_init(), so the likelihood of that conflicting with anything else is fairly small. Is that what you were proposing? Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 9/9] arm64: tegra: Enable SMMU support on Tegra194
From: Thierry Reding
Add the device tree node for the dual-SMMU found on Tegra194 and hook up
peripherals such as host1x, BPMP, HDA, SDMMC, EQOS and VIC.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 86
1 file changed, 86 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index ee71e0d9f895..94e1d8f1a79f 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -62,6 +62,7 @@ ethernet@249 {
interconnects = < TEGRA194_MEMORY_CLIENT_EQOSR >,
< TEGRA194_MEMORY_CLIENT_EQOSW >;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_EQOS>;
status = "disabled";
snps,write-requests = <1>;
@@ -733,6 +734,7 @@ sdmmc1: mmc@340 {
interconnects = < TEGRA194_MEMORY_CLIENT_SDMMCRA
>,
< TEGRA194_MEMORY_CLIENT_SDMMCWA
>;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_SDMMC1>;
nvidia,pad-autocal-pull-up-offset-3v3-timeout =
<0x07>;
nvidia,pad-autocal-pull-down-offset-3v3-timeout =
@@ -759,6 +761,7 @@ sdmmc3: mmc@344 {
interconnects = < TEGRA194_MEMORY_CLIENT_SDMMCR
>,
< TEGRA194_MEMORY_CLIENT_SDMMCW
>;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_SDMMC3>;
nvidia,pad-autocal-pull-up-offset-1v8 = <0x00>;
nvidia,pad-autocal-pull-down-offset-1v8 = <0x7a>;
nvidia,pad-autocal-pull-up-offset-3v3-timeout = <0x07>;
@@ -790,6 +793,7 @@ sdmmc4: mmc@346 {
interconnects = < TEGRA194_MEMORY_CLIENT_SDMMCRAB
>,
< TEGRA194_MEMORY_CLIENT_SDMMCWAB
>;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_SDMMC4>;
nvidia,pad-autocal-pull-up-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-down-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-up-offset-1v8-timeout = <0x0a>;
@@ -821,6 +825,7 @@ hda@351 {
interconnects = < TEGRA194_MEMORY_CLIENT_HDAR >,
< TEGRA194_MEMORY_CLIENT_HDAW >;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_HDA>;
status = "disabled";
};
@@ -1300,6 +1305,84 @@ pmc: pmc@c36 {
interrupt-controller;
};
+
[PATCH v3 8/9] arm64: tegra: Hook up memory controller to SMMU on Tegra186
From: Thierry Reding
On Tegra186 and later, the memory controller needs to be programmed in
coordination with any of the ARM SMMU instances to configure the stream
ID used for each memory client.
To support this, add a phandle reference to the memory controller to the
SMMU device tree node.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index a173f40256ae..d02f6bf3e2ca 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1152,6 +1152,8 @@ smmu: iommu@1200 {
stream-match-mask = <0x7f80>;
#global-interrupts = <1>;
#iommu-cells = <1>;
+
+ nvidia,memory-controller = <>;
};
host1x@13e0 {
--
2.31.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 7/9] arm64: tegra: Use correct compatible string for Tegra186 SMMU
From: Thierry Reding
The SMMU found on Tegra186 requires interoperation with the memory
controller in order to program stream ID overrides. The generic ARM SMMU
500 compatible is therefore inaccurate. Replace it with a more correct,
SoC-specific compatible string.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 9f75bbf00cf7..a173f40256ae 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1082,7 +1082,7 @@ pci@3,0 {
};
smmu: iommu@1200 {
- compatible = "arm,mmu-500";
+ compatible = "nvidia,tegra186-smmu", "nvidia,smmu-500";
reg = <0 0x1200 0 0x80>;
interrupts = ,
,
--
2.31.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 6/9] iommu/arm-smmu: Use Tegra implementation on Tegra186
From: Thierry Reding Tegra186 requires the same SID override programming as Tegra194 in order to seamlessly transition from the firmware framebuffer to the Linux framebuffer, so the Tegra implementation needs to be used on Tegra186 devices as well. Signed-off-by: Thierry Reding --- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 136872e77195..9f465e146799 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -211,7 +211,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) if (of_property_read_bool(np, "calxeda,smmu-secure-config-access")) smmu->impl = _impl; - if (of_device_is_compatible(np, "nvidia,tegra194-smmu")) + if (of_device_is_compatible(np, "nvidia,tegra194-smmu") || + of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); smmu = qcom_smmu_impl_init(smmu); -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 5/9] iommu/arm-smmu: tegra: Implement SID override programming
From: Thierry Reding
The secure firmware keeps some SID override registers set as passthrough
in order to allow devices such as the display controller to operate with
no knowledge of SMMU translations until an operating system driver takes
over. This is needed in order to seamlessly transition from the firmware
framebuffer to the OS framebuffer.
Upon successfully attaching a device to the SMMU and in the process
creating identity mappings for memory regions that are being accessed,
the Tegra implementation will call into the memory controller driver to
program the override SIDs appropriately.
Signed-off-by: Thierry Reding
---
Changes in v3:
- move .probe_finalize initialization to this patch
drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 33 ++--
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
index 23889090eb01..01e9b50b10a1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
@@ -7,6 +7,8 @@
#include
#include
+#include
+
#include "arm-smmu.h"
/*
@@ -15,10 +17,17 @@
* interleaved IOVA accesses across them and translates accesses from
* non-isochronous HW devices.
* Third one is used for translating accesses from isochronous HW devices.
+ *
+ * In addition, the SMMU driver needs to coordinate with the memory controller
+ * driver to ensure that the right SID override is programmed for any given
+ * memory client. This is necessary to allow for use-case such as seamlessly
+ * handing over the display controller configuration from the firmware to the
+ * kernel.
+ *
* This implementation supports programming of the two instances that must
- * be programmed identically.
- * The third instance usage is through standard arm-smmu driver itself and
- * is out of scope of this implementation.
+ * be programmed identically and takes care of invoking the memory controller
+ * driver for SID override programming after devices have been attached to an
+ * SMMU instance.
*/
#define MAX_SMMU_INSTANCES 2
@@ -26,6 +35,7 @@ struct nvidia_smmu {
struct arm_smmu_device smmu;
void __iomem *bases[MAX_SMMU_INSTANCES];
unsigned int num_instances;
+ struct tegra_mc *mc;
};
static inline struct nvidia_smmu *to_nvidia_smmu(struct arm_smmu_device *smmu)
@@ -237,6 +247,17 @@ static irqreturn_t nvidia_smmu_context_fault(int irq, void
*dev)
return ret;
}
+static void nvidia_smmu_probe_finalize(struct arm_smmu_device *smmu, struct
device *dev)
+{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
+ int err;
+
+ err = tegra_mc_probe_device(nvidia->mc, dev);
+ if (err < 0)
+ dev_err(smmu->dev, "memory controller probe failed for %s:
%d\n",
+ dev_name(dev), err);
+}
+
static const struct arm_smmu_impl nvidia_smmu_impl = {
.read_reg = nvidia_smmu_read_reg,
.write_reg = nvidia_smmu_write_reg,
@@ -246,9 +267,11 @@ static const struct arm_smmu_impl nvidia_smmu_impl = {
.tlb_sync = nvidia_smmu_tlb_sync,
.global_fault = nvidia_smmu_global_fault,
.context_fault = nvidia_smmu_context_fault,
+ .probe_finalize = nvidia_smmu_probe_finalize,
};
static const struct arm_smmu_impl nvidia_smmu_single_impl = {
+ .probe_finalize = nvidia_smmu_probe_finalize,
};
struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu)
@@ -263,6 +286,10 @@ struct arm_smmu_device *nvidia_smmu_impl_init(struct
arm_smmu_device *smmu)
if (!nvidia_smmu)
return ERR_PTR(-ENOMEM);
+ nvidia_smmu->mc = devm_tegra_memory_controller_get(dev);
+ if (IS_ERR(nvidia_smmu->mc))
+ return ERR_CAST(nvidia_smmu->mc);
+
/* Instance 0 is ioremapped by arm-smmu.c. */
nvidia_smmu->bases[0] = smmu->base;
nvidia_smmu->num_instances++;
--
2.31.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 3/9] iommu/arm-smmu: Implement ->probe_finalize()
From: Thierry Reding
Implement a ->probe_finalize() callback that can be used by vendor
implementations to perform extra programming necessary after devices
have been attached to the SMMU.
Signed-off-by: Thierry Reding
---
Changes in v2:
- remove unnecessarily paranoid check
drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 6f72c4d208ca..d20ce4d57df2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1450,6 +1450,18 @@ static void arm_smmu_release_device(struct device *dev)
iommu_fwspec_free(dev);
}
+static void arm_smmu_probe_finalize(struct device *dev)
+{
+ struct arm_smmu_master_cfg *cfg;
+ struct arm_smmu_device *smmu;
+
+ cfg = dev_iommu_priv_get(dev);
+ smmu = cfg->smmu;
+
+ if (smmu->impl->probe_finalize)
+ smmu->impl->probe_finalize(smmu, dev);
+}
+
static struct iommu_group *arm_smmu_device_group(struct device *dev)
{
struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
@@ -1569,6 +1581,7 @@ static struct iommu_ops arm_smmu_ops = {
.iova_to_phys = arm_smmu_iova_to_phys,
.probe_device = arm_smmu_probe_device,
.release_device = arm_smmu_release_device,
+ .probe_finalize = arm_smmu_probe_finalize,
.device_group = arm_smmu_device_group,
.enable_nesting = arm_smmu_enable_nesting,
.set_pgtable_quirks = arm_smmu_set_pgtable_quirks,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index c31a59d35c64..147c95e7c59c 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -439,6 +439,7 @@ struct arm_smmu_impl {
struct device *dev, int start);
void (*write_s2cr)(struct arm_smmu_device *smmu, int idx);
void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg);
+ void (*probe_finalize)(struct arm_smmu_device *smmu, struct device
*dev);
};
#define INVALID_SMENDX -1
--
2.31.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 4/9] iommu/arm-smmu: tegra: Detect number of instances at runtime
From: Thierry Reding
Parse the reg property in device tree and detect the number of instances
represented by a device tree node. This is subsequently needed in order
to support single-instance SMMUs with the Tegra implementation because
additional programming is needed to properly configure the SID override
registers in the memory controller.
Signed-off-by: Thierry Reding
---
Changes in v3:
- move .probe_finalize initialization to later patch
Changes in v2:
- provide a separate implementation to simplify single instances
drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 57 ++--
1 file changed, 41 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
index 29117444e5a0..23889090eb01 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
@@ -20,13 +20,19 @@
* The third instance usage is through standard arm-smmu driver itself and
* is out of scope of this implementation.
*/
-#define NUM_SMMU_INSTANCES 2
+#define MAX_SMMU_INSTANCES 2
struct nvidia_smmu {
- struct arm_smmu_device smmu;
- void __iomem*bases[NUM_SMMU_INSTANCES];
+ struct arm_smmu_device smmu;
+ void __iomem *bases[MAX_SMMU_INSTANCES];
+ unsigned int num_instances;
};
+static inline struct nvidia_smmu *to_nvidia_smmu(struct arm_smmu_device *smmu)
+{
+ return container_of(smmu, struct nvidia_smmu, smmu);
+}
+
static inline void __iomem *nvidia_smmu_page(struct arm_smmu_device *smmu,
unsigned int inst, int page)
{
@@ -47,9 +53,10 @@ static u32 nvidia_smmu_read_reg(struct arm_smmu_device *smmu,
static void nvidia_smmu_write_reg(struct arm_smmu_device *smmu,
int page, int offset, u32 val)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
void __iomem *reg = nvidia_smmu_page(smmu, i, page) + offset;
writel_relaxed(val, reg);
@@ -67,9 +74,10 @@ static u64 nvidia_smmu_read_reg64(struct arm_smmu_device
*smmu,
static void nvidia_smmu_write_reg64(struct arm_smmu_device *smmu,
int page, int offset, u64 val)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
void __iomem *reg = nvidia_smmu_page(smmu, i, page) + offset;
writeq_relaxed(val, reg);
@@ -79,6 +87,7 @@ static void nvidia_smmu_write_reg64(struct arm_smmu_device
*smmu,
static void nvidia_smmu_tlb_sync(struct arm_smmu_device *smmu, int page,
int sync, int status)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int delay;
arm_smmu_writel(smmu, page, sync, 0);
@@ -90,7 +99,7 @@ static void nvidia_smmu_tlb_sync(struct arm_smmu_device
*smmu, int page,
u32 val = 0;
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
void __iomem *reg;
reg = nvidia_smmu_page(smmu, i, page) + status;
@@ -112,9 +121,10 @@ static void nvidia_smmu_tlb_sync(struct arm_smmu_device
*smmu, int page,
static int nvidia_smmu_reset(struct arm_smmu_device *smmu)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
u32 val;
void __iomem *reg = nvidia_smmu_page(smmu, i, ARM_SMMU_GR0) +
ARM_SMMU_GR0_sGFSR;
@@ -157,8 +167,9 @@ static irqreturn_t nvidia_smmu_global_fault(int irq, void
*dev)
unsigned int inst;
irqreturn_t ret = IRQ_NONE;
struct arm_smmu_device *smmu = dev;
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
- for (inst = 0; inst < NUM_SMMU_INSTANCES; inst++) {
+ for (inst = 0; inst < nvidia->num_instances; inst++) {
irqreturn_t irq_ret;
irq_ret = nvidia_smmu_global_fault_inst(irq, smmu, inst);
@@ -202,11 +213,13 @@ static irqreturn_t nvidia_smmu_context_fault(int irq,
void *dev)
struct arm_smmu_device *smmu;
struct iommu_domain *domain = dev;
struct arm_smmu_domain *smmu_domain;
+ struct nvidia_smmu *nvidia;
smmu_domain = container_of(domain, struct arm_smmu_domain, domain);
smmu = smmu_domain->smmu;
+ nvidia = to_nvidia_smmu(smmu);
- for (inst = 0; inst <
[PATCH v3 2/9] dt-bindings: arm-smmu: Add Tegra186 compatible string
From: Thierry Reding The ARM SMMU instantiations found on Tegra186 and later need inter- operation with the memory controller in order to correctly program stream ID overrides. Furthermore, on Tegra194 multiple instances of the SMMU can gang up to achieve higher throughput. In order to do this, they have to be programmed identically so that the memory controller can interleave memory accesses between them. Add the Tegra186 compatible string to make sure the interoperation with the memory controller can be enabled on that SoC generation. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 9d27aa5111d4..1181b590db71 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -54,8 +54,14 @@ properties: - const: arm,mmu-500 - description: NVIDIA SoCs that program two ARM MMU-500s identically items: + - description: NVIDIA SoCs that require memory controller interaction + and may program multiple ARM MMU-500s identically with the memory + controller interleaving translations between multiple instances + for improved performance. +items: - enum: - - nvidia,tegra194-smmu + - const: nvidia,tegra194-smmu + - const: nvidia,tegra186-smmu - const: nvidia,smmu-500 - items: - const: arm,mmu-500 @@ -165,10 +171,11 @@ allOf: contains: enum: - nvidia,tegra194-smmu + - nvidia,tegra186-smmu then: properties: reg: - minItems: 2 + minItems: 1 maxItems: 2 else: properties: -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/9] memory: tegra: Implement SID override programming
From: Thierry Reding
Instead of programming all SID overrides during early boot, perform the
operation on-demand after the SMMU translations have been set up for a
device. This reuses data from device tree to match memory clients for a
device and programs the SID specified in device tree, which corresponds
to the SID used for the SMMU context banks for the device.
Signed-off-by: Thierry Reding
---
drivers/memory/tegra/mc.c | 9 +
drivers/memory/tegra/tegra186.c | 72 +
include/soc/tegra/mc.h | 3 ++
3 files changed, 84 insertions(+)
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 11b83de9361c..3c5aae7abf35 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -97,6 +97,15 @@ struct tegra_mc *devm_tegra_memory_controller_get(struct
device *dev)
}
EXPORT_SYMBOL_GPL(devm_tegra_memory_controller_get);
+int tegra_mc_probe_device(struct tegra_mc *mc, struct device *dev)
+{
+ if (mc->soc->ops && mc->soc->ops->probe_device)
+ return mc->soc->ops->probe_device(mc, dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra_mc_probe_device);
+
static int tegra_mc_block_dma_common(struct tegra_mc *mc,
const struct tegra_mc_reset *rst)
{
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 1f87915ccd62..e65eac5764d4 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -4,6 +4,7 @@
*/
#include
+#include
#include
#include
#include
@@ -15,6 +16,10 @@
#include
#endif
+#define MC_SID_STREAMID_OVERRIDE_MASK GENMASK(7, 0)
+#define MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED BIT(16)
+#define MC_SID_STREAMID_SECURITY_OVERRIDE BIT(8)
+
static void tegra186_mc_program_sid(struct tegra_mc *mc)
{
unsigned int i;
@@ -66,10 +71,77 @@ static int tegra186_mc_resume(struct tegra_mc *mc)
return 0;
}
+static void tegra186_mc_client_sid_override(struct tegra_mc *mc,
+ const struct tegra_mc_client
*client,
+ unsigned int sid)
+{
+ u32 value, old;
+
+ value = readl(mc->regs + client->regs.sid.security);
+ if ((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0) {
+ /*
+* If the secure firmware has locked this down the override
+* for this memory client, there's nothing we can do here.
+*/
+ if (value & MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED)
+ return;
+
+ /*
+* Otherwise, try to set the override itself. Typically the
+* secure firmware will never have set this configuration.
+* Instead, it will either have disabled write access to
+* this field, or it will already have set an explicit
+* override itself.
+*/
+ WARN_ON((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0);
+
+ value |= MC_SID_STREAMID_SECURITY_OVERRIDE;
+ writel(value, mc->regs + client->regs.sid.security);
+ }
+
+ value = readl(mc->regs + client->regs.sid.override);
+ old = value & MC_SID_STREAMID_OVERRIDE_MASK;
+
+ if (old != sid) {
+ dev_dbg(mc->dev, "overriding SID %x for %s with %x\n", old,
+ client->name, sid);
+ writel(sid, mc->regs + client->regs.sid.override);
+ }
+}
+
+static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
+{
+#if IS_ENABLED(CONFIG_IOMMU_API)
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct of_phandle_args args;
+ unsigned int i, index = 0;
+
+ while (!of_parse_phandle_with_args(dev->of_node, "interconnects",
"#interconnect-cells",
+ index, )) {
+ if (args.np == mc->dev->of_node && args.args_count != 0) {
+ for (i = 0; i < mc->soc->num_clients; i++) {
+ const struct tegra_mc_client *client =
>soc->clients[i];
+
+ if (client->id == args.args[0]) {
+ u32 sid = fwspec->ids[0] &
MC_SID_STREAMID_OVERRIDE_MASK;
+
+ tegra186_mc_client_sid_override(mc,
client, sid);
+ }
+ }
+ }
+
+ index++;
+ }
+#endif
+
+ return 0;
+}
+
const struct tegra_mc_ops tegra186_mc_ops = {
.probe = tegra186_mc_probe,
.remove = tegra186_mc_remove,
.resume = tegra186_mc_resume,
+ .probe_device = tegra186_mc_probe_device,
};
#if defined(CONFIG
[PATCH v3 0/9] arm64: tegra: Prevent early SMMU faults
From: Thierry Reding Hi, this is a set of patches that is the result of earlier discussions regarding early identity mappings that are needed to avoid SMMU faults during early boot. The goal here is to avoid early identity mappings altogether and instead postpone the need for the identity mappings to when devices are attached to the SMMU. This works by making the SMMU driver coordinate with the memory controller driver on when to start enforcing SMMU translations. This makes Tegra behave in a more standard way and pushes the code to deal with the Tegra-specific programming into the NVIDIA SMMU implementation. Compared to the original version of these patches, I've split the preparatory work into a separate patch series because it became very large and will be mostly uninteresting for this audience. Patch 1 provides a mechanism to program SID overrides at runtime. Patch 2 updates the ARM SMMU device tree bindings to include the Tegra186 compatible string as suggested by Robin during review. Patches 3 and 4 create the fundamentals in the SMMU driver to support this and also make this functionality available on Tegra186. Patch 5 hooks the ARM SMMU up to the memory controller so that the memory client stream ID overrides can be programmed at the right time. Patch 6 extends this mechanism to Tegra186 and patches 7-9 enable all of this through device tree updates. Patch 10 (that was included in earlier version to show how SMMU will be enabled for display controllers) has been dropped for now while waiting for the identity mappings support to land. The end result is that various peripherals will have SMMU enabled, while the display controllers will keep using passthrough, as initially set up by firmware. Once the device tree bindings have been accepted and the SMMU driver has been updated to create identity mappings for the display controllers, they can be hooked up to the SMMU and the code in this series will automatically program the SID overrides to enable SMMU translations at the right time. Will, Krzysztof: as discussed, it'd be best if Krzysztof picked up patch 1 into the memory controller tree on top of v3 of the driver unification series I sent out earlier today and then sent out a PR for Will to merge and apply patches 2-6. I can then take patches 7-9 in via the Tegra tree since there are no hard dependencies. Changes in v3: - move hunk from patch 4 to patch 5 to preserve bisectibility Changes in v2: - split off the preparatory work into a separate series (that needs to be applied first) - address review comments by Robin Thierry Thierry Reding (9): memory: tegra: Implement SID override programming dt-bindings: arm-smmu: Add Tegra186 compatible string iommu/arm-smmu: Implement ->probe_finalize() iommu/arm-smmu: tegra: Detect number of instances at runtime iommu/arm-smmu: tegra: Implement SID override programming iommu/arm-smmu: Use Tegra implementation on Tegra186 arm64: tegra: Use correct compatible string for Tegra186 SMMU arm64: tegra: Hook up memory controller to SMMU on Tegra186 arm64: tegra: Enable SMMU support on Tegra194 .../devicetree/bindings/iommu/arm,smmu.yaml | 11 ++- arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 86 ++ drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 +- drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 90 +++ drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +++ drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + drivers/memory/tegra/mc.c | 9 ++ drivers/memory/tegra/tegra186.c | 72 +++ include/soc/tegra/mc.h| 3 + 10 files changed, 269 insertions(+), 23 deletions(-) -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On Wed, Jun 02, 2021 at 12:40:49PM +0100, Will Deacon wrote: > On Wed, Jun 02, 2021 at 12:44:58PM +0200, Krzysztof Kozlowski wrote: > > On 02/06/2021 10:52, Thierry Reding wrote: > > > On Wed, Jun 02, 2021 at 09:35:13AM +0200, Krzysztof Kozlowski wrote: > > >> On 02/06/2021 09:33, Krzysztof Kozlowski wrote: > > >>> On 01/06/2021 20:08, Thierry Reding wrote: > > >>>> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: > > >>>>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: > > >>>>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: > > >>>>>>> From: Thierry Reding > > >>>>>>> > > >>>>> Probably best if I queue 3-6 on a separate branch once you send a v3, > > >>>>> then Krzysztof can pull that in if he needs it. > > >>>> > > >>>> Patch 5 has a build-time dependency on patch 1, so they need to go in > > >>>> together. The reason why I suggested Krzysztof pick these up is because > > >>>> there is a restructuring series that this depends on, which will go > > >>>> into > > >>>> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of > > >>>> other > > >>>> and mostly unrelated stuff as well. > > >>> > > >>> I missed that part... what other series are needed for this one? Except > > >>> Dmitry's power management set I do not have anything in my sight for > > >>> Tegras memory controllers. > > >>> > > >>> Anyway, I can take the memory bits and provide a stable tag with these. > > >>> Recently there was quite a lot work around Tegra memory controllers, so > > >>> this makes especially sense if new patches appear. > > >> > > >> OK, I think I have now the patchset you talked about - "memory: tegra: > > >> Driver unification" v2, right? > > > > > > Yes, that's the one. That series is fairly self-contained, but Dmitry's > > > power management set has dependencies that pull in the regulator, clock > > > and ARM SoC trees. > > > > > > I did a test merge of the driver unification series with a branch that > > > has Dmitry's patches and all the dependencies and there are no conflicts > > > so that, fortunately, doesn't further complicates things. > > > > > > Do you want me to send you a pull request with Dmitry's memory > > > controller changes? You could then apply the unification series on top, > > > which should allow this SMMU series to apply cleanly on top of that. > > > > Makes sense and it looks quite bulletproof for future changes. Let's do > > like this. I will apply your patch 1/10 from this v2 on top of it and > > driver unification later. > > Okey doke. Thierry -- please let me know which patches I can queue. Having > the SMMU driver changes in the IOMMU tree really helps in case of conflicts > with other SMMU changes. As I say, I can put stuff on a separate branch for > you if it helps. Given that the SMMU patches have a build-time dependency on the first patch in the series, and the series depends on the unification series, I think Krzysztof would have to pull the memory controller branch that I have with Dmitry's work, apply the unification series on top and then take patch 1 of this series on top of that. That's the state that you'd have to pull into the SMMU tree to get the right dependencies. The SMMU pieces are the leaf of the dependency tree, so technically no separate branch is needed, because I don't think anybody would have to pull from it. However, a separate branch might make it easier to back any of this work out if we have to. Krzysztof, I do plan on sending out a v3 of the unification series to address the two cleanups that Dmitry and you have pointed out. After that I can send out v3 of this series to fix the ordering issue that Krishna had mentioned. After all of those are applied, would you be able to provide a stable branch for Will's SMMU tree? Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On Wed, Jun 02, 2021 at 12:44:58PM +0200, Krzysztof Kozlowski wrote: > On 02/06/2021 10:52, Thierry Reding wrote: > > On Wed, Jun 02, 2021 at 09:35:13AM +0200, Krzysztof Kozlowski wrote: > >> On 02/06/2021 09:33, Krzysztof Kozlowski wrote: > >>> On 01/06/2021 20:08, Thierry Reding wrote: > >>>> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: > >>>>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: > >>>>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: > >>>>>>> From: Thierry Reding > >>>>>>> > >>>>> Probably best if I queue 3-6 on a separate branch once you send a v3, > >>>>> then Krzysztof can pull that in if he needs it. > >>>> > >>>> Patch 5 has a build-time dependency on patch 1, so they need to go in > >>>> together. The reason why I suggested Krzysztof pick these up is because > >>>> there is a restructuring series that this depends on, which will go into > >>>> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of other > >>>> and mostly unrelated stuff as well. > >>> > >>> I missed that part... what other series are needed for this one? Except > >>> Dmitry's power management set I do not have anything in my sight for > >>> Tegras memory controllers. > >>> > >>> Anyway, I can take the memory bits and provide a stable tag with these. > >>> Recently there was quite a lot work around Tegra memory controllers, so > >>> this makes especially sense if new patches appear. > >> > >> OK, I think I have now the patchset you talked about - "memory: tegra: > >> Driver unification" v2, right? > > > > Yes, that's the one. That series is fairly self-contained, but Dmitry's > > power management set has dependencies that pull in the regulator, clock > > and ARM SoC trees. > > > > I did a test merge of the driver unification series with a branch that > > has Dmitry's patches and all the dependencies and there are no conflicts > > so that, fortunately, doesn't further complicates things. > > > > Do you want me to send you a pull request with Dmitry's memory > > controller changes? You could then apply the unification series on top, > > which should allow this SMMU series to apply cleanly on top of that. > > Makes sense and it looks quite bulletproof for future changes. Let's do > like this. I will apply your patch 1/10 from this v2 on top of it and > driver unification later. The SMMU series here depends on the unification series, so the unification series needs to go first. It'd be a fair bit of work to reverse that because the ->probe_device() callback implemented by the first patch of this SMMU series is part of the tegra_mc_ops structure that's introduced in the unification series. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On Wed, Jun 02, 2021 at 09:35:13AM +0200, Krzysztof Kozlowski wrote: > On 02/06/2021 09:33, Krzysztof Kozlowski wrote: > > On 01/06/2021 20:08, Thierry Reding wrote: > >> On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: > >>> On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: > >>>> On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: > >>>>> From: Thierry Reding > >>>>> > >>> Probably best if I queue 3-6 on a separate branch once you send a v3, > >>> then Krzysztof can pull that in if he needs it. > >> > >> Patch 5 has a build-time dependency on patch 1, so they need to go in > >> together. The reason why I suggested Krzysztof pick these up is because > >> there is a restructuring series that this depends on, which will go into > >> Krzysztof's tree. So in order to pull in 3-6, you'd get a bunch of other > >> and mostly unrelated stuff as well. > > > > I missed that part... what other series are needed for this one? Except > > Dmitry's power management set I do not have anything in my sight for > > Tegras memory controllers. > > > > Anyway, I can take the memory bits and provide a stable tag with these. > > Recently there was quite a lot work around Tegra memory controllers, so > > this makes especially sense if new patches appear. > > OK, I think I have now the patchset you talked about - "memory: tegra: > Driver unification" v2, right? Yes, that's the one. That series is fairly self-contained, but Dmitry's power management set has dependencies that pull in the regulator, clock and ARM SoC trees. I did a test merge of the driver unification series with a branch that has Dmitry's patches and all the dependencies and there are no conflicts so that, fortunately, doesn't further complicates things. Do you want me to send you a pull request with Dmitry's memory controller changes? You could then apply the unification series on top, which should allow this SMMU series to apply cleanly on top of that. I can also carry all these changes in the Tegra tree and send a PR in a few days once this has seen a bit more testing in linux-next, which also makes sure it's got a bit more testing in our internal test farm. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On Tue, Jun 01, 2021 at 01:26:46PM +0100, Will Deacon wrote: > On Fri, May 28, 2021 at 07:05:28PM +0200, Thierry Reding wrote: > > On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > Hi, > > > > > > this is a set of patches that is the result of earlier discussions > > > regarding early identity mappings that are needed to avoid SMMU faults > > > during early boot. > > > > > > The goal here is to avoid early identity mappings altogether and instead > > > postpone the need for the identity mappings to when devices are attached > > > to the SMMU. This works by making the SMMU driver coordinate with the > > > memory controller driver on when to start enforcing SMMU translations. > > > This makes Tegra behave in a more standard way and pushes the code to > > > deal with the Tegra-specific programming into the NVIDIA SMMU > > > implementation. > > > > > > Compared to the original version of these patches, I've split the > > > preparatory work into a separate patch series because it became very > > > large and will be mostly uninteresting for this audience. > > > > > > Patch 1 provides a mechanism to program SID overrides at runtime. Patch > > > 2 updates the ARM SMMU device tree bindings to include the Tegra186 > > > compatible string as suggested by Robin during review. > > > > > > Patches 3 and 4 create the fundamentals in the SMMU driver to support > > > this and also make this functionality available on Tegra186. Patch 5 > > > hooks the ARM SMMU up to the memory controller so that the memory client > > > stream ID overrides can be programmed at the right time. > > > > > > Patch 6 extends this mechanism to Tegra186 and patches 7-9 enable all of > > > this through device tree updates. Patch 10 is included here to show how > > > SMMU will be enabled for display controllers. However, it cannot be > > > applied yet because the code to create identity mappings for potentially > > > live framebuffers hasn't been merged yet. > > > > > > The end result is that various peripherals will have SMMU enabled, while > > > the display controllers will keep using passthrough, as initially set up > > > by firmware. Once the device tree bindings have been accepted and the > > > SMMU driver has been updated to create identity mappings for the display > > > controllers, they can be hooked up to the SMMU and the code in this > > > series will automatically program the SID overrides to enable SMMU > > > translations at the right time. > > > > > > Note that the series creates a compile time dependency between the > > > memory controller and IOMMU trees. If it helps I can provide a branch > > > for each tree, modelling the dependency, once the series has been > > > reviewed. > > > > > > Changes in v2: > > > - split off the preparatory work into a separate series (that needs to > > > be applied first) > > > - address review comments by Robin > > > > > > Thierry > > > > > > Thierry Reding (10): > > > memory: tegra: Implement SID override programming > > > dt-bindings: arm-smmu: Add Tegra186 compatible string > > > iommu/arm-smmu: Implement ->probe_finalize() > > > iommu/arm-smmu: tegra: Detect number of instances at runtime > > > iommu/arm-smmu: tegra: Implement SID override programming > > > iommu/arm-smmu: Use Tegra implementation on Tegra186 > > > arm64: tegra: Use correct compatible string for Tegra186 SMMU > > > arm64: tegra: Hook up memory controller to SMMU on Tegra186 > > > arm64: tegra: Enable SMMU support on Tegra194 > > > arm64: tegra: Enable SMMU support for display on Tegra194 > > > > > > .../devicetree/bindings/iommu/arm,smmu.yaml | 11 +- > > > arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +- > > > arch/arm64/boot/dts/nvidia/tegra194.dtsi | 166 ++ > > > drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 +- > > > drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 90 -- > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++ > > > drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + > > > drivers/memory/tegra/mc.c | 9 + > > > drivers/memory/tegra/tegra186.c | 72 > > > include/soc/tegra/mc.h| 3 + > > > 10 files
Re: [PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
On Tue, Apr 20, 2021 at 07:26:09PM +0200, Thierry Reding wrote: > From: Thierry Reding > > Hi, > > this is a set of patches that is the result of earlier discussions > regarding early identity mappings that are needed to avoid SMMU faults > during early boot. > > The goal here is to avoid early identity mappings altogether and instead > postpone the need for the identity mappings to when devices are attached > to the SMMU. This works by making the SMMU driver coordinate with the > memory controller driver on when to start enforcing SMMU translations. > This makes Tegra behave in a more standard way and pushes the code to > deal with the Tegra-specific programming into the NVIDIA SMMU > implementation. > > Compared to the original version of these patches, I've split the > preparatory work into a separate patch series because it became very > large and will be mostly uninteresting for this audience. > > Patch 1 provides a mechanism to program SID overrides at runtime. Patch > 2 updates the ARM SMMU device tree bindings to include the Tegra186 > compatible string as suggested by Robin during review. > > Patches 3 and 4 create the fundamentals in the SMMU driver to support > this and also make this functionality available on Tegra186. Patch 5 > hooks the ARM SMMU up to the memory controller so that the memory client > stream ID overrides can be programmed at the right time. > > Patch 6 extends this mechanism to Tegra186 and patches 7-9 enable all of > this through device tree updates. Patch 10 is included here to show how > SMMU will be enabled for display controllers. However, it cannot be > applied yet because the code to create identity mappings for potentially > live framebuffers hasn't been merged yet. > > The end result is that various peripherals will have SMMU enabled, while > the display controllers will keep using passthrough, as initially set up > by firmware. Once the device tree bindings have been accepted and the > SMMU driver has been updated to create identity mappings for the display > controllers, they can be hooked up to the SMMU and the code in this > series will automatically program the SID overrides to enable SMMU > translations at the right time. > > Note that the series creates a compile time dependency between the > memory controller and IOMMU trees. If it helps I can provide a branch > for each tree, modelling the dependency, once the series has been > reviewed. > > Changes in v2: > - split off the preparatory work into a separate series (that needs to > be applied first) > - address review comments by Robin > > Thierry > > Thierry Reding (10): > memory: tegra: Implement SID override programming > dt-bindings: arm-smmu: Add Tegra186 compatible string > iommu/arm-smmu: Implement ->probe_finalize() > iommu/arm-smmu: tegra: Detect number of instances at runtime > iommu/arm-smmu: tegra: Implement SID override programming > iommu/arm-smmu: Use Tegra implementation on Tegra186 > arm64: tegra: Use correct compatible string for Tegra186 SMMU > arm64: tegra: Hook up memory controller to SMMU on Tegra186 > arm64: tegra: Enable SMMU support on Tegra194 > arm64: tegra: Enable SMMU support for display on Tegra194 > > .../devicetree/bindings/iommu/arm,smmu.yaml | 11 +- > arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +- > arch/arm64/boot/dts/nvidia/tegra194.dtsi | 166 ++ > drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 +- > drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 90 -- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++ > drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + > drivers/memory/tegra/mc.c | 9 + > drivers/memory/tegra/tegra186.c | 72 > include/soc/tegra/mc.h| 3 + > 10 files changed, 349 insertions(+), 23 deletions(-) Will, Robin, do you have any more comments on the ARM SMMU bits of this series? If not, can you guys provide an Acked-by so that Krzysztof can pick this (modulo the DT patches) up into the memory-controller tree for v5.14? I'll send out a v3 with the bisectibilitiy fix that Krishna pointed out. Thanks, Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
On Thu, May 20, 2021 at 05:03:06PM -0500, Rob Herring wrote:
> On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > Reserved memory region phandle references can be accompanied by a
> > specifier that provides additional information about how that specific
> > reference should be treated.
> >
> > One use-case is to mark a memory region as needing an identity mapping
> > in the system's IOMMU for the device that references the region. This is
> > needed for example when the bootloader has set up hardware (such as a
> > display controller) to actively access a memory region (e.g. a boot
> > splash screen framebuffer) during boot. The operating system can use the
> > identity mapping flag from the specifier to make sure an IOMMU identity
> > mapping is set up for the framebuffer before IOMMU translations are
> > enabled for the display controller.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > .../reserved-memory/reserved-memory.txt | 21 +++
> > include/dt-bindings/reserved-memory.h | 8 +++
> > 2 files changed, 29 insertions(+)
> > create mode 100644 include/dt-bindings/reserved-memory.h
>
> Sorry for being slow on this. I have 2 concerns.
>
> First, this creates an ABI issue. A DT with cells in 'memory-region'
> will not be understood by an existing OS. I'm less concerned about this
> if we address that with a stable fix. (Though I'm pretty sure we've
> naively added #?-cells in the past ignoring this issue.)
A while ago I had proposed adding memory-region*s* as an alternative
name for memory-region to make the naming more consistent with other
types of properties (think clocks, resets, gpios, ...). If we added
that, we could easily differentiate between the "legacy" cases where
no #memory-region-cells was allowed and the new cases where it was.
> Second, it could be the bootloader setting up the reserved region. If a
> node already has 'memory-region', then adding more regions is more
> complicated compared to adding new properties. And defining what each
> memory-region entry is or how many in schemas is impossible.
It's true that updating the property gets a bit complicated, but it's
not exactly rocket science. We really just need to splice the array. I
have a working implemention for this in U-Boot.
For what it's worth, we could run into the same issue with any new
property that we add. Even if we renamed this to iommu-memory-region,
it's still possible that a bootloader may have to update this property
if it already exists (it could be hard-coded in DT, or it could have
been added by some earlier bootloader or firmware).
> Both could be addressed with a new property. Perhaps something like
> 'iommu-memory-region = <>;'. I think the 'iommu' prefix is
> appropriate given this is entirely because of the IOMMU being in the
> mix. I might feel differently if we had other uses for cells, but I
> don't really see it in this case.
I'm afraid that down the road we'll end up with other cases and then we
might proliferate a number of *-memory-region properties with varying
prefixes.
I am aware of one other case where we might need something like this: on
some Tegra SoCs we have audio processors that will access memory buffers
using a DMA engine. These processors are booted from early firmware
using firmware from system memory. In order to avoid trashing the
firmware, we need to reserve memory. We can do this using reserved
memory nodes. However, the audio DMA engine also uses the SMMU, so we
need to make sure that the firmware memory is marked as reserved within
the SMMU. This is similar to the identity mapping case, but not exactly
the same. Instead of creating a 1:1 mapping, we just want that IOVA
region to be reserved (i.e. IOMMU_RESV_RESERVED instead of
IOMMU_RESV_DIRECT{,_RELAXABLE}).
That would also fall into the IOMMU domain, but we can't reuse the
iommu-memory-region property for that because then we don't have enough
information to decide which type of reservation we need.
We could obviously make iommu-memory-region take a specifier, but we
could just as well use memory-regions in that case since we have
something more generic anyway.
With the #memory-region-cells proposal, we can easily extend the cell in
the specifier with an additional MEMORY_REGION_IOMMU_RESERVE flag to
take that other use case into account. If we than also change to the new
memory-regions property name, we avoid the ABI issue (and we gain a bit
of consistency while at it).
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 01/10] memory: tegra: Implement SID override programming
On Mon, Apr 26, 2021 at 10:28:43AM +0200, Krzysztof Kozlowski wrote:
> On 20/04/2021 19:26, Thierry Reding wrote:
> > From: Thierry Reding
> >
> > Instead of programming all SID overrides during early boot, perform the
> > operation on-demand after the SMMU translations have been set up for a
> > device. This reuses data from device tree to match memory clients for a
> > device and programs the SID specified in device tree, which corresponds
> > to the SID used for the SMMU context banks for the device.
> >
> > Signed-off-by: Thierry Reding
> > ---
> > drivers/memory/tegra/mc.c | 9 +
> > drivers/memory/tegra/tegra186.c | 72 +
> > include/soc/tegra/mc.h | 3 ++
> > 3 files changed, 84 insertions(+)
> >
> > diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> > index c854639cf30c..bace5ecfe770 100644
> > --- a/drivers/memory/tegra/mc.c
> > +++ b/drivers/memory/tegra/mc.c
> > @@ -97,6 +97,15 @@ struct tegra_mc *devm_tegra_memory_controller_get(struct
> > device *dev)
> > }
> > EXPORT_SYMBOL_GPL(devm_tegra_memory_controller_get);
> >
> > +int tegra_mc_probe_device(struct tegra_mc *mc, struct device *dev)
> > +{
> > + if (mc->soc->ops && mc->soc->ops->probe_device)
> > + return mc->soc->ops->probe_device(mc, dev);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(tegra_mc_probe_device);
> > +
> > static int tegra_mc_block_dma_common(struct tegra_mc *mc,
> > const struct tegra_mc_reset *rst)
> > {
> > diff --git a/drivers/memory/tegra/tegra186.c
> > b/drivers/memory/tegra/tegra186.c
> > index 1f87915ccd62..e65eac5764d4 100644
> > --- a/drivers/memory/tegra/tegra186.c
> > +++ b/drivers/memory/tegra/tegra186.c
> > @@ -4,6 +4,7 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -15,6 +16,10 @@
> > #include
> > #endif
> >
> > +#define MC_SID_STREAMID_OVERRIDE_MASK GENMASK(7, 0)
> > +#define MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED BIT(16)
> > +#define MC_SID_STREAMID_SECURITY_OVERRIDE BIT(8)
> > +
> > static void tegra186_mc_program_sid(struct tegra_mc *mc)
> > {
> > unsigned int i;
> > @@ -66,10 +71,77 @@ static int tegra186_mc_resume(struct tegra_mc *mc)
> > return 0;
> > }
> >
> > +static void tegra186_mc_client_sid_override(struct tegra_mc *mc,
> > + const struct tegra_mc_client
> > *client,
> > + unsigned int sid)
> > +{
> > + u32 value, old;
> > +
> > + value = readl(mc->regs + client->regs.sid.security);
> > + if ((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0) {
> > + /*
> > +* If the secure firmware has locked this down the override
> > +* for this memory client, there's nothing we can do here.
> > +*/
> > + if (value & MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED)
> > + return;
> > +
> > + /*
> > +* Otherwise, try to set the override itself. Typically the
> > +* secure firmware will never have set this configuration.
> > +* Instead, it will either have disabled write access to
> > +* this field, or it will already have set an explicit
> > +* override itself.
> > +*/
> > + WARN_ON((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0);
> > +
> > + value |= MC_SID_STREAMID_SECURITY_OVERRIDE;
> > + writel(value, mc->regs + client->regs.sid.security);
> > + }
> > +
> > + value = readl(mc->regs + client->regs.sid.override);
> > + old = value & MC_SID_STREAMID_OVERRIDE_MASK;
> > +
> > + if (old != sid) {
> > + dev_dbg(mc->dev, "overriding SID %x for %s with %x\n", old,
> > + client->name, sid);
> > + writel(sid, mc->regs + client->regs.sid.override);
> > + }
> > +}
> > +
> > +static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device
> > *dev)
> > +{
> > +#if IS_ENABLED(CONFIG_IOMMU_API)
>
> Is this part really build-time dependent? I don't see here any uses of
> IOMMU specific fields, so maybe this should be runtime choice based on
> enabled inte
Re: [PATCH v1 2/2] iommu/tegra-smmu: Revert workaround that was needed for Nyan Big Chromebook
On Sat, Apr 24, 2021 at 11:27:10PM +0300, Dmitry Osipenko wrote: > 23.04.2021 18:23, Dmitry Osipenko пишет: > > 23.04.2021 18:01, Guillaume Tucker пишет: > >> On 02/04/2021 15:40, Dmitry Osipenko wrote: > >>> 01.04.2021 11:55, Nicolin Chen пишет: > On Mon, Mar 29, 2021 at 02:32:56AM +0300, Dmitry Osipenko wrote: > > The previous commit fixes problem where display client was attaching too > > early to IOMMU during kernel boot in a multi-platform kernel > > configuration > > which enables CONFIG_ARM_DMA_USE_IOMMU=y. The workaround that helped to > > defer the IOMMU attachment for Nyan Big Chromebook isn't needed anymore, > > revert it. > > Sorry for the late reply. I have been busy with downstream tasks. > > I will give them a try by the end of the week. Yet, probably it'd > be better to include Guillaume also as he has the Nyan platform. > > >>> > >>> Indeed, thanks. Although, I'm pretty sure that it's the same issue which > >>> I reproduced on Nexus 7. > >>> > >>> Guillaume, could you please give a test to these patches on Nyan Big? > >>> There should be no EMEM errors in the kernel log with this patches. > >>> > >>> https://patchwork.ozlabs.org/project/linux-tegra/list/?series=236215 > >> > >> So sorry for the very late reply. I have tried the patches but > >> hit some issues on linux-next, it's not reaching a login prompt > >> with next-20210422. So I then tried with next-20210419 which > >> does boot but shows the IOMMU error: > >> > >> <6>[2.995341] tegra-dc 5420.dc: Adding to iommu group 1 > >> <4>[3.001070] Failed to attached device 5420.dc to IOMMU_mapping > >> > >> https://lava.collabora.co.uk/scheduler/job/3570052#L1120 > >> > >> The branch I'm using with the patches applied can be found here: > >> > >> > >> https://gitlab.collabora.com/gtucker/linux/-/commits/next-20210419-nyan-big-drm-read/ > >> > >> Hope this helps, let me know if you need anything else to be > >> tested. > > > > > > Hello Guillaume, > > > > The current linux-next doesn't boot on all ARM (AFAIK), the older > > next-20210413 works. The above message should be unrelated to the boot > > problem. It should be okay to ignore that message as it should be > > harmless in yours case. > > > > Although, the 20210419 should be good. > > Thierry, do you know what those SOR and Nouveau issues are about? There's a use-after-free (though it's really a use-before-init) issue in linux-next at the moment, but a fix has been suggested. The fix for this along with an additional leak plug is here: http://patchwork.ozlabs.org/project/linux-tegra/list/?series=240569 I'm not aware of any Nouveau issues. What version and platform are those happening on? Are there any logs? I can't seem to find them in this thread. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 5/5] iommu/tegra-smmu: Support managed domains
From: Navneet Kumar
Allow creating identity and DMA API compatible IOMMU domains. When
creating a DMA API compatible domain, make sure to also create the
required cookie.
Signed-off-by: Navneet Kumar
Signed-off-by: Thierry Reding
---
drivers/iommu/tegra-smmu.c | 47 --
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 6bf7654371c5..40647e1f03ae 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -281,8 +282,11 @@ static bool tegra_smmu_capable(enum iommu_cap cap)
static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
{
struct tegra_smmu_as *as;
+ int ret;
- if (type != IOMMU_DOMAIN_UNMANAGED)
+ if (type != IOMMU_DOMAIN_UNMANAGED &&
+ type != IOMMU_DOMAIN_DMA &&
+ type != IOMMU_DOMAIN_IDENTITY)
return NULL;
as = kzalloc(sizeof(*as), GFP_KERNEL);
@@ -291,26 +295,23 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->attr = SMMU_PD_READABLE | SMMU_PD_WRITABLE | SMMU_PD_NONSECURE;
- as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
- if (!as->pd) {
- kfree(as);
- return NULL;
+ if (type == IOMMU_DOMAIN_DMA) {
+ ret = iommu_get_dma_cookie(>domain);
+ if (ret)
+ goto free_as;
}
+ as->pd = alloc_page(GFP_KERNEL | __GFP_DMA | __GFP_ZERO);
+ if (!as->pd)
+ goto put_dma_cookie;
+
as->count = kcalloc(SMMU_NUM_PDE, sizeof(u32), GFP_KERNEL);
- if (!as->count) {
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->count)
+ goto free_pd_range;
as->pts = kcalloc(SMMU_NUM_PDE, sizeof(*as->pts), GFP_KERNEL);
- if (!as->pts) {
- kfree(as->count);
- __free_page(as->pd);
- kfree(as);
- return NULL;
- }
+ if (!as->pts)
+ goto free_pts;
spin_lock_init(>lock);
@@ -320,6 +321,18 @@ static struct iommu_domain
*tegra_smmu_domain_alloc(unsigned type)
as->domain.geometry.force_aperture = true;
return >domain;
+
+free_pts:
+ kfree(as->pts);
+free_pd_range:
+ __free_page(as->pd);
+put_dma_cookie:
+ if (type == IOMMU_DOMAIN_DMA)
+ iommu_put_dma_cookie(>domain);
+free_as:
+ kfree(as);
+
+ return NULL;
}
static void tegra_smmu_domain_free(struct iommu_domain *domain)
@@ -1051,7 +1064,7 @@ static const struct iommu_ops tegra_smmu_ops = {
.map = tegra_smmu_map,
.unmap = tegra_smmu_unmap,
.iova_to_phys = tegra_smmu_iova_to_phys,
- .get_resv_regions = of_iommu_get_resv_regions,
+ .get_resv_regions = iommu_dma_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.apply_resv_region = tegra_smmu_apply_resv_region,
.of_xlate = tegra_smmu_of_xlate,
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 4/5] iommu/tegra-smmu: Add support for reserved regions
From: Thierry Reding
The Tegra DRM driver currently uses the IOMMU API explicitly. This means
that it has fine-grained control over when exactly the translation
through the IOMMU is enabled. This currently happens after the driver
probes, so the driver is in a DMA quiesced state when the IOMMU
translation is enabled.
During the transition of the Tegra DRM driver to use the DMA API instead
of the IOMMU API explicitly, it was observed that on certain platforms
the display controllers were still actively fetching from memory. When a
DMA IOMMU domain is created as part of the DMA/IOMMU API setup during
boot, the IOMMU translation for the display controllers can be enabled a
significant amount of time before the driver has had a chance to reset
the hardware into a sane state. This causes the SMMU to detect faults on
the addresses that the display controller is trying to fetch.
To avoid this, and as a byproduct paving the way for seamless transition
of display from the bootloader to the kernel, add support for reserved
regions in the Tegra SMMU driver. This is implemented using the standard
reserved memory device tree bindings, which let us describe regions of
memory which the kernel is forbidden from using for regular allocations.
The Tegra SMMU driver will parse the nodes associated with each device
via the "memory-region" property and return reserved regions that the
IOMMU core will then create direct mappings for prior to attaching the
IOMMU domains to the devices. This ensures that a 1:1 mapping is in
place when IOMMU translation starts and prevents the SMMU from detecting
any faults.
Signed-off-by: Thierry Reding
---
drivers/iommu/tegra-smmu.c | 76 ++
1 file changed, 76 insertions(+)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 0a281833f611..6bf7654371c5 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -539,6 +540,38 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as,
unsigned long iova,
struct tegra_smmu *smmu = as->smmu;
u32 *pd = page_address(as->pd);
unsigned long offset = pd_index * sizeof(*pd);
+ bool unmap = false;
+
+ /*
+* XXX Move this outside of this function. Perhaps add a struct
+* iommu_domain parameter to ->{get,put}_resv_regions() so that
+* the mapping can be done there.
+*
+* The problem here is that as->smmu is only known once we attach
+* the domain to a device (because then we look up the right SMMU
+* instance via the dev->archdata.iommu pointer). When the direct
+* mappings are created for reserved regions, the domain has not
+* been attached to a device yet, so we don't know. We currently
+* fix that up in ->apply_resv_regions() because that is the first
+* time where we have access to a struct device that will be used
+* with the IOMMU domain. However, that's asymmetric and doesn't
+* take care of the page directory mapping either, so we need to
+* come up with something better.
+*/
+ if (as->pd_dma == 0) {
+ as->pd_dma = dma_map_page(smmu->dev, as->pd, 0, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(smmu->dev, as->pd_dma))
+ return;
+
+ if (!smmu_dma_addr_valid(smmu, as->pd_dma)) {
+ dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ return;
+ }
+
+ unmap = true;
+ }
/* Set the page directory entry first */
pd[pd_index] = value;
@@ -551,6 +584,12 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as,
unsigned long iova,
smmu_flush_ptc(smmu, as->pd_dma, offset);
smmu_flush_tlb_section(smmu, as->id, iova);
smmu_flush(smmu);
+
+ if (unmap) {
+ dma_unmap_page(smmu->dev, as->pd_dma, SMMU_SIZE_PD,
+ DMA_TO_DEVICE);
+ as->pd_dma = 0;
+ }
}
static u32 *tegra_smmu_pte_offset(struct page *pt_page, unsigned long iova)
@@ -945,6 +984,40 @@ static struct iommu_group *tegra_smmu_device_group(struct
device *dev)
return group->group;
}
+static void tegra_smmu_apply_resv_region(struct device *dev,
+struct iommu_domain *domain,
+struct iommu_resv_region *region)
+{
+ struct tegra_smmu *smmu = dev_iommu_priv_get(dev);
+ struct tegra_smmu_as *as = to_smmu_as(domain);
+
+ /*
+* ->attach_dev() may not have been called yet at this point, so the
+* address space may not have b
[PATCH v2 3/5] iommu: dma: Use of_iommu_get_resv_regions()
From: Thierry Reding For device tree nodes, use the standard of_iommu_get_resv_regions() implementation to obtain the reserved memory regions associated with a device. Cc: Rob Herring Cc: Frank Rowand Cc: devicet...@vger.kernel.org Signed-off-by: Thierry Reding --- drivers/iommu/dma-iommu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 7bcdd1205535..52b424176241 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -190,6 +191,8 @@ void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) iort_iommu_msi_get_resv_regions(dev, list); + if (dev->of_node) + of_iommu_get_resv_regions(dev, list); } EXPORT_SYMBOL(iommu_dma_get_resv_regions); -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/5] iommu: Implement of_iommu_get_resv_regions()
From: Thierry Reding
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
Cc: devicet...@vger.kernel.org
Reviewed-by: Rob Herring
Signed-off-by: Thierry Reding
---
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 | 54
include/linux/of_iommu.h | 8 ++
2 files changed, 62 insertions(+)
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
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
+
#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(, 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(, args.args,
MAX_PHANDLE_ARGS);
+
+ err = of_address_to_resource(it.node, 0, );
+ 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(),
+IOMMU_READ |
IOMMU_WRITE,
+
IOMMU_RESV_DIRECT_RELAXABLE);
+ if (!region)
+ continue;
+
+ list_add_tail(>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 */
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
From: Thierry Reding Reserved memory region phandle references can be accompanied by a specifier that provides additional information about how that specific reference should be treated. One use-case is to mark a memory region as needing an identity mapping in the system's IOMMU for the device that references the region. This is needed for example when the bootloader has set up hardware (such as a display controller) to actively access a memory region (e.g. a boot splash screen framebuffer) during boot. The operating system can use the identity mapping flag from the specifier to make sure an IOMMU identity mapping is set up for the framebuffer before IOMMU translations are enabled for the display controller. Signed-off-by: Thierry Reding --- .../reserved-memory/reserved-memory.txt | 21 +++ include/dt-bindings/reserved-memory.h | 8 +++ 2 files changed, 29 insertions(+) create mode 100644 include/dt-bindings/reserved-memory.h diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt index e8d3096d922c..e9c2f80b441f 100644 --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt @@ -52,6 +52,11 @@ compatible (optional) - standard definition be used by an operating system to instantiate the necessary pool management subsystem if necessary. - vendor specific string in the form ,[-] +#memory-region-cells (optional) - +- Defines how many cells are used to form the memory region specifier. + The memory region specifier contains additional information on how a + reserved memory region referenced by the corresponding phandle will + be used in a specific context. no-map (optional) - empty property - Indicates the operating system must not create a virtual mapping of the region as part of its standard mapping of system memory, @@ -83,6 +88,22 @@ memory-region (optional) - phandle, specifier pairs to children of /reserved-mem memory-region-names (optional) - a list of names, one for each corresponding entry in the memory-region property +Reserved memory region references can be accompanied by a memory region +specifier, which provides additional information about how the memory region +will be used in that specific context. If a reserved memory region does not +have the #memory-region-cells property, 0 is implied and no information +besides the phandle is conveyed. For reserved memory regions that contain +#memory-region-cells = <1>, the following encoding applies if not otherwise +overridden by the bindings selected by the region's compatible string: + + - bit 0: If set, requests that the region be identity mapped if the system +uses an IOMMU for I/O virtual address translations. This is used, for +example, when a bootloader has configured a display controller to display +a boot splash. Once the OS takes over and enables the IOMMU for the given +display controller, the IOMMU may fault if the framebuffer hasn't been +mapped to the IOMMU at the address that the display controller tries to +access. + Example --- This example defines 3 contiguous regions are defined for Linux kernel: diff --git a/include/dt-bindings/reserved-memory.h b/include/dt-bindings/reserved-memory.h new file mode 100644 index ..174ca3448342 --- /dev/null +++ b/include/dt-bindings/reserved-memory.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */ + +#ifndef _DT_BINDINGS_RESERVED_MEMORY_H +#define _DT_BINDINGS_RESERVED_MEMORY_H + +#define MEMORY_REGION_IDENTITY_MAPPING 0x1 + +#endif -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 0/5] iommu: Support identity mappings of reserved-memory regions
From: Thierry Reding
Hi,
this is an updated proposal to solve the problem of passing memory
regions that are actively being accessed during boot. The particular
use-case that I need this for is when the bootloader has set up the
display controller to scan out a boot splash screen. During boot the
DMA/IOMMU glue code will attach devices to an IOMMU domain and by
doing so enable IOMMU translations. Typically this will be before a
device driver has had a chance to either disable the display
controller or set up a new framebuffer and map it to the IOMMU.
In that case, the IOMMU will start to fault because the accesses of
the display controller will be for memory addresses that are not mapped
in the IOMMU. The solution is obviously to create identity mappings for
such memory regions. From a device tree point of view, these memory
regions can be described using the reserved-memory device tree bindings
and hooked up to the consumer devices using the "memory-region"
property. On the kernel side, the IOMMU framework already supports the
concept of reserved regions, as well as a way of marking these regions
as requiring identity (a.k.a. direct) mappings.
Unfortunately, the current reserved-memory region bindings only allow
properties of the regions themselves to be described (such as whether a
kernel virtual mapping of the region is needed or not), but it doesn't
provide a way of associating extra information with any particular
reference to these regions. However, that's exactly what's needed for
this case because a given region may need to be identity mapped for a
specific device (such as the display controller scanning out from the
region) but referenced by multiple devices (e.g. if the memory is some
special carveout memory reserved for display purposes).
This series of patches proposes a simple solution: extend memory-region
properties to use an optional specifier, such as the ones already
commonly used for things like GPIOs or interrupts. The specifier needs
to be provided if the reserved-memory region has a non-zero
#memory-region-cells property (if the property is not present, zero is
the assumed default value). The specifier contains flags that specify
how the reference is to be treated. This series of patches introduces
the MEMORY_REGION_IDENTITY_MAPPING flag (value: 0x1) that marks the
specific reference to the memory region to require an identity mapping.
In practice, a device tree would look like this:
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
fb: framebuffer@92cb2000 {
reg = <0 0x92cb2000 0 0x0080>;
#memory-region-cells = <1>;
};
};
...
display@5240 {
...
memory-region = < MEMORY_REGION_IDENTITY_MAPPING>;
...
};
Note: While the above would be valid DTS content, it's more likely that
in practice this content would be dynamically generated by the
bootloader using runtime information (such as the framebuffer memory
location).
An operating system can derive from that pair that
the 8 MiB of memory at physical address 0x92cb2000 need to be identity
mapped to the same IO virtual address if the device is attached to an
IOMMU. If no IOMMU is enabled in the system, obviously no identity
mapping needs to be created, but the operating system may still use the
reference to transition to its own framebuffer using the existing memory
region.
Note that an earlier proposal was to use the existing simple-framebuffer
device tree bindings to transport this information. Unfortunately there
are cases where this is not enough. On Tegra SoCs, for example, the
bootloader will also set up a color space correction lookup table in the
system memory that the display controller will access during boot,
alongside the framebuffer. The simple-framebuffer DT bindings have no
way of describing this (and I guess one could argue that this particular
setup no longer is a "simple" framebuffer), so the above, more flexible
proposal was implemented.
I've made corresponding changes in the proprietary bootloader, added a
compatibility shim in U-Boot (which forwards information created by the
proprietary bootloader to the kernel) and the attached patches to test
this on Jetson TX1, Jetson TX2 and Jetson AGX Xavier.
Note that there will be no new releases of the bootloader for earlier
devices, so adding support for these new DT bindings will not be
practical. The bootloaders on those devices do pass information about
the active framebuffer via the kernel command-line, so we may want to
add code to create reserved regions in the IOMMU based on that.
Thierry
Navneet Kumar (1):
iommu/tegra-smmu: Support managed domains
Thierry Reding (4):
dt-bindings: reserved-memory: Document memory region specifier
iommu: Implement of_iommu_get_resv_regions()
i
[PATCH v2 09/10] arm64: tegra: Enable SMMU support on Tegra194
From: Thierry Reding
Add the device tree node for the dual-SMMU found on Tegra194 and hook up
peripherals such as host1x, BPMP, HDA, SDMMC, EQOS and VIC.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 86
1 file changed, 86 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 4d37ee0ea4d1..6ed296e27158 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -62,6 +62,7 @@ ethernet@249 {
interconnects = < TEGRA194_MEMORY_CLIENT_EQOSR >,
< TEGRA194_MEMORY_CLIENT_EQOSW >;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_EQOS>;
status = "disabled";
snps,write-requests = <1>;
@@ -733,6 +734,7 @@ sdmmc1: mmc@340 {
interconnects = < TEGRA194_MEMORY_CLIENT_SDMMCRA
>,
< TEGRA194_MEMORY_CLIENT_SDMMCWA
>;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_SDMMC1>;
nvidia,pad-autocal-pull-up-offset-3v3-timeout =
<0x07>;
nvidia,pad-autocal-pull-down-offset-3v3-timeout =
@@ -759,6 +761,7 @@ sdmmc3: mmc@344 {
interconnects = < TEGRA194_MEMORY_CLIENT_SDMMCR
>,
< TEGRA194_MEMORY_CLIENT_SDMMCW
>;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_SDMMC3>;
nvidia,pad-autocal-pull-up-offset-1v8 = <0x00>;
nvidia,pad-autocal-pull-down-offset-1v8 = <0x7a>;
nvidia,pad-autocal-pull-up-offset-3v3-timeout = <0x07>;
@@ -790,6 +793,7 @@ sdmmc4: mmc@346 {
interconnects = < TEGRA194_MEMORY_CLIENT_SDMMCRAB
>,
< TEGRA194_MEMORY_CLIENT_SDMMCWAB
>;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_SDMMC4>;
nvidia,pad-autocal-pull-up-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-down-offset-hs400 = <0x00>;
nvidia,pad-autocal-pull-up-offset-1v8-timeout = <0x0a>;
@@ -821,6 +825,7 @@ hda@351 {
interconnects = < TEGRA194_MEMORY_CLIENT_HDAR >,
< TEGRA194_MEMORY_CLIENT_HDAW >;
interconnect-names = "dma-mem", "write";
+ iommus = < TEGRA194_SID_HDA>;
status = "disabled";
};
@@ -1300,6 +1305,84 @@ pmc: pmc@c36 {
interrupt-controller;
};
+
[PATCH v2 10/10] arm64: tegra: Enable SMMU support for display on Tegra194
From: Thierry Reding
The display controllers are attached to a separate ARM SMMU instance
that is dedicated to servicing isochronous memory clients. Add this ISO
instance of the ARM SMMU to device tree and attach all four display
controllers to it.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra194.dtsi | 80
1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 6ed296e27158..00f8248f216e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -1305,6 +1305,82 @@ pmc: pmc@c36 {
interrupt-controller;
};
+ smmu_iso: iommu@1000 {
+ compatible = "nvidia,tegra194-smmu", "nvidia,smmu-500";
+ reg = <0x1000 0x80>;
+ interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+ stream-match-mask = <0x7f80>;
+ #global-interrupts = <1>;
+ #iommu-cells = <1>;
+
+ nvidia,memory-controller = <>;
+ status = "okay";
+ };
+
smmu: iommu@1200 {
compatible = "nvidia,tegra194-smmu", "nvidia,smmu-500";
reg = <0x1200 0x80>,
@@ -1441,6 +1517,7 @@ display@1520 {
interconnects = <
TEGRA194_MEMORY_CLIENT_NVDISPLAYR >,
<
TEGRA194_MEMORY_CLIENT_NVDISPLAYR1 >;
interconnect-names = "dma-mem",
"read-1";
+ iommus = <_iso
TEGRA194_SID_NVDISPLAY>;
nvidia,outputs = <
>;
nvidia,head = <0>;
@@ -1459,6 +1536,7 @@ display@1521 {
interconnects = <
TEGRA194_MEMORY_CLIENT_NVDISPLAYR >,
<
TEGRA194_MEMORY_CLIENT_NVDISPLAYR1 >;
interconnect-names = "dma-mem",
"read-1";
+ iommus = <_iso
TEGRA194_SID_NVDIS
[PATCH v2 08/10] arm64: tegra: Hook up memory controller to SMMU on Tegra186
From: Thierry Reding
On Tegra186 and later, the memory controller needs to be programmed in
coordination with any of the ARM SMMU instances to configure the stream
ID used for each memory client.
To support this, add a phandle reference to the memory controller to the
SMMU device tree node.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index a173f40256ae..d02f6bf3e2ca 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1152,6 +1152,8 @@ smmu: iommu@1200 {
stream-match-mask = <0x7f80>;
#global-interrupts = <1>;
#iommu-cells = <1>;
+
+ nvidia,memory-controller = <>;
};
host1x@13e0 {
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 07/10] arm64: tegra: Use correct compatible string for Tegra186 SMMU
From: Thierry Reding
The SMMU found on Tegra186 requires interoperation with the memory
controller in order to program stream ID overrides. The generic ARM SMMU
500 compatible is therefore inaccurate. Replace it with a more correct,
SoC-specific compatible string.
Signed-off-by: Thierry Reding
---
arch/arm64/boot/dts/nvidia/tegra186.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 9f75bbf00cf7..a173f40256ae 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1082,7 +1082,7 @@ pci@3,0 {
};
smmu: iommu@1200 {
- compatible = "arm,mmu-500";
+ compatible = "nvidia,tegra186-smmu", "nvidia,smmu-500";
reg = <0 0x1200 0 0x80>;
interrupts = ,
,
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 06/10] iommu/arm-smmu: Use Tegra implementation on Tegra186
From: Thierry Reding Tegra186 requires the same SID override programming as Tegra194 in order to seamlessly transition from the firmware framebuffer to the Linux framebuffer, so the Tegra implementation needs to be used on Tegra186 devices as well. Signed-off-by: Thierry Reding --- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c index 136872e77195..9f465e146799 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c @@ -211,7 +211,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu) if (of_property_read_bool(np, "calxeda,smmu-secure-config-access")) smmu->impl = _impl; - if (of_device_is_compatible(np, "nvidia,tegra194-smmu")) + if (of_device_is_compatible(np, "nvidia,tegra194-smmu") || + of_device_is_compatible(np, "nvidia,tegra186-smmu")) return nvidia_smmu_impl_init(smmu); smmu = qcom_smmu_impl_init(smmu); -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 05/10] iommu/arm-smmu: tegra: Implement SID override programming
From: Thierry Reding
The secure firmware keeps some SID override registers set as passthrough
in order to allow devices such as the display controller to operate with
no knowledge of SMMU translations until an operating system driver takes
over. This is needed in order to seamlessly transition from the firmware
framebuffer to the OS framebuffer.
Upon successfully attaching a device to the SMMU and in the process
creating identity mappings for memory regions that are being accessed,
the Tegra implementation will call into the memory controller driver to
program the override SIDs appropriately.
Signed-off-by: Thierry Reding
---
drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 32 ++--
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
index 0e547b29143d..01e9b50b10a1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
@@ -7,6 +7,8 @@
#include
#include
+#include
+
#include "arm-smmu.h"
/*
@@ -15,10 +17,17 @@
* interleaved IOVA accesses across them and translates accesses from
* non-isochronous HW devices.
* Third one is used for translating accesses from isochronous HW devices.
+ *
+ * In addition, the SMMU driver needs to coordinate with the memory controller
+ * driver to ensure that the right SID override is programmed for any given
+ * memory client. This is necessary to allow for use-case such as seamlessly
+ * handing over the display controller configuration from the firmware to the
+ * kernel.
+ *
* This implementation supports programming of the two instances that must
- * be programmed identically.
- * The third instance usage is through standard arm-smmu driver itself and
- * is out of scope of this implementation.
+ * be programmed identically and takes care of invoking the memory controller
+ * driver for SID override programming after devices have been attached to an
+ * SMMU instance.
*/
#define MAX_SMMU_INSTANCES 2
@@ -26,6 +35,7 @@ struct nvidia_smmu {
struct arm_smmu_device smmu;
void __iomem *bases[MAX_SMMU_INSTANCES];
unsigned int num_instances;
+ struct tegra_mc *mc;
};
static inline struct nvidia_smmu *to_nvidia_smmu(struct arm_smmu_device *smmu)
@@ -237,6 +247,17 @@ static irqreturn_t nvidia_smmu_context_fault(int irq, void
*dev)
return ret;
}
+static void nvidia_smmu_probe_finalize(struct arm_smmu_device *smmu, struct
device *dev)
+{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
+ int err;
+
+ err = tegra_mc_probe_device(nvidia->mc, dev);
+ if (err < 0)
+ dev_err(smmu->dev, "memory controller probe failed for %s:
%d\n",
+ dev_name(dev), err);
+}
+
static const struct arm_smmu_impl nvidia_smmu_impl = {
.read_reg = nvidia_smmu_read_reg,
.write_reg = nvidia_smmu_write_reg,
@@ -246,6 +267,7 @@ static const struct arm_smmu_impl nvidia_smmu_impl = {
.tlb_sync = nvidia_smmu_tlb_sync,
.global_fault = nvidia_smmu_global_fault,
.context_fault = nvidia_smmu_context_fault,
+ .probe_finalize = nvidia_smmu_probe_finalize,
};
static const struct arm_smmu_impl nvidia_smmu_single_impl = {
@@ -264,6 +286,10 @@ struct arm_smmu_device *nvidia_smmu_impl_init(struct
arm_smmu_device *smmu)
if (!nvidia_smmu)
return ERR_PTR(-ENOMEM);
+ nvidia_smmu->mc = devm_tegra_memory_controller_get(dev);
+ if (IS_ERR(nvidia_smmu->mc))
+ return ERR_CAST(nvidia_smmu->mc);
+
/* Instance 0 is ioremapped by arm-smmu.c. */
nvidia_smmu->bases[0] = smmu->base;
nvidia_smmu->num_instances++;
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 04/10] iommu/arm-smmu: tegra: Detect number of instances at runtime
From: Thierry Reding
Parse the reg property in device tree and detect the number of instances
represented by a device tree node. This is subsequently needed in order
to support single-instance SMMUs with the Tegra implementation because
additional programming is needed to properly configure the SID override
registers in the memory controller.
Signed-off-by: Thierry Reding
---
Changes in v2:
- provide a separate implementation to simplify single instances
---
drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 58 ++--
1 file changed, 42 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
index 29117444e5a0..0e547b29143d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c
@@ -20,13 +20,19 @@
* The third instance usage is through standard arm-smmu driver itself and
* is out of scope of this implementation.
*/
-#define NUM_SMMU_INSTANCES 2
+#define MAX_SMMU_INSTANCES 2
struct nvidia_smmu {
- struct arm_smmu_device smmu;
- void __iomem*bases[NUM_SMMU_INSTANCES];
+ struct arm_smmu_device smmu;
+ void __iomem *bases[MAX_SMMU_INSTANCES];
+ unsigned int num_instances;
};
+static inline struct nvidia_smmu *to_nvidia_smmu(struct arm_smmu_device *smmu)
+{
+ return container_of(smmu, struct nvidia_smmu, smmu);
+}
+
static inline void __iomem *nvidia_smmu_page(struct arm_smmu_device *smmu,
unsigned int inst, int page)
{
@@ -47,9 +53,10 @@ static u32 nvidia_smmu_read_reg(struct arm_smmu_device *smmu,
static void nvidia_smmu_write_reg(struct arm_smmu_device *smmu,
int page, int offset, u32 val)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
void __iomem *reg = nvidia_smmu_page(smmu, i, page) + offset;
writel_relaxed(val, reg);
@@ -67,9 +74,10 @@ static u64 nvidia_smmu_read_reg64(struct arm_smmu_device
*smmu,
static void nvidia_smmu_write_reg64(struct arm_smmu_device *smmu,
int page, int offset, u64 val)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
void __iomem *reg = nvidia_smmu_page(smmu, i, page) + offset;
writeq_relaxed(val, reg);
@@ -79,6 +87,7 @@ static void nvidia_smmu_write_reg64(struct arm_smmu_device
*smmu,
static void nvidia_smmu_tlb_sync(struct arm_smmu_device *smmu, int page,
int sync, int status)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int delay;
arm_smmu_writel(smmu, page, sync, 0);
@@ -90,7 +99,7 @@ static void nvidia_smmu_tlb_sync(struct arm_smmu_device
*smmu, int page,
u32 val = 0;
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
void __iomem *reg;
reg = nvidia_smmu_page(smmu, i, page) + status;
@@ -112,9 +121,10 @@ static void nvidia_smmu_tlb_sync(struct arm_smmu_device
*smmu, int page,
static int nvidia_smmu_reset(struct arm_smmu_device *smmu)
{
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
unsigned int i;
- for (i = 0; i < NUM_SMMU_INSTANCES; i++) {
+ for (i = 0; i < nvidia->num_instances; i++) {
u32 val;
void __iomem *reg = nvidia_smmu_page(smmu, i, ARM_SMMU_GR0) +
ARM_SMMU_GR0_sGFSR;
@@ -157,8 +167,9 @@ static irqreturn_t nvidia_smmu_global_fault(int irq, void
*dev)
unsigned int inst;
irqreturn_t ret = IRQ_NONE;
struct arm_smmu_device *smmu = dev;
+ struct nvidia_smmu *nvidia = to_nvidia_smmu(smmu);
- for (inst = 0; inst < NUM_SMMU_INSTANCES; inst++) {
+ for (inst = 0; inst < nvidia->num_instances; inst++) {
irqreturn_t irq_ret;
irq_ret = nvidia_smmu_global_fault_inst(irq, smmu, inst);
@@ -202,11 +213,13 @@ static irqreturn_t nvidia_smmu_context_fault(int irq,
void *dev)
struct arm_smmu_device *smmu;
struct iommu_domain *domain = dev;
struct arm_smmu_domain *smmu_domain;
+ struct nvidia_smmu *nvidia;
smmu_domain = container_of(domain, struct arm_smmu_domain, domain);
smmu = smmu_domain->smmu;
+ nvidia = to_nvidia_smmu(smmu);
- for (inst = 0; inst < NUM_SMMU_INSTANCES; inst++) {
+
[PATCH v2 03/10] iommu/arm-smmu: Implement ->probe_finalize()
From: Thierry Reding
Implement a ->probe_finalize() callback that can be used by vendor
implementations to perform extra programming necessary after devices
have been attached to the SMMU.
Signed-off-by: Thierry Reding
---
Changes in v2:
-remove unnecessarily paranoid check
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 6f72c4d208ca..d20ce4d57df2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1450,6 +1450,18 @@ static void arm_smmu_release_device(struct device *dev)
iommu_fwspec_free(dev);
}
+static void arm_smmu_probe_finalize(struct device *dev)
+{
+ struct arm_smmu_master_cfg *cfg;
+ struct arm_smmu_device *smmu;
+
+ cfg = dev_iommu_priv_get(dev);
+ smmu = cfg->smmu;
+
+ if (smmu->impl->probe_finalize)
+ smmu->impl->probe_finalize(smmu, dev);
+}
+
static struct iommu_group *arm_smmu_device_group(struct device *dev)
{
struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
@@ -1569,6 +1581,7 @@ static struct iommu_ops arm_smmu_ops = {
.iova_to_phys = arm_smmu_iova_to_phys,
.probe_device = arm_smmu_probe_device,
.release_device = arm_smmu_release_device,
+ .probe_finalize = arm_smmu_probe_finalize,
.device_group = arm_smmu_device_group,
.enable_nesting = arm_smmu_enable_nesting,
.set_pgtable_quirks = arm_smmu_set_pgtable_quirks,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index c31a59d35c64..147c95e7c59c 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -439,6 +439,7 @@ struct arm_smmu_impl {
struct device *dev, int start);
void (*write_s2cr)(struct arm_smmu_device *smmu, int idx);
void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg);
+ void (*probe_finalize)(struct arm_smmu_device *smmu, struct device
*dev);
};
#define INVALID_SMENDX -1
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 02/10] dt-bindings: arm-smmu: Add Tegra186 compatible string
From: Thierry Reding The ARM SMMU instantiations found on Tegra186 and later need inter- operation with the memory controller in order to correctly program stream ID overrides. Furthermore, on Tegra194 multiple instances of the SMMU can gang up to achieve higher throughput. In order to do this, they have to be programmed identically so that the memory controller can interleave memory accesses between them. Add the Tegra186 compatible string to make sure the interoperation with the memory controller can be enabled on that SoC generation. Signed-off-by: Thierry Reding --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 9d27aa5111d4..1181b590db71 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -54,8 +54,14 @@ properties: - const: arm,mmu-500 - description: NVIDIA SoCs that program two ARM MMU-500s identically items: + - description: NVIDIA SoCs that require memory controller interaction + and may program multiple ARM MMU-500s identically with the memory + controller interleaving translations between multiple instances + for improved performance. +items: - enum: - - nvidia,tegra194-smmu + - const: nvidia,tegra194-smmu + - const: nvidia,tegra186-smmu - const: nvidia,smmu-500 - items: - const: arm,mmu-500 @@ -165,10 +171,11 @@ allOf: contains: enum: - nvidia,tegra194-smmu + - nvidia,tegra186-smmu then: properties: reg: - minItems: 2 + minItems: 1 maxItems: 2 else: properties: -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 01/10] memory: tegra: Implement SID override programming
From: Thierry Reding
Instead of programming all SID overrides during early boot, perform the
operation on-demand after the SMMU translations have been set up for a
device. This reuses data from device tree to match memory clients for a
device and programs the SID specified in device tree, which corresponds
to the SID used for the SMMU context banks for the device.
Signed-off-by: Thierry Reding
---
drivers/memory/tegra/mc.c | 9 +
drivers/memory/tegra/tegra186.c | 72 +
include/soc/tegra/mc.h | 3 ++
3 files changed, 84 insertions(+)
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index c854639cf30c..bace5ecfe770 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -97,6 +97,15 @@ struct tegra_mc *devm_tegra_memory_controller_get(struct
device *dev)
}
EXPORT_SYMBOL_GPL(devm_tegra_memory_controller_get);
+int tegra_mc_probe_device(struct tegra_mc *mc, struct device *dev)
+{
+ if (mc->soc->ops && mc->soc->ops->probe_device)
+ return mc->soc->ops->probe_device(mc, dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tegra_mc_probe_device);
+
static int tegra_mc_block_dma_common(struct tegra_mc *mc,
const struct tegra_mc_reset *rst)
{
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 1f87915ccd62..e65eac5764d4 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -4,6 +4,7 @@
*/
#include
+#include
#include
#include
#include
@@ -15,6 +16,10 @@
#include
#endif
+#define MC_SID_STREAMID_OVERRIDE_MASK GENMASK(7, 0)
+#define MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED BIT(16)
+#define MC_SID_STREAMID_SECURITY_OVERRIDE BIT(8)
+
static void tegra186_mc_program_sid(struct tegra_mc *mc)
{
unsigned int i;
@@ -66,10 +71,77 @@ static int tegra186_mc_resume(struct tegra_mc *mc)
return 0;
}
+static void tegra186_mc_client_sid_override(struct tegra_mc *mc,
+ const struct tegra_mc_client
*client,
+ unsigned int sid)
+{
+ u32 value, old;
+
+ value = readl(mc->regs + client->regs.sid.security);
+ if ((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0) {
+ /*
+* If the secure firmware has locked this down the override
+* for this memory client, there's nothing we can do here.
+*/
+ if (value & MC_SID_STREAMID_SECURITY_WRITE_ACCESS_DISABLED)
+ return;
+
+ /*
+* Otherwise, try to set the override itself. Typically the
+* secure firmware will never have set this configuration.
+* Instead, it will either have disabled write access to
+* this field, or it will already have set an explicit
+* override itself.
+*/
+ WARN_ON((value & MC_SID_STREAMID_SECURITY_OVERRIDE) == 0);
+
+ value |= MC_SID_STREAMID_SECURITY_OVERRIDE;
+ writel(value, mc->regs + client->regs.sid.security);
+ }
+
+ value = readl(mc->regs + client->regs.sid.override);
+ old = value & MC_SID_STREAMID_OVERRIDE_MASK;
+
+ if (old != sid) {
+ dev_dbg(mc->dev, "overriding SID %x for %s with %x\n", old,
+ client->name, sid);
+ writel(sid, mc->regs + client->regs.sid.override);
+ }
+}
+
+static int tegra186_mc_probe_device(struct tegra_mc *mc, struct device *dev)
+{
+#if IS_ENABLED(CONFIG_IOMMU_API)
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct of_phandle_args args;
+ unsigned int i, index = 0;
+
+ while (!of_parse_phandle_with_args(dev->of_node, "interconnects",
"#interconnect-cells",
+ index, )) {
+ if (args.np == mc->dev->of_node && args.args_count != 0) {
+ for (i = 0; i < mc->soc->num_clients; i++) {
+ const struct tegra_mc_client *client =
>soc->clients[i];
+
+ if (client->id == args.args[0]) {
+ u32 sid = fwspec->ids[0] &
MC_SID_STREAMID_OVERRIDE_MASK;
+
+ tegra186_mc_client_sid_override(mc,
client, sid);
+ }
+ }
+ }
+
+ index++;
+ }
+#endif
+
+ return 0;
+}
+
const struct tegra_mc_ops tegra186_mc_ops = {
.probe = tegra186_mc_probe,
.remove = tegra186_mc_remove,
.resume = tegra186_mc_resume,
+ .probe_device = tegra186_mc_probe_device,
};
#if defined(CONFIG
[PATCH v2 00/10] arm64: tegra: Prevent early SMMU faults
From: Thierry Reding Hi, this is a set of patches that is the result of earlier discussions regarding early identity mappings that are needed to avoid SMMU faults during early boot. The goal here is to avoid early identity mappings altogether and instead postpone the need for the identity mappings to when devices are attached to the SMMU. This works by making the SMMU driver coordinate with the memory controller driver on when to start enforcing SMMU translations. This makes Tegra behave in a more standard way and pushes the code to deal with the Tegra-specific programming into the NVIDIA SMMU implementation. Compared to the original version of these patches, I've split the preparatory work into a separate patch series because it became very large and will be mostly uninteresting for this audience. Patch 1 provides a mechanism to program SID overrides at runtime. Patch 2 updates the ARM SMMU device tree bindings to include the Tegra186 compatible string as suggested by Robin during review. Patches 3 and 4 create the fundamentals in the SMMU driver to support this and also make this functionality available on Tegra186. Patch 5 hooks the ARM SMMU up to the memory controller so that the memory client stream ID overrides can be programmed at the right time. Patch 6 extends this mechanism to Tegra186 and patches 7-9 enable all of this through device tree updates. Patch 10 is included here to show how SMMU will be enabled for display controllers. However, it cannot be applied yet because the code to create identity mappings for potentially live framebuffers hasn't been merged yet. The end result is that various peripherals will have SMMU enabled, while the display controllers will keep using passthrough, as initially set up by firmware. Once the device tree bindings have been accepted and the SMMU driver has been updated to create identity mappings for the display controllers, they can be hooked up to the SMMU and the code in this series will automatically program the SID overrides to enable SMMU translations at the right time. Note that the series creates a compile time dependency between the memory controller and IOMMU trees. If it helps I can provide a branch for each tree, modelling the dependency, once the series has been reviewed. Changes in v2: - split off the preparatory work into a separate series (that needs to be applied first) - address review comments by Robin Thierry Thierry Reding (10): memory: tegra: Implement SID override programming dt-bindings: arm-smmu: Add Tegra186 compatible string iommu/arm-smmu: Implement ->probe_finalize() iommu/arm-smmu: tegra: Detect number of instances at runtime iommu/arm-smmu: tegra: Implement SID override programming iommu/arm-smmu: Use Tegra implementation on Tegra186 arm64: tegra: Use correct compatible string for Tegra186 SMMU arm64: tegra: Hook up memory controller to SMMU on Tegra186 arm64: tegra: Enable SMMU support on Tegra194 arm64: tegra: Enable SMMU support for display on Tegra194 .../devicetree/bindings/iommu/arm,smmu.yaml | 11 +- arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +- arch/arm64/boot/dts/nvidia/tegra194.dtsi | 166 ++ drivers/iommu/arm/arm-smmu/arm-smmu-impl.c| 3 +- drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 90 -- drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++ drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + drivers/memory/tegra/mc.c | 9 + drivers/memory/tegra/tegra186.c | 72 include/soc/tegra/mc.h| 3 + 10 files changed, 349 insertions(+), 23 deletions(-) -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
