| Message ID | 20230102115241.25733-11-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: OpenSBI boot test and cleanups | expand |
On Mon, Jan 2, 2023 at 7:55 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > the same steps when '-kernel' is used: > > - execute load_kernel() > - load init_rd() > - write kernel_cmdline > > Let's fold everything inside riscv_load_kernel() to avoid code > repetition. To not change the behavior of boards that aren't calling > riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and typo: should be riscv_load_initrd() > allow these boards to opt out from initrd loading. > > Cc: Palmer Dabbelt <palmer@dabbelt.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/boot.c | 22 +++++++++++++++++++--- > hw/riscv/microchip_pfsoc.c | 12 ++---------- > hw/riscv/opentitan.c | 2 +- > hw/riscv/sifive_e.c | 3 ++- > hw/riscv/sifive_u.c | 12 ++---------- > hw/riscv/spike.c | 11 +---------- > hw/riscv/virt.c | 12 ++---------- > include/hw/riscv/boot.h | 1 + > 8 files changed, 30 insertions(+), 45 deletions(-) > Otherwise, Reviewed-by: Bin Meng <bmeng@tinylab.org>
On 1/8/23 00:33, Bin Meng wrote: > On Mon, Jan 2, 2023 at 7:55 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing >> the same steps when '-kernel' is used: >> >> - execute load_kernel() >> - load init_rd() >> - write kernel_cmdline >> >> Let's fold everything inside riscv_load_kernel() to avoid code >> repetition. To not change the behavior of boards that aren't calling >> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and > typo: should be riscv_load_initrd() > >> allow these boards to opt out from initrd loading. >> >> Cc: Palmer Dabbelt <palmer@dabbelt.com> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> hw/riscv/boot.c | 22 +++++++++++++++++++--- >> hw/riscv/microchip_pfsoc.c | 12 ++---------- >> hw/riscv/opentitan.c | 2 +- >> hw/riscv/sifive_e.c | 3 ++- >> hw/riscv/sifive_u.c | 12 ++---------- >> hw/riscv/spike.c | 11 +---------- >> hw/riscv/virt.c | 12 ++---------- >> include/hw/riscv/boot.h | 1 + >> 8 files changed, 30 insertions(+), 45 deletions(-) >> > Otherwise, > Reviewed-by: Bin Meng <bmeng@tinylab.org> Thanks! Alistair, let me know if you want me to send another version with the commit message typo fixed. I might as well take the change to rebase it with riscv-to-apply.next. Daniel
On 1/10/23 08:43, Daniel Henrique Barboza wrote: > > > On 1/8/23 00:33, Bin Meng wrote: >> On Mon, Jan 2, 2023 at 7:55 PM Daniel Henrique Barboza >> <dbarboza@ventanamicro.com> wrote: >>> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing >>> the same steps when '-kernel' is used: >>> >>> - execute load_kernel() >>> - load init_rd() >>> - write kernel_cmdline >>> >>> Let's fold everything inside riscv_load_kernel() to avoid code >>> repetition. To not change the behavior of boards that aren't calling >>> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and >> typo: should be riscv_load_initrd() >> >>> allow these boards to opt out from initrd loading. >>> >>> Cc: Palmer Dabbelt <palmer@dabbelt.com> >>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>> --- >>> hw/riscv/boot.c | 22 +++++++++++++++++++--- >>> hw/riscv/microchip_pfsoc.c | 12 ++---------- >>> hw/riscv/opentitan.c | 2 +- >>> hw/riscv/sifive_e.c | 3 ++- >>> hw/riscv/sifive_u.c | 12 ++---------- >>> hw/riscv/spike.c | 11 +---------- >>> hw/riscv/virt.c | 12 ++---------- >>> include/hw/riscv/boot.h | 1 + >>> 8 files changed, 30 insertions(+), 45 deletions(-) >>> >> Otherwise, >> Reviewed-by: Bin Meng <bmeng@tinylab.org> > > Thanks! > > Alistair, let me know if you want me to send another version with the commit > message typo fixed. I might as well take the change to rebase it with > riscv-to-apply.next. While rebasing these patches on top of riscv-to-apply.next, the avocado tests I've introduced here started to fail both sifive_u tests: tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv32_sifive_u: INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: ... (5.07 s) (09/18) tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv64_sifive_u: INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: ... (5.05 s) I proposed a fix here: https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg02035.html I can re-send this series after we get that problem figure out. Otherwise we're going to add 2 avocado tests that are failing right from the start hehe. Thanks, Daniel > > > Daniel >
On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > the same steps when '-kernel' is used: > > - execute load_kernel() > - load init_rd() > - write kernel_cmdline > > Let's fold everything inside riscv_load_kernel() to avoid code > repetition. To not change the behavior of boards that aren't calling > riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and > allow these boards to opt out from initrd loading. > > Cc: Palmer Dabbelt <palmer@dabbelt.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/riscv/boot.c | 22 +++++++++++++++++++--- > hw/riscv/microchip_pfsoc.c | 12 ++---------- > hw/riscv/opentitan.c | 2 +- > hw/riscv/sifive_e.c | 3 ++- > hw/riscv/sifive_u.c | 12 ++---------- > hw/riscv/spike.c | 11 +---------- > hw/riscv/virt.c | 12 ++---------- > include/hw/riscv/boot.h | 1 + > 8 files changed, 30 insertions(+), 45 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 2594276223..4888d5c1e0 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -175,10 +175,12 @@ target_ulong riscv_load_firmware(const char *firmware_filename, > > target_ulong riscv_load_kernel(MachineState *machine, > target_ulong kernel_start_addr, > + bool load_initrd, > symbol_fn_t sym_cb) > { > const char *kernel_filename = machine->kernel_filename; > uint64_t kernel_load_base, kernel_entry; > + void *fdt = machine->fdt; > > g_assert(kernel_filename != NULL); > > @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > NULL, &kernel_load_base, NULL, NULL, 0, > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > - return kernel_load_base; > + kernel_entry = kernel_load_base; > + goto out; > } > > if (load_uimage_as(kernel_filename, &kernel_entry, NULL, NULL, > NULL, NULL, NULL) > 0) { > - return kernel_entry; > + goto out; > } > > if (load_image_targphys_as(kernel_filename, kernel_start_addr, > current_machine->ram_size, NULL) > 0) { > - return kernel_start_addr; > + kernel_entry = kernel_start_addr; > + goto out; > } > > error_report("could not load kernel '%s'", kernel_filename); > exit(1); > + > +out: > + if (load_initrd && machine->initrd_filename) { > + riscv_load_initrd(machine, kernel_entry); > + } > + > + if (fdt && machine->kernel_cmdline && *machine->kernel_cmdline) { > + qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", > + machine->kernel_cmdline); > + } > + > + return kernel_entry; > } > > void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry) > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > index 82ae5e7023..c45023a2b1 100644 > --- a/hw/riscv/microchip_pfsoc.c > +++ b/hw/riscv/microchip_pfsoc.c > @@ -629,16 +629,8 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) > kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus, > firmware_end_addr); > > - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); > - > - if (machine->initrd_filename) { > - riscv_load_initrd(machine, kernel_entry); > - } > - > - if (machine->kernel_cmdline && *machine->kernel_cmdline) { > - qemu_fdt_setprop_string(machine->fdt, "/chosen", > - "bootargs", machine->kernel_cmdline); > - } > + kernel_entry = riscv_load_kernel(machine, kernel_start_addr, > + true, NULL); > > /* Compute the fdt load address in dram */ > fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base, > diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c > index 64d5d435b9..f6fd9725a5 100644 > --- a/hw/riscv/opentitan.c > +++ b/hw/riscv/opentitan.c > @@ -101,7 +101,7 @@ static void opentitan_board_init(MachineState *machine) > } > > if (machine->kernel_filename) { > - riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, NULL); > + riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, false, NULL); > } > } > > diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c > index 3e3f4b0088..6835d1c807 100644 > --- a/hw/riscv/sifive_e.c > +++ b/hw/riscv/sifive_e.c > @@ -114,7 +114,8 @@ static void sifive_e_machine_init(MachineState *machine) > memmap[SIFIVE_E_DEV_MROM].base, &address_space_memory); > > if (machine->kernel_filename) { > - riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, NULL); > + riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, > + false, NULL); > } > } > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > index bac394c959..9a75d4aa62 100644 > --- a/hw/riscv/sifive_u.c > +++ b/hw/riscv/sifive_u.c > @@ -598,16 +598,8 @@ static void sifive_u_machine_init(MachineState *machine) > kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus, > firmware_end_addr); > > - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); > - > - if (machine->initrd_filename) { > - riscv_load_initrd(machine, kernel_entry); > - } > - > - if (machine->kernel_cmdline && *machine->kernel_cmdline) { > - qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs", > - machine->kernel_cmdline); > - } > + kernel_entry = riscv_load_kernel(machine, kernel_start_addr, > + true, NULL); > } else { > /* > * If dynamic firmware is used, it doesn't know where is the next mode > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > index bff9475686..c517885e6e 100644 > --- a/hw/riscv/spike.c > +++ b/hw/riscv/spike.c > @@ -308,16 +308,7 @@ static void spike_board_init(MachineState *machine) > firmware_end_addr); > > kernel_entry = riscv_load_kernel(machine, kernel_start_addr, > - htif_symbol_callback); > - > - if (machine->initrd_filename) { > - riscv_load_initrd(machine, kernel_entry); > - } > - > - if (machine->kernel_cmdline && *machine->kernel_cmdline) { > - qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs", > - machine->kernel_cmdline); > - } > + true, htif_symbol_callback); > } else { > /* > * If dynamic firmware is used, it doesn't know where is the next mode > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index c8e35f861e..a931ed05ab 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -1281,16 +1281,8 @@ static void virt_machine_done(Notifier *notifier, void *data) > kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0], > firmware_end_addr); > > - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); > - > - if (machine->initrd_filename) { > - riscv_load_initrd(machine, kernel_entry); > - } > - > - if (machine->kernel_cmdline && *machine->kernel_cmdline) { > - qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs", > - machine->kernel_cmdline); > - } > + kernel_entry = riscv_load_kernel(machine, kernel_start_addr, > + true, NULL); > } else { > /* > * If dynamic firmware is used, it doesn't know where is the next mode > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h > index f94653a09b..c3de897371 100644 > --- a/include/hw/riscv/boot.h > +++ b/include/hw/riscv/boot.h > @@ -45,6 +45,7 @@ target_ulong riscv_load_firmware(const char *firmware_filename, > symbol_fn_t sym_cb); > target_ulong riscv_load_kernel(MachineState *machine, > target_ulong firmware_end_addr, > + bool load_initrd, > symbol_fn_t sym_cb); > void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry); > uint64_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt); > -- > 2.39.0 > >
On Wed, Jan 11, 2023 at 6:21 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 1/10/23 08:43, Daniel Henrique Barboza wrote: > > > > > > On 1/8/23 00:33, Bin Meng wrote: > >> On Mon, Jan 2, 2023 at 7:55 PM Daniel Henrique Barboza > >> <dbarboza@ventanamicro.com> wrote: > >>> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > >>> the same steps when '-kernel' is used: > >>> > >>> - execute load_kernel() > >>> - load init_rd() > >>> - write kernel_cmdline > >>> > >>> Let's fold everything inside riscv_load_kernel() to avoid code > >>> repetition. To not change the behavior of boards that aren't calling > >>> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and > >> typo: should be riscv_load_initrd() > >> > >>> allow these boards to opt out from initrd loading. > >>> > >>> Cc: Palmer Dabbelt <palmer@dabbelt.com> > >>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > >>> --- > >>> hw/riscv/boot.c | 22 +++++++++++++++++++--- > >>> hw/riscv/microchip_pfsoc.c | 12 ++---------- > >>> hw/riscv/opentitan.c | 2 +- > >>> hw/riscv/sifive_e.c | 3 ++- > >>> hw/riscv/sifive_u.c | 12 ++---------- > >>> hw/riscv/spike.c | 11 +---------- > >>> hw/riscv/virt.c | 12 ++---------- > >>> include/hw/riscv/boot.h | 1 + > >>> 8 files changed, 30 insertions(+), 45 deletions(-) > >>> > >> Otherwise, > >> Reviewed-by: Bin Meng <bmeng@tinylab.org> > > > > Thanks! > > > > Alistair, let me know if you want me to send another version with the commit > > message typo fixed. I might as well take the change to rebase it with > > riscv-to-apply.next. > > While rebasing these patches on top of riscv-to-apply.next, the avocado tests > I've introduced here started to fail both sifive_u tests: > > tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv32_sifive_u: INTERRUPTED: > Test interrupted by SIGTERM\nRunner error occurred: ... (5.07 s) > (09/18) tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv64_sifive_u: INTERRUPTED: > Test interrupted by SIGTERM\nRunner error occurred: ... (5.05 s) > > > I proposed a fix here: > > https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg02035.html Thanks! I generally push riscv-to-apply.next before running tests, so it's possible to break. I'm seeing similar failures. Generally when I see failures from a series I just drop the series, but if you have a fix that's even better :) Alistair > > I can re-send this series after we get that problem figure out. Otherwise we're > going to add 2 avocado tests that are failing right from the start hehe. > > Thanks, > > Daniel > > > > > > > > Daniel > > > >
On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > the same steps when '-kernel' is used: > > - execute load_kernel() > - load init_rd() > - write kernel_cmdline > > Let's fold everything inside riscv_load_kernel() to avoid code > repetition. To not change the behavior of boards that aren't calling > riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and > allow these boards to opt out from initrd loading. > > Cc: Palmer Dabbelt <palmer@dabbelt.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/boot.c | 22 +++++++++++++++++++--- > hw/riscv/microchip_pfsoc.c | 12 ++---------- > hw/riscv/opentitan.c | 2 +- > hw/riscv/sifive_e.c | 3 ++- > hw/riscv/sifive_u.c | 12 ++---------- > hw/riscv/spike.c | 11 +---------- > hw/riscv/virt.c | 12 ++---------- > include/hw/riscv/boot.h | 1 + > 8 files changed, 30 insertions(+), 45 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 2594276223..4888d5c1e0 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -175,10 +175,12 @@ target_ulong riscv_load_firmware(const char *firmware_filename, > > target_ulong riscv_load_kernel(MachineState *machine, > target_ulong kernel_start_addr, > + bool load_initrd, > symbol_fn_t sym_cb) > { > const char *kernel_filename = machine->kernel_filename; > uint64_t kernel_load_base, kernel_entry; > + void *fdt = machine->fdt; > > g_assert(kernel_filename != NULL); > > @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > NULL, &kernel_load_base, NULL, NULL, 0, > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > - return kernel_load_base; > + kernel_entry = kernel_load_base; This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest we get a value of 0xffffffff80000000. Previously the top bits would be lost as we return a target_ulong from this function, but with this change we pass the value 0xffffffff80000000 to riscv_load_initrd() which causes failures. This diff fixes the failure for me diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index 4888d5c1e0..f08ed44b97 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, NULL, &kernel_load_base, NULL, NULL, 0, EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { - kernel_entry = kernel_load_base; + kernel_entry = (target_ulong) kernel_load_base; goto out; } but I don't think that's the right fix. We should instead look at the CPU XLEN and drop the high bits if required. I'm going to drop this patch, do you mind looking into a proper fix? Alistair
On 1/11/23 21:34, Alistair Francis wrote: > On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing >> the same steps when '-kernel' is used: >> >> - execute load_kernel() >> - load init_rd() >> - write kernel_cmdline >> >> Let's fold everything inside riscv_load_kernel() to avoid code >> repetition. To not change the behavior of boards that aren't calling >> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and >> allow these boards to opt out from initrd loading. >> >> Cc: Palmer Dabbelt <palmer@dabbelt.com> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> hw/riscv/boot.c | 22 +++++++++++++++++++--- >> hw/riscv/microchip_pfsoc.c | 12 ++---------- >> hw/riscv/opentitan.c | 2 +- >> hw/riscv/sifive_e.c | 3 ++- >> hw/riscv/sifive_u.c | 12 ++---------- >> hw/riscv/spike.c | 11 +---------- >> hw/riscv/virt.c | 12 ++---------- >> include/hw/riscv/boot.h | 1 + >> 8 files changed, 30 insertions(+), 45 deletions(-) >> >> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c >> index 2594276223..4888d5c1e0 100644 >> --- a/hw/riscv/boot.c >> +++ b/hw/riscv/boot.c >> @@ -175,10 +175,12 @@ target_ulong riscv_load_firmware(const char *firmware_filename, >> >> target_ulong riscv_load_kernel(MachineState *machine, >> target_ulong kernel_start_addr, >> + bool load_initrd, >> symbol_fn_t sym_cb) >> { >> const char *kernel_filename = machine->kernel_filename; >> uint64_t kernel_load_base, kernel_entry; >> + void *fdt = machine->fdt; >> >> g_assert(kernel_filename != NULL); >> >> @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, >> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, >> NULL, &kernel_load_base, NULL, NULL, 0, >> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { >> - return kernel_load_base; >> + kernel_entry = kernel_load_base; > This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest > we get a value of 0xffffffff80000000. > > Previously the top bits would be lost as we return a target_ulong from > this function, but with this change we pass the value > 0xffffffff80000000 to riscv_load_initrd() which causes failures. > > This diff fixes the failure for me > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 4888d5c1e0..f08ed44b97 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > NULL, &kernel_load_base, NULL, NULL, 0, > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > - kernel_entry = kernel_load_base; > + kernel_entry = (target_ulong) kernel_load_base; > goto out; > } > > > but I don't think that's the right fix. We should instead look at the > CPU XLEN and drop the high bits if required. > > I'm going to drop this patch, do you mind looking into a proper fix? Thanks for looking this up! I 'll fix it and resend patches 10 and 11. Daniel > > Alistair
Hi Alistair, On Thu, Jan 12, 2023 at 8:36 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: > > > > The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > > the same steps when '-kernel' is used: > > > > - execute load_kernel() > > - load init_rd() > > - write kernel_cmdline > > > > Let's fold everything inside riscv_load_kernel() to avoid code > > repetition. To not change the behavior of boards that aren't calling > > riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and > > allow these boards to opt out from initrd loading. > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com> > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > --- > > hw/riscv/boot.c | 22 +++++++++++++++++++--- > > hw/riscv/microchip_pfsoc.c | 12 ++---------- > > hw/riscv/opentitan.c | 2 +- > > hw/riscv/sifive_e.c | 3 ++- > > hw/riscv/sifive_u.c | 12 ++---------- > > hw/riscv/spike.c | 11 +---------- > > hw/riscv/virt.c | 12 ++---------- > > include/hw/riscv/boot.h | 1 + > > 8 files changed, 30 insertions(+), 45 deletions(-) > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > index 2594276223..4888d5c1e0 100644 > > --- a/hw/riscv/boot.c > > +++ b/hw/riscv/boot.c > > @@ -175,10 +175,12 @@ target_ulong riscv_load_firmware(const char *firmware_filename, > > > > target_ulong riscv_load_kernel(MachineState *machine, > > target_ulong kernel_start_addr, > > + bool load_initrd, > > symbol_fn_t sym_cb) > > { > > const char *kernel_filename = machine->kernel_filename; > > uint64_t kernel_load_base, kernel_entry; > > + void *fdt = machine->fdt; > > > > g_assert(kernel_filename != NULL); > > > > @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, > > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > > NULL, &kernel_load_base, NULL, NULL, 0, > > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > > - return kernel_load_base; > > + kernel_entry = kernel_load_base; > > This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest > we get a value of 0xffffffff80000000. Shouldn't the bug be the 32-bit Xvisor image? How can a 32-bit image return an address of 0xffffffff80000000? > > Previously the top bits would be lost as we return a target_ulong from > this function, but with this change we pass the value > 0xffffffff80000000 to riscv_load_initrd() which causes failures. > > This diff fixes the failure for me > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 4888d5c1e0..f08ed44b97 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > NULL, &kernel_load_base, NULL, NULL, 0, > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > - kernel_entry = kernel_load_base; > + kernel_entry = (target_ulong) kernel_load_base; > goto out; > } > > > but I don't think that's the right fix. We should instead look at the > CPU XLEN and drop the high bits if required. > > I'm going to drop this patch, do you mind looking into a proper fix? > Regards, Bin
On 12/1/23 01:34, Alistair Francis wrote: > On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> >> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing >> the same steps when '-kernel' is used: >> >> - execute load_kernel() >> - load init_rd() >> - write kernel_cmdline >> >> Let's fold everything inside riscv_load_kernel() to avoid code >> repetition. To not change the behavior of boards that aren't calling >> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and >> allow these boards to opt out from initrd loading. >> >> Cc: Palmer Dabbelt <palmer@dabbelt.com> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> hw/riscv/boot.c | 22 +++++++++++++++++++--- >> hw/riscv/microchip_pfsoc.c | 12 ++---------- >> hw/riscv/opentitan.c | 2 +- >> hw/riscv/sifive_e.c | 3 ++- >> hw/riscv/sifive_u.c | 12 ++---------- >> hw/riscv/spike.c | 11 +---------- >> hw/riscv/virt.c | 12 ++---------- >> include/hw/riscv/boot.h | 1 + >> 8 files changed, 30 insertions(+), 45 deletions(-) >> @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, >> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, >> NULL, &kernel_load_base, NULL, NULL, 0, >> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { >> - return kernel_load_base; >> + kernel_entry = kernel_load_base; > > This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest > we get a value of 0xffffffff80000000. > > Previously the top bits would be lost as we return a target_ulong from > this function, but with this change we pass the value > 0xffffffff80000000 to riscv_load_initrd() which causes failures. > > This diff fixes the failure for me > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 4888d5c1e0..f08ed44b97 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, > if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > NULL, &kernel_load_base, NULL, NULL, 0, > EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > - kernel_entry = kernel_load_base; > + kernel_entry = (target_ulong) kernel_load_base; > goto out; > } > > > but I don't think that's the right fix. We should instead look at the > CPU XLEN and drop the high bits if required. Ah, that is what should be done in load_elf_ram_sym()'s missing translate_fn() handler.
On 1/13/23 04:16, Philippe Mathieu-Daudé wrote: > On 12/1/23 01:34, Alistair Francis wrote: >> On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza >> <dbarboza@ventanamicro.com> wrote: >>> >>> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing >>> the same steps when '-kernel' is used: >>> >>> - execute load_kernel() >>> - load init_rd() >>> - write kernel_cmdline >>> >>> Let's fold everything inside riscv_load_kernel() to avoid code >>> repetition. To not change the behavior of boards that aren't calling >>> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and >>> allow these boards to opt out from initrd loading. >>> >>> Cc: Palmer Dabbelt <palmer@dabbelt.com> >>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>> --- >>> hw/riscv/boot.c | 22 +++++++++++++++++++--- >>> hw/riscv/microchip_pfsoc.c | 12 ++---------- >>> hw/riscv/opentitan.c | 2 +- >>> hw/riscv/sifive_e.c | 3 ++- >>> hw/riscv/sifive_u.c | 12 ++---------- >>> hw/riscv/spike.c | 11 +---------- >>> hw/riscv/virt.c | 12 ++---------- >>> include/hw/riscv/boot.h | 1 + >>> 8 files changed, 30 insertions(+), 45 deletions(-) > >>> @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, >>> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, >>> NULL, &kernel_load_base, NULL, NULL, 0, >>> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { >>> - return kernel_load_base; >>> + kernel_entry = kernel_load_base; >> >> This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest >> we get a value of 0xffffffff80000000. >> >> Previously the top bits would be lost as we return a target_ulong from >> this function, but with this change we pass the value >> 0xffffffff80000000 to riscv_load_initrd() which causes failures. >> >> This diff fixes the failure for me >> >> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c >> index 4888d5c1e0..f08ed44b97 100644 >> --- a/hw/riscv/boot.c >> +++ b/hw/riscv/boot.c >> @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, >> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, >> NULL, &kernel_load_base, NULL, NULL, 0, >> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { >> - kernel_entry = kernel_load_base; >> + kernel_entry = (target_ulong) kernel_load_base; >> goto out; >> } >> >> >> but I don't think that's the right fix. We should instead look at the >> CPU XLEN and drop the high bits if required. > > Ah, that is what should be done in load_elf_ram_sym()'s missing > translate_fn() handler. Interesting. I'll try it again and re-send. Daniel
On Fri, Jan 13, 2023 at 6:23 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 1/13/23 04:16, Philippe Mathieu-Daudé wrote: > > On 12/1/23 01:34, Alistair Francis wrote: > >> On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza > >> <dbarboza@ventanamicro.com> wrote: > >>> > >>> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > >>> the same steps when '-kernel' is used: > >>> > >>> - execute load_kernel() > >>> - load init_rd() > >>> - write kernel_cmdline > >>> > >>> Let's fold everything inside riscv_load_kernel() to avoid code > >>> repetition. To not change the behavior of boards that aren't calling > >>> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and > >>> allow these boards to opt out from initrd loading. > >>> > >>> Cc: Palmer Dabbelt <palmer@dabbelt.com> > >>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > >>> --- > >>> hw/riscv/boot.c | 22 +++++++++++++++++++--- > >>> hw/riscv/microchip_pfsoc.c | 12 ++---------- > >>> hw/riscv/opentitan.c | 2 +- > >>> hw/riscv/sifive_e.c | 3 ++- > >>> hw/riscv/sifive_u.c | 12 ++---------- > >>> hw/riscv/spike.c | 11 +---------- > >>> hw/riscv/virt.c | 12 ++---------- > >>> include/hw/riscv/boot.h | 1 + > >>> 8 files changed, 30 insertions(+), 45 deletions(-) > > > >>> @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, > >>> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > >>> NULL, &kernel_load_base, NULL, NULL, 0, > >>> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > >>> - return kernel_load_base; > >>> + kernel_entry = kernel_load_base; > >> > >> This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest > >> we get a value of 0xffffffff80000000. > >> > >> Previously the top bits would be lost as we return a target_ulong from > >> this function, but with this change we pass the value > >> 0xffffffff80000000 to riscv_load_initrd() which causes failures. > >> > >> This diff fixes the failure for me > >> > >> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > >> index 4888d5c1e0..f08ed44b97 100644 > >> --- a/hw/riscv/boot.c > >> +++ b/hw/riscv/boot.c > >> @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, > >> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, > >> NULL, &kernel_load_base, NULL, NULL, 0, > >> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { > >> - kernel_entry = kernel_load_base; > >> + kernel_entry = (target_ulong) kernel_load_base; > >> goto out; > >> } > >> > >> > >> but I don't think that's the right fix. We should instead look at the > >> CPU XLEN and drop the high bits if required. > > > > Ah, that is what should be done in load_elf_ram_sym()'s missing > > translate_fn() handler. > > Interesting. I'll try it again and re-send. > If that fixes the problem, it should be a separate patch. I still don't understand why 32-bit xvisor image has a 64-bit address encoded? Regards, Bin
On 1/13/23 07:30, Bin Meng wrote: > On Fri, Jan 13, 2023 at 6:23 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> >> >> On 1/13/23 04:16, Philippe Mathieu-Daudé wrote: >>> On 12/1/23 01:34, Alistair Francis wrote: >>>> On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza >>>> <dbarboza@ventanamicro.com> wrote: >>>>> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing >>>>> the same steps when '-kernel' is used: >>>>> >>>>> - execute load_kernel() >>>>> - load init_rd() >>>>> - write kernel_cmdline >>>>> >>>>> Let's fold everything inside riscv_load_kernel() to avoid code >>>>> repetition. To not change the behavior of boards that aren't calling >>>>> riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and >>>>> allow these boards to opt out from initrd loading. >>>>> >>>>> Cc: Palmer Dabbelt <palmer@dabbelt.com> >>>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>>>> --- >>>>> hw/riscv/boot.c | 22 +++++++++++++++++++--- >>>>> hw/riscv/microchip_pfsoc.c | 12 ++---------- >>>>> hw/riscv/opentitan.c | 2 +- >>>>> hw/riscv/sifive_e.c | 3 ++- >>>>> hw/riscv/sifive_u.c | 12 ++---------- >>>>> hw/riscv/spike.c | 11 +---------- >>>>> hw/riscv/virt.c | 12 ++---------- >>>>> include/hw/riscv/boot.h | 1 + >>>>> 8 files changed, 30 insertions(+), 45 deletions(-) >>>>> @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, >>>>> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, >>>>> NULL, &kernel_load_base, NULL, NULL, 0, >>>>> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { >>>>> - return kernel_load_base; >>>>> + kernel_entry = kernel_load_base; >>>> This breaks 32-bit Xvisor loading. It seems that for the 32-bit guest >>>> we get a value of 0xffffffff80000000. >>>> >>>> Previously the top bits would be lost as we return a target_ulong from >>>> this function, but with this change we pass the value >>>> 0xffffffff80000000 to riscv_load_initrd() which causes failures. >>>> >>>> This diff fixes the failure for me >>>> >>>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c >>>> index 4888d5c1e0..f08ed44b97 100644 >>>> --- a/hw/riscv/boot.c >>>> +++ b/hw/riscv/boot.c >>>> @@ -194,7 +194,7 @@ target_ulong riscv_load_kernel(MachineState *machine, >>>> if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, >>>> NULL, &kernel_load_base, NULL, NULL, 0, >>>> EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { >>>> - kernel_entry = kernel_load_base; >>>> + kernel_entry = (target_ulong) kernel_load_base; >>>> goto out; >>>> } >>>> >>>> >>>> but I don't think that's the right fix. We should instead look at the >>>> CPU XLEN and drop the high bits if required. >>> Ah, that is what should be done in load_elf_ram_sym()'s missing >>> translate_fn() handler. >> Interesting. I'll try it again and re-send. >> > If that fixes the problem, it should be a separate patch. Fair enough. I'll keep this patch as is and fix it in a separated patch. Daniel > > I still don't understand why 32-bit xvisor image has a 64-bit address encoded? > > Regards, > Bin
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index 2594276223..4888d5c1e0 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -175,10 +175,12 @@ target_ulong riscv_load_firmware(const char *firmware_filename, target_ulong riscv_load_kernel(MachineState *machine, target_ulong kernel_start_addr, + bool load_initrd, symbol_fn_t sym_cb) { const char *kernel_filename = machine->kernel_filename; uint64_t kernel_load_base, kernel_entry; + void *fdt = machine->fdt; g_assert(kernel_filename != NULL); @@ -192,21 +194,35 @@ target_ulong riscv_load_kernel(MachineState *machine, if (load_elf_ram_sym(kernel_filename, NULL, NULL, NULL, NULL, &kernel_load_base, NULL, NULL, 0, EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) { - return kernel_load_base; + kernel_entry = kernel_load_base; + goto out; } if (load_uimage_as(kernel_filename, &kernel_entry, NULL, NULL, NULL, NULL, NULL) > 0) { - return kernel_entry; + goto out; } if (load_image_targphys_as(kernel_filename, kernel_start_addr, current_machine->ram_size, NULL) > 0) { - return kernel_start_addr; + kernel_entry = kernel_start_addr; + goto out; } error_report("could not load kernel '%s'", kernel_filename); exit(1); + +out: + if (load_initrd && machine->initrd_filename) { + riscv_load_initrd(machine, kernel_entry); + } + + if (fdt && machine->kernel_cmdline && *machine->kernel_cmdline) { + qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", + machine->kernel_cmdline); + } + + return kernel_entry; } void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry) diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c index 82ae5e7023..c45023a2b1 100644 --- a/hw/riscv/microchip_pfsoc.c +++ b/hw/riscv/microchip_pfsoc.c @@ -629,16 +629,8 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus, firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); - - if (machine->initrd_filename) { - riscv_load_initrd(machine, kernel_entry); - } - - if (machine->kernel_cmdline && *machine->kernel_cmdline) { - qemu_fdt_setprop_string(machine->fdt, "/chosen", - "bootargs", machine->kernel_cmdline); - } + kernel_entry = riscv_load_kernel(machine, kernel_start_addr, + true, NULL); /* Compute the fdt load address in dram */ fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base, diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c index 64d5d435b9..f6fd9725a5 100644 --- a/hw/riscv/opentitan.c +++ b/hw/riscv/opentitan.c @@ -101,7 +101,7 @@ static void opentitan_board_init(MachineState *machine) } if (machine->kernel_filename) { - riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, NULL); + riscv_load_kernel(machine, memmap[IBEX_DEV_RAM].base, false, NULL); } } diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c index 3e3f4b0088..6835d1c807 100644 --- a/hw/riscv/sifive_e.c +++ b/hw/riscv/sifive_e.c @@ -114,7 +114,8 @@ static void sifive_e_machine_init(MachineState *machine) memmap[SIFIVE_E_DEV_MROM].base, &address_space_memory); if (machine->kernel_filename) { - riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, NULL); + riscv_load_kernel(machine, memmap[SIFIVE_E_DEV_DTIM].base, + false, NULL); } } diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index bac394c959..9a75d4aa62 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -598,16 +598,8 @@ static void sifive_u_machine_init(MachineState *machine) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc.u_cpus, firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); - - if (machine->initrd_filename) { - riscv_load_initrd(machine, kernel_entry); - } - - if (machine->kernel_cmdline && *machine->kernel_cmdline) { - qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs", - machine->kernel_cmdline); - } + kernel_entry = riscv_load_kernel(machine, kernel_start_addr, + true, NULL); } else { /* * If dynamic firmware is used, it doesn't know where is the next mode diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index bff9475686..c517885e6e 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -308,16 +308,7 @@ static void spike_board_init(MachineState *machine) firmware_end_addr); kernel_entry = riscv_load_kernel(machine, kernel_start_addr, - htif_symbol_callback); - - if (machine->initrd_filename) { - riscv_load_initrd(machine, kernel_entry); - } - - if (machine->kernel_cmdline && *machine->kernel_cmdline) { - qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs", - machine->kernel_cmdline); - } + true, htif_symbol_callback); } else { /* * If dynamic firmware is used, it doesn't know where is the next mode diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index c8e35f861e..a931ed05ab 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1281,16 +1281,8 @@ static void virt_machine_done(Notifier *notifier, void *data) kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0], firmware_end_addr); - kernel_entry = riscv_load_kernel(machine, kernel_start_addr, NULL); - - if (machine->initrd_filename) { - riscv_load_initrd(machine, kernel_entry); - } - - if (machine->kernel_cmdline && *machine->kernel_cmdline) { - qemu_fdt_setprop_string(machine->fdt, "/chosen", "bootargs", - machine->kernel_cmdline); - } + kernel_entry = riscv_load_kernel(machine, kernel_start_addr, + true, NULL); } else { /* * If dynamic firmware is used, it doesn't know where is the next mode diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h index f94653a09b..c3de897371 100644 --- a/include/hw/riscv/boot.h +++ b/include/hw/riscv/boot.h @@ -45,6 +45,7 @@ target_ulong riscv_load_firmware(const char *firmware_filename, symbol_fn_t sym_cb); target_ulong riscv_load_kernel(MachineState *machine, target_ulong firmware_end_addr, + bool load_initrd, symbol_fn_t sym_cb); void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry); uint64_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
The microchip_icicle_kit, sifive_u, spike and virt boards are now doing the same steps when '-kernel' is used: - execute load_kernel() - load init_rd() - write kernel_cmdline Let's fold everything inside riscv_load_kernel() to avoid code repetition. To not change the behavior of boards that aren't calling riscv_load_init(), add an 'load_initrd' flag to riscv_load_kernel() and allow these boards to opt out from initrd loading. Cc: Palmer Dabbelt <palmer@dabbelt.com> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- hw/riscv/boot.c | 22 +++++++++++++++++++--- hw/riscv/microchip_pfsoc.c | 12 ++---------- hw/riscv/opentitan.c | 2 +- hw/riscv/sifive_e.c | 3 ++- hw/riscv/sifive_u.c | 12 ++---------- hw/riscv/spike.c | 11 +---------- hw/riscv/virt.c | 12 ++---------- include/hw/riscv/boot.h | 1 + 8 files changed, 30 insertions(+), 45 deletions(-)