Hi Alistair, On Thu, Jan 12, 2023 at 8:36 AM Alistair Francis <alistai...@gmail.com> wrote: > > On Mon, Jan 2, 2023 at 9:55 PM Daniel Henrique Barboza > <dbarb...@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 <pal...@dabbelt.com> > > Signed-off-by: Daniel Henrique Barboza <dbarb...@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