All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/4] target/arm: Add support for FEAT_DIT, Data Independent Timing
@ 2021-02-03  4:58 Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 1/4] " Rebecca Cran
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Rebecca Cran @ 2021-02-03  4:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Rebecca Cran, Richard Henderson

Add support for FEAT_DIT. DIT (Data Independent Timing) is a required
feature for ARMv8.4.

Changes from v3 to v4:

o Fixed AA32 DIT/PSTATE_SS patch following review feedback.

Rebecca Cran (4):
  target/arm: Add support for FEAT_DIT, Data Independent Timing
  target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into
    env->pstate
  target/arm: Set ID_AA64PFR0.DIT and ID_PFR0.DIT to 1 for "max" AA64
    CPU
  target/arm: Set ID_PFR0.DIT to 1 for "max" 32-bit CPU

 target/arm/cpu.c           |  4 ++
 target/arm/cpu.h           | 12 +++++
 target/arm/cpu64.c         |  5 ++
 target/arm/helper-a64.c    | 32 +++++++++++--
 target/arm/helper.c        | 49 +++++++++++++++++---
 target/arm/internals.h     |  6 +++
 target/arm/op_helper.c     |  9 +---
 target/arm/translate-a64.c | 12 +++++
 8 files changed, 110 insertions(+), 19 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4 1/4] target/arm: Add support for FEAT_DIT, Data Independent Timing
  2021-02-03  4:58 [PATCH v4 0/4] target/arm: Add support for FEAT_DIT, Data Independent Timing Rebecca Cran
@ 2021-02-03  4:58 ` Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate Rebecca Cran
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2021-02-03  4:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Rebecca Cran, Richard Henderson

Add support for FEAT_DIT. DIT (Data Independent Timing) is a required
feature for ARMv8.4. Since virtual machine execution is largely
nondeterministic and TCG is outside of the security domain, it's
implemented as a NOP.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h           | 12 +++++++++++
 target/arm/helper.c        | 22 ++++++++++++++++++++
 target/arm/internals.h     |  6 ++++++
 target/arm/translate-a64.c | 12 +++++++++++
 4 files changed, 52 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index d080239863c0..2e5853928474 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1243,6 +1243,7 @@ void pmu_init(ARMCPU *cpu);
 #define CPSR_IT_2_7 (0xfc00U)
 #define CPSR_GE (0xfU << 16)
 #define CPSR_IL (1U << 20)
+#define CPSR_DIT (1U << 21)
 #define CPSR_PAN (1U << 22)
 #define CPSR_J (1U << 24)
 #define CPSR_IT_0_1 (3U << 25)
@@ -1310,6 +1311,7 @@ void pmu_init(ARMCPU *cpu);
 #define PSTATE_SS (1U << 21)
 #define PSTATE_PAN (1U << 22)
 #define PSTATE_UAO (1U << 23)
+#define PSTATE_DIT (1U << 24)
 #define PSTATE_TCO (1U << 25)
 #define PSTATE_V (1U << 28)
 #define PSTATE_C (1U << 29)
@@ -3876,6 +3878,11 @@ static inline bool isar_feature_aa32_tts2uxn(const ARMISARegisters *id)
     return FIELD_EX32(id->id_mmfr4, ID_MMFR4, XNX) != 0;
 }
 
+static inline bool isar_feature_aa32_dit(const ARMISARegisters *id)
+{
+    return FIELD_EX32(id->id_pfr0, ID_PFR0, DIT) != 0;
+}
+
 /*
  * 64-bit feature tests via id registers.
  */
@@ -4120,6 +4127,11 @@ static inline bool isar_feature_aa64_tts2uxn(const ARMISARegisters *id)
     return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, XNX) != 0;
 }
 
+static inline bool isar_feature_aa64_dit(const ARMISARegisters *id)
+{
+    return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, DIT) != 0;
+}
+
 /*
  * Feature tests for "does this exist in either 32-bit or 64-bit?"
  */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 47e266d7e64f..0aad6d79dcb1 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4419,6 +4419,24 @@ static const ARMCPRegInfo uao_reginfo = {
     .readfn = aa64_uao_read, .writefn = aa64_uao_write
 };
 
+static uint64_t aa64_dit_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return env->pstate & PSTATE_DIT;
+}
+
+static void aa64_dit_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                           uint64_t value)
+{
+    env->pstate = (env->pstate & ~PSTATE_DIT) | (value & PSTATE_DIT);
+}
+
+static const ARMCPRegInfo dit_reginfo = {
+    .name = "DIT", .state = ARM_CP_STATE_AA64,
+    .opc0 = 3, .opc1 = 3, .crn = 4, .crm = 2, .opc2 = 5,
+    .type = ARM_CP_NO_RAW, .access = PL0_RW,
+    .readfn = aa64_dit_read, .writefn = aa64_dit_write
+};
+
 static CPAccessResult aa64_cacheop_poc_access(CPUARMState *env,
                                               const ARMCPRegInfo *ri,
                                               bool isread)
@@ -8212,6 +8230,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_one_arm_cp_reg(cpu, &uao_reginfo);
     }
 
+    if (cpu_isar_feature(aa64_dit, cpu)) {
+        define_one_arm_cp_reg(cpu, &dit_reginfo);
+    }
+
     if (arm_feature(env, ARM_FEATURE_EL2) && cpu_isar_feature(aa64_vh, cpu)) {
         define_arm_cp_regs(cpu, vhe_reginfo);
     }
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 853fa88fd616..3d11e42d8e1b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1222,6 +1222,9 @@ static inline uint32_t aarch32_cpsr_valid_mask(uint64_t features,
     if (isar_feature_aa32_pan(id)) {
         valid |= CPSR_PAN;
     }
+    if (isar_feature_aa32_dit(id)) {
+        valid |= CPSR_DIT;
+    }
 
     return valid;
 }
@@ -1240,6 +1243,9 @@ static inline uint32_t aarch64_pstate_valid_mask(const ARMISARegisters *id)
     if (isar_feature_aa64_uao(id)) {
         valid |= PSTATE_UAO;
     }
+    if (isar_feature_aa64_dit(id)) {
+        valid |= PSTATE_DIT;
+    }
     if (isar_feature_aa64_mte(id)) {
         valid |= PSTATE_TCO;
     }
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index ffc060e5d70c..1c4b8d02f3b8 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1700,6 +1700,18 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
         tcg_temp_free_i32(t1);
         break;
 
+    case 0x1a: /* DIT */
+        if (!dc_isar_feature(aa64_dit, s)) {
+            goto do_unallocated;
+        }
+        if (crm & 1) {
+            set_pstate_bits(PSTATE_DIT);
+        } else {
+            clear_pstate_bits(PSTATE_DIT);
+        }
+        /* There's no need to rebuild hflags because DIT is a nop */
+        break;
+
     case 0x1e: /* DAIFSet */
         t1 = tcg_const_i32(crm);
         gen_helper_msr_i_daifset(cpu_env, t1);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate
  2021-02-03  4:58 [PATCH v4 0/4] target/arm: Add support for FEAT_DIT, Data Independent Timing Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 1/4] " Rebecca Cran
@ 2021-02-03  4:58 ` Rebecca Cran
  2021-02-03 17:17   ` Richard Henderson
  2021-02-03 17:19   ` Richard Henderson
  2021-02-03  4:58 ` [PATCH v4 3/4] target/arm: Set ID_AA64PFR0.DIT and ID_PFR0.DIT to 1 for "max" AA64 CPU Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 4/4] target/arm: Set ID_PFR0.DIT to 1 for "max" 32-bit CPU Rebecca Cran
  3 siblings, 2 replies; 10+ messages in thread
From: Rebecca Cran @ 2021-02-03  4:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Rebecca Cran, Richard Henderson

cpsr has been treated as being the same as spsr, but it isn't.
Since PSTATE_SS isn't in cpsr, remove it and move it into env->pstate.

This allows us to add support for CPSR_DIT, adding helper functions
to merge SPSR_ELx to and from CPSR.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
---
 target/arm/helper-a64.c | 32 +++++++++++++++++---
 target/arm/helper.c     | 27 ++++++++++++-----
 target/arm/op_helper.c  |  9 +-----
 3 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index c426c23d2c4e..be5d3f6e75cb 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -945,11 +945,31 @@ static int el_from_spsr(uint32_t spsr)
     }
 }
 
+static void cpsr_write_from_spsr_elx(CPUARMState *env,
+                                     uint32_t val)
+{
+    uint32_t mask;
+
+    /* Save SPSR_ELx.SS into PSTATE. */
+    env->pstate = (env->pstate & ~PSTATE_SS) | (val & PSTATE_SS);
+    val &= ~PSTATE_SS;
+
+    /* Move DIT to the correct location for CPSR */
+    if (val & PSTATE_DIT) {
+        val &= ~PSTATE_DIT;
+        val |= CPSR_DIT;
+    }
+
+    mask = aarch32_cpsr_valid_mask(env->features, \
+        &env_archcpu(env)->isar);
+    cpsr_write(env, val, mask, CPSRWriteRaw);
+}
+
 void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
 {
     int cur_el = arm_current_el(env);
     unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el);
-    uint32_t mask, spsr = env->banked_spsr[spsr_idx];
+    uint32_t spsr = env->banked_spsr[spsr_idx];
     int new_el;
     bool return_to_aa64 = (spsr & PSTATE_nRW) == 0;
 
@@ -998,11 +1018,13 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
          * will sort the register banks out for us, and we've already
          * caught all the bad-mode cases in el_from_spsr().
          */
-        mask = aarch32_cpsr_valid_mask(env->features, &env_archcpu(env)->isar);
-        cpsr_write(env, spsr, mask, CPSRWriteRaw);
+        cpsr_write_from_spsr_elx(env, spsr);
         if (!arm_singlestep_active(env)) {
-            env->uncached_cpsr &= ~PSTATE_SS;
+            env->pstate &= ~PSTATE_SS;
+        } else {
+            env->pstate |= PSTATE_SS;
         }
+
         aarch64_sync_64_to_32(env);
 
         if (spsr & CPSR_T) {
@@ -1022,6 +1044,8 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
         pstate_write(env, spsr);
         if (!arm_singlestep_active(env)) {
             env->pstate &= ~PSTATE_SS;
+        } else {
+            env->pstate |= PSTATE_SS;
         }
         aarch64_restore_sp(env, new_el);
         helper_rebuild_hflags_a64(env, new_el);
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0aad6d79dcb1..a31f37e2a257 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9420,6 +9420,21 @@ void aarch64_sync_64_to_32(CPUARMState *env)
     env->regs[15] = env->pc;
 }
 
+static uint32_t cpsr_read_for_spsr_elx(CPUARMState *env)
+{
+    uint32_t ret = cpsr_read(env);
+
+    /* Move DIT to the correct location for SPSR_ELx */
+    if (ret & CPSR_DIT) {
+        ret &= ~CPSR_DIT;
+        ret |= PSTATE_DIT;
+    }
+    /* Merge PSTATE.SS into SPSR_ELx */
+    ret |= env->pstate & PSTATE_SS;
+
+    return ret;
+}
+
 static void take_aarch32_exception(CPUARMState *env, int new_mode,
                                    uint32_t mask, uint32_t offset,
                                    uint32_t newpc)
@@ -9433,8 +9448,9 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode,
      * For exceptions taken to AArch32 we must clear the SS bit in both
      * PSTATE and in the old-state value we save to SPSR_<mode>, so zero it now.
      */
-    env->uncached_cpsr &= ~PSTATE_SS;
-    env->spsr = cpsr_read(env);
+    env->pstate &= ~PSTATE_SS;
+    env->spsr = cpsr_read_for_spsr_elx(env);
+
     /* Clear IT bits.  */
     env->condexec_bits = 0;
     /* Switch to the new mode, and to the correct instruction set.  */
@@ -9911,7 +9927,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
         aarch64_save_sp(env, arm_current_el(env));
         env->elr_el[new_el] = env->pc;
     } else {
-        old_mode = cpsr_read(env);
+        old_mode = cpsr_read_for_spsr_elx(env);
         env->elr_el[new_el] = env->regs[15];
 
         aarch64_sync_32_to_64(env);
@@ -13201,7 +13217,6 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
                           target_ulong *cs_base, uint32_t *pflags)
 {
     uint32_t flags = env->hflags;
-    uint32_t pstate_for_ss;
 
     *cs_base = 0;
     assert_hflags_rebuild_correctly(env);
@@ -13211,7 +13226,6 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
         if (cpu_isar_feature(aa64_bti, env_archcpu(env))) {
             flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
         }
-        pstate_for_ss = env->pstate;
     } else {
         *pc = env->regs[15];
 
@@ -13259,7 +13273,6 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
 
         flags = FIELD_DP32(flags, TBFLAG_AM32, THUMB, env->thumb);
         flags = FIELD_DP32(flags, TBFLAG_AM32, CONDEXEC, env->condexec_bits);
-        pstate_for_ss = env->uncached_cpsr;
     }
 
     /*
@@ -13272,7 +13285,7 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
      * SS_ACTIVE is set in hflags; PSTATE_SS is computed every TB.
      */
     if (FIELD_EX32(flags, TBFLAG_ANY, SS_ACTIVE) &&
-        (pstate_for_ss & PSTATE_SS)) {
+        (env->pstate & PSTATE_SS)) {
         flags = FIELD_DP32(flags, TBFLAG_ANY, PSTATE_SS, 1);
     }
 
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 5e0f123043b5..65cb37d088f8 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -389,14 +389,7 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome)
 
 uint32_t HELPER(cpsr_read)(CPUARMState *env)
 {
-    /*
-     * We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr.
-     * This is convenient for populating SPSR_ELx, but must be
-     * hidden from aarch32 mode, where it is not visible.
-     *
-     * TODO: ARMv8.4-DIT -- need to move SS somewhere else.
-     */
-    return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS);
+    return cpsr_read(env) & ~CPSR_EXEC;
 }
 
 void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 3/4] target/arm: Set ID_AA64PFR0.DIT and ID_PFR0.DIT to 1 for "max" AA64 CPU
  2021-02-03  4:58 [PATCH v4 0/4] target/arm: Add support for FEAT_DIT, Data Independent Timing Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 1/4] " Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate Rebecca Cran
@ 2021-02-03  4:58 ` Rebecca Cran
  2021-02-03  4:58 ` [PATCH v4 4/4] target/arm: Set ID_PFR0.DIT to 1 for "max" 32-bit CPU Rebecca Cran
  3 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2021-02-03  4:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Rebecca Cran, Richard Henderson

Enable FEAT_DIT for the "max" AARCH64 CPU.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu64.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 5e851028c592..9a5cfd4fc632 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -666,6 +666,7 @@ static void aarch64_max_initfn(Object *obj)
         t = FIELD_DP64(t, ID_AA64PFR0, FP, 1);
         t = FIELD_DP64(t, ID_AA64PFR0, ADVSIMD, 1);
         t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1);
+        t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1);
         cpu->isar.id_aa64pfr0 = t;
 
         t = cpu->isar.id_aa64pfr1;
@@ -715,6 +716,10 @@ static void aarch64_max_initfn(Object *obj)
         u = FIELD_DP32(u, ID_ISAR6, SPECRES, 1);
         cpu->isar.id_isar6 = u;
 
+        u = cpu->isar.id_pfr0;
+        u = FIELD_DP32(u, ID_PFR0, DIT, 1);
+        cpu->isar.id_pfr0 = u;
+
         u = cpu->isar.id_mmfr3;
         u = FIELD_DP32(u, ID_MMFR3, PAN, 2); /* ATS1E1 */
         cpu->isar.id_mmfr3 = u;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v4 4/4] target/arm: Set ID_PFR0.DIT to 1 for "max" 32-bit CPU
  2021-02-03  4:58 [PATCH v4 0/4] target/arm: Add support for FEAT_DIT, Data Independent Timing Rebecca Cran
                   ` (2 preceding siblings ...)
  2021-02-03  4:58 ` [PATCH v4 3/4] target/arm: Set ID_AA64PFR0.DIT and ID_PFR0.DIT to 1 for "max" AA64 CPU Rebecca Cran
@ 2021-02-03  4:58 ` Rebecca Cran
  3 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2021-02-03  4:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Rebecca Cran, Richard Henderson

Enable FEAT_DIT for the "max" 32-bit CPU.

Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 40142ac141e5..c98f44624423 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2197,6 +2197,10 @@ static void arm_max_initfn(Object *obj)
         t = FIELD_DP32(t, ID_MMFR4, CNP, 1); /* TTCNP */
         t = FIELD_DP32(t, ID_MMFR4, XNX, 1); /* TTS2UXN */
         cpu->isar.id_mmfr4 = t;
+
+        t = cpu->isar.id_pfr0;
+        t = FIELD_DP32(t, ID_PFR0, DIT, 1);
+        cpu->isar.id_pfr0 = t;
     }
 #endif
 }
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate
  2021-02-03  4:58 ` [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate Rebecca Cran
@ 2021-02-03 17:17   ` Richard Henderson
  2021-02-03 20:28     ` Rebecca Cran
  2021-02-03 17:19   ` Richard Henderson
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2021-02-03 17:17 UTC (permalink / raw)
  To: Rebecca Cran, qemu-devel; +Cc: Peter Maydell

On 2/2/21 6:58 PM, Rebecca Cran wrote:
>          if (!arm_singlestep_active(env)) {
> -            env->uncached_cpsr &= ~PSTATE_SS;
> +            env->pstate &= ~PSTATE_SS;
> +        } else {
> +            env->pstate |= PSTATE_SS;
>          }

Where did this addition come from?


r~


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate
  2021-02-03  4:58 ` [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate Rebecca Cran
  2021-02-03 17:17   ` Richard Henderson
@ 2021-02-03 17:19   ` Richard Henderson
  2021-02-08  6:50     ` Rebecca Cran
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2021-02-03 17:19 UTC (permalink / raw)
  To: Rebecca Cran, qemu-devel; +Cc: Peter Maydell

On 2/2/21 6:58 PM, Rebecca Cran wrote:
> @@ -9433,8 +9448,9 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode,
>       * For exceptions taken to AArch32 we must clear the SS bit in both
>       * PSTATE and in the old-state value we save to SPSR_<mode>, so zero it now.
>       */
> -    env->uncached_cpsr &= ~PSTATE_SS;
> -    env->spsr = cpsr_read(env);
> +    env->pstate &= ~PSTATE_SS;
> +    env->spsr = cpsr_read_for_spsr_elx(env);
> +

Again, this is the aarch32 exception path, and should not use
cpsr_read_for_spsr_elx.


r~


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate
  2021-02-03 17:17   ` Richard Henderson
@ 2021-02-03 20:28     ` Rebecca Cran
  2021-02-03 20:36       ` Richard Henderson
  0 siblings, 1 reply; 10+ messages in thread
From: Rebecca Cran @ 2021-02-03 20:28 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Peter Maydell

On 2/3/21 10:17 AM, Richard Henderson wrote:
> On 2/2/21 6:58 PM, Rebecca Cran wrote:
>>           if (!arm_singlestep_active(env)) {
>> -            env->uncached_cpsr &= ~PSTATE_SS;
>> +            env->pstate &= ~PSTATE_SS;
>> +        } else {
>> +            env->pstate |= PSTATE_SS;
>>           }
> 
> Where did this addition come from?

I thought this was needed given your comment:

"This is missing the restore of PSTATE_SS for when singlestep *is* active."

-- 
Rebecca Cran


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate
  2021-02-03 20:28     ` Rebecca Cran
@ 2021-02-03 20:36       ` Richard Henderson
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2021-02-03 20:36 UTC (permalink / raw)
  To: Rebecca Cran, qemu-devel; +Cc: Peter Maydell

On 2/3/21 10:28 AM, Rebecca Cran wrote:
> On 2/3/21 10:17 AM, Richard Henderson wrote:
>> On 2/2/21 6:58 PM, Rebecca Cran wrote:
>>>           if (!arm_singlestep_active(env)) {
>>> -            env->uncached_cpsr &= ~PSTATE_SS;
>>> +            env->pstate &= ~PSTATE_SS;
>>> +        } else {
>>> +            env->pstate |= PSTATE_SS;
>>>           }
>>
>> Where did this addition come from?
> 
> I thought this was needed given your comment:
> 
> "This is missing the restore of PSTATE_SS for when singlestep *is* active."

No, that was this:

> +    /* Save SPSR_ELx.SS into PSTATE. */
> +    env->pstate = (env->pstate & ~PSTATE_SS) | (val & PSTATE_SS);
> +    val &= ~PSTATE_SS;

which is a restore, not an unconditional enable as you do above.


r~


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate
  2021-02-03 17:19   ` Richard Henderson
@ 2021-02-08  6:50     ` Rebecca Cran
  0 siblings, 0 replies; 10+ messages in thread
From: Rebecca Cran @ 2021-02-08  6:50 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: Peter Maydell

On 2/3/21 10:19 AM, Richard Henderson wrote:
> On 2/2/21 6:58 PM, Rebecca Cran wrote:
>> @@ -9433,8 +9448,9 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode,
>>        * For exceptions taken to AArch32 we must clear the SS bit in both
>>        * PSTATE and in the old-state value we save to SPSR_<mode>, so zero it now.
>>        */
>> -    env->uncached_cpsr &= ~PSTATE_SS;
>> -    env->spsr = cpsr_read(env);
>> +    env->pstate &= ~PSTATE_SS;
>> +    env->spsr = cpsr_read_for_spsr_elx(env);
>> +
> 
> Again, this is the aarch32 exception path, and should not use
> cpsr_read_for_spsr_elx.

Yeah, sorry I'm not sure why/how that got in.
I'm hoping the v5 series that I'm sending out in a few minutes fixes 
these issues.

-- 
Rebecca Cran


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-02-08 11:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03  4:58 [PATCH v4 0/4] target/arm: Add support for FEAT_DIT, Data Independent Timing Rebecca Cran
2021-02-03  4:58 ` [PATCH v4 1/4] " Rebecca Cran
2021-02-03  4:58 ` [PATCH v4 2/4] target/arm: Support AA32 DIT by moving PSTATE_SS from cpsr into env->pstate Rebecca Cran
2021-02-03 17:17   ` Richard Henderson
2021-02-03 20:28     ` Rebecca Cran
2021-02-03 20:36       ` Richard Henderson
2021-02-03 17:19   ` Richard Henderson
2021-02-08  6:50     ` Rebecca Cran
2021-02-03  4:58 ` [PATCH v4 3/4] target/arm: Set ID_AA64PFR0.DIT and ID_PFR0.DIT to 1 for "max" AA64 CPU Rebecca Cran
2021-02-03  4:58 ` [PATCH v4 4/4] target/arm: Set ID_PFR0.DIT to 1 for "max" 32-bit CPU Rebecca Cran

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.