[Qemu-devel] [PATCH] target-arm: Fix general-purpose register mapping between AArch32 state and AArch64 state

[Qemu-devel] [PATCH] target-arm: Fix general-purpose register mapping between AArch32 state and AArch64 state

[Sergey Sorokin]

There is an error in functions aarch64_sync_32_to_64() and
aarch64_sync_64_to_32() with mapping of registers between AArch32 and AArch64.
The patch fixes it according to ARM ARM D1.20.1 (table D1-77).

Signed-off-by: Sergey Sorokin <afarallax@yandex.ru>
---
 target-arm/helper.c | 64 ++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7df1f06..1d52e5e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5322,35 +5322,35 @@ void aarch64_sync_32_to_64(CPUARMState *env)
     }
 
     if (mode == ARM_CPU_MODE_IRQ) {
-        env->xregs[16] = env->regs[13];
-        env->xregs[17] = env->regs[14];
+        env->xregs[16] = env->regs[14];
+        env->xregs[17] = env->regs[13];
     } else {
-        env->xregs[16] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
-        env->xregs[17] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
+        env->xregs[16] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];
+        env->xregs[17] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];
     }
 
     if (mode == ARM_CPU_MODE_SVC) {
-        env->xregs[18] = env->regs[13];
-        env->xregs[19] = env->regs[14];
+        env->xregs[18] = env->regs[14];
+        env->xregs[19] = env->regs[13];
     } else {
-        env->xregs[18] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
-        env->xregs[19] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
+        env->xregs[18] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];
+        env->xregs[19] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];
     }
 
     if (mode == ARM_CPU_MODE_ABT) {
-        env->xregs[20] = env->regs[13];
-        env->xregs[21] = env->regs[14];
+        env->xregs[20] = env->regs[14];
+        env->xregs[21] = env->regs[13];
     } else {
-        env->xregs[20] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
-        env->xregs[21] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
+        env->xregs[20] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];
+        env->xregs[21] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];
     }
 
     if (mode == ARM_CPU_MODE_UND) {
-        env->xregs[22] = env->regs[13];
-        env->xregs[23] = env->regs[14];
+        env->xregs[22] = env->regs[14];
+        env->xregs[23] = env->regs[13];
     } else {
-        env->xregs[22] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
-        env->xregs[23] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
+        env->xregs[22] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];
+        env->xregs[23] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];
     }
 
     /* Registers x24-x30 are mapped to r8-r14 in FIQ mode.  If we are in FIQ
@@ -5427,35 +5427,35 @@ void aarch64_sync_64_to_32(CPUARMState *env)
     }
 
     if (mode == ARM_CPU_MODE_IRQ) {
-        env->regs[13] = env->xregs[16];
-        env->regs[14] = env->xregs[17];
+        env->regs[14] = env->xregs[16];
+        env->regs[13] = env->xregs[17];
     } else {
-        env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];
-        env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[17];
+        env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];
+        env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[17];
     }
 
     if (mode == ARM_CPU_MODE_SVC) {
-        env->regs[13] = env->xregs[18];
-        env->regs[14] = env->xregs[19];
+        env->regs[14] = env->xregs[18];
+        env->regs[13] = env->xregs[19];
     } else {
-        env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];
-        env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[19];
+        env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];
+        env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[19];
     }
 
     if (mode == ARM_CPU_MODE_ABT) {
-        env->regs[13] = env->xregs[20];
-        env->regs[14] = env->xregs[21];
+        env->regs[14] = env->xregs[20];
+        env->regs[13] = env->xregs[21];
     } else {
-        env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];
-        env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[21];
+        env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];
+        env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[21];
     }
 
     if (mode == ARM_CPU_MODE_UND) {
-        env->regs[13] = env->xregs[22];
-        env->regs[14] = env->xregs[23];
+        env->regs[14] = env->xregs[22];
+        env->regs[13] = env->xregs[23];
     } else {
-        env->banked_r13[bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];
-        env->banked_r14[bank_number(ARM_CPU_MODE_UND)] = env->xregs[23];
+        env->banked_r14[bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];
+        env->banked_r13[bank_number(ARM_CPU_MODE_UND)] = env->xregs[23];
     }
 
     /* Registers x24-x30 are mapped to r8-r14 in FIQ mode.  If we are in FIQ
-- 
1.9.3

Re: [Qemu-devel] [PATCH] target-arm: Fix general-purpose register mapping between AArch32 state and AArch64 state

[Peter Maydell]

On 28 August 2015 at 22:14, Sergey Sorokin <afarallax@yandex.ru> wrote:
> There is an error in functions aarch64_sync_32_to_64() and
> aarch64_sync_64_to_32() with mapping of registers between AArch32 and AArch64.
> The patch fixes it according to ARM ARM D1.20.1 (table D1-77).
>
> Signed-off-by: Sergey Sorokin <afarallax@yandex.ru>

Thanks, nice catch.

Applied to target-arm.next (with minor tweaking of the commit message;
it would be nice if you could keep the lines short enough that they
don't line-wrap on an 80-column terminal).

-- PMM

Re: [Qemu-devel] [PATCH] target-arm: Fix general-purpose register mapping between AArch32 state and AArch64 state

[Sergey Sorokin]

I did so. As I see, the longest line in the message is 78 chars. How short should it be in the patches?

01.09.2015, 19:57, "Peter Maydell" <peter.maydell@linaro.org>:
> On 28 August 2015 at 22:14, Sergey Sorokin <afarallax@yandex.ru> wrote:
>>  There is an error in functions aarch64_sync_32_to_64() and
>>  aarch64_sync_64_to_32() with mapping of registers between AArch32 and AArch64.
>>  The patch fixes it according to ARM ARM D1.20.1 (table D1-77).
>>
>>  Signed-off-by: Sergey Sorokin <afarallax@yandex.ru>
>
> Thanks, nice catch.
>
> Applied to target-arm.next (with minor tweaking of the commit message;
> it would be nice if you could keep the lines short enough that they
> don't line-wrap on an 80-column terminal).
>
> -- PMM

Re: [Qemu-devel] [PATCH] target-arm: Fix general-purpose register mapping between AArch32 state and AArch64 state

[Peter Maydell]

On 2 September 2015 at 10:53, Sergey Sorokin <afarallax@yandex.ru> wrote:
> I did so. As I see, the longest line in the message is 78 chars.
> How short should it be in the patches?

If you look at the output of "git log" you'll see that git formats
the commit messages with a four-character left margin. So you need
to keep them short enough that they don't overflow when indented a
bit -- 70..75 chars is probably about as wide as you want.

thanks
-- PMM