On 10/2/24 11:44 AM, Mark Cave-Ayland wrote:
On 02/10/2024 15:17, Daniel Henrique Barboza wrote:
Phil, this patch breaks 'make check-avocado' in my env:
AVOCADO tests/avocado
JOB ID : 2e98b3ea8b63d22f092ff73bdadfd975cbc27026
JOB LOG :
/home/danielhb/work/qemu/build/tests/results/job-2024-10-02T10.48-2e98b3e/job.log
(01/15) tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv32_spike:
STARTED
(01/15) tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv32_spike:
INTERRUPTED: Test interrupted: Timeout reached (5.07 s)
Interrupting job (failfast).
RESULTS : PASS 0 | ERROR 0 | FAIL 0 | SKIP 14 | WARN 0 | INTERRUPT 1 |
CANCEL 0
JOB TIME : 7.57 s
Test summary:
01-tests/avocado/riscv_opensbi.py:RiscvOpenSBI.test_riscv32_spike: INTERRUPTED
make[1]: *** [/home/danielhb/work/qemu/tests/Makefile.include:141:
check-avocado] Error 8
In fact, if you try to execute the 'spike' machine with --nographics, you're
supposed to see the OpenSBI banner and boot. With this patch I don't see
anything:
$ ./build/qemu-system-riscv32 -M spike --nographic
(---nothing---)
For reference this is what I applied on top of master to test it:
9de01b7f1c (HEAD -> review) hw/riscv/spike: Replace tswap64() by ldq_endian_p()
202986f968 hw/net/tulip: Use ld/st_endian_pci_dma() API
30aacb872e hw/pci/pci_device: Introduce ld/st_endian_pci_dma() API
8ee9a2310b hw/pci/pci_device: Add PCI_DMA_DEFINE_LDST_END() macro
54271f92e5 hw/virtio/virtio-access: Use ld/st_endian_phys() API
54ff063593 exec/memory_ldst_phys: Introduce ld/st_endian_phys() API
5749a411cc hw/xtensa/xtfpga: Replace memcpy()+tswap32() by stl_endian_p()
75b7a99a5d hw/xtensa/xtfpga: Remove TARGET_BIG_ENDIAN #ifdef'ry
652016da1e tests/tcg/plugins: Use the ld/st_endian_p() API
d1e4d2544a hw/mips: Add cpu_is_bigendian field to BlCpuCfg structure
6a7b3e09bb hw/mips: Pass BlCpuCfg argument to bootloader API
0466217d0e target/arm/ptw: Use the ld/st_endian_p() API
920a241f00 hw/virtio/virtio-access: Use the ld/st_endian_p() API
e5fc1a2224 qemu/bswap: Introduce ld/st_endian_p() API
062cfce8d4 (origin/master, origin/HEAD, master) Merge tag
'pull-target-arm-20241001' of https://git.linaro.org/people/pmaydell/qemu-arm
into staging
Let me know if I did something wrong. Thanks,
Daniel
On 9/30/24 9:48 AM, Philippe Mathieu-Daudé wrote:
Hold the target endianness in HTIFState::target_is_bigendian.
Pass the target endianness as argument to htif_mm_init().
Replace tswap64() calls by ldq_endian_p() ones.
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
Based-on: <20240930073450.33195-2-phi...@linaro.org>
"qemu/bswap: Introduce ld/st_endian_p() API"
Note: this is a non-QOM device!
---
include/hw/char/riscv_htif.h | 4 +++-
hw/char/riscv_htif.c | 17 +++++++++++------
hw/riscv/spike.c | 2 +-
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
index df493fdf6b..24868ddfe1 100644
--- a/include/hw/char/riscv_htif.h
+++ b/include/hw/char/riscv_htif.h
@@ -35,6 +35,7 @@ typedef struct HTIFState {
hwaddr tohost_offset;
hwaddr fromhost_offset;
MemoryRegion mmio;
+ bool target_is_bigendian;
CharBackend chr;
uint64_t pending_read;
@@ -49,6 +50,7 @@ void htif_symbol_callback(const char *st_name, int st_info,
uint64_t st_value,
/* legacy pre qom */
HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
- uint64_t nonelf_base, bool custom_base);
+ uint64_t nonelf_base, bool custom_base,
+ bool target_is_bigendian);
#endif
diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 9bef60def1..77951f3c76 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -30,7 +30,6 @@
#include "qemu/timer.h"
#include "qemu/error-report.h"
#include "exec/address-spaces.h"
-#include "exec/tswap.h"
#include "sysemu/dma.h"
#include "sysemu/runstate.h"
@@ -211,13 +210,17 @@ static void htif_handle_tohost_write(HTIFState *s,
uint64_t val_written)
SHUTDOWN_CAUSE_GUEST_SHUTDOWN, exit_code);
return;
} else {
+ bool be = s->target_is_bigendian;
uint64_t syscall[8];
+
cpu_physical_memory_read(payload, syscall, sizeof(syscall));
- if (tswap64(syscall[0]) == PK_SYS_WRITE &&
- tswap64(syscall[1]) == HTIF_DEV_CONSOLE &&
- tswap64(syscall[3]) == HTIF_CONSOLE_CMD_PUTC) {
+ if (ldq_endian_p(be, &syscall[0]) == PK_SYS_WRITE &&
+ ldq_endian_p(be, &syscall[1]) == HTIF_DEV_CONSOLE &&
+ ldq_endian_p(be, &syscall[3]) == HTIF_CONSOLE_CMD_PUTC) {
uint8_t ch;
- cpu_physical_memory_read(tswap64(syscall[2]), &ch, 1);
+
+ cpu_physical_memory_read(ldl_endian_p(be, &syscall[2]),
^^^^^^^^^^^^
Shouldn't this be ldq_endian_p() for a 64-bit value?
Bingo! This change fixes make check-avocado and the OpenSBI boot:
$ git diff
diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 77951f3c76..0ed038a70c 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -219,7 +219,7 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t
val_written)
ldq_endian_p(be, &syscall[3]) == HTIF_CONSOLE_CMD_PUTC) {
uint8_t ch;
- cpu_physical_memory_read(ldl_endian_p(be, &syscall[2]),
+ cpu_physical_memory_read(ldq_endian_p(be, &syscall[2]),
&ch, 1);
qemu_chr_fe_write(&s->chr, &ch, 1);
resp = 0x100 | (uint8_t)payload;
$ ./build/qemu-system-riscv32 -M spike --nographic
OpenSBI v1.5.1
____ _____ ____ _____
/ __ \ / ____| _ \_ _|
| | | |_ __ ___ _ __ | (___ | |_) || |
| | | | '_ \ / _ \ '_ \ \___ \| _ < | |
| |__| | |_) | __/ | | |____) | |_) || |_
\____/| .__/ \___|_| |_|_____/|____/_____|
| |
|_|
(...)
Thanks,
Daniel
+ &ch, 1);
qemu_chr_fe_write(&s->chr, &ch, 1);
resp = 0x100 | (uint8_t)payload;
} else {
@@ -320,7 +323,8 @@ static const MemoryRegionOps htif_mm_ops = {
};
HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
- uint64_t nonelf_base, bool custom_base)
+ uint64_t nonelf_base, bool custom_base,
+ bool target_is_bigendian)
{
uint64_t base, size, tohost_offset, fromhost_offset;
@@ -345,6 +349,7 @@ HTIFState *htif_mm_init(MemoryRegion *address_space,
Chardev *chr,
s->pending_read = 0;
s->allow_tohost = 0;
s->fromhost_inprogress = 0;
+ s->target_is_bigendian = target_is_bigendian;
qemu_chr_fe_init(&s->chr, chr, &error_abort);
qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
htif_be_change, s, NULL, true);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 64074395bc..0989cd4a41 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -327,7 +327,7 @@ static void spike_board_init(MachineState *machine)
/* initialize HTIF using symbols found in load_kernel */
htif_mm_init(system_memory, serial_hd(0), memmap[SPIKE_HTIF].base,
- htif_custom_base);
+ htif_custom_base, TARGET_BIG_ENDIAN);
}
static void spike_set_signature(Object *obj, const char *val, Error **errp)
ATB,
Mark.
