IOMMU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA
@ 2020-10-10 15:12 Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() Nicolas Saenz Julienne
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-10 15:12 UTC (permalink / raw)
  To: robh+dt, catalin.marinas, hch, ardb, linux-kernel
  Cc: devicetree, linux-mm, jeremy.linton, iommu, linux-rpi-kernel,
	robin.murphy, linux-arm-kernel

I realized this morning after reading Ard's patch fixing the same issue
in ACPI that we can move the zone_dma_bits initialization later in the
init process. This permits the use of OF to parse dma-ranges in the
system. Something we though we couldn't do on previous iterations of
this.

The series sits on top of Ard's patch "arm64: mm: set ZONE_DMA size
based on early IORT scan."

--- Original cover letter

Using two distinct DMA zones turned out to be problematic. Here's an
attempt go back to a saner default.

I tested this on both a RPi4 and QEMU.

---

Changes since v1:
 - Parse dma-ranges instead of using machine compatible string

Nicolas Saenz Julienne (5):
  arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
  of/address: Introduce of_dma_lower_bus_limit()
  dma-direct: Turn zone_dma_bits default value into a define
  arm64: mm: Dynamically resize zone_dma_bits based on system's
    constraints
  mm: Update DMA zones description

 arch/arm64/include/asm/processor.h |  1 +
 arch/arm64/mm/init.c               | 12 ++++-------
 drivers/of/address.c               | 34 ++++++++++++++++++++++++++++++
 include/linux/dma-direct.h         |  3 +++
 include/linux/mmzone.h             |  5 +++--
 include/linux/of.h                 |  7 ++++++
 kernel/dma/direct.c                |  2 +-
 7 files changed, 53 insertions(+), 11 deletions(-)

-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
  2020-10-10 15:12 [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA Nicolas Saenz Julienne
@ 2020-10-10 15:12 ` Nicolas Saenz Julienne
  2020-10-12 11:37   ` Catalin Marinas
  2020-10-10 15:12 ` [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit() Nicolas Saenz Julienne
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-10 15:12 UTC (permalink / raw)
  To: robh+dt, catalin.marinas, hch, ardb, linux-kernel
  Cc: devicetree, Will Deacon, jeremy.linton, iommu, linux-rpi-kernel,
	robin.murphy, linux-arm-kernel

There no use for initializing it earlier in arm64_memblock_init().

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/arm64/mm/init.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index f6902a2b4ea6..0eca5865dcb1 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -196,14 +196,16 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 	unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
+	zone_dma_bits = ARM64_ZONE_DMA_BITS;
+
 	if (IS_ENABLED(CONFIG_ACPI)) {
 		extern unsigned int acpi_iort_get_zone_dma_size(void);
 
 		zone_dma_bits = min(zone_dma_bits,
 				    acpi_iort_get_zone_dma_size());
-		arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
 	}
 
+	arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
 	max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
 #endif
 #ifdef CONFIG_ZONE_DMA32
@@ -394,11 +396,6 @@ void __init arm64_memblock_init(void)
 
 	early_init_fdt_scan_reserved_mem();
 
-	if (IS_ENABLED(CONFIG_ZONE_DMA)) {
-		zone_dma_bits = ARM64_ZONE_DMA_BITS;
-		arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
-	}
-
 	if (IS_ENABLED(CONFIG_ZONE_DMA32))
 		arm64_dma32_phys_limit = max_zone_phys(32);
 	else
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
  2020-10-10 15:12 [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() Nicolas Saenz Julienne
@ 2020-10-10 15:12 ` Nicolas Saenz Julienne
  2020-10-11  7:47   ` Ard Biesheuvel
  2020-10-12 15:25   ` Rob Herring
  2020-10-10 15:12 ` [PATCH v2 3/5] dma-direct: Turn zone_dma_bits default value into a define Nicolas Saenz Julienne
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-10 15:12 UTC (permalink / raw)
  To: robh+dt, catalin.marinas, hch, ardb, linux-kernel, Frank Rowand
  Cc: devicetree, jeremy.linton, iommu, linux-rpi-kernel, robin.murphy,
	linux-arm-kernel

The function provides the CPU physical address addressable by the most
constrained bus in the system. It might be useful in order to
dynamically set up memory zones during boot.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/of.h   |  7 +++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index eb9ab4f1e80b..755e97b65096 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
 }
 #endif /* CONFIG_HAS_DMA */
 
+/**
+ * of_dma_safe_phys_limit - Get system wide DMA safe address space
+ *
+ * Gets the CPU physical address limit for safe DMA addressing system wide by
+ * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
+ */
+u64 __init of_dma_safe_phys_limit(void)
+{
+	struct device_node *np = NULL;
+	struct of_range_parser parser;
+	const __be32 *ranges = NULL;
+	u64 phys_dma_limit = ~0ULL;
+	struct of_range range;
+	int len;
+
+	for_each_of_allnodes(np) {
+		dma_addr_t cpu_end = 0;
+
+		ranges = of_get_property(np, "dma-ranges", &len);
+		if (!ranges || !len)
+			continue;
+
+		of_dma_range_parser_init(&parser, np);
+		for_each_of_range(&parser, &range)
+			if (range.cpu_addr + range.size > cpu_end)
+				cpu_end = range.cpu_addr + range.size;
+
+		if (phys_dma_limit > cpu_end)
+			phys_dma_limit = cpu_end;
+	}
+
+	return phys_dma_limit;
+}
+
 /**
  * of_dma_is_coherent - Check if device is coherent
  * @np:	device node
diff --git a/include/linux/of.h b/include/linux/of.h
index 481ec0467285..958c64cffa92 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
 	       const char *map_name, const char *map_mask_name,
 	       struct device_node **target, u32 *id_out);
 
+u64 of_dma_safe_phys_limit(void);
+
 #else /* CONFIG_OF */
 
 static inline void of_core_init(void)
@@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
 	return -EINVAL;
 }
 
+static inline u64 of_dma_safe_phys_limit(void)
+{
+	return ~0ULL;
+}
+
 #define of_match_ptr(_ptr)	NULL
 #define of_match_node(_matches, _node)	NULL
 #endif /* CONFIG_OF */
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 3/5] dma-direct: Turn zone_dma_bits default value into a define
  2020-10-10 15:12 [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit() Nicolas Saenz Julienne
@ 2020-10-10 15:12 ` Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 4/5] arm64: mm: Dynamically resize zone_dma_bits based on system's constraints Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 5/5] mm: Update DMA zones description Nicolas Saenz Julienne
  4 siblings, 0 replies; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-10 15:12 UTC (permalink / raw)
  To: robh+dt, catalin.marinas, hch, ardb, linux-kernel,
	Marek Szyprowski, Robin Murphy
  Cc: linux-arm-kernel, devicetree, jeremy.linton, iommu, linux-rpi-kernel

So as for architecture code to set their own default values when
relevant.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---

Note: This is not really needed, but I think it nicer having
architectures use this than setting zone_dma_bits in a random place in
arch code. That said, I'll hapily edit it out if you don't agree.

 include/linux/dma-direct.h | 3 +++
 kernel/dma/direct.c        | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 18aade195884..e433d90cbacf 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -12,6 +12,9 @@
 #include <linux/mem_encrypt.h>
 #include <linux/swiotlb.h>
 
+#ifndef ZONE_DMA_BITS_DEFAULT
+#define ZONE_DMA_BITS_DEFAULT	24
+#endif
 extern unsigned int zone_dma_bits;
 
 /*
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 06c111544f61..c0d97f536e93 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -20,7 +20,7 @@
  * it for entirely different regions. In that case the arch code needs to
  * override the variable below for dma-direct to work properly.
  */
-unsigned int zone_dma_bits __ro_after_init = 24;
+unsigned int zone_dma_bits __ro_after_init = ZONE_DMA_BITS_DEFAULT;
 
 static inline dma_addr_t phys_to_dma_direct(struct device *dev,
 		phys_addr_t phys)
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 4/5] arm64: mm: Dynamically resize zone_dma_bits based on system's constraints
  2020-10-10 15:12 [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA Nicolas Saenz Julienne
                   ` (2 preceding siblings ...)
  2020-10-10 15:12 ` [PATCH v2 3/5] dma-direct: Turn zone_dma_bits default value into a define Nicolas Saenz Julienne
@ 2020-10-10 15:12 ` Nicolas Saenz Julienne
  2020-10-10 15:12 ` [PATCH v2 5/5] mm: Update DMA zones description Nicolas Saenz Julienne
  4 siblings, 0 replies; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-10 15:12 UTC (permalink / raw)
  To: robh+dt, catalin.marinas, hch, ardb, linux-kernel
  Cc: devicetree, Will Deacon, jeremy.linton, iommu, linux-rpi-kernel,
	robin.murphy, linux-arm-kernel

With the help of of_dma_safe_phys_limit() we can get the topmost
physical address accessible for DMA to the whole system and use that
information to properly setup zone_dma_bits.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 arch/arm64/include/asm/processor.h | 1 +
 arch/arm64/mm/init.c               | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index fce8cbecd6bc..c09d3f1a9a6b 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -97,6 +97,7 @@
 
 extern phys_addr_t arm64_dma_phys_limit;
 #define ARCH_LOW_ADDRESS_LIMIT	(arm64_dma_phys_limit - 1)
+#define ZONE_DMA_BITS_DEFAULT	32
 
 struct debug_info {
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 0eca5865dcb1..5934df93bf4d 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -42,8 +42,6 @@
 #include <asm/tlb.h>
 #include <asm/alternative.h>
 
-#define ARM64_ZONE_DMA_BITS	30
-
 /*
  * We need to be able to catch inadvertent references to memstart_addr
  * that occur (potentially in generic code) before arm64_memblock_init()
@@ -196,7 +194,8 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
 	unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
 
 #ifdef CONFIG_ZONE_DMA
-	zone_dma_bits = ARM64_ZONE_DMA_BITS;
+	zone_dma_bits = min(zone_dma_bits,
+			    (unsigned int)ilog2(of_dma_safe_phys_limit()));
 
 	if (IS_ENABLED(CONFIG_ACPI)) {
 		extern unsigned int acpi_iort_get_zone_dma_size(void);
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 5/5] mm: Update DMA zones description
  2020-10-10 15:12 [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA Nicolas Saenz Julienne
                   ` (3 preceding siblings ...)
  2020-10-10 15:12 ` [PATCH v2 4/5] arm64: mm: Dynamically resize zone_dma_bits based on system's constraints Nicolas Saenz Julienne
@ 2020-10-10 15:12 ` Nicolas Saenz Julienne
  4 siblings, 0 replies; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-10 15:12 UTC (permalink / raw)
  To: robh+dt, catalin.marinas, hch, ardb, linux-kernel, Andrew Morton
  Cc: devicetree, linux-mm, jeremy.linton, iommu, linux-rpi-kernel,
	robin.murphy, linux-arm-kernel

The default behavior for arm64 changed, so reflect that.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 include/linux/mmzone.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fb3bf696c05e..4ee2306351b9 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -363,8 +363,9 @@ enum zone_type {
 	 *  - arm only uses ZONE_DMA, the size, up to 4G, may vary depending on
 	 *    the specific device.
 	 *
-	 *  - arm64 has a fixed 1G ZONE_DMA and ZONE_DMA32 for the rest of the
-	 *    lower 4G.
+	 *  - arm64 uses a single 4GB ZONE_DMA, except on the Raspberry Pi 4,
+	 *    in which ZONE_DMA covers the first GB and ZONE_DMA32 the rest of
+	 *    the lower 4GB.
 	 *
 	 *  - powerpc only uses ZONE_DMA, the size, up to 2G, may vary
 	 *    depending on the specific device.
-- 
2.28.0

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
  2020-10-10 15:12 ` [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit() Nicolas Saenz Julienne
@ 2020-10-11  7:47   ` Ard Biesheuvel
  2020-10-14 11:34     ` Nicolas Saenz Julienne
  2020-10-12 15:25   ` Rob Herring
  1 sibling, 1 reply; 13+ messages in thread
From: Ard Biesheuvel @ 2020-10-11  7:47 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Catalin Marinas, Robin Murphy, Jeremy Linton,
	Linux Kernel Mailing List, iommu, Rob Herring, linux-rpi-kernel,
	Frank Rowand, Christoph Hellwig, Linux ARM

Hi Nicolas,

$SUBJECT is out of sync with the patch below. Also, for legibility, it
helps if the commit log is intelligible by itself, rather than relying
on $SUBJECT being the first line of the first paragraph.

On Sat, 10 Oct 2020 at 17:12, Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> The function provides the CPU physical address addressable by the most
> constrained bus in the system. It might be useful in order to
> dynamically set up memory zones during boot.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/of.h   |  7 +++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index eb9ab4f1e80b..755e97b65096 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
>  }
>  #endif /* CONFIG_HAS_DMA */
>
> +/**
> + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> + *
> + * Gets the CPU physical address limit for safe DMA addressing system wide by
> + * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
> + */
> +u64 __init of_dma_safe_phys_limit(void)

I don't think 'safe' strikes the right tone here. You are looking for
the highest CPU address that is addressable by all DMA masters in the
system.

Something like

of_dma_get_max_cpu_address(void)

perhaps? Also, since this is generic code, phys_addr_t is probably a
better type to return.


> +{
> +       struct device_node *np = NULL;
> +       struct of_range_parser parser;
> +       const __be32 *ranges = NULL;

I think you can drop these NULL initializers.

> +       u64 phys_dma_limit = ~0ULL;

PHYS_ADDR_MAX

> +       struct of_range range;
> +       int len;
> +
> +       for_each_of_allnodes(np) {
> +               dma_addr_t cpu_end = 0;
> +
> +               ranges = of_get_property(np, "dma-ranges", &len);
> +               if (!ranges || !len)
> +                       continue;
> +
> +               of_dma_range_parser_init(&parser, np);
> +               for_each_of_range(&parser, &range)
> +                       if (range.cpu_addr + range.size > cpu_end)
> +                               cpu_end = range.cpu_addr + range.size;
> +
> +               if (phys_dma_limit > cpu_end)
> +                       phys_dma_limit = cpu_end;
> +       }
> +
> +       return phys_dma_limit;
> +}
> +
>  /**
>   * of_dma_is_coherent - Check if device is coherent
>   * @np:        device node
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 481ec0467285..958c64cffa92 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
>                const char *map_name, const char *map_mask_name,
>                struct device_node **target, u32 *id_out);
>
> +u64 of_dma_safe_phys_limit(void);
> +
>  #else /* CONFIG_OF */
>
>  static inline void of_core_init(void)
> @@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
>         return -EINVAL;
>  }
>
> +static inline u64 of_dma_safe_phys_limit(void)
> +{
> +       return ~0ULL;
> +}
> +
>  #define of_match_ptr(_ptr)     NULL
>  #define of_match_node(_matches, _node) NULL
>  #endif /* CONFIG_OF */
> --
> 2.28.0
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
  2020-10-10 15:12 ` [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() Nicolas Saenz Julienne
@ 2020-10-12 11:37   ` Catalin Marinas
  2020-10-12 14:23     ` Ard Biesheuvel
  0 siblings, 1 reply; 13+ messages in thread
From: Catalin Marinas @ 2020-10-12 11:37 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: devicetree, Will Deacon, linux-kernel, jeremy.linton, ardb,
	iommu, robh+dt, linux-rpi-kernel, robin.murphy, hch,
	linux-arm-kernel

On Sat, Oct 10, 2020 at 05:12:31PM +0200, Nicolas Saenz Julienne wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index f6902a2b4ea6..0eca5865dcb1 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -196,14 +196,16 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
>  	unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
>  
>  #ifdef CONFIG_ZONE_DMA
> +	zone_dma_bits = ARM64_ZONE_DMA_BITS;
> +
>  	if (IS_ENABLED(CONFIG_ACPI)) {
>  		extern unsigned int acpi_iort_get_zone_dma_size(void);
>  
>  		zone_dma_bits = min(zone_dma_bits,
>  				    acpi_iort_get_zone_dma_size());
> -		arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
>  	}
>  
> +	arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
>  	max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
>  #endif
>  #ifdef CONFIG_ZONE_DMA32
> @@ -394,11 +396,6 @@ void __init arm64_memblock_init(void)
>  
>  	early_init_fdt_scan_reserved_mem();
>  
> -	if (IS_ENABLED(CONFIG_ZONE_DMA)) {
> -		zone_dma_bits = ARM64_ZONE_DMA_BITS;
> -		arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
> -	}

arm64_dma_phys_limit is used by memblock_alloc_low() (via
ARCH_LOW_ADDRESS_LIMIT). I think it's too late to leave its
initialisation to zone_sizes_init().

-- 
Catalin
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()
  2020-10-12 11:37   ` Catalin Marinas
@ 2020-10-12 14:23     ` Ard Biesheuvel
  0 siblings, 0 replies; 13+ messages in thread
From: Ard Biesheuvel @ 2020-10-12 14:23 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Will Deacon, Linux Kernel Mailing List, Jeremy Linton, iommu,
	Rob Herring, Robin Murphy, Christoph Hellwig, Linux ARM,
	linux-rpi-kernel

On Mon, 12 Oct 2020 at 13:37, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Sat, Oct 10, 2020 at 05:12:31PM +0200, Nicolas Saenz Julienne wrote:
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index f6902a2b4ea6..0eca5865dcb1 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -196,14 +196,16 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> >       unsigned long max_zone_pfns[MAX_NR_ZONES]  = {0};
> >
> >  #ifdef CONFIG_ZONE_DMA
> > +     zone_dma_bits = ARM64_ZONE_DMA_BITS;
> > +
> >       if (IS_ENABLED(CONFIG_ACPI)) {
> >               extern unsigned int acpi_iort_get_zone_dma_size(void);
> >
> >               zone_dma_bits = min(zone_dma_bits,
> >                                   acpi_iort_get_zone_dma_size());
> > -             arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
> >       }
> >
> > +     arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
> >       max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> >  #endif
> >  #ifdef CONFIG_ZONE_DMA32
> > @@ -394,11 +396,6 @@ void __init arm64_memblock_init(void)
> >
> >       early_init_fdt_scan_reserved_mem();
> >
> > -     if (IS_ENABLED(CONFIG_ZONE_DMA)) {
> > -             zone_dma_bits = ARM64_ZONE_DMA_BITS;
> > -             arm64_dma_phys_limit = max_zone_phys(ARM64_ZONE_DMA_BITS);
> > -     }
>
> arm64_dma_phys_limit is used by memblock_alloc_low() (via
> ARCH_LOW_ADDRESS_LIMIT). I think it's too late to leave its
> initialisation to zone_sizes_init().
>

The only generic caller of memblock_alloc_low() is swiotlb_init(),
which is called much later. So at that point, we definitely need
ARCH_LOW_ADDRESS_LIMIT to be set correctly, but that means doing it in
zone_sizes_init() is early enough.

So the only problematic reference seems to be crashkernel_reserve() afaict.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
  2020-10-10 15:12 ` [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit() Nicolas Saenz Julienne
  2020-10-11  7:47   ` Ard Biesheuvel
@ 2020-10-12 15:25   ` Rob Herring
  2020-10-14 11:52     ` Nicolas Saenz Julienne
  1 sibling, 1 reply; 13+ messages in thread
From: Rob Herring @ 2020-10-12 15:25 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: devicetree, Frank Rowand, Catalin Marinas, linux-kernel,
	Jeremy Linton, Ard Biesheuvel, Linux IOMMU,
	moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, Robin Murphy,
	Christoph Hellwig, linux-arm-kernel

On Sat, Oct 10, 2020 at 10:12 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> The function provides the CPU physical address addressable by the most
> constrained bus in the system. It might be useful in order to
> dynamically set up memory zones during boot.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/of.h   |  7 +++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index eb9ab4f1e80b..755e97b65096 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
>  }
>  #endif /* CONFIG_HAS_DMA */
>
> +/**
> + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> + *
> + * Gets the CPU physical address limit for safe DMA addressing system wide by
> + * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
> + */
> +u64 __init of_dma_safe_phys_limit(void)
> +{
> +       struct device_node *np = NULL;
> +       struct of_range_parser parser;
> +       const __be32 *ranges = NULL;
> +       u64 phys_dma_limit = ~0ULL;
> +       struct of_range range;
> +       int len;
> +
> +       for_each_of_allnodes(np) {
> +               dma_addr_t cpu_end = 0;
> +
> +               ranges = of_get_property(np, "dma-ranges", &len);
> +               if (!ranges || !len)
> +                       continue;
> +
> +               of_dma_range_parser_init(&parser, np);
> +               for_each_of_range(&parser, &range)
> +                       if (range.cpu_addr + range.size > cpu_end)
> +                               cpu_end = range.cpu_addr + range.size;

This doesn't work if you have more than one level of dma-ranges. The
address has to be translated first. It should be okay to do that on
the start or end address (if not, your DT is broken).

Please add/extend a unittest for this.

> +
> +               if (phys_dma_limit > cpu_end)
> +                       phys_dma_limit = cpu_end;
> +       }
> +
> +       return phys_dma_limit;
> +}
> +
>  /**
>   * of_dma_is_coherent - Check if device is coherent
>   * @np:        device node
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 481ec0467285..958c64cffa92 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -558,6 +558,8 @@ int of_map_id(struct device_node *np, u32 id,
>                const char *map_name, const char *map_mask_name,
>                struct device_node **target, u32 *id_out);
>
> +u64 of_dma_safe_phys_limit(void);
> +
>  #else /* CONFIG_OF */
>
>  static inline void of_core_init(void)
> @@ -995,6 +997,11 @@ static inline int of_map_id(struct device_node *np, u32 id,
>         return -EINVAL;
>  }
>
> +static inline u64 of_dma_safe_phys_limit(void)
> +{
> +       return ~0ULL;
> +}
> +
>  #define of_match_ptr(_ptr)     NULL
>  #define of_match_node(_matches, _node) NULL
>  #endif /* CONFIG_OF */
> --
> 2.28.0
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
  2020-10-11  7:47   ` Ard Biesheuvel
@ 2020-10-14 11:34     ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-14 11:34 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Catalin Marinas, Robin Murphy, Jeremy Linton,
	Linux Kernel Mailing List, iommu, Rob Herring, linux-rpi-kernel,
	Frank Rowand, Christoph Hellwig, Linux ARM

[-- Attachment #1.1: Type: text/plain, Size: 1927 bytes --]

On Sun, 2020-10-11 at 09:47 +0200, Ard Biesheuvel wrote:
> Hi Nicolas,
> 
> $SUBJECT is out of sync with the patch below. Also, for legibility, it
> helps if the commit log is intelligible by itself, rather than relying
> on $SUBJECT being the first line of the first paragraph.

Noted, I'll update all commit logs.

> On Sat, 10 Oct 2020 at 17:12, Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > The function provides the CPU physical address addressable by the most
> > constrained bus in the system. It might be useful in order to
> > dynamically set up memory zones during boot.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> >  drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++
> >  include/linux/of.h   |  7 +++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index eb9ab4f1e80b..755e97b65096 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> >  }
> >  #endif /* CONFIG_HAS_DMA */
> > 
> > +/**
> > + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> > + *
> > + * Gets the CPU physical address limit for safe DMA addressing system wide by
> > + * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
> > + */
> > +u64 __init of_dma_safe_phys_limit(void)
> 
> I don't think 'safe' strikes the right tone here. You are looking for
> the highest CPU address that is addressable by all DMA masters in the
> system.
> 
> Something like
> 
> of_dma_get_max_cpu_address(void)
> 
> perhaps? Also, since this is generic code, phys_addr_t is probably a
> better type to return.

Sonds good to me, I dindn't like the name I used either.

Will use with phys_addr_t.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
  2020-10-12 15:25   ` Rob Herring
@ 2020-10-14 11:52     ` Nicolas Saenz Julienne
  2020-10-14 12:23       ` Rob Herring
  0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Saenz Julienne @ 2020-10-14 11:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Frank Rowand, Catalin Marinas, linux-kernel,
	Jeremy Linton, Ard Biesheuvel, Linux IOMMU,
	moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, Robin Murphy,
	Christoph Hellwig, linux-arm-kernel

[-- Attachment #1.1: Type: text/plain, Size: 2436 bytes --]

Hi Rob,

On Mon, 2020-10-12 at 10:25 -0500, Rob Herring wrote:
> On Sat, Oct 10, 2020 at 10:12 AM Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> > The function provides the CPU physical address addressable by the most
> > constrained bus in the system. It might be useful in order to
> > dynamically set up memory zones during boot.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > ---
> >  drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++
> >  include/linux/of.h   |  7 +++++++
> >  2 files changed, 41 insertions(+)
> > 
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index eb9ab4f1e80b..755e97b65096 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> >  }
> >  #endif /* CONFIG_HAS_DMA */
> > 
> > +/**
> > + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> > + *
> > + * Gets the CPU physical address limit for safe DMA addressing system wide by
> > + * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
> > + */
> > +u64 __init of_dma_safe_phys_limit(void)
> > +{
> > +       struct device_node *np = NULL;
> > +       struct of_range_parser parser;
> > +       const __be32 *ranges = NULL;
> > +       u64 phys_dma_limit = ~0ULL;
> > +       struct of_range range;
> > +       int len;
> > +
> > +       for_each_of_allnodes(np) {
> > +               dma_addr_t cpu_end = 0;
> > +
> > +               ranges = of_get_property(np, "dma-ranges", &len);
> > +               if (!ranges || !len)
> > +                       continue;
> > +
> > +               of_dma_range_parser_init(&parser, np);
> > +               for_each_of_range(&parser, &range)
> > +                       if (range.cpu_addr + range.size > cpu_end)
> > +                               cpu_end = range.cpu_addr + range.size;
> 
> This doesn't work if you have more than one level of dma-ranges. The
> address has to be translated first. It should be okay to do that on
> the start or end address (if not, your DT is broken).

for_each_of_range() calls of_pci_range_parser_one() which utimately populates
range.cpu_addr with of_translate_dma_address() results. Isn't that good enough?

> Please add/extend a unittest for this.

Will do.

Regards,
Nicolas


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()
  2020-10-14 11:52     ` Nicolas Saenz Julienne
@ 2020-10-14 12:23       ` Rob Herring
  0 siblings, 0 replies; 13+ messages in thread
From: Rob Herring @ 2020-10-14 12:23 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: devicetree, Frank Rowand, Catalin Marinas, linux-kernel,
	Jeremy Linton, Ard Biesheuvel, Linux IOMMU,
	moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, Robin Murphy,
	Christoph Hellwig, linux-arm-kernel

On Wed, Oct 14, 2020 at 6:52 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi Rob,
>
> On Mon, 2020-10-12 at 10:25 -0500, Rob Herring wrote:
> > On Sat, Oct 10, 2020 at 10:12 AM Nicolas Saenz Julienne
> > <nsaenzjulienne@suse.de> wrote:
> > > The function provides the CPU physical address addressable by the most
> > > constrained bus in the system. It might be useful in order to
> > > dynamically set up memory zones during boot.
> > >
> > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > > ---
> > >  drivers/of/address.c | 34 ++++++++++++++++++++++++++++++++++
> > >  include/linux/of.h   |  7 +++++++
> > >  2 files changed, 41 insertions(+)
> > >
> > > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > > index eb9ab4f1e80b..755e97b65096 100644
> > > --- a/drivers/of/address.c
> > > +++ b/drivers/of/address.c
> > > @@ -1024,6 +1024,40 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
> > >  }
> > >  #endif /* CONFIG_HAS_DMA */
> > >
> > > +/**
> > > + * of_dma_safe_phys_limit - Get system wide DMA safe address space
> > > + *
> > > + * Gets the CPU physical address limit for safe DMA addressing system wide by
> > > + * searching for the most constraining dma-range. Otherwise it returns ~0ULL.
> > > + */
> > > +u64 __init of_dma_safe_phys_limit(void)
> > > +{
> > > +       struct device_node *np = NULL;
> > > +       struct of_range_parser parser;
> > > +       const __be32 *ranges = NULL;
> > > +       u64 phys_dma_limit = ~0ULL;
> > > +       struct of_range range;
> > > +       int len;
> > > +
> > > +       for_each_of_allnodes(np) {
> > > +               dma_addr_t cpu_end = 0;
> > > +
> > > +               ranges = of_get_property(np, "dma-ranges", &len);
> > > +               if (!ranges || !len)
> > > +                       continue;
> > > +
> > > +               of_dma_range_parser_init(&parser, np);
> > > +               for_each_of_range(&parser, &range)
> > > +                       if (range.cpu_addr + range.size > cpu_end)
> > > +                               cpu_end = range.cpu_addr + range.size;
> >
> > This doesn't work if you have more than one level of dma-ranges. The
> > address has to be translated first. It should be okay to do that on
> > the start or end address (if not, your DT is broken).
>
> for_each_of_range() calls of_pci_range_parser_one() which utimately populates
> range.cpu_addr with of_translate_dma_address() results. Isn't that good enough?

Indeed. I guess I was remembering the cases not using
for_each_of_range which forgot to do that...

Rob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-10 15:12 [PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA Nicolas Saenz Julienne
2020-10-10 15:12 ` [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init() Nicolas Saenz Julienne
2020-10-12 11:37   ` Catalin Marinas
2020-10-12 14:23     ` Ard Biesheuvel
2020-10-10 15:12 ` [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit() Nicolas Saenz Julienne
2020-10-11  7:47   ` Ard Biesheuvel
2020-10-14 11:34     ` Nicolas Saenz Julienne
2020-10-12 15:25   ` Rob Herring
2020-10-14 11:52     ` Nicolas Saenz Julienne
2020-10-14 12:23       ` Rob Herring
2020-10-10 15:12 ` [PATCH v2 3/5] dma-direct: Turn zone_dma_bits default value into a define Nicolas Saenz Julienne
2020-10-10 15:12 ` [PATCH v2 4/5] arm64: mm: Dynamically resize zone_dma_bits based on system's constraints Nicolas Saenz Julienne
2020-10-10 15:12 ` [PATCH v2 5/5] mm: Update DMA zones description Nicolas Saenz Julienne

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org
	public-inbox-index linux-iommu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git