[PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA

[PATCH v2 0/5] arm64: Default to 32-bit wide ZONE_DMA

[Nicolas Saenz Julienne]

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

[PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()

[Nicolas Saenz Julienne]

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

[PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

[Nicolas Saenz Julienne]

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

[PATCH v2 3/5] dma-direct: Turn zone_dma_bits default value into a define

[Nicolas Saenz Julienne]

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

[PATCH v2 4/5] arm64: mm: Dynamically resize zone_dma_bits based on system's constraints

[Nicolas Saenz Julienne]

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

[PATCH v2 5/5] mm: Update DMA zones description

[Nicolas Saenz Julienne]

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

Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

[Ard Biesheuvel]

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

Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()

[Catalin Marinas]

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

Re: [PATCH v2 1/5] arm64: mm: Move zone_dma_bits initialization into zone_sizes_init()

[Ard Biesheuvel]

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

Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

[Rob Herring]

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

Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

[Nicolas Saenz Julienne]

[-- 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

Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

[Nicolas Saenz Julienne]

[-- 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

Re: [PATCH v2 2/5] of/address: Introduce of_dma_lower_bus_limit()

[Rob Herring]

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