On Fri, Jan 13, 2023 at 6:23 PM Daniel Henrique Barboza <dbarb...@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 > >> <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(-) > > > >>> @@ -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