Executing fmovem.l fpstate, %fpcr/%fpsr results in incorrect values
being loaded into the fpu control registers.

When fpu control registers are moved to/from memory, they must follow a
strict order regardless of the addressing mode: FPCR is always at the
lowest address, FPSR at the next, and FPIAR at the highest address.

The current implementation in gen_op_fmove_fcr() processes the bitmask
incorrectly and uses the wrong loop direction for pre-decrement mode,
mapping the registers to the wrong memory addresses.

Fix this by correcting the loop iteration order and mask checking to
follow the architectural memory layout.

Fixes: ba62494483ab ("target-m68k: add FPCR and FPSR")
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Closes: https://lore.kernel.org/qemu-devel/87cxyv4390.fsf@igel.home
Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
---
 target/m68k/translate.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 138c89d3e5..8d8531b1ad 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4837,24 +4837,26 @@ static void gen_op_fmove_fcr(CPUM68KState *env, DisasContext *s,
      */
 
     if (is_write && mode == 4) {
-        for (i = 2; i >= 0; i--, mask >>= 1) {
-            if (mask & 1) {
+        for (i = 0; i < 3; i++) {
+            if (mask & (1 << i)) {
                 gen_qemu_store_fcr(s, addr, 1 << i);
-                if (mask != 1) {
+                mask &= ~(1 << i);
+                if (mask != 0) {
                     tcg_gen_subi_i32(addr, addr, opsize_bytes(OS_LONG));
                 }
             }
        }
        tcg_gen_mov_i32(AREG(insn, 0), addr);
     } else {
-        for (i = 0; i < 3; i++, mask >>= 1) {
-            if (mask & 1) {
+        for (i = 2; i >= 0; i--) {
+            if (mask & (1 << i)) {
                 if (is_write) {
                     gen_qemu_store_fcr(s, addr, 1 << i);
                 } else {
                     gen_qemu_load_fcr(s, addr, 1 << i);
                 }
-                if (mask != 1 || mode == 3) {
+                mask &= ~(1 << i);
+                if (mask != 0 || mode == 3) {
                     tcg_gen_addi_i32(addr, addr, opsize_bytes(OS_LONG));
                 }
             }
-- 
2.54.0.563.g4f69b47b94-goog

Le 17/05/2026 à 21:17, Kuan-Wei Chiu a écrit :
> Executing fmovem.l fpstate, %fpcr/%fpsr results in incorrect values
> being loaded into the fpu control registers.
> 
> When fpu control registers are moved to/from memory, they must follow a
> strict order regardless of the addressing mode: FPCR is always at the
> lowest address, FPSR at the next, and FPIAR at the highest address.
> 
> The current implementation in gen_op_fmove_fcr() processes the bitmask
> incorrectly and uses the wrong loop direction for pre-decrement mode,
> mapping the registers to the wrong memory addresses.
> 
> Fix this by correcting the loop iteration order and mask checking to
> follow the architectural memory layout.
> 
> Fixes: ba62494483ab ("target-m68k: add FPCR and FPSR")
> Reported-by: Andreas Schwab <schwab@linux-m68k.org>
> Closes: https://lore.kernel.org/qemu-devel/87cxyv4390.fsf@igel.home
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

> ---
>   target/m68k/translate.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 138c89d3e5..8d8531b1ad 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4837,24 +4837,26 @@ static void gen_op_fmove_fcr(CPUM68KState *env, DisasContext *s,
>        */
>   
>       if (is_write && mode == 4) {
> -        for (i = 2; i >= 0; i--, mask >>= 1) {
> -            if (mask & 1) {
> +        for (i = 0; i < 3; i++) {
> +            if (mask & (1 << i)) {
>                   gen_qemu_store_fcr(s, addr, 1 << i);
> -                if (mask != 1) {
> +                mask &= ~(1 << i);
> +                if (mask != 0) {
>                       tcg_gen_subi_i32(addr, addr, opsize_bytes(OS_LONG));
>                   }
>               }
>          }
>          tcg_gen_mov_i32(AREG(insn, 0), addr);
>       } else {
> -        for (i = 0; i < 3; i++, mask >>= 1) {
> -            if (mask & 1) {
> +        for (i = 2; i >= 0; i--) {
> +            if (mask & (1 << i)) {
>                   if (is_write) {
>                       gen_qemu_store_fcr(s, addr, 1 << i);
>                   } else {
>                       gen_qemu_load_fcr(s, addr, 1 << i);
>                   }
> -                if (mask != 1 || mode == 3) {
> +                mask &= ~(1 << i);
> +                if (mask != 0 || mode == 3) {
>                       tcg_gen_addi_i32(addr, addr, opsize_bytes(OS_LONG));
>                   }
>               }