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:
> > On Thu, Mar 06, 2025 at 08:51:20PM +0200, Mike Rapoport wrote:
> > > From: "Mike Rapoport (Microsoft)" <rppt@xxxxxxxxxx>
> > >
> > > 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.
