Re: [PATCH 1/4] of/fdt: Update zone_dma_bits when running in bcm2711
From: Jeremy Linton
Date: Thu Oct 08 2020 - 23:59:19 EST
On 10/8/20 2:43 PM, Ard Biesheuvel wrote:
(+ Lorenzo)
On Thu, 8 Oct 2020 at 12:14, Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
On Thu, Oct 08, 2020 at 12:05:25PM +0200, Nicolas Saenz Julienne wrote:
On Fri, 2020-10-02 at 12:55 +0100, Catalin Marinas wrote:
On Thu, Oct 01, 2020 at 07:31:19PM +0200, Nicolas Saenz Julienne wrote:
On Thu, 2020-10-01 at 18:23 +0100, Catalin Marinas wrote:
On Thu, Oct 01, 2020 at 06:15:01PM +0100, Catalin Marinas wrote:
On Thu, Oct 01, 2020 at 06:17:37PM +0200, Nicolas Saenz Julienne wrote:
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 4602e467ca8b..cd0d115ef329 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -25,6 +25,7 @@
#include <linux/serial_core.h>
#include <linux/sysfs.h>
#include <linux/random.h>
+#include <linux/dma-direct.h> /* for zone_dma_bits */
#include <asm/setup.h> /* for COMMAND_LINE_SIZE */
#include <asm/page.h>
@@ -1198,6 +1199,14 @@ void __init early_init_dt_scan_nodes(void)
of_scan_flat_dt(early_init_dt_scan_memory, NULL);
}
+void __init early_init_dt_update_zone_dma_bits(void)
+{
+ unsigned long dt_root = of_get_flat_dt_root();
+
+ if (of_flat_dt_is_compatible(dt_root, "brcm,bcm2711"))
+ zone_dma_bits = 30;
+}
I think we could keep this entirely in the arm64 setup_machine_fdt() and
not pollute the core code with RPi4-specific code.
Actually, even better, could we not move the check to
arm64_memblock_init() when we initialise zone_dma_bits?
I did it this way as I vaguely remembered Rob saying he wanted to centralise
all early boot fdt code in one place. But I'll be happy to move it there.
I can see Rob replied and I'm fine if that's his preference. However,
what I don't particularly like is that in the arm64 code, if
zone_dma_bits == 24, we set it to 32 assuming that it wasn't touched by
the early_init_dt_update_zone_dma_bits(). What if at some point we'll
get a platform that actually needs 24 here (I truly hope not, but just
the principle of relying on magic values)?
So rather than guessing, I'd prefer if the arch code can override
ZONE_DMA_BITS_DEFAULT. Then, in arm64, we'll just set it to 32 and no
need to explicitly touch the zone_dma_bits variable.
Yes, sonds like the way to go. TBH I wasn't happy with that solution either,
but couldn't think of a nicer alternative.
Sadly I just realised that the series is incomplete, we have RPi4 users that
want to boot unsing ACPI, and this series would break things for them. I'll
have a word with them to see what we can do for their use-case.
Is there a way to get some SoC information from ACPI?
This is unfortunate. We used ACPI _DMA methods as they were designed
to communicate the DMA limit of the XHCI controller to the OS.
It shouldn't be too hard to match the OEM id field in the DSDT, and
switch to the smaller mask. But it sucks to have to add a quirk like
that.
It also requires delaying setting the arm64_dma_phy_limit a bit, but
that doesn't appear to be causing a problem. I've been boot/compiling
with a patch like:
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index cada0b816c8a..9dfe776c1c75 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <linux/cpumask.h>
+#include <linux/dma-direct.h>
#include <linux/efi.h>
#include <linux/efi-bgrt.h>
#include <linux/init.h>
@@ -168,6 +169,11 @@ static int __init acpi_fadt_sanity_check(void)
ret = -EINVAL;
}
+ if (!strncmp(table->oem_id, "RPIFDN", ACPI_OEM_ID_SIZE) &&
+ !strncmp(table->oem_table_id, "RPI4 ",
ACPI_OEM_TABLE_ID_SIZE) &&
+ table->oem_revision <= 0x200) {
+ zone_dma_bits = 30;
+ }
out:
/*
* acpi_get_table() creates FADT table mapping that
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index cd5caca8a929..6c8aaf1570ce 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -195,6 +195,7 @@ 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
+ 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
@@ -393,7 +394,6 @@ void __init arm64_memblock_init(void)
*/
if (zone_dma_bits == ZONE_DMA_BITS_DEFAULT)
zone_dma_bits = 32;
- arm64_dma_phys_limit = max_zone_phys(zone_dma_bits);
}
if (IS_ENABLED(CONFIG_ZONE_DMA32))