Re: [PATCH 00/12] hw/riscv: Improve Spike HTIF emulation fidelity

[Daniel Henrique Barboza]

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(-)