diff mbox series

[v3,2/7] target/arm: Honor the HCR_EL2.{TVM,TRVM} bits

Message ID 20200218190958.745-3-richard.henderson@linaro.org
State New
Headers show
Series target/arm: Honor more HCR_EL2 traps | expand

Commit Message

Richard Henderson Feb. 18, 2020, 7:09 p.m. UTC 
These bits trap EL1 access to various virtual memory controls.

Buglink: https://bugs.launchpad.net/bugs/1855072
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v2: Include TTBCR.
---
 target/arm/helper.c | 77 ++++++++++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 25 deletions(-)

-- 
2.20.1

Comments

Peter Maydell Feb. 25, 2020, 11:44 a.m. UTC | #1 
On Tue, 18 Feb 2020 at 19:10, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> These bits trap EL1 access to various virtual memory controls.

>

> Buglink: https://bugs.launchpad.net/bugs/1855072

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

> ---

> v2: Include TTBCR.

> ---

>  target/arm/helper.c | 77 ++++++++++++++++++++++++++++++---------------

>  1 file changed, 52 insertions(+), 25 deletions(-)


> +/* Check for traps from EL1 due to HCR_EL2.TVM and HCR_EL2.TRVM.  */

> +static CPAccessResult access_tvm_trvm(CPUARMState *env, const ARMCPRegInfo *ri,

> +                                      bool isread)

> +{

> +    if (arm_current_el(env) == 1) {

> +        uint64_t trap = isread ? HCR_TRVM : HCR_TVM;

> +        if (arm_hcr_el2_eff(env) & trap) {

> +            return CP_ACCESS_TRAP_EL2;

> +        }

> +    }

> +    return CP_ACCESS_OK;

> +}


v7 doesn't have HCR_TRVM -- we should stop the guest being
able to write to that bit if we don't want to do a version
check on the CPU here to see whether to honour it.

> +

>  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)

>  {

>      ARMCPU *cpu = env_archcpu(env);

> @@ -785,7 +798,8 @@ static const ARMCPRegInfo cp_reginfo[] = {

>       */

>      { .name = "CONTEXTIDR_EL1", .state = ARM_CP_STATE_BOTH,

>        .opc0 = 3, .opc1 = 0, .crn = 13, .crm = 0, .opc2 = 1,

> -      .access = PL1_RW, .secure = ARM_CP_SECSTATE_NS,

> +      .access = PL1_RW, .accessfn = access_tvm_trvm,

> +      .secure = ARM_CP_SECSTATE_NS,

>        .fieldoffset = offsetof(CPUARMState, cp15.contextidr_el[1]),

>        .resetvalue = 0, .writefn = contextidr_write, .raw_writefn = raw_write, },

>      { .name = "CONTEXTIDR_S", .state = ARM_CP_STATE_AA32,


We could I guess add the accessfn to CONTEXTIDR_S, which will
do nothing now but would save us forgetting it if we ever
implement emulation of secure EL2... (For the other regs
touched by this patch this happens automatically because they
don't specify a secure-state and so one regdef does both.)

> @@ -877,9 +891,11 @@ static const ARMCPRegInfo not_v7_cp_reginfo[] = {

>        .opc1 = CP_ANY, .opc2 = 3, .access = PL1_W, .writefn = tlbimvaa_write,

>        .type = ARM_CP_NO_RAW },

>      { .name = "PRRR", .cp = 15, .crn = 10, .crm = 2,

> -      .opc1 = 0, .opc2 = 0, .access = PL1_RW, .type = ARM_CP_NOP },

> +      .opc1 = 0, .opc2 = 0, .access = PL1_RW, .accessfn = access_tvm_trvm,

> +      .type = ARM_CP_NOP },

>      { .name = "NMRR", .cp = 15, .crn = 10, .crm = 2,

> -      .opc1 = 0, .opc2 = 1, .access = PL1_RW, .type = ARM_CP_NOP },

> +      .opc1 = 0, .opc2 = 1, .access = PL1_RW, .accessfn = access_tvm_trvm,

> +      .type = ARM_CP_NOP },


Why are we adding an accessfn that checks bits in a v7-and-later
register to regdefs in the "not_v7_cp_reginfo" array? These only
get used for v6 and earlier CPUs...

> @@ -4158,12 +4185,12 @@ static const ARMCPRegInfo lpae_cp_reginfo[] = {

>      /* NOP AMAIR0/1 */

>      { .name = "AMAIR0", .state = ARM_CP_STATE_BOTH,

>        .opc0 = 3, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 0,

> -      .access = PL1_RW, .type = ARM_CP_CONST,

> -      .resetvalue = 0 },

> +      .access = PL1_RW, .accessfn = access_tvm_trvm,

> +      .type = ARM_CP_CONST, .resetvalue = 0 },

>      /* AMAIR1 is mapped to AMAIR_EL1[63:32] */

>      { .name = "AMAIR1", .cp = 15, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 1,

> -      .access = PL1_RW, .type = ARM_CP_CONST,

> -      .resetvalue = 0 },

> +      .access = PL1_RW, .accessfn = access_tvm_trvm,

> +      .type = ARM_CP_CONST, .resetvalue = 0 },

>      { .name = "PAR", .cp = 15, .crm = 7, .opc1 = 0,

>        .access = PL1_RW, .type = ARM_CP_64BIT, .resetvalue = 0,

>        .bank_fieldoffsets = { offsetof(CPUARMState, cp15.par_s),


I think you have missed adding the accessfn to the 64-bit
TTBR0 and TTBR1 regdefs in lpae_cp_reginfo[].

> @@ -4889,7 +4916,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {

>        .type = ARM_CP_NOP, .access = PL1_W },

>      /* MMU Domain access control / MPU write buffer control */

>      { .name = "DACR", .cp = 15, .opc1 = 0, .crn = 3, .crm = 0, .opc2 = 0,

> -      .access = PL1_RW, .resetvalue = 0,

> +      .access = PL1_RW, .accessfn = access_tvm_trvm, .resetvalue = 0,

>        .writefn = dacr_write, .raw_writefn = raw_write,

>        .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.dacr_s),

>                               offsetoflow32(CPUARMState, cp15.dacr_ns) } },


There is also a DACR definition in not_v8_cp_reginfo[] which
I think needs the accessfn as well.

> @@ -7716,7 +7743,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)

>          ARMCPRegInfo sctlr = {

>              .name = "SCTLR", .state = ARM_CP_STATE_BOTH,

>              .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 0,

> -            .access = PL1_RW,

> +            .access = PL1_RW, .accessfn = access_tvm_trvm,

>              .bank_fieldoffsets = { offsetof(CPUARMState, cp15.sctlr_s),

>                                     offsetof(CPUARMState, cp15.sctlr_ns) },

>              .writefn = sctlr_write, .resetvalue = cpu->reset_sctlr,


thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 513f4edbb4..8abbc4e991 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -530,6 +530,19 @@  static CPAccessResult access_tpm(CPUARMState *env, const ARMCPRegInfo *ri,
     return CP_ACCESS_OK;
 }
 
+/* Check for traps from EL1 due to HCR_EL2.TVM and HCR_EL2.TRVM.  */
+static CPAccessResult access_tvm_trvm(CPUARMState *env, const ARMCPRegInfo *ri,
+                                      bool isread)
+{
+    if (arm_current_el(env) == 1) {
+        uint64_t trap = isread ? HCR_TRVM : HCR_TVM;
+        if (arm_hcr_el2_eff(env) & trap) {
+            return CP_ACCESS_TRAP_EL2;
+        }
+    }
+    return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = env_archcpu(env);
@@ -785,7 +798,8 @@  static const ARMCPRegInfo cp_reginfo[] = {
      */
     { .name = "CONTEXTIDR_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 0, .crn = 13, .crm = 0, .opc2 = 1,
-      .access = PL1_RW, .secure = ARM_CP_SECSTATE_NS,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .secure = ARM_CP_SECSTATE_NS,
       .fieldoffset = offsetof(CPUARMState, cp15.contextidr_el[1]),
       .resetvalue = 0, .writefn = contextidr_write, .raw_writefn = raw_write, },
     { .name = "CONTEXTIDR_S", .state = ARM_CP_STATE_AA32,
@@ -877,9 +891,11 @@  static const ARMCPRegInfo not_v7_cp_reginfo[] = {
       .opc1 = CP_ANY, .opc2 = 3, .access = PL1_W, .writefn = tlbimvaa_write,
       .type = ARM_CP_NO_RAW },
     { .name = "PRRR", .cp = 15, .crn = 10, .crm = 2,
-      .opc1 = 0, .opc2 = 0, .access = PL1_RW, .type = ARM_CP_NOP },
+      .opc1 = 0, .opc2 = 0, .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_NOP },
     { .name = "NMRR", .cp = 15, .crn = 10, .crm = 2,
-      .opc1 = 0, .opc2 = 1, .access = PL1_RW, .type = ARM_CP_NOP },
+      .opc1 = 0, .opc2 = 1, .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_NOP },
     REGINFO_SENTINEL
 };
 
@@ -997,7 +1013,7 @@  static const ARMCPRegInfo v6_cp_reginfo[] = {
     { .name = "DMB", .cp = 15, .crn = 7, .crm = 10, .opc1 = 0, .opc2 = 5,
       .access = PL0_W, .type = ARM_CP_NOP },
     { .name = "IFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 2,
-      .access = PL1_RW,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.ifar_s),
                              offsetof(CPUARMState, cp15.ifar_ns) },
       .resetvalue = 0, },
@@ -2209,16 +2225,19 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
      */
     { .name = "AFSR0_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 1, .opc2 = 0,
-      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "AFSR1_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 0, .crn = 5, .crm = 1, .opc2 = 1,
-      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     /* MAIR can just read-as-written because we don't implement caches
      * and so don't need to care about memory attributes.
      */
     { .name = "MAIR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0,
-      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[1]),
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .fieldoffset = offsetof(CPUARMState, cp15.mair_el[1]),
       .resetvalue = 0 },
     { .name = "MAIR_EL3", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 6, .crn = 10, .crm = 2, .opc2 = 0,
@@ -2232,12 +2251,14 @@  static const ARMCPRegInfo v7_cp_reginfo[] = {
       * handled in the field definitions.
       */
     { .name = "MAIR0", .state = ARM_CP_STATE_AA32,
-      .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0, .access = PL1_RW,
+      .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 0,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair0_s),
                              offsetof(CPUARMState, cp15.mair0_ns) },
       .resetfn = arm_cp_reset_ignore },
     { .name = "MAIR1", .state = ARM_CP_STATE_AA32,
-      .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 1, .access = PL1_RW,
+      .cp = 15, .opc1 = 0, .crn = 10, .crm = 2, .opc2 = 1,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.mair1_s),
                              offsetof(CPUARMState, cp15.mair1_ns) },
       .resetfn = arm_cp_reset_ignore },
@@ -3887,20 +3908,21 @@  static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
 
 static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
     { .name = "DFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW, .type = ARM_CP_ALIAS,
+      .access = PL1_RW, .accessfn = access_tvm_trvm, .type = ARM_CP_ALIAS,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.dfsr_s),
                              offsetoflow32(CPUARMState, cp15.dfsr_ns) }, },
     { .name = "IFSR", .cp = 15, .crn = 5, .crm = 0, .opc1 = 0, .opc2 = 1,
-      .access = PL1_RW, .resetvalue = 0,
+      .access = PL1_RW, .accessfn = access_tvm_trvm, .resetvalue = 0,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.ifsr_s),
                              offsetoflow32(CPUARMState, cp15.ifsr_ns) } },
     { .name = "DFAR", .cp = 15, .opc1 = 0, .crn = 6, .crm = 0, .opc2 = 0,
-      .access = PL1_RW, .resetvalue = 0,
+      .access = PL1_RW, .accessfn = access_tvm_trvm, .resetvalue = 0,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.dfar_s),
                              offsetof(CPUARMState, cp15.dfar_ns) } },
     { .name = "FAR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.far_el[1]),
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .fieldoffset = offsetof(CPUARMState, cp15.far_el[1]),
       .resetvalue = 0, },
     REGINFO_SENTINEL
 };
@@ -3908,25 +3930,29 @@  static const ARMCPRegInfo vmsa_pmsa_cp_reginfo[] = {
 static const ARMCPRegInfo vmsa_cp_reginfo[] = {
     { .name = "ESR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .crn = 5, .crm = 2, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
       .fieldoffset = offsetof(CPUARMState, cp15.esr_el[1]), .resetvalue = 0, },
     { .name = "TTBR0_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 0, .opc2 = 0,
-      .access = PL1_RW, .writefn = vmsa_ttbr_write, .resetvalue = 0,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .writefn = vmsa_ttbr_write, .resetvalue = 0,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.ttbr0_s),
                              offsetof(CPUARMState, cp15.ttbr0_ns) } },
     { .name = "TTBR1_EL1", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 0, .crn = 2, .crm = 0, .opc2 = 1,
-      .access = PL1_RW, .writefn = vmsa_ttbr_write, .resetvalue = 0,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .writefn = vmsa_ttbr_write, .resetvalue = 0,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.ttbr1_s),
                              offsetof(CPUARMState, cp15.ttbr1_ns) } },
     { .name = "TCR_EL1", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 2,
-      .access = PL1_RW, .writefn = vmsa_tcr_el12_write,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .writefn = vmsa_tcr_el12_write,
       .resetfn = vmsa_ttbcr_reset, .raw_writefn = raw_write,
       .fieldoffset = offsetof(CPUARMState, cp15.tcr_el[1]) },
     { .name = "TTBCR", .cp = 15, .crn = 2, .crm = 0, .opc1 = 0, .opc2 = 2,
-      .access = PL1_RW, .type = ARM_CP_ALIAS, .writefn = vmsa_ttbcr_write,
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_ALIAS, .writefn = vmsa_ttbcr_write,
       .raw_writefn = vmsa_ttbcr_raw_write,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.tcr_el[3]),
                              offsetoflow32(CPUARMState, cp15.tcr_el[1])} },
@@ -3938,7 +3964,8 @@  static const ARMCPRegInfo vmsa_cp_reginfo[] = {
  */
 static const ARMCPRegInfo ttbcr2_reginfo = {
     .name = "TTBCR2", .cp = 15, .opc1 = 0, .crn = 2, .crm = 0, .opc2 = 3,
-    .access = PL1_RW, .type = ARM_CP_ALIAS,
+    .access = PL1_RW, .accessfn = access_tvm_trvm,
+    .type = ARM_CP_ALIAS,
     .bank_fieldoffsets = { offsetofhigh32(CPUARMState, cp15.tcr_el[3]),
                            offsetofhigh32(CPUARMState, cp15.tcr_el[1]) },
 };
@@ -4158,12 +4185,12 @@  static const ARMCPRegInfo lpae_cp_reginfo[] = {
     /* NOP AMAIR0/1 */
     { .name = "AMAIR0", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW, .type = ARM_CP_CONST,
-      .resetvalue = 0 },
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     /* AMAIR1 is mapped to AMAIR_EL1[63:32] */
     { .name = "AMAIR1", .cp = 15, .crn = 10, .crm = 3, .opc1 = 0, .opc2 = 1,
-      .access = PL1_RW, .type = ARM_CP_CONST,
-      .resetvalue = 0 },
+      .access = PL1_RW, .accessfn = access_tvm_trvm,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "PAR", .cp = 15, .crm = 7, .opc1 = 0,
       .access = PL1_RW, .type = ARM_CP_64BIT, .resetvalue = 0,
       .bank_fieldoffsets = { offsetof(CPUARMState, cp15.par_s),
@@ -4889,7 +4916,7 @@  static const ARMCPRegInfo v8_cp_reginfo[] = {
       .type = ARM_CP_NOP, .access = PL1_W },
     /* MMU Domain access control / MPU write buffer control */
     { .name = "DACR", .cp = 15, .opc1 = 0, .crn = 3, .crm = 0, .opc2 = 0,
-      .access = PL1_RW, .resetvalue = 0,
+      .access = PL1_RW, .accessfn = access_tvm_trvm, .resetvalue = 0,
       .writefn = dacr_write, .raw_writefn = raw_write,
       .bank_fieldoffsets = { offsetoflow32(CPUARMState, cp15.dacr_s),
                              offsetoflow32(CPUARMState, cp15.dacr_ns) } },
@@ -7716,7 +7743,7 @@  void register_cp_regs_for_features(ARMCPU *cpu)
         ARMCPRegInfo sctlr = {
             .name = "SCTLR", .state = ARM_CP_STATE_BOTH,
             .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 0,
-            .access = PL1_RW,
+            .access = PL1_RW, .accessfn = access_tvm_trvm,
             .bank_fieldoffsets = { offsetof(CPUARMState, cp15.sctlr_s),
                                    offsetof(CPUARMState, cp15.sctlr_ns) },
             .writefn = sctlr_write, .resetvalue = cpu->reset_sctlr,