Re: [PATCH v3 2/4] target/arm: Add support for FEAT_DIT, Data Independent Timing

From: Rebecca Cran <rebecca@nuviainc.com>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH v3 2/4] target/arm: Add support for FEAT_DIT, Data Independent Timing
Date: Tue, 2 Feb 2021 15:21:44 -0700	[thread overview]
Message-ID: <621ad496-705e-3fc6-0ff9-f37e1836a267@nuviainc.com> (raw)
In-Reply-To: <5ac0bb98-107b-004c-4ef3-b4ee0e15530a@linaro.org>

On 1/27/21 10:06 PM, Richard Henderson wrote:
> On 1/27/21 6:45 PM, Rebecca Cran wrote:
>> 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>
> 
> 
> This misses the convert from AA32 CPSR to AA64 SPSR on exception entry (and
> vice-versa on return).
> 
> In particular: CPSR.DIT (bit 21) -> SPSR_EL1.DIT (bit 24), and merging
> PSTATE.SS into SPSR_EL1.SS (bit 21).

Thanks. I _think_ I'm understanding it better now. Would the following 
work? I don't see where I need to map PSTATE.SS into SPSR_EL1.SS though, 
because isn't that handled automatically since PSTATE maps onto SPSR?

diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c
index a6b162049806..c1ff24d42f32 100644
--- a/target/arm/helper-a64.c
+++ b/target/arm/helper-a64.c
@@ -1003,6 +1003,11 @@ void HELPER(exception_return)(CPUARMState *env, 
uint64_t new_pc)
          if (!arm_singlestep_active(env)) {
              env->pstate &= ~PSTATE_SS;
          }
+
+        if (spsr & PSTATE_DIT) {
+            env->uncached_cpsr |= CPSR_DIT;
+        }
+
          aarch64_sync_64_to_32(env);

          if (spsr & CPSR_T) {
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 54ac1f476940..1ecfd63d8f70 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9426,6 +9426,12 @@ static void take_aarch32_exception(CPUARMState 
*env, int new_mode,
       */
      env->pstate &= ~PSTATE_SS;
      env->spsr = cpsr_read(env);
+
+    if (env->uncached_cpsr & CPSR_DIT) {
+        env->spsr |= PSTATE_DIT;
+        env->spsr &= ~PSTATE_SS;
+    }
+
      /* Clear IT bits.  */
      env->condexec_bits = 0;
      /* Switch to the new mode, and to the correct instruction set.  */
@@ -9905,6 +9911,11 @@ static void arm_cpu_do_interrupt_aarch64(CPUState 
*cs)
          old_mode = cpsr_read(env);
          env->elr_el[new_el] = env->regs[15];

+        if (old_mode & CPSR_DIT) {
+            old_mode |= PSTATE_DIT;
+            old_mode &= ~PSTATE_SS;
+        }
+
          aarch64_sync_32_to_64(env);

          env->condexec_bits = 0;

