2020-10-21 13:59:28

by Nicolas Saenz Julienne

[permalink] [raw]
Subject: [PATCH v4 6/7] arm64: mm: Set ZONE_DMA size based on early IORT scan

From: Ard Biesheuvel <[email protected]>

We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
incorporating masters that can address less than 32 bits of DMA, in
particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
peripherals that can only address up to 1 GB (and its PCIe host
bridge can only access the bottom 3 GB)

Instructing the DMA layer about these limitations is straight-forward,
even though we had to fix some issues regarding memory limits set in
the IORT for named components, and regarding the handling of ACPI _DMA
methods. However, the DMA layer also needs to be able to allocate
memory that is guaranteed to meet those DMA constraints, for bounce
buffering as well as allocating the backing for consistent mappings.

This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
problems with kdump, and potentially in other places where allocations
cannot cross zone boundaries. Therefore, we should avoid having two
separate DMA zones when possible.

So let's do an early scan of the IORT, and only create the ZONE_DMA
if we encounter any devices that need it. This puts the burden on
the firmware to describe such limitations in the IORT, which may be
redundant (and less precise) if _DMA methods are also being provided.
However, it should be noted that this situation is highly unusual for
arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
the _DMA method if implemented, and so we will not lose the ability to
perform streaming DMA outside the ZONE_DMA if the _DMA method permits
it.

Cc: Jeremy Linton <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Nicolas Saenz Julienne <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Hanjun Guo <[email protected]>
Cc: Sudeep Holla <[email protected]>
Cc: Anshuman Khandual <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
[nsaenz: Rebased, removed documentation change and add declaration in acpi_iort.h]
Signed-off-by: Nicolas Saenz Julienne <[email protected]>

---

Changes since v3:
- Use min_not_zero()
- Check ACPI revision
- Remove unnecessary #ifdef in zone_sizes_init()

arch/arm64/mm/init.c | 3 ++-
drivers/acpi/arm64/iort.c | 52 +++++++++++++++++++++++++++++++++++++++
include/linux/acpi_iort.h | 4 +++
3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 94e38f99748b..f5d4f85506e4 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -29,6 +29,7 @@
#include <linux/kexec.h>
#include <linux/crash_dump.h>
#include <linux/hugetlb.h>
+#include <linux/acpi_iort.h>

#include <asm/boot.h>
#include <asm/fixmap.h>
@@ -190,7 +191,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)

#ifdef CONFIG_ZONE_DMA
dt_zone_dma_bits = ilog2(of_dma_get_max_cpu_address(NULL));
- zone_dma_bits = min(32U, dt_zone_dma_bits);
+ zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_iort_get_zone_dma_size());
arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
#endif
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 9929ff50c0c0..05fe4a076bab 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1718,3 +1718,55 @@ void __init acpi_iort_init(void)

iort_init_platform_devices();
}
+
+#ifdef CONFIG_ZONE_DMA
+/*
+ * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
+ * If so, return the smallest value encountered, or 32 otherwise.
+ */
+unsigned int __init acpi_iort_get_zone_dma_size(void)
+{
+ struct acpi_table_iort *iort;
+ struct acpi_iort_node *node, *end;
+ acpi_status status;
+ u8 limit = 32;
+ int i;
+
+ if (acpi_disabled)
+ return limit;
+
+ status = acpi_get_table(ACPI_SIG_IORT, 0,
+ (struct acpi_table_header **)&iort);
+ if (ACPI_FAILURE(status))
+ return limit;
+
+ node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
+ end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
+
+ for (i = 0; i < iort->node_count; i++) {
+ if (node >= end)
+ break;
+
+ switch (node->type) {
+ struct acpi_iort_named_component *ncomp;
+ struct acpi_iort_root_complex *rc;
+
+ case ACPI_IORT_NODE_NAMED_COMPONENT:
+ ncomp = (struct acpi_iort_named_component *)node->node_data;
+ limit = min_not_zero(limit, ncomp->memory_address_limit);
+ break;
+
+ case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
+ if (node->revision < 1)
+ break;
+
+ rc = (struct acpi_iort_root_complex *)node->node_data;
+ limit = min_not_zero(limit, rc->memory_address_limit);
+ break;
+ }
+ node = ACPI_ADD_PTR(struct acpi_iort_node, node, node->length);
+ }
+ acpi_put_table(&iort->header);
+ return limit;
+}
+#endif
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 20a32120bb88..7d2e184f0d4d 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
const u32 *id_in);
int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
+unsigned int acpi_iort_get_zone_dma_size(void);
#else
static inline void acpi_iort_init(void) { }
static inline u32 iort_msi_map_id(struct device *dev, u32 id)
@@ -55,6 +56,9 @@ static inline const struct iommu_ops *iort_iommu_configure_id(
static inline
int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
{ return 0; }
+
+static inline unsigned int acpi_iort_get_zone_dma_size(void)
+{ return 32; }
#endif

#endif /* __ACPI_IORT_H__ */
--
2.28.0


2020-10-28 07:17:20

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] arm64: mm: Set ZONE_DMA size based on early IORT scan

On Wed, Oct 21, 2020 at 02:34:36PM +0200, Nicolas Saenz Julienne wrote:
> From: Ard Biesheuvel <[email protected]>
>
> We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
> incorporating masters that can address less than 32 bits of DMA, in
> particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
> peripherals that can only address up to 1 GB (and its PCIe host
> bridge can only access the bottom 3 GB)
>
> Instructing the DMA layer about these limitations is straight-forward,
> even though we had to fix some issues regarding memory limits set in
> the IORT for named components, and regarding the handling of ACPI _DMA
> methods. However, the DMA layer also needs to be able to allocate
> memory that is guaranteed to meet those DMA constraints, for bounce
> buffering as well as allocating the backing for consistent mappings.
>
> This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
> it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
> problems with kdump, and potentially in other places where allocations
> cannot cross zone boundaries. Therefore, we should avoid having two
> separate DMA zones when possible.
>
> So let's do an early scan of the IORT, and only create the ZONE_DMA
> if we encounter any devices that need it. This puts the burden on
> the firmware to describe such limitations in the IORT, which may be
> redundant (and less precise) if _DMA methods are also being provided.
> However, it should be noted that this situation is highly unusual for
> arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
> the _DMA method if implemented, and so we will not lose the ability to
> perform streaming DMA outside the ZONE_DMA if the _DMA method permits
> it.
>
> Cc: Jeremy Linton <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Nicolas Saenz Julienne <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Sudeep Holla <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> [nsaenz: Rebased, removed documentation change and add declaration in acpi_iort.h]
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>
> ---
>
> Changes since v3:
> - Use min_not_zero()
> - Check ACPI revision
> - Remove unnecessary #ifdef in zone_sizes_init()
>
> arch/arm64/mm/init.c | 3 ++-
> drivers/acpi/arm64/iort.c | 52 +++++++++++++++++++++++++++++++++++++++
> include/linux/acpi_iort.h | 4 +++
> 3 files changed, 58 insertions(+), 1 deletion(-)

Acked-by: Lorenzo Pieralisi <[email protected]>

> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 94e38f99748b..f5d4f85506e4 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -29,6 +29,7 @@
> #include <linux/kexec.h>
> #include <linux/crash_dump.h>
> #include <linux/hugetlb.h>
> +#include <linux/acpi_iort.h>
>
> #include <asm/boot.h>
> #include <asm/fixmap.h>
> @@ -190,7 +191,7 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
>
> #ifdef CONFIG_ZONE_DMA
> dt_zone_dma_bits = ilog2(of_dma_get_max_cpu_address(NULL));
> - zone_dma_bits = min(32U, dt_zone_dma_bits);
> + zone_dma_bits = min3(32U, dt_zone_dma_bits, acpi_iort_get_zone_dma_size());
> arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
> max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> #endif
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 9929ff50c0c0..05fe4a076bab 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1718,3 +1718,55 @@ void __init acpi_iort_init(void)
>
> iort_init_platform_devices();
> }
> +
> +#ifdef CONFIG_ZONE_DMA
> +/*
> + * Check the IORT whether any devices exist whose DMA mask is < 32 bits.
> + * If so, return the smallest value encountered, or 32 otherwise.
> + */
> +unsigned int __init acpi_iort_get_zone_dma_size(void)
> +{
> + struct acpi_table_iort *iort;
> + struct acpi_iort_node *node, *end;
> + acpi_status status;
> + u8 limit = 32;
> + int i;
> +
> + if (acpi_disabled)
> + return limit;
> +
> + status = acpi_get_table(ACPI_SIG_IORT, 0,
> + (struct acpi_table_header **)&iort);
> + if (ACPI_FAILURE(status))
> + return limit;
> +
> + node = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->node_offset);
> + end = ACPI_ADD_PTR(struct acpi_iort_node, iort, iort->header.length);
> +
> + for (i = 0; i < iort->node_count; i++) {
> + if (node >= end)
> + break;
> +
> + switch (node->type) {
> + struct acpi_iort_named_component *ncomp;
> + struct acpi_iort_root_complex *rc;
> +
> + case ACPI_IORT_NODE_NAMED_COMPONENT:
> + ncomp = (struct acpi_iort_named_component *)node->node_data;
> + limit = min_not_zero(limit, ncomp->memory_address_limit);
> + break;
> +
> + case ACPI_IORT_NODE_PCI_ROOT_COMPLEX:
> + if (node->revision < 1)
> + break;
> +
> + rc = (struct acpi_iort_root_complex *)node->node_data;
> + limit = min_not_zero(limit, rc->memory_address_limit);
> + break;
> + }
> + node = ACPI_ADD_PTR(struct acpi_iort_node, node, node->length);
> + }
> + acpi_put_table(&iort->header);
> + return limit;
> +}
> +#endif
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index 20a32120bb88..7d2e184f0d4d 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
> const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
> const u32 *id_in);
> int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
> +unsigned int acpi_iort_get_zone_dma_size(void);
> #else
> static inline void acpi_iort_init(void) { }
> static inline u32 iort_msi_map_id(struct device *dev, u32 id)
> @@ -55,6 +56,9 @@ static inline const struct iommu_ops *iort_iommu_configure_id(
> static inline
> int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
> { return 0; }
> +
> +static inline unsigned int acpi_iort_get_zone_dma_size(void)
> +{ return 32; }
> #endif
>
> #endif /* __ACPI_IORT_H__ */
> --
> 2.28.0
>

2020-10-29 00:55:34

by Hanjun Guo

[permalink] [raw]
Subject: Re: [PATCH v4 6/7] arm64: mm: Set ZONE_DMA size based on early IORT scan

On 2020/10/21 20:34, Nicolas Saenz Julienne wrote:
> From: Ard Biesheuvel <[email protected]>
>
> We recently introduced a 1 GB sized ZONE_DMA to cater for platforms
> incorporating masters that can address less than 32 bits of DMA, in
> particular the Raspberry Pi 4, which has 4 or 8 GB of DRAM, but has
> peripherals that can only address up to 1 GB (and its PCIe host
> bridge can only access the bottom 3 GB)
>
> Instructing the DMA layer about these limitations is straight-forward,
> even though we had to fix some issues regarding memory limits set in
> the IORT for named components, and regarding the handling of ACPI _DMA
> methods. However, the DMA layer also needs to be able to allocate
> memory that is guaranteed to meet those DMA constraints, for bounce
> buffering as well as allocating the backing for consistent mappings.
>
> This is why the 1 GB ZONE_DMA was introduced recently. Unfortunately,
> it turns out the having a 1 GB ZONE_DMA as well as a ZONE_DMA32 causes
> problems with kdump, and potentially in other places where allocations
> cannot cross zone boundaries. Therefore, we should avoid having two
> separate DMA zones when possible.
>
> So let's do an early scan of the IORT, and only create the ZONE_DMA
> if we encounter any devices that need it. This puts the burden on
> the firmware to describe such limitations in the IORT, which may be
> redundant (and less precise) if _DMA methods are also being provided.
> However, it should be noted that this situation is highly unusual for
> arm64 ACPI machines. Also, the DMA subsystem still gives precedence to
> the _DMA method if implemented, and so we will not lose the ability to
> perform streaming DMA outside the ZONE_DMA if the _DMA method permits
> it.
>
> Cc: Jeremy Linton <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Nicolas Saenz Julienne <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Hanjun Guo <[email protected]>
> Cc: Sudeep Holla <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> [nsaenz: Rebased, removed documentation change and add declaration in acpi_iort.h]
> Signed-off-by: Nicolas Saenz Julienne <[email protected]>
>
> ---
>
> Changes since v3:
> - Use min_not_zero()
> - Check ACPI revision
> - Remove unnecessary #ifdef in zone_sizes_init()
>
> arch/arm64/mm/init.c | 3 ++-
> drivers/acpi/arm64/iort.c | 52 +++++++++++++++++++++++++++++++++++++++
> include/linux/acpi_iort.h | 4 +++

Acked-by: Hanjun Guo <[email protected]>