Re: [PATCH v4 2/2] RISC-V: Setup initial page tables in two stages

From: Christoph Hellwig
Date: Mon May 20 2019 - 07:46:19 EST


> void __init parse_dtb(unsigned int hartid, void *dtb)
> {
> - if (early_init_dt_scan(__va(dtb)))
> + dtb = (void *)fix_to_virt(FIX_FDT) + ((uintptr_t)dtb & ~PAGE_MASK);
> + if (early_init_dt_scan(dtb))

FYI, parse_dtb in mainline now lost the hartid argument and takes a
phys_addr_t for the dtb address.

That being said I find the above way to magic. So we take the fixmap
address and then only the offset from something passed as a pointer?
This just looks very weird. The way FIX_FDT is defined to add to my
confusion, which might partially be due to not understanding fixmaps
very well. But it seems like at very least we should set up an
actual kernel pointer for the dtb in setup_vm based on what that
gets passed and stop passing any arguments to parse_dtb to keep
that magic in one place. And possibly add some comment.

> +#if MAX_EARLY_MAPPING_SIZE < PGDIR_SIZE

It seems MAX_EARLY_MAPPING_SIZE is defined to a fix constant,
why do we need these conditionals?