On Mon, Jan 30, 2023 at 8:19 AM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote: > > On Mon, Jan 30, 2023 at 07:48:04AM -0600, Rob Herring wrote: > > On Wed, Jan 25, 2023 at 6:13 AM Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> wrote: > > > > > > On Wed, Jan 25, 2023 at 11:41 AM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote: > > > > > > > > On Mon, Jan 23, 2023 at 03:25:54PM +0100, Andrew Jones wrote: > > > > > On Mon, Jan 23, 2023 at 12:28:02PM +0100, Alexandre Ghiti wrote: > > > > > > During the early page table creation, we used to set the mapping for > > > > > > PAGE_OFFSET to the kernel load address: but the kernel load address is > > > > > > always offseted by PMD_SIZE which makes it impossible to use PUD/P4D/PGD > > > > > > pages as this physical address is not aligned on PUD/P4D/PGD size (whereas > > > > > > PAGE_OFFSET is). > > > > [...] > > > > > > > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > > > > > > index f08b25195ae7..58107bd56f8f 100644 > > > > > > --- a/drivers/of/fdt.c > > > > > > +++ b/drivers/of/fdt.c > > > > > > @@ -891,12 +891,13 @@ const void * __init of_flat_dt_match_machine(const void *default_match, > > > > > > static void __early_init_dt_declare_initrd(unsigned long start, > > > > > > unsigned long end) > > > > > > { > > > > > > - /* ARM64 would cause a BUG to occur here when CONFIG_DEBUG_VM is > > > > > > - * enabled since __va() is called too early. ARM64 does make use > > > > > > - * of phys_initrd_start/phys_initrd_size so we can skip this > > > > > > - * conversion. > > > > > > + /* > > > > > > + * __va() is not yet available this early on some platforms. In that > > > > > > + * case, the platform uses phys_initrd_start/phys_initrd_size instead > > > > > > + * and does the VA conversion itself. > > > > > > */ > > > > > > - if (!IS_ENABLED(CONFIG_ARM64)) { > > > > > > + if (!IS_ENABLED(CONFIG_ARM64) && > > > > > > + !(IS_ENABLED(CONFIG_RISCV) && IS_ENABLED(CONFIG_64BIT))) { > > > > > > > > > > There are now two architectures, so maybe it's time for a new config > > > > > symbol which would be selected by arm64 and riscv64 and then used here, > > > > > e.g. > > > > > > > > > > if (!IS_ENABLED(CONFIG_NO_EARLY_LINEAR_MAP)) { > > > > > > > > I see v5 left this as it was. Any comment on this suggestion? > > > > > > Introducing a config for this only use case sounds excessive to me, > > > but I'll let Rob decide what he wants to see here. > > > > Agreed. Can we just keep it as is here. > > > > > > > > initrd_start = (unsigned long)__va(start); > > > > > > initrd_end = (unsigned long)__va(end); > > > > I think long term, we should just get rid of needing to do this part > > in the DT code and let the initrd code do this. > > initrd code provides reserve_initrd_mem() for this and riscv calls > it later on. afaict, this early setting in OF code is a convenience > which architectures could be taught not to depend on, and then it > could be removed. But, until then, some architectures will need to > avoid it. As I commented downthread, I also don't want to go with > a config anymore, but it'd be nice to keep arch-specifics out of > here, so I've posted a patch changing __early_init_dt_declare_initrd > to be a weak function. If this was *always* going to be architecture specific, then I'd agree. But I think there are better paths to refactor this. I don't want to go back to a weak function and encourage more implementations of __early_init_dt_declare_initrd(). Rob