| Message ID | 20221227064812.1903326-1-bmeng@tinylab.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/riscv: Improve Spike HTIF emulation fidelity | expand |
On 12/27/22 03:48, Bin Meng wrote: > At present the 32-bit OpenSBI generic firmware image does not boot on > Spike, only 64-bit image can. This is due to the HTIF emulation does > not implement the proxy syscall interface which is required for the > 32-bit HTIF console output. > > An OpenSBI bug fix [1] is also needed when booting the plain binary image. > > With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF > images can boot on QEMU 'spike' machine. > > [1] https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bmeng@tinylab.org/ Aside from a nit in patch 12/12, LGTM. I've tested with a patched version of Opensbi including [1] and I can get terminal output with riscv32 spike: $ ./qemu-system-riscv32 -M spike -display none -nographic -bios ../../opensbi/build/platform/generic/firmware/fw_payload.bin OpenSBI v1.1-112-g6ce00f8 ____ _____ ____ _____ / __ \ / ____| _ \_ _| | | | |_ __ ___ _ __ | (___ | |_) || | | | | | '_ \ / _ \ '_ \ \___ \| _ < | | | |__| | |_) | __/ | | |____) | |_) || |_ \____/| .__/ \___|_| |_|_____/|____/_____| | | |_| (.......) Speaking of [1], it seems like the fix went a bit too late for the opensbi 1.2 release. Assuming that [1] is accepted, it would be nice if we could bake in this fix on top of the 1.2 release when updating the QEMU roms. Thanks, Daniel > > > Bin Meng (10): > hw/char: riscv_htif: Avoid using magic numbers > hw/char: riscv_htif: Drop {to,from}host_size in HTIFState > hw/char: riscv_htif: Drop useless assignment of memory region > hw/char: riscv_htif: Use conventional 's' for HTIFState > hw/char: riscv_htif: Move registers from CPUArchState to HTIFState > hw/char: riscv_htif: Remove forward declarations for non-existent > variables > hw/char: riscv_htif: Support console output via proxy syscall > hw/riscv: spike: Remove the out-of-date comments > hw/riscv/boot.c: Introduce riscv_find_firmware() > hw/riscv: spike: Decouple create_fdt() dependency to ELF loading > > Daniel Henrique Barboza (2): > hw/riscv/boot.c: make riscv_find_firmware() static > hw/riscv/boot.c: introduce riscv_default_firmware_name() > > include/hw/char/riscv_htif.h | 19 +--- > include/hw/riscv/boot.h | 4 +- > target/riscv/cpu.h | 4 - > hw/char/riscv_htif.c | 172 +++++++++++++++++++++-------------- > hw/riscv/boot.c | 76 ++++++++++------ > hw/riscv/sifive_u.c | 11 +-- > hw/riscv/spike.c | 59 ++++++++---- > hw/riscv/virt.c | 10 +- > target/riscv/machine.c | 6 +- > 9 files changed, 212 insertions(+), 149 deletions(-) >
Hi Daniel, On Wed, Dec 28, 2022 at 1:52 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 12/27/22 03:48, Bin Meng wrote: > > At present the 32-bit OpenSBI generic firmware image does not boot on > > Spike, only 64-bit image can. This is due to the HTIF emulation does > > not implement the proxy syscall interface which is required for the > > 32-bit HTIF console output. > > > > An OpenSBI bug fix [1] is also needed when booting the plain binary image. > > > > With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF > > images can boot on QEMU 'spike' machine. > > > > [1] https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bmeng@tinylab.org/ > > Aside from a nit in patch 12/12, LGTM. I've tested with a patched version of > Opensbi including [1] and I can get terminal output with riscv32 spike: > > $ ./qemu-system-riscv32 -M spike -display none -nographic > -bios ../../opensbi/build/platform/generic/firmware/fw_payload.bin > > OpenSBI v1.1-112-g6ce00f8 > ____ _____ ____ _____ > / __ \ / ____| _ \_ _| > | | | |_ __ ___ _ __ | (___ | |_) || | > | | | | '_ \ / _ \ '_ \ \___ \| _ < | | > | |__| | |_) | __/ | | |____) | |_) || |_ > \____/| .__/ \___|_| |_|_____/|____/_____| > | | > |_| > (.......) > > > Speaking of [1], it seems like the fix went a bit too late for the opensbi 1.2 release. > Assuming that [1] is accepted, it would be nice if we could bake in this fix on top of the > 1.2 release when updating the QEMU roms. > Thanks for the review and testing! Regarding whether we can cherry-pick the fix on top of OpenSBI 1.2, I am not sure if that's allowed by the policy. Alistair, do you know? Regards, Bin
On Wed, Dec 28, 2022 at 1:59 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Daniel, > > On Wed, Dec 28, 2022 at 1:52 AM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: > > > > > > > > On 12/27/22 03:48, Bin Meng wrote: > > > At present the 32-bit OpenSBI generic firmware image does not boot on > > > Spike, only 64-bit image can. This is due to the HTIF emulation does > > > not implement the proxy syscall interface which is required for the > > > 32-bit HTIF console output. > > > > > > An OpenSBI bug fix [1] is also needed when booting the plain binary image. > > > > > > With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF > > > images can boot on QEMU 'spike' machine. > > > > > > [1] https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bmeng@tinylab.org/ > > > > Aside from a nit in patch 12/12, LGTM. I've tested with a patched version of > > Opensbi including [1] and I can get terminal output with riscv32 spike: > > > > $ ./qemu-system-riscv32 -M spike -display none -nographic > > -bios ../../opensbi/build/platform/generic/firmware/fw_payload.bin > > > > OpenSBI v1.1-112-g6ce00f8 > > ____ _____ ____ _____ > > / __ \ / ____| _ \_ _| > > | | | |_ __ ___ _ __ | (___ | |_) || | > > | | | | '_ \ / _ \ '_ \ \___ \| _ < | | > > | |__| | |_) | __/ | | |____) | |_) || |_ > > \____/| .__/ \___|_| |_|_____/|____/_____| > > | | > > |_| > > (.......) > > > > > > Speaking of [1], it seems like the fix went a bit too late for the opensbi 1.2 release. > > Assuming that [1] is accepted, it would be nice if we could bake in this fix on top of the > > 1.2 release when updating the QEMU roms. > > > > Thanks for the review and testing! > > Regarding whether we can cherry-pick the fix on top of OpenSBI 1.2, I > am not sure if that's allowed by the policy. It doesn't seem like a good idea. Maybe it's worth pinging Anup to see if we can get a 1.2.1 with the fix? Alistair > > Alistair, do you know? > > Regards, > Bin >
At present the 32-bit OpenSBI generic firmware image does not boot on Spike, only 64-bit image can. This is due to the HTIF emulation does not implement the proxy syscall interface which is required for the 32-bit HTIF console output. An OpenSBI bug fix [1] is also needed when booting the plain binary image. With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF images can boot on QEMU 'spike' machine. [1] https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bmeng@tinylab.org/ Bin Meng (10): hw/char: riscv_htif: Avoid using magic numbers hw/char: riscv_htif: Drop {to,from}host_size in HTIFState hw/char: riscv_htif: Drop useless assignment of memory region hw/char: riscv_htif: Use conventional 's' for HTIFState hw/char: riscv_htif: Move registers from CPUArchState to HTIFState hw/char: riscv_htif: Remove forward declarations for non-existent variables hw/char: riscv_htif: Support console output via proxy syscall hw/riscv: spike: Remove the out-of-date comments hw/riscv/boot.c: Introduce riscv_find_firmware() hw/riscv: spike: Decouple create_fdt() dependency to ELF loading Daniel Henrique Barboza (2): hw/riscv/boot.c: make riscv_find_firmware() static hw/riscv/boot.c: introduce riscv_default_firmware_name() include/hw/char/riscv_htif.h | 19 +--- include/hw/riscv/boot.h | 4 +- target/riscv/cpu.h | 4 - hw/char/riscv_htif.c | 172 +++++++++++++++++++++-------------- hw/riscv/boot.c | 76 ++++++++++------ hw/riscv/sifive_u.c | 11 +-- hw/riscv/spike.c | 59 ++++++++---- hw/riscv/virt.c | 10 +- target/riscv/machine.c | 6 +- 9 files changed, 212 insertions(+), 149 deletions(-)