On 30/09/2025 07:59, Heinrich Schuchardt wrote:
On 9/29/25 16:37, Philippe Mathieu-Daudé wrote:
Hi,
On 29/9/25 13:55, Valentin Haudiquet wrote:
From: vhaudiquet <[email protected]>
Three instructions were not using the endianness swap flag, which
resulted in a bug on big-endian architectures.
I suppose you mean "big-endian host architectures".
If so, please clarify.
Hello Phil,
Ubuntu is providing QEMU to emulate RISC-V both on little-endian hosts
like X86 and ARM as well as on big-endian hosts like the IBM S/390.
The Unprivileged RISC-V ISA Specification has this sentence:
"The base ISA has been defined to have a little-endian memory system,
with big-endian or bi-endian as non-standard variants."
According to the Privileged RISC-V ISA Specification the mstatus and
mstatush register may be used to set the endianness at runtime.
"The MBE, SBE, and UBE bits in mstatus and mstatush are WARL fields that
control the endianness of memory accesses other than instruction
fetches. Instruction fetches are always little-endian."
Currently RISC-V work focuses on little-endian targets but there is
active community work to enable big-endian Linux for RISC-V.
Therefore a solution is required that considers both the host and the
target endianness.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
Signed-off-by: Valentin Haudiquet <[email protected]>
---
target/riscv/insn_trans/trans_rvzce.c.inc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc b/target/
riscv/ insn_trans/trans_rvzce.c.inc
index c77c2b927b..dd15af0f54 100644
--- a/target/riscv/insn_trans/trans_rvzce.c.inc
+++ b/target/riscv/insn_trans/trans_rvzce.c.inc
@@ -88,13 +88,13 @@ static bool trans_c_lbu(DisasContext *ctx,
arg_c_lbu *a)
static bool trans_c_lhu(DisasContext *ctx, arg_c_lhu *a)
{
REQUIRE_ZCB(ctx);
- return gen_load(ctx, a, MO_UW);
+ return gen_load(ctx, a, MO_TEUW);
NAck.
Please do not use MO_TE* anymore. Use explicit endianness.
So far all our RISC-V targets are little-endian:
$ git grep TARGET_BIG_ENDIAN configs/targets/riscv*
$
If you are not worried about RISCV core running in BE mode
(as we currently don't check MSTATUS_[USM]BE bits), your change
should be:
- return gen_load(ctx, a, MO_UW);
+ return gen_load(ctx, a, MO_UW | MO_LE);
I saw your patches to remove MO_TE from little-endian only targets like
i386 but RISC-V is different.
We should foresee that in future RISC-V targets run in either little-
endian or big-endian mode and implement our code accordingly.
When big-endian RISC-V comes upon QEMU, we should avoid duplicating the
target code but reuse what we have.
MO_UW | MO_LE looks wrong in this context.
We should stay consistent with
target/riscv/insn_trans/trans_rvi.c.inc
target/riscv/insn_trans/trans_rvzfh.c.inc
target/riscv/insn_trans/trans_xthead.c.inc
which currently use MO_TEUW.
For the time being, I would suggest to stick to MO_TE* to maintain the
information in which code locations we need to consider the target
endianness.
Targets that can switch endianness at runtime (e.g. MIPS) use an
architecture specific implemenation of mo_endian(ctx). When implementing
big-endian RISC-V support in future, we can use the MO_TE occurrences as
indicator where to use mo_endian() instead.
With these considerations in mind the current patch looks good to me.
Reviewed-by: Heinrich Schuchardt <[email protected]>
You've reminded me that we still haven't managed to get movement on
the big-endian runtime patches.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
