Re: [PATCH -fixes v2 2/3] riscv: Do not set initial_boot_params to the linear address of the dtb

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]

On Wed, Mar 29, 2023 at 04:52:45PM +0200, Alexandre Ghiti wrote:
> On Wed, Mar 29, 2023 at 4:37 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Wed, Mar 29, 2023 at 10:19:31AM +0200, Alexandre Ghiti wrote:
> > > early_init_dt_verify() is already called in parse_dtb() and since the dtb
> > > address does not change anymore (it is now in the fixmap region), no need
> > > to reset initial_boot_params by calling early_init_dt_verify() again.
> > >
> > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > ---
> > >  arch/riscv/kernel/setup.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > > index 542eed85ad2c..a059b73f4ddb 100644
> > > --- a/arch/riscv/kernel/setup.c
> > > +++ b/arch/riscv/kernel/setup.c
> > > @@ -278,10 +278,7 @@ void __init setup_arch(char **cmdline_p)
> > >  #if IS_ENABLED(CONFIG_BUILTIN_DTB)
> > >       unflatten_and_copy_device_tree();
> > >  #else
> > > -     if (early_init_dt_verify(__va(XIP_FIXUP(dtb_early_pa))))
> > > -             unflatten_device_tree();
> >
> > Silly question maybe, but since it isn't explicitly mentioned, the
> > XIP_FIXUP bits no longer matter?
> 
> The XIP_FIXUP is only needed when translating virtual to physical
> addresses, but that does not mean I did not break it, I haven't
> considered XIP at all...

So, what currently happens is that, during early boot, we call
parse_dtb() right at the beginning of setup_arch().
That calls early_init_dt_scan(dtb_early_pa), which in turn calls
early_init_dt_verify(dtb_early_pa).

Here, relatively late during boot, we are coming along and calling the
function again. This existed prior to the XIP stuff landing, but the
specific XIP_FIXUP handling looks to be the fallout from:
https://lore.kernel.org/linux-riscv/82a05081-5662-c787-44e4-d480774ce31c@ghiti.fr/

The check in the first place was added by Anup's move away from fixmap
for dtb stuff, which makes me wonder - should this actually be part of
1/3? Something, something we no longer need to do this because these
addresses no longer change as per 1/3?

> > Also, in related news, I assume you don't have a QEMU setup that can do
> > boot an XIP kernel?
> 
> I haven't booted a XIP kernel for a long time now, here are my notes
> from that time:
> https://github.com/AlexGhiti/alexghiti.github.io/blob/main/xip/XIP.md

Right, I'll have to get around to trying that. We never put any
investment into QEMU internally, nor run any CI against it, so the HSS
doesn't actually work in QEMU anymore.
Assertion failures due to missing peripheral emulation :(
Probably don't need the HSS though and can do a direct kernel boot, I'll
have to see if that works for this XIP stuff, I know it does for regular
kernels.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]