On Tue, Mar 11, 2025 at 09:59:32PM +0000, Russell King (Oracle) wrote:

On Tue, Mar 11, 2025 at 05:51:06PM +0000, Mark Brown wrote:

quoted

On Thu, Mar 06, 2025 at 08:51:20PM +0200, Mike Rapoport wrote:

quoted

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

high_memory defines upper bound on the directly mapped memory.
This bound is defined by the beginning of ZONE_HIGHMEM when a system has
high memory and by the end of memory otherwise.

All this is known to generic memory management initialization code that
can set high_memory while initializing core mm structures.

Remove per-architecture calculation of high_memory and add a generic
version to free_area_init().

This patch appears to be causing breakage on a number of 32 bit arm
platforms, including qemu's virt-2.11,gic-version=3.  Affected platforms
die on boot with no output, a bisect with qemu points at this commit and
those for physical platforms appear to be converging on the same place.

I'm not convinced that the old and the new code is doing the same
thing.

The new code:

+       phys_addr_t highmem = memblock_end_of_DRAM();
+
+#ifdef CONFIG_HIGHMEM
+       unsigned long pfn = arch_zone_lowest_possible_pfn[ZONE_HIGHMEM];
+
+       if (arch_has_descending_max_zone_pfns() || highmem > PFN_PHYS(pfn))
+               highmem = PFN_PHYS(pfn);
+#endif
+
+       high_memory = phys_to_virt(highmem - 1) + 1;

First, when CONFIG_HIGHMEM is disabled, this code assumes that the last
byte of DRAM declared to memblock is the highmem limit. This _could_
overflow phys_to_virt() and lead to an invalid value for high_memory.

Second, arch_zone_lowest_possible_pfn[ZONE_HIGHMEM] is the _start_ of
highmem. This is not what arch code sets high_memory to - because
the start of highmem may not contiguously follow on from lowmem.

In arch/arm/mm/mmu.c, lowmem_limit is computed to be the highest + 1
physical address that lowmem can possibly be, taking into account the
amount of vmalloc memory that is required. This is used to set
high_memory.

We also limit the amount of usable RAM via memblock_set_current_limit()
which memblock_end_of_DRAM() doesn't respect.

I don't think the proposed generic version is suitable for 32-bit arm.

Unless I'm missing something, both memblock.current_limit and start of
ZONE_HIGHMEM are set to arm_lowmem_limit which will be different from
memblock_end_of_DRAM() only for machines with more than nearly 4GiB of RAM
and those will supposedly use HIGHMEM anyway.

But this does not matter anyway because failures Mark reported happen
because 32-bit arm uses high_memory before mem_init() and that what causes
the hangs. 

Here's the fix I have, I'll send v2 shortly.

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e492d58a0386..f02f872ea8a9 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c

@@ -1250,6 +1250,8 @@ void __init adjust_lowmem_bounds(void)
 
 	arm_lowmem_limit = lowmem_limit;
 
+	high_memory = __va(arm_lowmem_limit - 1) + 1;
+
 	if (!memblock_limit)
 		memblock_limit = arm_lowmem_limit;
 

diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 65903ed5e80d..1a8f6914ee59 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c

@@ -146,6 +146,7 @@ void __init adjust_lowmem_bounds(void)
 	phys_addr_t end;
 	adjust_lowmem_bounds_mpu();
 	end = memblock_end_of_DRAM();
+	high_memory = __va(end - 1) + 1;
 	memblock_set_current_limit(end);
 }
 

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 545e11f1a3ba..0aef4bef93c4 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c

@@ -1765,14 +1765,20 @@ static bool arch_has_descending_max_zone_pfns(void)
 
 static void set_high_memory(void)
 {
+	unsigned long pfn = arch_zone_lowest_possible_pfn[ZONE_HIGHMEM];
 	phys_addr_t highmem = memblock_end_of_DRAM();
 
-#ifdef CONFIG_HIGHMEM
-	unsigned long pfn = arch_zone_lowest_possible_pfn[ZONE_HIGHMEM];
+	/*
+	 * Some architectures (e.g. ARM) set high_memory very early and
+	 * use it in arch setup code.
+	 * If an architecture already set high_memory don't overwrite it
+	 */
+	if (high_memory)
+		return;
 
-	if (arch_has_descending_max_zone_pfns() || highmem > PFN_PHYS(pfn))
+	if (IS_ENABLED(CONFIG_HIGHMEM) &&
+	    (arch_has_descending_max_zone_pfns() || highmem > PFN_PHYS(pfn)))
 		highmem = PFN_PHYS(pfn);
-#endif
 
 	high_memory = phys_to_virt(highmem - 1) + 1;
 }

-- 
Sincerely yours,
Mike.