[U-Boot] [PATCH 03/12] riscv: bootm: Correct the 1st kernel argument to hart id

[Bin Meng]

Hi Lukas,

On Tue, Sep 4, 2018 at 5:41 AM Auer, Lukas
<lukas.auer at aisec.fraunhofer.de> wrote:
>
> On Thu, 2018-08-30 at 00:54 -0700, Bin Meng wrote:
> > The first argument of Linux kernel is the risc-v core hart id,
> > from which the kernel is booted from. It is not the mach_id,
> > which seems to be copied from arm.
> >
> > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> > ---
> >
> >  arch/riscv/lib/bootm.c | 18 +++++-------------
> >  1 file changed, 5 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/riscv/lib/bootm.c b/arch/riscv/lib/bootm.c
> > index 6662aff..754bbff 100644
> > --- a/arch/riscv/lib/bootm.c
> > +++ b/arch/riscv/lib/bootm.c
> > @@ -25,10 +25,7 @@ int arch_fixup_fdt(void *blob)
> >
> >  int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t
> > *images)
> >  {
> > -     bd_t    *bd = gd->bd;
> > -     char    *s;
> > -     int     machid = bd->bi_arch_number;
> > -     void    (*theKernel)(int arch, uint params);
> > +     void    (*kernel)(int hart, uint dtb);
>
> This probably does not cause any issues in u-boot, but the second
> parameter should be 32 bit or 64 bit depending on the architecture.
> Since you are already changing the kernel arguments it would make sense
> to also change dtb from uint to ulong or void *.
>

Yes. Will address this in v2.

> >
> >       /*
> >        * allow the PREP bootm subcommand, it is required for bootm to
> > work
> > @@ -39,18 +36,12 @@ int do_bootm_linux(int flag, int argc, char
> > *argv[], bootm_headers_t *images)
> >       if ((flag != 0) && (flag != BOOTM_STATE_OS_GO))
> >               return 1;
> >
> > -     theKernel = (void (*)(int, uint))images->ep;
> > -
> > -     s = env_get("machid");
> > -     if (s) {
> > -             machid = simple_strtoul(s, NULL, 16);
> > -             printf("Using machid 0x%x from environment\n", machid);
> > -     }
> > +     kernel = (void (*)(int, uint))images->ep;
> >
> >       bootstage_mark(BOOTSTAGE_ID_RUN_OS);
> >
> >       debug("## Transferring control to Linux (at address %08lx)
> > ...\n",
> > -            (ulong)theKernel);
> > +            (ulong)kernel);
> >
> >       if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) {
> >  #ifdef CONFIG_OF_LIBFDT
> > @@ -66,8 +57,9 @@ int do_bootm_linux(int flag, int argc, char
> > *argv[], bootm_headers_t *images)
> >       printf("\nStarting kernel ...\n\n");
> >
> >       cleanup_before_linux();
> > +     /* TODO: hardcode the hart id to zero for now */
> >       if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len)
> > -             theKernel(machid, (unsigned long)images->ft_addr);
> > +             kernel(0, (unsigned long)images->ft_addr);
> >
>
> You can use the mhartid CSR to get the hart id. This will limit u-boot
> to running in machine mode however. Alternatively you can also use the
> hart id, which is passed in a0 by the bootloader.
>

If the goal is to use U-Boot to directly boot Linux, I think we need
more changes. I believe we should allow only one hart to execute this
function. For other harts, they should be waked up and jump to kernel
directly.

Regards,
Bin