Re: [Qemu-devel] [RFC v1 0/5] RISC-V: Add firmware loading support and default

[Alistair Francis]

On Wed, Jun 19, 2019 at 7:42 AM Bin Meng <address@hidden> wrote:
>
> On Wed, Jun 19, 2019 at 10:30 PM Alistair Francis <address@hidden> wrote:
> >
> > On Wed, Jun 19, 2019 at 7:26 AM Bin Meng <address@hidden> wrote:
> > >
> > > On Wed, Jun 19, 2019 at 8:56 AM Alistair Francis
> > > <address@hidden> wrote:
> > > >
> > > > This is an RFC as it will break ALL current users! See below for 
> > > > details.
> > > >
> > > > This series consolidates the current RISC-V kernel loading
> > > > impelementation while also adding support for the -bios option and more
> > > > advanced kernel image types.
> > > >
> > > > After consolidating the kernel loading we can extend the boot loader to
> > > > support a -bios option. We can also extend the kernel loading options to
> > > > support not just ELF files but other standard formats.
> > > >
> > > > Finally we can include the OpenSBI firmware by default for QEMU users.
> > > >
> > > > At the end of this series we are in the good place of no longer
> > > > requiring users to build firmware to boot a kernel. Instead users can
> > > > just run QEMu with the -kernel option and everything will work. They can
> > >
> > > This is great. I like booting kernel directly for testing without
> > > bothering the firmware stuff.
> >
> > That's the goal! Now we just need to not break everything.
> >
> > >
> > > > also override the firmware with their own using the -bios option. Using
> > > > "-bios none" will result in no firmware being loaded (as it is today).
> > > >
> > > > !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
> > > >
> > > > Unfortunately this series (patch 5 specifically) results in all current
> > > > Linux boots being broken as users are already loading in their own
> > > > firmware, which overlaps with the now included default. They will
> > > > see this error:
> > > >
> > > >   rom: requested regions overlap (rom phdr #0: 
> > > > ./images/qemuriscv64/fw_jump.elf. free=0x0000000080008090, 
> > > > addr=0x0000000080000000)
> > > >   qemu-system-riscv64: rom check and register reset failed
> > > >
> > > > If a current user specieifies "-bios none" their flow will continue
> > > > working. A user can instead no longer load their firmware and
> > > > use the default or they can load their firmware with the -bios option.
> > > >
> > > > At the moment the best idea I have to not break all users is to only
> > > > include the default firmware if the user specifies "-bios opensbi".
> > > > That is change the default to not loading the firmware. Then we can work
> > > > on updating documentation and maybe in future change the default to
> > > > include a firmware and anyone who doesn't want a default firmware can
> > > > specify "-bios none".
> > > >
> > > > Any other ideas on how to not break everything?
> > > >
> > > >
> > > >
> > > > Alistair Francis (5):
> > > >   hw/riscv: Split out the boot functions
> > > >   hw/riscv: Add support for loading a firmware
> > > >   hw/riscv: Extend the kernel loading support
> > > >   roms: Add OpenSBI version 0.3
> > > >   hw/riscv: Load OpenSBI as the default firmware
> > > >
> > > >  .gitmodules                         |   3 +
> > > >  Makefile                            |   3 +-
> > > >  configure                           |   1 +
> > > >  hw/riscv/Makefile.objs              |   1 +
> > > >  hw/riscv/boot.c                     | 143 ++++++++++++++++++++++++++++
> > > >  hw/riscv/sifive_e.c                 |  17 +---
> > > >  hw/riscv/sifive_u.c                 |  19 +---
> > > >  hw/riscv/spike.c                    |  21 +---
> > > >  hw/riscv/virt.c                     |  54 ++---------
> > > >  include/hw/riscv/boot.h             |  30 ++++++
> > > >  pc-bios/opensbi-riscv32-fw_jump.elf | Bin 0 -> 197988 bytes
> > > >  pc-bios/opensbi-riscv64-fw_jump.elf | Bin 0 -> 200192 bytes
> > >
> > > Since we are considering adding "bios" images, I prefer to add the
> > > pure binary images instead of ELF images here.
> >
> > I didn't think about that. Can we just boot them in QEMU like we do
> > with the ELFs?
>
> Yes, use load_image_targphys() instead of load_elf().

Ah, that is obvious. I'll update it to use the bin files then.

Alistair

>
> Regards,
> Bin