Re: [PATCH 1/2] target/arm: add support for FEAT_DIT, Data Independent Timing

[Richard Henderson]

On 12/10/20 11:13 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, it's implemented as a NOP.

Alternately, or additionally, TCG is outside of the security domain (only
hardware accelerators like KVM are inside), and so we may implement this as a 
NOP.

> 
> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
> ---
>  target/arm/cpu.h           | 20 +++++++++++++-
>  target/arm/helper.c        | 28 +++++++++++++++++++-
>  target/arm/internals.h     |  6 +++++
>  target/arm/translate-a64.c | 14 ++++++++++
>  4 files changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 4c9cbfbd9975..862be662cef7 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -269,6 +269,7 @@ typedef struct CPUARMState {
>      uint32_t NF; /* N is bit 31. All other bits are undefined.  */
>      uint32_t ZF; /* Z set if zero.  */
>      uint32_t QF; /* 0 or 1 */
> +    uint32_t DIT; /* 0 or 1 */

You don't need to add this.  Leave the DIT bit in uncached_cpsr.

> +++ b/target/arm/translate-a64.c
> @@ -1696,6 +1696,20 @@ 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);
> +        }
> +        t1 = tcg_const_i32(s->current_el);
> +        gen_helper_rebuild_hflags_a64(cpu_env, t1);
> +        tcg_temp_free_i32(t1);
> +        break;

You don't need to rebuild hflags, because the implementation of DIT is a nop.
All you need is to record the pstate change.  You may wish to add a comment
here about that, reminding the reader.


r~