Re: [PATCH] riscv/mm: use physical alignment for vmemmap_start_pfn

[David Hildenbrand (Arm)]

On 7/16/26 12:53, Jiakai Xu wrote:
> RISC-V computes vmemmap_start_pfn by rounding phys_ram_base down to
> VMEMMAP_ADDR_ALIGN. That alignment must therefore be expressed in the
> physical-address domain.
> 
> Commit 476849b0fba4 ("riscv/mm: align vmemmap to maximal folio size")
> attempted to account for the maximal folio alignment by feeding
> MAX_FOLIO_VMEMMAP_ALIGN directly into VMEMMAP_ADDR_ALIGN. However,
> MAX_FOLIO_VMEMMAP_ALIGN is measured in bytes of struct page storage,
> whereas VMEMMAP_ADDR_ALIGN is used to align a physical address.

So in sparse_init(), we check

VM_WARN_ON_ONCE(!IS_ALIGNED((unsigned long) pfn_to_page(0),
		MAX_FOLIO_VMEMMAP_ALIGN));

> 
> The mask-based compound_info encoding requires pfn_to_page(0) to be
> naturally aligned to MAX_FOLIO_VMEMMAP_ALIGN. Commit 9f94db4c7eaa
> ("mm/sparse: check memmap alignment for compound_info_has_mask()")
> added a check for that requirement and exposed the unit mismatch on
> systems such as QEMU virt, where the DRAM base is not aligned to
> MAX_FOLIO_NR_PAGES * PAGE_SIZE.
> 
> Convert MAX_FOLIO_VMEMMAP_ALIGN to the equivalent physical alignment
> before using it in VMEMMAP_ADDR_ALIGN. This keeps the existing
> round_down() logic while making the resulting vmemmap base satisfy the
> mask-alignment requirement.
> 
> Fixes: 476849b0fba4 ("riscv/mm: align vmemmap to maximal folio size")
> Signed-off-by: Jiakai Xu <xujiakai2025@xxxxxxxxxxx>
> Assisted-by: YuanSheng:DeepSeek-V4-Flash
> ---
>  arch/riscv/mm/init.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 3e450890be07..422efa11824b 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -63,7 +63,8 @@ EXPORT_SYMBOL(phys_ram_base);
>  
>  #ifdef CONFIG_SPARSEMEM_VMEMMAP
>  #define VMEMMAP_ADDR_ALIGN	max(1ULL << SECTION_SIZE_BITS, \
> -				    MAX_FOLIO_VMEMMAP_ALIGN)
> +				    PFN_PHYS(MAX_FOLIO_VMEMMAP_ALIGN / \
> +					     sizeof(struct page)))

I'm still confused, but I think it might be more elegant to make this

#define VMEMMAP_PFN_ALIGN	max(PAGES_PER_SECTION, \
	    			    MAX_FOLIO_VMEMMAP_ALIGN / sizeof(struct page))

And then do

	round_down(PHYS_PFN(phys_ram_base), VMEMMAP_PFN_ALIGN);

-- 
Cheers,

David