Re: [Qemu-devel] [RFC v1 3/5] hw/riscv: Extend the kernel loading support

[Alistair Francis <alistair23@gmail.com>]

On Wed, Jun 19, 2019 at 8:16 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jun 19, 2019 at 8:56 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Extend the RISC-V kernel loader to support uImage and Image files.
> > A Linux kernel can now be booted with:
> >
> >     qemu-system-riscv64 -machine virt -bios fw_jump.elf -kernel Image
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/riscv/boot.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 392ca0cb2e..7f68035a3f 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -67,13 +67,22 @@ target_ulong riscv_load_kernel(MachineState *machine,
> >      uint64_t kernel_entry, kernel_high;
> >
> >      if (load_elf(kernel_filename, NULL, kernel_translate, machine,
> > -                 &kernel_entry, NULL, &kernel_high,
> > -                 0, EM_RISCV, 1, 0) < 0) {
> > -        error_report("could not load kernel '%s'", kernel_filename);
> > -        exit(1);
> > +                 &kernel_entry, NULL, &kernel_high, 0, EM_RISCV, 1, 0) > 0) {
> > +        return kernel_entry;
> > +    }
> > +
> > +    if (load_uimage_as(kernel_filename, &kernel_entry, NULL, NULL,
> > +                       kernel_translate, machine, NULL) > 0) {
>
> We should not set the 'kernel_translate' here for uImage.
>
> In fact, the whole kernel_translate() is not necessary.

I have removed the kernel_translate() function. I tested loading
uImage files though and they don't seem to work as the image is loaded
at the wrong address.

I have removed uImage loading support from this series. We can look at
it in the future if we decide we want it.

Alistair

>
> > +        return kernel_entry;
> > +    }
> > +
>
> Regards,
> Bin