iommu: Support mappings/reservations in reserved-memory regions

From: Thierry Reding <[email protected]>

DMA parent devices can define separate DMA busses via the "dma-ranges" and "#address-cells" and "#size-cells" properties. If the DMA bus has different cell counts than its parent, this can cause the translation of DMA address to fails (e.g. truncation from 2 to 1 address cells).

Avoid this by stopping to search for DMA parents when a parent without a "dma-ranges" property is encountered. Also, since it is the DMA parent that defines the DMA bus, use the bus' cell counts instead of its parent cell counts.

Signed-off-by Thierry Reding <[email protected]>

---
Changes in v10:
- new patch to avoid address truncation when traversing a bus hierarchy with mismatching #address-cells properties

Example from Tegra194 (redacted for clarity):

	reserved-memory {
		#address-cells = <2>;
		#size-cells = <2>;
		ranges;

		framebuffer@0,0 {
			compatible = "framebuffer";
			reg = <0x2 0x57320000 0x0 0x00800000>;
			iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
		};
	};

	bus@0 {

		/* truncation happens here */
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0 0x0 0x0 0x40000000>;

		mc: memory-controller@2c00000 {
			#address-cells = <2>;
			#size-cells = <2>;

			/*
			 * memory controller provides access to 512 GiB
			 * of system RAM (root of the DMA bus)
			 */
			dma-ranges = <0x0 0x0 0x0 0x80 0x0>;
		};

		host1x@13e00000 {
			display-hub@15200000 {
				display@15200000 {
					interconnect-names = "dma-mem", ...;
					interconnects = <&mc ...>;
					memory-region = <&fb>;
				};
			};
		};
	};

During DMA address translation, the framebuffer address (0x257320000) will first be translated to the DMA parent's DMA bus, which yields the same value. After that, the current translation code will switch to the control bus of bus@0 and then the address will be truncated to 0x57320000 due to #address-cells = <1>.

The idea of this patch is to interrupt DMA address translation at &mc because it is the root of the DMA bus (i.e. its parent does not have a dma-ranges property) so that the control bus' #address-cells doesn't truncate the DMA address.

 drivers/of/address.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

drivers/of/address.c +14 -3

index 14f137a21b0c..e2f45bdbc41a 100644
@@ -475,6 +475,7 @@ static u64 __of_translate_address(struct device_node *dev,
 				  const __be32 *in_addr, const char *rprop,
 				  struct device_node **host)
 {
+	bool dma = rprop && !strcmp(rprop, "dma-ranges");
 	struct device_node *parent = NULL;
 	struct of_bus *bus, *pbus;
 	__be32 addr[OF_MAX_ADDR_CELLS];

@@ -494,7 +495,12 @@ static u64 __of_translate_address(struct device_node *dev,
 	bus = of_match_bus(parent);
 
 	/* Count address cells & copy address locally */
-	bus->count_cells(dev, &na, &ns);
+	if (dma) {
+		na = of_bus_n_addr_cells(parent);
+		ns = of_bus_n_size_cells(parent);
+	} else {
+		bus->count_cells(dev, &na, &ns);
+	}
 	if (!OF_CHECK_COUNTS(na, ns)) {
 		pr_debug("Bad cell count for %pOF\n", dev);
 		goto bail;

@@ -515,7 +521,7 @@ static u64 __of_translate_address(struct device_node *dev,
 		parent = get_parent(dev);
 
 		/* If root, we have finished */
-		if (parent == NULL) {
+		if (parent == NULL || (dma && !of_get_property(parent, "dma-ranges", NULL))) {
 			pr_debug("reached root node\n");
 			result = of_read_number(addr, na);
 			break;

@@ -536,7 +542,12 @@ static u64 __of_translate_address(struct device_node *dev,
 
 		/* Get new parent bus and counts */
 		pbus = of_match_bus(parent);
-		pbus->count_cells(dev, &pna, &pns);
+		if (dma) {
+			pna = of_bus_n_addr_cells(parent);
+			pns = of_bus_n_size_cells(parent);
+		} else {
+			pbus->count_cells(dev, &pna, &pns);
+		}
 		if (!OF_CHECK_COUNTS(pna, pns)) {
 			pr_err("Bad cell count for %pOF\n", dev);
 			break;

Signature · 1 line

2.38.1

Quoted 6 lines · Thierry Reding

From: Thierry Reding <[email protected]>

DMA parent devices can define separate DMA busses via the "dma-ranges" and "#address-cells" and "#size-cells" properties. If the DMA bus has different cell counts than its parent, this can cause the translation of DMA address to fails (e.g. truncation from 2 to 1 address cells).

My assumption in this case was that the parent cell sizes should be increased to 2 cells. That tends to be what people want to do anyways (64-bit everywhere on 64-bit CPUs).

Quoted 4 lines

Avoid this by stopping to search for DMA parents when a parent without a "dma-ranges" property is encountered. Also, since it is the DMA parent that defines the DMA bus, use the bus' cell counts instead of its parent cell counts.

We treat no 'dma-ranges' as equivalent to 'dma-ranges;'. IIRC, the spec even says that because I hit that case.

Is this going to work for 'dma-device' with something like this?:

  bus@0 {
    dma-ranges = <...>;
    child-bus@... {
      dma-device@... {
      };
    };
  };

Quoted 27 lines

Signed-off-by Thierry Reding <[email protected]>

---
Changes in v10:
- new patch to avoid address truncation when traversing a bus hierarchy with mismatching #address-cells properties

Example from Tegra194 (redacted for clarity):

	reserved-memory {
		#address-cells = <2>;
		#size-cells = <2>;
		ranges;

		framebuffer@0,0 {
			compatible = "framebuffer";
			reg = <0x2 0x57320000 0x0 0x00800000>;
			iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
		};
	};

	bus@0 {
		/* truncation happens here */
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0 0x0 0x0 0x40000000>;

Quoted 3 lines

		mc: memory-controller@2c00000 {
			#address-cells = <2>;
			#size-cells = <2>;

I think this is wrong. The parent should have more or equal number of cells.

Quoted 57 lines · Rob Herring

Quoted 6 lines · Thierry Reding

From: Thierry Reding <[email protected]>

DMA parent devices can define separate DMA busses via the "dma-ranges" and "#address-cells" and "#size-cells" properties. If the DMA bus has different cell counts than its parent, this can cause the translation of DMA address to fails (e.g. truncation from 2 to 1 address cells).

My assumption in this case was that the parent cell sizes should be increased to 2 cells. That tends to be what people want to do anyways (64-bit everywhere on 64-bit CPUs).

Quoted 4 lines

Avoid this by stopping to search for DMA parents when a parent without a "dma-ranges" property is encountered. Also, since it is the DMA parent that defines the DMA bus, use the bus' cell counts instead of its parent cell counts.

We treat no 'dma-ranges' as equivalent to 'dma-ranges;'. IIRC, the spec even says that because I hit that case.

Is this going to work for 'dma-device' with something like this?:

  bus@0 {
    dma-ranges = <...>;
    child-bus@... {
      dma-device@... {
      };
    };
  };

Quoted 27 lines

Signed-off-by Thierry Reding <[email protected]>

---
Changes in v10:
- new patch to avoid address truncation when traversing a bus hierarchy with mismatching #address-cells properties

Example from Tegra194 (redacted for clarity):

    reserved-memory {
            #address-cells = <2>;
            #size-cells = <2>;
            ranges;

            framebuffer@0,0 {
                    compatible = "framebuffer";
                    reg = <0x2 0x57320000 0x0 0x00800000>;
                    iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
            };
    };

    bus@0 {
            /* truncation happens here */
            #address-cells = <1>;
            #size-cells = <1>;
            ranges = <0x0 0x0 0x0 0x40000000>;

Quoted 6 lines

Quoted 3 lines

            mc: memory-controller@2c00000 {
                    #address-cells = <2>;
                    #size-cells = <2>;

I think this is wrong. The parent should have more or equal number of cells.

I was half suspecting that. The reason why I hesitated is that I recall having the opposite discussion a while ago when we were adding bus@0 to 64-bit Tegra devices. We had at some point (probably around Tegra114 or Tegra124, 32-bit ARM chips that support LPAE) started to set #address- cells = <2> precisely because the CPU could address more than 32-bit addresses. We then did the same thing transitioning to 64-bit ARM. When we then started discussing bus@0, someone (might have been you) had argued that all these peripherals could be addressed with a single cell so there'd be no need for #address-cells = <2>, so then we went with that.

Reverting back to #address-cells = <2> is now going to cause quite a bit of churn, but I guess if it's the right thing, so be it.

Another possible alternative would be to move the memory-controller node from the bus@0 to the top-level. Not sure if that's any better.

Quoted 66 lines · Rob Herring

Quoted 57 lines · Thierry Reding

Quoted 6 lines

From: Thierry Reding <[email protected]>

DMA parent devices can define separate DMA busses via the "dma-ranges" and "#address-cells" and "#size-cells" properties. If the DMA bus has different cell counts than its parent, this can cause the translation of DMA address to fails (e.g. truncation from 2 to 1 address cells).

My assumption in this case was that the parent cell sizes should be increased to 2 cells. That tends to be what people want to do anyways (64-bit everywhere on 64-bit CPUs).

Quoted 4 lines

Avoid this by stopping to search for DMA parents when a parent without a "dma-ranges" property is encountered. Also, since it is the DMA parent that defines the DMA bus, use the bus' cell counts instead of its parent cell counts.

We treat no 'dma-ranges' as equivalent to 'dma-ranges;'. IIRC, the spec even says that because I hit that case.

Is this going to work for 'dma-device' with something like this?:

  bus@0 {
    dma-ranges = <...>;
    child-bus@... {
      dma-device@... {
      };
    };
  };

Quoted 27 lines

Signed-off-by Thierry Reding <[email protected]>

---
Changes in v10:
- new patch to avoid address truncation when traversing a bus hierarchy with mismatching #address-cells properties

Example from Tegra194 (redacted for clarity):

    reserved-memory {
            #address-cells = <2>;
            #size-cells = <2>;
            ranges;

            framebuffer@0,0 {
                    compatible = "framebuffer";
                    reg = <0x2 0x57320000 0x0 0x00800000>;
                    iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
            };
    };

    bus@0 {
            /* truncation happens here */
            #address-cells = <1>;
            #size-cells = <1>;
            ranges = <0x0 0x0 0x0 0x40000000>;

Quoted 6 lines

Quoted 3 lines

            mc: memory-controller@2c00000 {
                    #address-cells = <2>;
                    #size-cells = <2>;

I think this is wrong. The parent should have more or equal number of cells.

Quoted 10 lines

I was half suspecting that. The reason why I hesitated is that I recall having the opposite discussion a while ago when we were adding bus@0 to 64-bit Tegra devices. We had at some point (probably around Tegra114 or Tegra124, 32-bit ARM chips that support LPAE) started to set #address- cells = <2> precisely because the CPU could address more than 32-bit addresses. We then did the same thing transitioning to 64-bit ARM. When we then started discussing bus@0, someone (might have been you) had argued that all these peripherals could be addressed with a single cell so there'd be no need for #address-cells = <2>, so then we went with that.

I may have not thinking about the DMA side of things.

Quoted 3 lines

Reverting back to #address-cells = <2> is now going to cause quite a bit of churn, but I guess if it's the right thing, so be it.

Quoted 2 lines

Another possible alternative would be to move the memory-controller node from the bus@0 to the top-level. Not sure if that's any better.

I stumbled upon 'ibm,#dma-address-cells' and 'ibm,#dma-size-cells' while reviewing this. Those seem to be for the same purpose AFAICT. We could consider adding those (w/o 'ibm') to handle this situation.

Am Dienstag, dem 08.11.2022 um 10:25 -0600 schrieb Rob Herring:

Quoted 83 lines · Thierry Reding

Quoted 66 lines · Rob Herring

Quoted 57 lines · Thierry Reding

Quoted 6 lines

From: Thierry Reding <[email protected]>

DMA parent devices can define separate DMA busses via the "dma-ranges" and "#address-cells" and "#size-cells" properties. If the DMA bus has different cell counts than its parent, this can cause the translation of DMA address to fails (e.g. truncation from 2 to 1 address cells).

My assumption in this case was that the parent cell sizes should be increased to 2 cells. That tends to be what people want to do anyways (64-bit everywhere on 64-bit CPUs).

Quoted 4 lines

Avoid this by stopping to search for DMA parents when a parent without a "dma-ranges" property is encountered. Also, since it is the DMA parent that defines the DMA bus, use the bus' cell counts instead of its parent cell counts.

We treat no 'dma-ranges' as equivalent to 'dma-ranges;'. IIRC, the spec even says that because I hit that case.

Is this going to work for 'dma-device' with something like this?:

  bus@0 {
    dma-ranges = <...>;
    child-bus@... {
      dma-device@... {
      };
    };
  };

Quoted 27 lines

Signed-off-by Thierry Reding <[email protected]>

---
Changes in v10:
- new patch to avoid address truncation when traversing a bus hierarchy with mismatching #address-cells properties

Example from Tegra194 (redacted for clarity):

    reserved-memory {
            #address-cells = <2>;
            #size-cells = <2>;
            ranges;

            framebuffer@0,0 {
                    compatible = "framebuffer";
                    reg = <0x2 0x57320000 0x0 0x00800000>;
                    iommu-addresses = <&dc0 0x2 0x57320000 0x0 0x00800000>;
            };
    };

    bus@0 {
            /* truncation happens here */
            #address-cells = <1>;
            #size-cells = <1>;
            ranges = <0x0 0x0 0x0 0x40000000>;

Quoted 6 lines

Quoted 3 lines

            mc: memory-controller@2c00000 {
                    #address-cells = <2>;
                    #size-cells = <2>;

I think this is wrong. The parent should have more or equal number of cells.

Quoted 10 lines

I was half suspecting that. The reason why I hesitated is that I recall having the opposite discussion a while ago when we were adding bus@0 to 64-bit Tegra devices. We had at some point (probably around Tegra114 or Tegra124, 32-bit ARM chips that support LPAE) started to set #address- cells = <2> precisely because the CPU could address more than 32-bit addresses. We then did the same thing transitioning to 64-bit ARM. When we then started discussing bus@0, someone (might have been you) had argued that all these peripherals could be addressed with a single cell so there'd be no need for #address-cells = <2>, so then we went with that.

I may have not thinking about the DMA side of things.

Quoted 3 lines

Reverting back to #address-cells = <2> is now going to cause quite a bit of churn, but I guess if it's the right thing, so be it.

Quoted 6 lines

Quoted 2 lines

Another possible alternative would be to move the memory-controller node from the bus@0 to the top-level. Not sure if that's any better.

I stumbled upon 'ibm,#dma-address-cells' and 'ibm,#dma-size-cells' while reviewing this. Those seem to be for the same purpose AFAICT. We could consider adding those (w/o 'ibm') to handle this situation.

I would appreciate this. We have the same situation on some of the NXP i.MX8 SoCs right now: all the MMIO is addressable with 32bit, so all the busses have a single address and size cell right now, but we would need to extend the address-cells to 64bit just to properly describe the DMA addressing capabilities of the devices.