Re: [PATCH v3 24/29] arch, mm: consolidate initialization of SPARSE memory model

[Mike Rapoport <rppt@kernel.org>]

Hi Thomas,

On Mon, Feb 23, 2026 at 02:52:45PM +0100, Thomas Weißschuh wrote:
> Hi everyone,
> 
> On Sun, Jan 11, 2026 at 10:20:58AM +0200, Mike Rapoport wrote:
> > Every architecture calls sparse_init() during setup_arch() although the
> > data structures created by sparse_init() are not used until the
> > initialization of the core MM.
> > 
> > Beside the code duplication, calling sparse_init() from architecture
> > specific code causes ordering differences of vmemmap and HVO initialization
> > on different architectures.
> > 
> > Move the call to sparse_init() from architecture specific code to
> > free_area_init() to ensure that vmemmap and HVO initialization order is
> > always the same.
> 
> This broke the boot on RISC-V 32-bit (rv32_defconfig) for me.
> 
> Specifically if sparse_init() is *not* called before the following callchain,
> the kernel dies at that point.
> 
> start_kernel()
>   setup_arch()
>     apply_boot_alternatives()
>       _apply_alternatives()
>         riscv_cpufeature_patch_func()
>           patch_text_nosync()
>           riscv_alternative_fix_offsets()

Hm, most architectures do alternatives patching much later in the boot,
when much more subsystems (including mm) is already initialized.

Any particular reason riscv does it that early? 
 
> Simple reproducer, using kunit:
> 
> ./tools/testing/kunit/kunit.py run --raw_output=all --make_options LLVM=1 --arch riscv32 --kconfig_add CONFIG_SPARSEMEM_MANUAL=y --kconfig_add CONFIG_SPARSEMEM=y

Looking at patch_map it's quite clear why movement of sparse_init() cased a
crash:

	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr))
		page = phys_to_page(__pa_symbol(addr));

phys_to_page() with CONFIG_SPARSEMEM=y will try to access memory section
that are initialized in sparse_init().

What I don't understand is why patch_map() needs a struct page for kernel
text patching at all, __pa_symbol() should work just fine.
And the BUG_ON(!page) is completely bogus for phys_to_page() conversion,
because that one is pure arithmetics.

If moving apply_boot_alternatives() is not an option for riscv, something
like the patch below should fix the issue with access to nonexistent
memory sections. But I think moving apply_boot_alternatives() later in boot
would make things less fragile.

diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index db13c9ddf9e3..89b3c13f2865 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -43,18 +43,19 @@ static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
 {
 	uintptr_t uintaddr = (uintptr_t) addr;
 	struct page *page;
+	phys_addr_t phys;
 
-	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr))
-		page = phys_to_page(__pa_symbol(addr));
-	else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
+	if (core_kernel_text(uintaddr) || is_kernel_exittext(uintaddr)) {
+		phys = __pa_symbol(addr);
+	} else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
 		page = vmalloc_to_page(addr);
-	else
+		BUG_ON(!page);
+		phys = page_to_phys(page);
+	} else {
 		return addr;
+	}
 
-	BUG_ON(!page);
-
-	return (void *)set_fixmap_offset(fixmap, page_to_phys(page) +
-					 offset_in_page(addr));
+	return (void *)set_fixmap_offset(fixmap, phys + offset_in_page(addr));
 }
 
 static void patch_unmap(int fixmap)

-- 
Sincerely yours,
Mike.