[PATCH v2 0/5] target/arm: More EL2 trapping fixes

[PATCH v2 0/5] target/arm: More EL2 trapping fixes

[Marc Zyngier]

Hi all,

This series is a follow-up on [1], which tried to address the
remaining missing HCR_EL2.TIDx traps. I've hopefully now adressed the
comments that Peter and Edgar raised.

I've also tried to tackle missing traps generated by HSTR_EL2, which
got completely ignored so far. Note that this results in the use of a
new TB bit, which I understand is a rare resource. I'd welcome
comments on how to handle it differently if at all possible.

Finally, and as a bonus non-feature, I've added support for the
missing Jazelle registers, giving me the opportunity to allow trapping
of JIDR to EL2 using HCR_EL2.TID0. Yay, Christmas! ;-)

I'm now going back to kernel stuff. I swear!

[1] https://patchew.org/QEMU/20191128161718.24361-1-maz@kernel.org/

Marc Zyngier (5):
  target/arm: Honor HCR_EL2.TID2 trapping requirements
  target/arm: Honor HCR_EL2.TID1 trapping requirements
  target/arm: Handle trapping to EL2 of AArch32 VMRS instructions
  target/arm: Handle AArch32 CP15 trapping via HSTR_EL2
  target/arm: Add support for missing Jazelle system registers

 target/arm/cpu.h               |   2 +
 target/arm/helper-a64.h        |   2 +
 target/arm/helper.c            | 100 ++++++++++++++++++++++++++++++---
 target/arm/op_helper.c         |  21 +++++++
 target/arm/translate-vfp.inc.c |  18 +++++-
 target/arm/translate.c         |   3 +-
 target/arm/translate.h         |   2 +
 target/arm/vfp_helper.c        |  29 ++++++++++
 8 files changed, 165 insertions(+), 12 deletions(-)

-- 
2.20.1

[PATCH v2 1/5] target/arm: Honor HCR_EL2.TID2 trapping requirements

[Marc Zyngier]

HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1,
CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU
completely ignores it, making it impossible for hypervisors to
virtualize the cache hierarchy.

Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 target/arm/helper.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0bf8f53d4b..1e546096b8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1910,6 +1910,17 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
     raw_write(env, ri, value);
 }
 
+static CPAccessResult access_aa64_tid2(CPUARMState *env,
+                                       const ARMCPRegInfo *ri,
+                                       bool isread)
+{
+    if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID2)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+
+    return CP_ACCESS_OK;
+}
+
 static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     ARMCPU *cpu = env_archcpu(env);
@@ -2110,10 +2121,14 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
       .writefn = pmintenclr_write },
     { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
-      .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
+      .access = PL1_R,
+      .accessfn = access_aa64_tid2,
+      .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
     { .name = "CSSELR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 2, .opc2 = 0,
-      .access = PL1_RW, .writefn = csselr_write, .resetvalue = 0,
+      .access = PL1_RW,
+      .accessfn = access_aa64_tid2,
+      .writefn = csselr_write, .resetvalue = 0,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.csselr_s),
                              offsetof(CPUARMState, cp15.csselr_ns) } },
     /* Auxiliary ID register: this actually has an IMPDEF value but for now
@@ -5204,6 +5219,11 @@ static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri,
     if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCT)) {
         return CP_ACCESS_TRAP;
     }
+
+    if (arm_current_el(env) < 2 && arm_hcr_el2_eff(env) & HCR_TID2) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+
     return CP_ACCESS_OK;
 }
 
@@ -6184,7 +6204,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         ARMCPRegInfo clidr = {
             .name = "CLIDR", .state = ARM_CP_STATE_BOTH,
             .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,
-            .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->clidr
+            .access = PL1_R, .type = ARM_CP_CONST,
+            .accessfn = access_aa64_tid2,
+            .resetvalue = cpu->clidr
         };
         define_one_arm_cp_reg(cpu, &clidr);
         define_arm_cp_regs(cpu, v7_cp_reginfo);
@@ -6717,7 +6739,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             /* These are common to v8 and pre-v8 */
             { .name = "CTR",
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
+              .access = PL1_R, .accessfn = ctr_el0_access,
+              .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
             { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
               .access = PL0_R, .accessfn = ctr_el0_access,
-- 
2.20.1

[PATCH v2 2/5] target/arm: Honor HCR_EL2.TID1 trapping requirements

[Marc Zyngier]

HCR_EL2.TID1 mandates that access from EL1 to REVIDR_EL1, AIDR_EL1
(and their 32bit equivalents) as well as TCMTR, TLBTR are trapped
to EL2. QEMU ignores it, making it harder for a hypervisor to
virtualize the HW (though to be fair, no known hypervisor actually
cares).

Do the right thing by trapping to EL2 if HCR_EL2.TID1 is set.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1e546096b8..93ecab27c0 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1973,6 +1973,26 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
     return ret;
 }
 
+static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
+                                       bool isread)
+{
+    if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID1)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+
+    return CP_ACCESS_OK;
+}
+
+static CPAccessResult access_aa32_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
+                                       bool isread)
+{
+    if (arm_feature(env, ARM_FEATURE_V8)) {
+        return access_aa64_tid1(env, ri, isread);
+    }
+
+    return CP_ACCESS_OK;
+}
+
 static const ARMCPRegInfo v7_cp_reginfo[] = {
     /* the old v6 WFI, UNPREDICTABLE in v7 but we choose to NOP */
     { .name = "NOP", .cp = 15, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 4,
@@ -2136,7 +2156,9 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
      */
     { .name = "AIDR", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 1, .crn = 0, .crm = 0, .opc2 = 7,
-      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL1_R, .type = ARM_CP_CONST,
+      .accessfn = access_aa64_tid1,
+      .resetvalue = 0 },
     /* Auxiliary fault status registers: these also are IMPDEF, and we
      * choose to RAZ/WI for all cores.
      */
@@ -6732,7 +6754,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .resetvalue = cpu->midr },
             { .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
+              .access = PL1_R,
+              .accessfn = access_aa64_tid1,
+              .type = ARM_CP_CONST, .resetvalue = cpu->revidr },
             REGINFO_SENTINEL
         };
         ARMCPRegInfo id_cp_reginfo[] = {
@@ -6748,14 +6772,18 @@ void register_cp_regs_for_features(ARMCPU *cpu)
             /* TCMTR and TLBTR exist in v8 but have no 64-bit versions */
             { .name = "TCMTR",
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+              .access = PL1_R,
+              .accessfn = access_aa32_tid1,
+              .type = ARM_CP_CONST, .resetvalue = 0 },
             REGINFO_SENTINEL
         };
         /* TLBTR is specific to VMSA */
         ARMCPRegInfo id_tlbtr_reginfo = {
               .name = "TLBTR",
               .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0,
+              .access = PL1_R,
+              .accessfn = access_aa32_tid1,
+              .type = ARM_CP_CONST, .resetvalue = 0,
         };
         /* MPUIR is specific to PMSA V6+ */
         ARMCPRegInfo id_mpuir_reginfo = {
-- 
2.20.1

[PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Marc Zyngier]

HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to
EL2, and HCR_EL2.TID0 does the same for reads of FPSID.
In order to handle this, introduce a new TCG helper function that
checks for these control bits before executing the VMRC instruction.

Tested with a hacked-up version of KVM/arm64 that sets the control
bits for 32bit guests.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 target/arm/helper-a64.h        |  2 ++
 target/arm/translate-vfp.inc.c | 18 +++++++++++++++---
 target/arm/vfp_helper.c        | 29 +++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
index a915c1247f..0af44dc814 100644
--- a/target/arm/helper-a64.h
+++ b/target/arm/helper-a64.h
@@ -102,3 +102,5 @@ DEF_HELPER_FLAGS_3(autda, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
 DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
 DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
+
+DEF_HELPER_3(check_hcr_el2_trap, void, env, i32, i32)
diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index 85c5ef897b..bf90ac0e5b 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -761,13 +761,25 @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS *a)
     if (a->l) {
         /* VMRS, move VFP special register to gp register */
         switch (a->reg) {
+        case ARM_VFP_MVFR0:
+        case ARM_VFP_MVFR1:
+        case ARM_VFP_MVFR2:
         case ARM_VFP_FPSID:
+            if (s->current_el == 1) {
+                TCGv_i32 tcg_reg, tcg_rt;
+
+                gen_set_condexec(s);
+                gen_set_pc_im(s, s->pc_curr);
+                tcg_reg = tcg_const_i32(a->reg);
+                tcg_rt = tcg_const_i32(a->rt);
+                gen_helper_check_hcr_el2_trap(cpu_env, tcg_rt, tcg_reg);
+                tcg_temp_free_i32(tcg_reg);
+                tcg_temp_free_i32(tcg_rt);
+            }
+            /* fall through */
         case ARM_VFP_FPEXC:
         case ARM_VFP_FPINST:
         case ARM_VFP_FPINST2:
-        case ARM_VFP_MVFR0:
-        case ARM_VFP_MVFR1:
-        case ARM_VFP_MVFR2:
             tmp = load_cpu_field(vfp.xregs[a->reg]);
             break;
         case ARM_VFP_FPSCR:
diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c
index 9710ef1c3e..0ae7d4f34a 100644
--- a/target/arm/vfp_helper.c
+++ b/target/arm/vfp_helper.c
@@ -1322,4 +1322,33 @@ float64 HELPER(frint64_d)(float64 f, void *fpst)
     return frint_d(f, fpst, 64);
 }
 
+void HELPER(check_hcr_el2_trap)(CPUARMState *env, uint32_t rt, uint32_t reg)
+{
+    uint32_t syndrome;
+
+    switch (reg) {
+    case ARM_VFP_MVFR0:
+    case ARM_VFP_MVFR1:
+    case ARM_VFP_MVFR2:
+        if (!(arm_hcr_el2_eff(env) & HCR_TID3)) {
+            return;
+        }
+        break;
+    case ARM_VFP_FPSID:
+        if (!(arm_hcr_el2_eff(env) & HCR_TID0)) {
+            return;
+        }
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    syndrome = ((EC_FPIDTRAP << ARM_EL_EC_SHIFT)
+                | ARM_EL_IL
+                | (1 << 24) | (0xe << 20) | (7 << 14)
+                | (reg << 10) | (rt << 5) | 1);
+
+    raise_exception(env, EXCP_HYP_TRAP, syndrome, 2);
+}
+
 #endif
-- 
2.20.1

[PATCH v2 4/5] target/arm: Handle AArch32 CP15 trapping via HSTR_EL2

[Marc Zyngier]

HSTR_EL2 offers a way to trap ranges of CP15 system register
accesses to EL2, and it looks like this register is completely
ignored by QEMU.

To avoid adding extra .accessfn filters all over the place (which
would have a direct performance impact), let's add a new TB flag
that gets set whenever HSTR_EL2 is non-zero and that QEMU translates
a context where this trap has a chance to apply, and only generate
the extra access check if the hypervisor is actively using this feature.

Tested with a hand-crafted KVM guest accessing CBAR.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 target/arm/cpu.h       |  2 ++
 target/arm/helper.c    |  6 ++++++
 target/arm/op_helper.c | 21 +++++++++++++++++++++
 target/arm/translate.c |  3 ++-
 target/arm/translate.h |  2 ++
 5 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 83a809d4ba..cebb3511a5 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3215,6 +3215,8 @@ FIELD(TBFLAG_A32, NS, 6, 1)
 FIELD(TBFLAG_A32, VFPEN, 7, 1)          /* Partially cached, minus FPEXC. */
 FIELD(TBFLAG_A32, CONDEXEC, 8, 8)       /* Not cached. */
 FIELD(TBFLAG_A32, SCTLR_B, 16, 1)
+FIELD(TBFLAG_A32, HSTR_ACTIVE, 17, 1)
+
 /* For M profile only, set if FPCCR.LSPACT is set */
 FIELD(TBFLAG_A32, LSPACT, 18, 1)        /* Not cached. */
 /* For M profile only, set if we must create a new FP context */
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 93ecab27c0..0ba08d550a 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11283,6 +11283,12 @@ static uint32_t rebuild_hflags_a32(CPUARMState *env, int fp_el,
     if (arm_el_is_aa64(env, 1)) {
         flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1);
     }
+
+    if (arm_current_el(env) < 2 && env->cp15.hstr_el2 &&
+        (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) {
+        flags = FIELD_DP32(flags, TBFLAG_A32, HSTR_ACTIVE, 1);
+    }
+
     return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags);
 }
 
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index b529d6c1bf..681c5f3681 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -603,6 +603,26 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome,
         raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env));
     }
 
+    /* Check for an EL2 trap due to HSTR_EL2. We expect EL0 accesses
+     * to sysregs non accessible at EL0 to have UNDEF-ed already.
+     */
+    if (!env->aarch64 && arm_current_el(env) < 2 && ri->cp == 15 &&
+        (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) {
+        uint32_t mask = 1 << ri->crn;
+
+        if (ri->type & ARM_CP_64BIT) {
+            mask = 1 << ri->crm;
+        }
+
+        /* T4 and T14 are RES0 */
+        mask &= ~((1 << 4) | (1 << 14));
+
+        if (env->cp15.hstr_el2 & mask) {
+            target_el = 2;
+            goto exept;
+        }
+    }
+
     if (!ri->accessfn) {
         return;
     }
@@ -652,6 +672,7 @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome,
         g_assert_not_reached();
     }
 
+exept:
     raise_exception(env, EXCP_UDEF, syndrome, target_el);
 }
 
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 4d5d4bd888..f162be8434 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -6897,7 +6897,7 @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn)
             return 1;
         }
 
-        if (ri->accessfn ||
+        if (s->hstr_active || ri->accessfn ||
             (arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) {
             /* Emit code to perform further access permissions checks at
              * runtime; this may result in an exception.
@@ -10843,6 +10843,7 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
                                !arm_el_is_aa64(env, 3);
     dc->thumb = FIELD_EX32(tb_flags, TBFLAG_A32, THUMB);
     dc->sctlr_b = FIELD_EX32(tb_flags, TBFLAG_A32, SCTLR_B);
+    dc->hstr_active = FIELD_EX32(tb_flags, TBFLAG_A32, HSTR_ACTIVE);
     dc->be_data = FIELD_EX32(tb_flags, TBFLAG_ANY, BE_DATA) ? MO_BE : MO_LE;
     condexec = FIELD_EX32(tb_flags, TBFLAG_A32, CONDEXEC);
     dc->condexec_mask = (condexec & 0xf) << 1;
diff --git a/target/arm/translate.h b/target/arm/translate.h
index dd24f91f26..b837b7fcbf 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -77,6 +77,8 @@ typedef struct DisasContext {
     bool pauth_active;
     /* True with v8.5-BTI and SCTLR_ELx.BT* set.  */
     bool bt;
+    /* True if any CP15 access is trapped by HSTR_EL2 */
+    bool hstr_active;
     /*
      * >= 0, a copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI.
      *  < 0, set by the current instruction.
-- 
2.20.1

[PATCH v2 5/5] target/arm: Add support for missing Jazelle system registers

[Marc Zyngier]

QEMU lacks the minimum Jazelle implementation that is required
by the architecture (everything is RAZ or RAZ/WI). Add it
together with the HCR_EL2.TID0 trapping that goes with it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 target/arm/helper.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0ba08d550a..d6fc198a97 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6040,6 +6040,16 @@ static CPAccessResult access_aa32_tid3(CPUARMState *env, const ARMCPRegInfo *ri,
     return CP_ACCESS_OK;
 }
 
+static CPAccessResult access_jazelle(CPUARMState *env, const ARMCPRegInfo *ri,
+                                     bool isread)
+{
+    if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID0)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+
+    return CP_ACCESS_OK;
+}
+
 void register_cp_regs_for_features(ARMCPU *cpu)
 {
     /* Register all the coprocessor registers based on feature bits */
@@ -6057,6 +6067,23 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, not_v8_cp_reginfo);
     }
 
+    if (cpu_isar_feature(jazelle, cpu)) {
+        ARMCPRegInfo jazelle_regs[] = {
+            { .name = "JIDR",
+              .cp = 14, .crn = 0, .crm = 0, .opc1 = 7, .opc2 = 0,
+              .access = PL1_R, .accessfn = access_jazelle,
+              .type = ARM_CP_CONST, .resetvalue = 0 },
+            { .name = "JOSCR",
+              .cp = 14, .crn = 1, .crm = 0, .opc1 = 7, .opc2 = 0,
+              .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+            { .name = "JMCR",
+              .cp = 14, .crn = 2, .crm = 0, .opc1 = 7, .opc2 = 0,
+              .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+            REGINFO_SENTINEL
+        };
+
+        define_arm_cp_regs(cpu, jazelle_regs);
+    }
     if (arm_feature(env, ARM_FEATURE_V6)) {
         /* The ID registers all have impdef reset values */
         ARMCPRegInfo v6_idregs[] = {
-- 
2.20.1

Re: [PATCH v2 1/5] target/arm: Honor HCR_EL2.TID2 trapping requirements

[Edgar E. Iglesias]

On Sun, Dec 01, 2019 at 12:20:14PM +0000, Marc Zyngier wrote:
> HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1,
> CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU
> completely ignores it, making it impossible for hypervisors to
> virtualize the cache hierarchy.
> 
> Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  target/arm/helper.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0bf8f53d4b..1e546096b8 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1910,6 +1910,17 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>      raw_write(env, ri, value);
>  }
>  
> +static CPAccessResult access_aa64_tid2(CPUARMState *env,
> +                                       const ARMCPRegInfo *ri,
> +                                       bool isread)
> +{
> +    if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID2)) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }
> +
> +    return CP_ACCESS_OK;
> +}
> +
>  static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      ARMCPU *cpu = env_archcpu(env);
> @@ -2110,10 +2121,14 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>        .writefn = pmintenclr_write },
>      { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> -      .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
> +      .access = PL1_R,
> +      .accessfn = access_aa64_tid2,
> +      .readfn = ccsidr_read, .type = ARM_CP_NO_RAW },
>      { .name = "CSSELR", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 2, .opc2 = 0,
> -      .access = PL1_RW, .writefn = csselr_write, .resetvalue = 0,
> +      .access = PL1_RW,
> +      .accessfn = access_aa64_tid2,
> +      .writefn = csselr_write, .resetvalue = 0,
>        .bank_fieldoffsets = { offsetof(CPUARMState, cp15.csselr_s),
>                               offsetof(CPUARMState, cp15.csselr_ns) } },
>      /* Auxiliary ID register: this actually has an IMPDEF value but for now
> @@ -5204,6 +5219,11 @@ static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri,
>      if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCT)) {
>          return CP_ACCESS_TRAP;
>      }
> +
> +    if (arm_current_el(env) < 2 && arm_hcr_el2_eff(env) & HCR_TID2) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }
> +
>      return CP_ACCESS_OK;
>  }
>  
> @@ -6184,7 +6204,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>          ARMCPRegInfo clidr = {
>              .name = "CLIDR", .state = ARM_CP_STATE_BOTH,
>              .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1,
> -            .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->clidr
> +            .access = PL1_R, .type = ARM_CP_CONST,
> +            .accessfn = access_aa64_tid2,
> +            .resetvalue = cpu->clidr
>          };
>          define_one_arm_cp_reg(cpu, &clidr);
>          define_arm_cp_regs(cpu, v7_cp_reginfo);
> @@ -6717,7 +6739,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>              /* These are common to v8 and pre-v8 */
>              { .name = "CTR",
>                .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
> -              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
> +              .access = PL1_R, .accessfn = ctr_el0_access,
> +              .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
>              { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
>                .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
>                .access = PL0_R, .accessfn = ctr_el0_access,
> -- 
> 2.20.1
> 
> 

Re: [PATCH v2 5/5] target/arm: Add support for missing Jazelle system registers

[Edgar E. Iglesias]

On Sun, Dec 01, 2019 at 12:20:18PM +0000, Marc Zyngier wrote:
> QEMU lacks the minimum Jazelle implementation that is required
> by the architecture (everything is RAZ or RAZ/WI). Add it
> together with the HCR_EL2.TID0 trapping that goes with it.


Looks good to me:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  target/arm/helper.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0ba08d550a..d6fc198a97 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6040,6 +6040,16 @@ static CPAccessResult access_aa32_tid3(CPUARMState *env, const ARMCPRegInfo *ri,
>      return CP_ACCESS_OK;
>  }
>  
> +static CPAccessResult access_jazelle(CPUARMState *env, const ARMCPRegInfo *ri,
> +                                     bool isread)
> +{
> +    if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID0)) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }
> +
> +    return CP_ACCESS_OK;
> +}
> +
>  void register_cp_regs_for_features(ARMCPU *cpu)
>  {
>      /* Register all the coprocessor registers based on feature bits */
> @@ -6057,6 +6067,23 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>          define_arm_cp_regs(cpu, not_v8_cp_reginfo);
>      }
>  
> +    if (cpu_isar_feature(jazelle, cpu)) {
> +        ARMCPRegInfo jazelle_regs[] = {
> +            { .name = "JIDR",
> +              .cp = 14, .crn = 0, .crm = 0, .opc1 = 7, .opc2 = 0,
> +              .access = PL1_R, .accessfn = access_jazelle,
> +              .type = ARM_CP_CONST, .resetvalue = 0 },
> +            { .name = "JOSCR",
> +              .cp = 14, .crn = 1, .crm = 0, .opc1 = 7, .opc2 = 0,
> +              .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> +            { .name = "JMCR",
> +              .cp = 14, .crn = 2, .crm = 0, .opc1 = 7, .opc2 = 0,
> +              .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> +            REGINFO_SENTINEL
> +        };
> +
> +        define_arm_cp_regs(cpu, jazelle_regs);
> +    }
>      if (arm_feature(env, ARM_FEATURE_V6)) {
>          /* The ID registers all have impdef reset values */
>          ARMCPRegInfo v6_idregs[] = {
> -- 
> 2.20.1
> 
> 

Re: [PATCH v2 1/5] target/arm: Honor HCR_EL2.TID2 trapping requirements

[Richard Henderson]

On 12/1/19 12:20 PM, Marc Zyngier wrote:
> HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1,
> CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU
> completely ignores it, making it impossible for hypervisors to
> virtualize the cache hierarchy.
> 
> Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  target/arm/helper.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)

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


r~

Re: [PATCH v2 2/5] target/arm: Honor HCR_EL2.TID1 trapping requirements

[Richard Henderson]

On 12/1/19 12:20 PM, Marc Zyngier wrote:
> HCR_EL2.TID1 mandates that access from EL1 to REVIDR_EL1, AIDR_EL1
> (and their 32bit equivalents) as well as TCMTR, TLBTR are trapped
> to EL2. QEMU ignores it, making it harder for a hypervisor to
> virtualize the HW (though to be fair, no known hypervisor actually
> cares).
> 
> Do the right thing by trapping to EL2 if HCR_EL2.TID1 is set.
> 
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)

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


r~

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Richard Henderson]

On 12/1/19 12:20 PM, Marc Zyngier wrote:
> HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to
> EL2, and HCR_EL2.TID0 does the same for reads of FPSID.
> In order to handle this, introduce a new TCG helper function that
> checks for these control bits before executing the VMRC instruction.
> 
> Tested with a hacked-up version of KVM/arm64 that sets the control
> bits for 32bit guests.
> 
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  target/arm/helper-a64.h        |  2 ++
>  target/arm/translate-vfp.inc.c | 18 +++++++++++++++---
>  target/arm/vfp_helper.c        | 29 +++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+), 3 deletions(-)

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

Annoying that there's a bug in the manual -- FPSID is listed as group 0 in
plenty of places, except in the pseudo-code for Accessing the FPSID which uses
TID3.


r~

Re: [PATCH v2 4/5] target/arm: Handle AArch32 CP15 trapping via HSTR_EL2

[Richard Henderson]

On 12/1/19 12:20 PM, Marc Zyngier wrote:
> +    /* Check for an EL2 trap due to HSTR_EL2. We expect EL0 accesses
> +     * to sysregs non accessible at EL0 to have UNDEF-ed already.
> +     */

We're enforcing

    /*
     * Multi-line comment
     */

for qemu now; checkpatch should be reporting on that.

> +    if (!env->aarch64 && arm_current_el(env) < 2 && ri->cp == 15 &&
> +        (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) {

Use is_a64(env) not env->aarch64 directly.

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


r~

Re: [PATCH v2 5/5] target/arm: Add support for missing Jazelle system registers

[Richard Henderson]

On 12/1/19 12:20 PM, Marc Zyngier wrote:
> +    if (cpu_isar_feature(jazelle, cpu)) {
> +        ARMCPRegInfo jazelle_regs[] = {

static const.

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


r~

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Marc Zyngier]

On 2019-12-02 15:35, Richard Henderson wrote:
> On 12/1/19 12:20 PM, Marc Zyngier wrote:
>> HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to
>> EL2, and HCR_EL2.TID0 does the same for reads of FPSID.
>> In order to handle this, introduce a new TCG helper function that
>> checks for these control bits before executing the VMRC instruction.
>>
>> Tested with a hacked-up version of KVM/arm64 that sets the control
>> bits for 32bit guests.
>>
>> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  target/arm/helper-a64.h        |  2 ++
>>  target/arm/translate-vfp.inc.c | 18 +++++++++++++++---
>>  target/arm/vfp_helper.c        | 29 +++++++++++++++++++++++++++++
>>  3 files changed, 46 insertions(+), 3 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> Annoying that there's a bug in the manual -- FPSID is listed as group 
> 0 in
> plenty of places, except in the pseudo-code for Accessing the FPSID
> which uses TID3.

Are you sure? I'm looking at DDI0487E_a, and the pseudo-code for
AArch32.CheckAdvSIMDOrFPRegisterTraps has this check:

<quote>
if (tid0 == '1' && reg == '0000')                           // FPSID
   || (tid3 == '1' && reg IN {'0101', '0110', '0111'}) then  // MVFRx
     if ELUsingAArch32(EL2) then
       AArch32.SystemAccessTrap(M32_Hyp, 0x8);    // 
Exception_AdvSIMDFPAccessTrap
     else
       AArch64.AArch32SystemAccessTrap(EL2, 0x8); // 
Exception_AdvSIMDFPAccessTrap
</quote>

which seems to do the right thing. Or have you spotted a discrepancy
somewhere else (which would be oh-so-surprising...)?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Richard Henderson]

On 12/2/19 4:45 PM, Marc Zyngier wrote:
>> Annoying that there's a bug in the manual -- FPSID is listed as group 0 in
>> plenty of places, except in the pseudo-code for Accessing the FPSID
>> which uses TID3.
> 
> Are you sure? I'm looking at DDI0487E_a,
...
> Or have you spotted a discrepancy
> somewhere else (which would be oh-so-surprising...)?

In DDI0487E_a, page G8-6028:

> elsif EL2Enabled() && !ELUsingAArch32(EL2) && HCR_EL2.TID3 == '1' then
>   AArch64.AArch32SystemAccessTrap(EL2, 0x08);
> elsif EL2Enabled() && ELUsingAArch32(EL2) && HCR.TID3 == '1' then
>   AArch32.TakeHypTrapException(0x08);
> else
>   return FPSID;

within the summary documentation for FPSID.


r~

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Marc Zyngier]

On 2019-12-02 16:56, Richard Henderson wrote:
> On 12/2/19 4:45 PM, Marc Zyngier wrote:
>>> Annoying that there's a bug in the manual -- FPSID is listed as 
>>> group 0 in
>>> plenty of places, except in the pseudo-code for Accessing the FPSID
>>> which uses TID3.
>>
>> Are you sure? I'm looking at DDI0487E_a,
> ...
>> Or have you spotted a discrepancy
>> somewhere else (which would be oh-so-surprising...)?
>
> In DDI0487E_a, page G8-6028:
>
>> elsif EL2Enabled() && !ELUsingAArch32(EL2) && HCR_EL2.TID3 == '1' 
>> then
>>   AArch64.AArch32SystemAccessTrap(EL2, 0x08);
>> elsif EL2Enabled() && ELUsingAArch32(EL2) && HCR.TID3 == '1' then
>>   AArch32.TakeHypTrapException(0x08);
>> else
>>   return FPSID;
>
> within the summary documentation for FPSID.

Ah, that was too obvious for me to find ;-). Indeed, this looks totally
bogus. I'll try and poke a few people...

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 5/5] target/arm: Add support for missing Jazelle system registers

[Peter Maydell]

On Mon, 2 Dec 2019 at 15:58, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 12/1/19 12:20 PM, Marc Zyngier wrote:
> > +    if (cpu_isar_feature(jazelle, cpu)) {
> > +        ARMCPRegInfo jazelle_regs[] = {
>
> static const.

If this can be static const we should just declare it
at file scope. The only arrays we put inline in this
function are the ones which need some non-const
fields.

thanks
-- PMM

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Peter Maydell]

On Sun, 1 Dec 2019 at 12:20, Marc Zyngier <maz@kernel.org> wrote:
>
> HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to
> EL2, and HCR_EL2.TID0 does the same for reads of FPSID.
> In order to handle this, introduce a new TCG helper function that
> checks for these control bits before executing the VMRC instruction.
>
> Tested with a hacked-up version of KVM/arm64 that sets the control
> bits for 32bit guests.
>
> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  target/arm/helper-a64.h        |  2 ++
>  target/arm/translate-vfp.inc.c | 18 +++++++++++++++---
>  target/arm/vfp_helper.c        | 29 +++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
> index a915c1247f..0af44dc814 100644
> --- a/target/arm/helper-a64.h
> +++ b/target/arm/helper-a64.h
> @@ -102,3 +102,5 @@ DEF_HELPER_FLAGS_3(autda, TCG_CALL_NO_WG, i64, env, i64, i64)
>  DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
>  DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
> +
> +DEF_HELPER_3(check_hcr_el2_trap, void, env, i32, i32)

This has to be in helper.h, not helper-a64.h, otherwise
the arm-softmmu target won't build. helper-a64.h is for
helper functions which only exist in the aarch64 binary.

thanks
-- PMM

Re: [PATCH v2 0/5] target/arm: More EL2 trapping fixes

[Peter Maydell]

On Sun, 1 Dec 2019 at 12:20, Marc Zyngier <maz@kernel.org> wrote:
>
> Hi all,
>
> This series is a follow-up on [1], which tried to address the
> remaining missing HCR_EL2.TIDx traps. I've hopefully now adressed the
> comments that Peter and Edgar raised.
>
> I've also tried to tackle missing traps generated by HSTR_EL2, which
> got completely ignored so far. Note that this results in the use of a
> new TB bit, which I understand is a rare resource. I'd welcome
> comments on how to handle it differently if at all possible.
>
> Finally, and as a bonus non-feature, I've added support for the
> missing Jazelle registers, giving me the opportunity to allow trapping
> of JIDR to EL2 using HCR_EL2.TID0. Yay, Christmas! ;-)
>
> I'm now going back to kernel stuff. I swear!

To save you from having to roll a v3, I've fixed up the
handful of nits Richard and I found as I applied this
series to target-arm.next.

thanks
-- PMM

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Marc Zyngier]

On 2019-12-06 14:08, Peter Maydell wrote:
> On Sun, 1 Dec 2019 at 12:20, Marc Zyngier <maz@kernel.org> wrote:
>>
>> HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to
>> EL2, and HCR_EL2.TID0 does the same for reads of FPSID.
>> In order to handle this, introduce a new TCG helper function that
>> checks for these control bits before executing the VMRC instruction.
>>
>> Tested with a hacked-up version of KVM/arm64 that sets the control
>> bits for 32bit guests.
>>
>> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  target/arm/helper-a64.h        |  2 ++
>>  target/arm/translate-vfp.inc.c | 18 +++++++++++++++---
>>  target/arm/vfp_helper.c        | 29 +++++++++++++++++++++++++++++
>>  3 files changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h
>> index a915c1247f..0af44dc814 100644
>> --- a/target/arm/helper-a64.h
>> +++ b/target/arm/helper-a64.h
>> @@ -102,3 +102,5 @@ DEF_HELPER_FLAGS_3(autda, TCG_CALL_NO_WG, i64, 
>> env, i64, i64)
>>  DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
>>  DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
>>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
>> +
>> +DEF_HELPER_3(check_hcr_el2_trap, void, env, i32, i32)
>
> This has to be in helper.h, not helper-a64.h, otherwise
> the arm-softmmu target won't build. helper-a64.h is for
> helper functions which only exist in the aarch64 binary.

Ah, fair enough. I guess I should build all targets rather than
limit myself to aarch64...

I'll fix that and repost the series, having hopefully addressed
Richard's comments.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 0/5] target/arm: More EL2 trapping fixes

[Marc Zyngier]

On 2019-12-06 14:13, Peter Maydell wrote:
> On Sun, 1 Dec 2019 at 12:20, Marc Zyngier <maz@kernel.org> wrote:
>>
>> Hi all,
>>
>> This series is a follow-up on [1], which tried to address the
>> remaining missing HCR_EL2.TIDx traps. I've hopefully now adressed 
>> the
>> comments that Peter and Edgar raised.
>>
>> I've also tried to tackle missing traps generated by HSTR_EL2, which
>> got completely ignored so far. Note that this results in the use of 
>> a
>> new TB bit, which I understand is a rare resource. I'd welcome
>> comments on how to handle it differently if at all possible.
>>
>> Finally, and as a bonus non-feature, I've added support for the
>> missing Jazelle registers, giving me the opportunity to allow 
>> trapping
>> of JIDR to EL2 using HCR_EL2.TID0. Yay, Christmas! ;-)
>>
>> I'm now going back to kernel stuff. I swear!
>
> To save you from having to roll a v3, I've fixed up the
> handful of nits Richard and I found as I applied this
> series to target-arm.next.

Ah, brilliant. Thanks a lot Peter.

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 3/5] target/arm: Handle trapping to EL2 of AArch32 VMRS instructions

[Richard Henderson]

On 12/6/19 6:08 AM, Peter Maydell wrote:
>>  DEF_HELPER_FLAGS_3(autdb, TCG_CALL_NO_WG, i64, env, i64, i64)
>>  DEF_HELPER_FLAGS_2(xpaci, TCG_CALL_NO_RWG_SE, i64, env, i64)
>>  DEF_HELPER_FLAGS_2(xpacd, TCG_CALL_NO_RWG_SE, i64, env, i64)
>> +
>> +DEF_HELPER_3(check_hcr_el2_trap, void, env, i32, i32)
> 
> This has to be in helper.h, not helper-a64.h, otherwise
> the arm-softmmu target won't build. helper-a64.h is for
> helper functions which only exist in the aarch64 binary.

Oh, while we're at it,

  DEF_HELPER_FLAGS_3(..., TCG_CALL_NO_WG, ...)

The helper does not modify tcg globals (on successful return).
It does read globals (via the exception path), and of course it has side
effects (the exception).


r~