1. Patchew
  2. QEMU
  3. View series

[PATCH] target/arm: mark the 32bit alias of PAR when LPAE enabled

Alex Bennée posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231107105145.2916124-1-alex.bennee@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/helper.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
[PATCH] target/arm: mark the 32bit alias of PAR when LPAE enabled
Posted by Alex Bennée 1 year ago 
We also mark it ARM_CP_NO_GDB so we avoid duplicate PAR's in the
system register XML we send to gdb.

Suggested-by: <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - only set ARM_CP_NO_GDB when no LPAE enabled
  - also mark as AP_CP_ALIAS
---
 target/arm/helper.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 5dc0d20a84..80a35b29bb 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3722,20 +3722,6 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
 }
 #endif
 
-static const ARMCPRegInfo vapa_cp_reginfo[] = {
-    { .name = "PAR", .cp = 15, .crn = 7, .crm = 4, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW, .resetvalue = 0,
-      .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
-                             offsetoflow32(CPUARMState, cp15.par_ns) },
-      .writefn = par_write },
-#ifndef CONFIG_USER_ONLY
-    /* This underdecoding is safe because the reginfo is NO_RAW. */
-    { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
-      .access = PL1_W, .accessfn = ats_access,
-      .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },
-#endif
-};
-
 /* Return basic MPU access permission bits.  */
 static uint32_t simple_mpu_ap_bits(uint32_t val)
 {
@@ -8904,6 +8890,24 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, generic_timer_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_VAPA)) {
+        ARMCPRegInfo vapa_cp_reginfo[] = {
+            { .name = "PAR", .cp = 15, .crn = 7, .crm = 4, .opc1 = 0, .opc2 = 0,
+              .access = PL1_RW, .resetvalue = 0,
+              .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
+                                     offsetoflow32(CPUARMState, cp15.par_ns) },
+              .writefn = par_write},
+#ifndef CONFIG_USER_ONLY
+            /* This underdecoding is safe because the reginfo is NO_RAW. */
+            { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
+              .access = PL1_W, .accessfn = ats_access,
+              .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },
+#endif
+        };
+
+        /* When LPAE exists the PAR register is an alias */
+        if (arm_feature(env, ARM_FEATURE_LPAE)) {
+            vapa_cp_reginfo[0].type = ARM_CP_ALIAS | ARM_CP_NO_GDB;
+        }
         define_arm_cp_regs(cpu, vapa_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_CACHE_TEST_CLEAN)) {
-- 
2.39.2
Re: [PATCH] target/arm: mark the 32bit alias of PAR when LPAE enabled
Posted by Richard Henderson 1 year ago 
On 11/7/23 02:51, Alex Bennée wrote:
> We also mark it ARM_CP_NO_GDB so we avoid duplicate PAR's in the
> system register XML we send to gdb.
> 
> Suggested-by: <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>    - only set ARM_CP_NO_GDB when no LPAE enabled
>    - also mark as AP_CP_ALIAS
> ---
>   target/arm/helper.c | 32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
> 

With Peter's comment improvement,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Re: [PATCH] target/arm: mark the 32bit alias of PAR when LPAE enabled
Posted by Peter Maydell 1 year ago 
On Tue, 7 Nov 2023 at 10:51, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> We also mark it ARM_CP_NO_GDB so we avoid duplicate PAR's in the
> system register XML we send to gdb.
>
> Suggested-by: <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
>   - only set ARM_CP_NO_GDB when no LPAE enabled
>   - also mark as AP_CP_ALIAS
> ---
>  target/arm/helper.c | 32 ++++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 5dc0d20a84..80a35b29bb 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -3722,20 +3722,6 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
>  }
>  #endif
>
> -static const ARMCPRegInfo vapa_cp_reginfo[] = {
> -    { .name = "PAR", .cp = 15, .crn = 7, .crm = 4, .opc1 = 0, .opc2 = 0,
> -      .access = PL1_RW, .resetvalue = 0,
> -      .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
> -                             offsetoflow32(CPUARMState, cp15.par_ns) },
> -      .writefn = par_write },
> -#ifndef CONFIG_USER_ONLY
> -    /* This underdecoding is safe because the reginfo is NO_RAW. */
> -    { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
> -      .access = PL1_W, .accessfn = ats_access,
> -      .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },
> -#endif
> -};
> -
>  /* Return basic MPU access permission bits.  */
>  static uint32_t simple_mpu_ap_bits(uint32_t val)
>  {
> @@ -8904,6 +8890,24 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>          define_arm_cp_regs(cpu, generic_timer_cp_reginfo);
>      }
>      if (arm_feature(env, ARM_FEATURE_VAPA)) {
> +        ARMCPRegInfo vapa_cp_reginfo[] = {
> +            { .name = "PAR", .cp = 15, .crn = 7, .crm = 4, .opc1 = 0, .opc2 = 0,
> +              .access = PL1_RW, .resetvalue = 0,
> +              .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.par_s),
> +                                     offsetoflow32(CPUARMState, cp15.par_ns) },
> +              .writefn = par_write},
> +#ifndef CONFIG_USER_ONLY
> +            /* This underdecoding is safe because the reginfo is NO_RAW. */
> +            { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
> +              .access = PL1_W, .accessfn = ats_access,
> +              .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },
> +#endif
> +        };
> +
> +        /* When LPAE exists the PAR register is an alias */

We could be a little more explicit:
 /*
  * When LPAE exists this 32-bit PAR register is an alias of the
  * 64-bit AArch32 PAR register defined in lpae_cp_reginfo[]
  */

> +        if (arm_feature(env, ARM_FEATURE_LPAE)) {
> +            vapa_cp_reginfo[0].type = ARM_CP_ALIAS | ARM_CP_NO_GDB;
> +        }
>          define_arm_cp_regs(cpu, vapa_cp_reginfo);
>      }
>      if (arm_feature(env, ARM_FEATURE_CACHE_TEST_CLEAN)) {

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.