[PATCH] target/riscv: Fix endianness swap on compressed instructions

[PATCH] target/riscv: Fix endianness swap on compressed instructions

[Valentin Haudiquet]

From: vhaudiquet <vhaudiquet343@hotmail.fr>

Three instructions were not using the endianness swap flag, which resulted in a bug on big-endian architectures.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828

Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
---
 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);
 }
 
 static bool trans_c_lh(DisasContext *ctx, arg_c_lh *a)
 {
     REQUIRE_ZCB(ctx);
-    return gen_load(ctx, a, MO_SW);
+    return gen_load(ctx, a, MO_TESW);
 }
 
 static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
@@ -106,7 +106,7 @@ static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
 static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
 {
     REQUIRE_ZCB(ctx);
-    return gen_store(ctx, a, MO_UW);
+    return gen_store(ctx, a, MO_TEUW);
 }
 
 #define X_S0    8
-- 
2.51.0

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Anton Johansson via]

On 29/09/25, Valentin Haudiquet wrote:
> From: vhaudiquet <vhaudiquet343@hotmail.fr>
> 
> Three instructions were not using the endianness swap flag, which resulted in a bug on big-endian architectures.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
> 
> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
> ---
>  target/riscv/insn_trans/trans_rvzce.c.inc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Daniel Henrique Barboza]

On 9/29/25 8:55 AM, Valentin Haudiquet wrote:
> From: vhaudiquet <vhaudiquet343@hotmail.fr>
> 
> Three instructions were not using the endianness swap flag, which resulted in a bug on big-endian architectures.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
> 
> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   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);
>   }
>   
>   static bool trans_c_lh(DisasContext *ctx, arg_c_lh *a)
>   {
>       REQUIRE_ZCB(ctx);
> -    return gen_load(ctx, a, MO_SW);
> +    return gen_load(ctx, a, MO_TESW);
>   }
>   
>   static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
> @@ -106,7 +106,7 @@ static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
>   static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
>   {
>       REQUIRE_ZCB(ctx);
> -    return gen_store(ctx, a, MO_UW);
> +    return gen_store(ctx, a, MO_TEUW);
>   }
>   
>   #define X_S0    8

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Philippe Mathieu-Daudé]

Hi,


On 29/9/25 13:55, Valentin Haudiquet wrote:
> From: vhaudiquet <vhaudiquet343@hotmail.fr>
> 
> 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.

> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
> 
> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
> ---
>   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);

>   }

Regards,

Phil.

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Richard Henderson]

On 9/29/25 04:55, Valentin Haudiquet wrote:
> From: vhaudiquet <vhaudiquet343@hotmail.fr>
> 
> Three instructions were not using the endianness swap flag, which resulted in a bug on big-endian architectures.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
> 
> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
> ---
>   target/riscv/insn_trans/trans_rvzce.c.inc | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Cc: qemu-stable@nongnu.org
Fixes: e0a3054f18e ("target/riscv: add support for Zcb extension")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> 
> 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);
>   }
>   
>   static bool trans_c_lh(DisasContext *ctx, arg_c_lh *a)
>   {
>       REQUIRE_ZCB(ctx);
> -    return gen_load(ctx, a, MO_SW);
> +    return gen_load(ctx, a, MO_TESW);
>   }
>   
>   static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
> @@ -106,7 +106,7 @@ static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
>   static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
>   {
>       REQUIRE_ZCB(ctx);
> -    return gen_store(ctx, a, MO_UW);
> +    return gen_store(ctx, a, MO_TEUW);
>   }
>   
>   #define X_S0    8

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Anton Johansson via]

On 29/09/25, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> 
> On 29/9/25 13:55, Valentin Haudiquet wrote:
> > From: vhaudiquet <vhaudiquet343@hotmail.fr>
> > 
> > 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.
> 
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
> > Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
> > 
> > Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
> > ---
> >   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);
> 
> >   }
> 
> Regards,
> 
> Phil.

Right, I forgot about the MO_TE changes when reviewing. My bad.

//Anton

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Philippe Mathieu-Daudé]

On 29/9/25 17:12, Anton Johansson wrote:
> On 29/09/25, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>>
>> On 29/9/25 13:55, Valentin Haudiquet wrote:
>>> From: vhaudiquet <vhaudiquet343@hotmail.fr>
>>>
>>> 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.
>>
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
>>> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
>>>
>>> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
>>> ---
>>>    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);
>>
>>>    }
>>
>> Regards,
>>
>> Phil.
> 
> Right, I forgot about the MO_TE changes when reviewing. My bad.

Well, since this is a bugfix for current tree, maybe let's go with
this patch and remove MO_TE on top...

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Philippe Mathieu-Daudé]

On 29/9/25 17:19, Philippe Mathieu-Daudé wrote:
> On 29/9/25 17:12, Anton Johansson wrote:
>> On 29/09/25, Philippe Mathieu-Daudé wrote:
>>> Hi,
>>>
>>>
>>> On 29/9/25 13:55, Valentin Haudiquet wrote:
>>>> From: vhaudiquet <vhaudiquet343@hotmail.fr>
>>>>
>>>> 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.
>>>
>>>>
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
>>>> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
>>>>
>>>> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>
>>>> ---
>>>>    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);
>>>
>>>>    }
>>>
>>> Regards,
>>>
>>> Phil.
>>
>> Right, I forgot about the MO_TE changes when reviewing. My bad.
> 
> Well, since this is a bugfix for current tree, maybe let's go with
> this patch and remove MO_TE on top...

Also, as Valentin noted, there is no "official MO_TE" deprecation,
so my Nack is really aggressive here, and I apologize for it.

Regards,

Phil.

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Heinrich Schuchardt]

On 9/29/25 16:37, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> 
> On 29/9/25 13:55, Valentin Haudiquet wrote:
>> From: vhaudiquet <vhaudiquet343@hotmail.fr>
>>
>> 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 <valentin.haudiquet@canonical.com>
>> ---
>>   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 <heinrich.schuchardt@canonical.com>

> 
>>   }
> 
> Regards,
> 
> Phil.
> 
> 

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Ben Dooks]

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 <vhaudiquet343@hotmail.fr>
>>>
>>> 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 <valentin.haudiquet@canonical.com>
>>> ---
>>>   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 <heinrich.schuchardt@canonical.com>

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

Re: [PATCH] target/riscv: Fix endianness swap on compressed instructions

[Alistair Francis]

On Mon, Sep 29, 2025 at 11:02 PM Valentin Haudiquet
<valentin.haudiquet@canonical.com> wrote:
>
> From: vhaudiquet <vhaudiquet343@hotmail.fr>
>
> Three instructions were not using the endianness swap flag, which resulted in a bug on big-endian architectures.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3131
> Buglink: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2123828
>
> Signed-off-by: Valentin Haudiquet <valentin.haudiquet@canonical.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  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);
>  }
>
>  static bool trans_c_lh(DisasContext *ctx, arg_c_lh *a)
>  {
>      REQUIRE_ZCB(ctx);
> -    return gen_load(ctx, a, MO_SW);
> +    return gen_load(ctx, a, MO_TESW);
>  }
>
>  static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
> @@ -106,7 +106,7 @@ static bool trans_c_sb(DisasContext *ctx, arg_c_sb *a)
>  static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
>  {
>      REQUIRE_ZCB(ctx);
> -    return gen_store(ctx, a, MO_UW);
> +    return gen_store(ctx, a, MO_TEUW);
>  }
>
>  #define X_S0    8
> --
> 2.51.0
>
>