[PATCH 0/3] target/arm: Fix various FEAT_MOPS bugs

[PATCH 0/3] target/arm: Fix various FEAT_MOPS bugs

[Peter Maydell]

This patchset fixes some bugs in FEAT_MOPS that were encountered
by somebody trying to use it in qemu user-mode:
 * we forgot to set the SCTLR bit in CPU reset, so all the
   insns UNDEF
 * we weren't reporting the hwcap bit because of a uint32_t
   vs uint64_t mixup
 * we didn't handle the case of SET* with Xs == XZR correctly

This set of patches fix these issues.

thanks
-- PMM

Peter Maydell (3):
  target/arm: Enable FEAT_MOPS insns in user-mode emulation
  linux-user: Report AArch64 hwcap2 fields above bit 31
  target/arm: Make FEAT_MOPS SET* insns handle Xs == XZR correctly

 linux-user/loader.h         |  2 +-
 linux-user/elfload.c        |  8 ++++----
 target/arm/cpu.c            |  2 ++
 target/arm/tcg/helper-a64.c | 15 ++++++++++++---
 4 files changed, 19 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH 1/3] target/arm: Enable FEAT_MOPS insns in user-mode emulation

[Peter Maydell]

In user-mode emulation, we need to set the SCTLR_EL1.MSCEn
bit to avoid all the FEAT_MOPS insns UNDEFing.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index aa4e006f21a..cdb37ce5512 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -295,6 +295,8 @@ static void arm_cpu_reset_hold(Object *obj)
         env->cp15.sctlr_el[1] |= SCTLR_TSCXT;
         /* Disable access to Debug Communication Channel (DCC). */
         env->cp15.mdscr_el1 |= 1 << 12;
+        /* Enable FEAT_MOPS */
+        env->cp15.sctlr_el[1] |= SCTLR_MSCEN;
 #else
         /* Reset into the highest available EL */
         if (arm_feature(env, ARM_FEATURE_EL3)) {
-- 
2.34.1

[PATCH 2/3] linux-user: Report AArch64 hwcap2 fields above bit 31

[Peter Maydell]

The AArch64 ELF hwcap2 field is 64 bits, but our get_elf_hwcap2()
works with uint32_t, so it accidentally fails to report any hwcaps
over bit 31.  Use uint64_t here.

The Arm hwcap2 is only 32 bits (because the ELF format makes these
fields be the size of "long" in the ABI), but since it shares the
prototype declaration for get_elf_hwcap2() it is easier to also
expand it to 64 bits.

The only hwcap fields we implement already that are affected by this
are the HBC and MOPS ones, neither of which were implemented in a
previous release, so this doesn't need backporting to older stable
branches.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/loader.h  | 2 +-
 linux-user/elfload.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/linux-user/loader.h b/linux-user/loader.h
index 324e5c872af..9be00da40a4 100644
--- a/linux-user/loader.h
+++ b/linux-user/loader.h
@@ -61,7 +61,7 @@ uint32_t get_elf_hwcap(void);
 const char *elf_hwcap_str(uint32_t bit);
 #endif
 #if defined(TARGET_AARCH64) || defined(TARGET_ARM)
-uint32_t get_elf_hwcap2(void);
+uint64_t get_elf_hwcap2(void);
 const char *elf_hwcap2_str(uint32_t bit);
 #endif
 
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 2e3809f03c4..6fb44206fab 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -520,10 +520,10 @@ uint32_t get_elf_hwcap(void)
     return hwcaps;
 }
 
-uint32_t get_elf_hwcap2(void)
+uint64_t get_elf_hwcap2(void)
 {
     ARMCPU *cpu = ARM_CPU(thread_cpu);
-    uint32_t hwcaps = 0;
+    uint64_t hwcaps = 0;
 
     GET_FEATURE_ID(aa32_aes, ARM_HWCAP2_ARM_AES);
     GET_FEATURE_ID(aa32_pmull, ARM_HWCAP2_ARM_PMULL);
@@ -784,10 +784,10 @@ uint32_t get_elf_hwcap(void)
     return hwcaps;
 }
 
-uint32_t get_elf_hwcap2(void)
+uint64_t get_elf_hwcap2(void)
 {
     ARMCPU *cpu = ARM_CPU(thread_cpu);
-    uint32_t hwcaps = 0;
+    uint64_t hwcaps = 0;
 
     GET_FEATURE_ID(aa64_dcpodp, ARM_HWCAP2_A64_DCPODP);
     GET_FEATURE_ID(aa64_sve2, ARM_HWCAP2_A64_SVE2);
-- 
2.34.1

[PATCH 3/3] target/arm: Make FEAT_MOPS SET* insns handle Xs == XZR correctly

[Peter Maydell]

Most of the registers used by the FEAT_MOPS instructions cannot use
31 as a register field value; this is CONSTRAINED UNPREDICTABLE to
NOP or UNDEF (we UNDEF).  However, it is permitted for the "source
value" register for the memset insns SET* to be 31, which (as usual
for most data-processing insns) means it should be the zero register
XZR. We forgot to handle this case, with the effect that trying to
set memory to zero with a "SET* Xd, Xn, XZR" sets the memory to
the value that happens to be in the low byte of SP.

Handle XZR when getting the SET* data value from the register file.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/helper-a64.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index 84f54750fc2..ce4800b8d13 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -1206,6 +1206,15 @@ static void check_setg_alignment(CPUARMState *env, uint64_t ptr, uint64_t size,
     }
 }
 
+static uint64_t arm_reg_or_xzr(CPUARMState *env, int reg)
+{
+    /*
+     * Runtime equivalent of cpu_reg() -- return the CPU register value,
+     * for contexts when index 31 means XZR (not SP).
+     */
+    return reg == 31 ? 0 : env->xregs[reg];
+}
+
 /*
  * For the Memory Set operation, our implementation chooses
  * always to use "option A", where we update Xd to the final
@@ -1226,7 +1235,7 @@ static void do_setp(CPUARMState *env, uint32_t syndrome, uint32_t mtedesc,
     int rd = mops_destreg(syndrome);
     int rs = mops_srcreg(syndrome);
     int rn = mops_sizereg(syndrome);
-    uint8_t data = env->xregs[rs];
+    uint8_t data = arm_reg_or_xzr(env, rs);
     uint32_t memidx = FIELD_EX32(mtedesc, MTEDESC, MIDX);
     uint64_t toaddr = env->xregs[rd];
     uint64_t setsize = env->xregs[rn];
@@ -1286,7 +1295,7 @@ static void do_setm(CPUARMState *env, uint32_t syndrome, uint32_t mtedesc,
     int rd = mops_destreg(syndrome);
     int rs = mops_srcreg(syndrome);
     int rn = mops_sizereg(syndrome);
-    uint8_t data = env->xregs[rs];
+    uint8_t data = arm_reg_or_xzr(env, rs);
     uint64_t toaddr = env->xregs[rd] + env->xregs[rn];
     uint64_t setsize = -env->xregs[rn];
     uint32_t memidx = FIELD_EX32(mtedesc, MTEDESC, MIDX);
@@ -1349,7 +1358,7 @@ static void do_sete(CPUARMState *env, uint32_t syndrome, uint32_t mtedesc,
     int rd = mops_destreg(syndrome);
     int rs = mops_srcreg(syndrome);
     int rn = mops_sizereg(syndrome);
-    uint8_t data = env->xregs[rs];
+    uint8_t data = arm_reg_or_xzr(env, rs);
     uint64_t toaddr = env->xregs[rd] + env->xregs[rn];
     uint64_t setsize = -env->xregs[rn];
     uint32_t memidx = FIELD_EX32(mtedesc, MTEDESC, MIDX);
-- 
2.34.1

Re: [PATCH 0/3] target/arm: Fix various FEAT_MOPS bugs

[Richard Henderson]

On 10/30/23 10:39, Peter Maydell wrote:
> This patchset fixes some bugs in FEAT_MOPS that were encountered
> by somebody trying to use it in qemu user-mode:
>   * we forgot to set the SCTLR bit in CPU reset, so all the
>     insns UNDEF
>   * we weren't reporting the hwcap bit because of a uint32_t
>     vs uint64_t mixup
>   * we didn't handle the case of SET* with Xs == XZR correctly
> 
> This set of patches fix these issues.
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>    target/arm: Enable FEAT_MOPS insns in user-mode emulation
>    linux-user: Report AArch64 hwcap2 fields above bit 31
>    target/arm: Make FEAT_MOPS SET* insns handle Xs == XZR correctly

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 2/3] linux-user: Report AArch64 hwcap2 fields above bit 31

[Philippe Mathieu-Daudé]

On 30/10/23 18:39, Peter Maydell wrote:
> The AArch64 ELF hwcap2 field is 64 bits, but our get_elf_hwcap2()
> works with uint32_t, so it accidentally fails to report any hwcaps
> over bit 31.  Use uint64_t here.
> 
> The Arm hwcap2 is only 32 bits (because the ELF format makes these
> fields be the size of "long" in the ABI), but since it shares the
> prototype declaration for get_elf_hwcap2() it is easier to also
> expand it to 64 bits.
> 
> The only hwcap fields we implement already that are affected by this
> are the HBC and MOPS ones, neither of which were implemented in a
> previous release, so this doesn't need backporting to older stable
> branches.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/loader.h  | 2 +-
>   linux-user/elfload.c | 8 ++++----
>   2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>