qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 5/5] target/arm: Correctly implement Feat_DoubleLock


From: Richard Henderson
Subject: Re: [PATCH 5/5] target/arm: Correctly implement Feat_DoubleLock
Date: Tue, 5 Jul 2022 22:46:20 +0530
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1

On 7/5/22 21:06, Peter Maydell wrote:
On Sat, 2 Jul 2022 at 15:19, Richard Henderson
<richard.henderson@linaro.org> wrote:

On 7/1/22 01:11, Peter Maydell wrote:
+static inline bool isar_feature_any_doublelock(const ARMISARegisters *id)
+{
+    /*
+     * We can't just OR together the aa32 and aa64 checks, because
+     * if there is no AArch64 support the aa64 function will default
+     * to returning true for a zero id_aa64dfr0.
+     * We use "is id_aa64pfr0 non-zero" as a proxy for "do we have
+     * the AArch64 ID register values in id", because it's always the
+     * case that ID_AA64PFR0_EL1.EL0 at least will be non-zero.
+     */
+    if (id->id_aa64pfr0) {
+        return isar_feature_aa64_doublelock(id);
+    }
+    return isar_feature_aa32_doublelock(id);
+}

If you're going to rely on this, you need to clear this register for -cpu 
aarch64=off.
It's probably easier to drop this function...

+static void osdlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                        uint64_t value)
+{
+    /*
+     * Only defined bit is bit 0 (DLK); if Feat_DoubleLock is not
+     * implemented this is RAZ/WI.
+     */
+    if (cpu_isar_feature(any_doublelock, env_archcpu(env))) {

... and use

      if (arm_feature(env, ARM_FEATURE_AARCH64)
          ? cpu_isar_feature(aa64_doublelock, cpu)
          : cpu_isar_feature(aa32_doublelock, cpu)) {

since it's just used once.

If I squash in this change:

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 069dfe1d308..1f4f3e0485c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -4218,22 +4218,6 @@ static inline bool isar_feature_any_ras(const
ARMISARegisters *id)
      return isar_feature_aa64_ras(id) || isar_feature_aa32_ras(id);
  }

-static inline bool isar_feature_any_doublelock(const ARMISARegisters *id)
-{
-    /*
-     * We can't just OR together the aa32 and aa64 checks, because
-     * if there is no AArch64 support the aa64 function will default
-     * to returning true for a zero id_aa64dfr0.
-     * We use "is id_aa64pfr0 non-zero" as a proxy for "do we have
-     * the AArch64 ID register values in id", because it's always the
-     * case that ID_AA64PFR0_EL1.EL0 at least will be non-zero.
-     */
-    if (id->id_aa64pfr0) {
-        return isar_feature_aa64_doublelock(id);
-    }
-    return isar_feature_aa32_doublelock(id);
-}
-
  /*
   * Forward to the above feature tests given an ARMCPU pointer.
   */
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 62bd8f85383..d09fccb0a4f 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -617,11 +617,14 @@ static void oslar_write(CPUARMState *env, const
ARMCPRegInfo *ri,
  static void osdlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
                          uint64_t value)
  {
+    ARMCPU *cpu = env_archcpu(env);
      /*
       * Only defined bit is bit 0 (DLK); if Feat_DoubleLock is not
       * implemented this is RAZ/WI.
       */
-    if (cpu_isar_feature(any_doublelock, env_archcpu(env))) {
+    if(arm_feature(env, ARM_FEATURE_AARCH64)
+       ? cpu_isar_feature(aa64_doublelock, cpu)
+       : cpu_isar_feature(aa32_doublelock, cpu)) {
          env->cp15.osdlr_el1 = value & 1;
      }
  }




can I have a reviewed-by? Would save me doing a respin.

You may.

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

r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]