[PATCH v5 1/6] riscv: Add boot hartid to Device tree

[Auer, Lukas]

Hi Heinrich, hi Bin,

On Tue, 2020-04-14 at 07:08 +0800, Bin Meng wrote:
> Hi Heinrich,
> 
> On Tue, Apr 14, 2020 at 7:05 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > On 4/6/20 10:44 PM, Atish Patra wrote:
> > > Linux booting protocol mandates that register "a0" contains the hartid.
> > > However, U-boot can not pass the hartid via a0 during via standard UEFI
> > > protocol. DT nodes are commonly used to pass such information to the OS.
> > > 
> > > Add a DT node under chosen node to indicate the boot hartid. EFI stub
> > > in Linux kernel will parse this node and pass it to the real kernel
> > > in "a0" before jumping to it.
> > > 
> > > Signed-off-by: Atish Patra <atish.patra at wdc.com>
> > > Reviewed-by: Rick Chen <rick at andestech.com>
> > > ---
> > >  arch/riscv/lib/bootm.c | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > > index fad16901c5f2..87cadad5016d 100644
> > > --- a/arch/riscv/lib/bootm.c
> > > +++ b/arch/riscv/lib/bootm.c
> > > @@ -28,6 +28,28 @@ __weak void board_quiesce_devices(void)
> > > 
> > >  int arch_fixup_fdt(void *blob)
> > >  {
> > > +#ifdef CONFIG_EFI_LOADER
> > > +     int err;
> > > +     u32 size;
> > > +     int chosen_offset;
> > > +
> > > +     size = fdt_totalsize(blob);
> > > +     err  = fdt_open_into(blob, blob, size + 32);
> > > +     if (err < 0) {
> > > +             printf("Device Tree can't be expanded to accommodate new node");
> > > +             return err;
> > > +     }
> > > +     chosen_offset = fdt_path_offset(blob, "/chosen");
> > > +     if (chosen_offset < 0) {
> > > +             err = fdt_add_subnode(blob, 0, "chosen");
> > > +             if (err < 0) {
> > > +                     printf("chosen node can not be added\n");
> > > +                     return err;
> > > +             }
> > > +     }
> > > +     /* Overwrite the boot-hartid as U-Boot is the last stage BL */
> > > +     fdt_setprop_u32(blob, chosen_offset, "boot-hartid", gd->arch.boot_hart);
> > > +#endif
> > >       return 0;
> > >  }
> > > 
> > > 
> > 
> > I have tested this on qemu-riscv64_defconfig by comparing the device
> > tree before and after running helloworld.efi:
> > 
> > => fdt addr $fdtcontroladdr
> > => fdt print /chosen
> > chosen {
> >         bootargs = "";
> >         stdout-path = "/uart at 10000000";
> > };
> > ...
> > => dhcp $kernel_addr_r helloworld.efi
> > => bootefi $kernel_addr_r
> > ...
> > => fdt addr 0x87F00000
> > => fdt print /chosen
> > chosen {
> >         riscv,kernel-end = <0x00000000 0x00000000>;
> >         riscv,kernel-start = <0x00000000 0x00000000>;
> >         boot-hartid = <0x00000000>;
> >         bootargs = "";
> >         stdout-path = "/uart at 10000000";
> > };
> > 
> > The entry for boot-hardid seems to be ok.
> > 
> > But the riscv,kernel-end and riscv,kernel-start values are just some
> > dummy values introduced in:
> > 
> > board/emulation/qemu-riscv/qemu-riscv.c:84
> > commit 897206c5cc5c ("riscv: qemu: clear kernel-start/-end in device
> > tree as workaround for BBL)"
> > 
> > @Lukas:
> > Why are these values set to zero and not deleted (using fdt_delprop())
> > from the device tree? I cannot see that we need them when booting via UEFI.
> > 
> 

That was a workaround for BBL, which required them to be set if I
remember correctly.

> This should be removed as BBL is legacy and we only support working
> with OpenSBI.
> 

Absolutely correct, this is not required anymore since we are using
OpenSBI now. I will send a patch later today reverting the workaround.

Regards,
Lukas

> > Tested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> 
> Regards,
> Bin