Re: [Qemu-devel] [PATCH 6/7] target-arm: Implement NSACR trapping behaviour

2016-02-06 Thread Edgar E. Iglesias
On Wed, Feb 03, 2016 at 01:38:40PM +, Peter Maydell wrote:
> Implement some corner cases of the behaviour of the NSACR
> register on ARMv8:
>  * if EL3 is AArch64 then accessing the NSACR from Secure EL1
>with AArch32 should trap to EL3
>  * if EL3 is not present or is AArch64 then reads from NS EL1 and
>NS EL2 return constant 0xc00
> 
> It would in theory be possible to implement all these with
> a single reginfo definition, but for clarity we use three
> separate definitions for the three cases and install the
> right one based on the CPU feature flags.
> 
> Signed-off-by: Peter Maydell 

AFAICT it looks OK

Reviewed-by: Edgar E. Iglesias 


> ---
>  target-arm/helper.c | 62 
> +
>  1 file changed, 58 insertions(+), 4 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 8bc3ea3..34dc144 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3562,6 +3562,25 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>  REGINFO_SENTINEL
>  };
>  
> +static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri,
> +   bool isread)
> +{
> +/* The NSACR is RW at EL3, and RO for NS EL1 and NS EL2.
> + * At Secure EL1 it traps to EL3.
> + */
> +if (arm_current_el(env) == 3) {
> +return CP_ACCESS_OK;
> +}
> +if (arm_is_secure_below_el3(env)) {
> +return CP_ACCESS_TRAP_EL3;
> +}
> +/* Accesses from EL1 NS and EL2 NS are UNDEF for write but allow reads. 
> */
> +if (isread) {
> +return CP_ACCESS_OK;
> +}
> +return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +
>  static const ARMCPRegInfo el3_cp_reginfo[] = {
>  { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
>.opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> @@ -3587,10 +3606,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 1,
>.access = PL3_RW, .resetvalue = 0,
>.fieldoffset = offsetoflow32(CPUARMState, cp15.sder) },
> -  /* TODO: Implement NSACR trapping of secure EL1 accesses to EL3 */
> -{ .name = "NSACR", .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
> -  .access = PL3_W | PL1_R, .resetvalue = 0,
> -  .fieldoffset = offsetof(CPUARMState, cp15.nsacr) },
>  { .name = "MVBAR", .cp = 15, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
>.access = PL1_RW, .accessfn = access_trap_aa32s_el1,
>.writefn = vbar_write, .resetvalue = 0,
> @@ -4361,6 +4376,45 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>  };
>  define_one_arm_cp_reg(cpu, );
>  }
> +/* The behaviour of NSACR is sufficiently various that we don't
> + * try to describe it in a single reginfo:
> + *  if EL3 is 64 bit, then trap to EL3 from S EL1,
> + * reads as constant 0xc00 from NS EL1 and NS EL2
> + *  if EL3 is 32 bit, then RW at EL3, RO at NS EL1 and NS EL2
> + *  if v7 without EL3, register doesn't exist
> + *  if v8 without EL3, reads as constant 0xc00 from NS EL1 and NS EL2
> + */
> +if (arm_feature(env, ARM_FEATURE_EL3)) {
> +if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> +ARMCPRegInfo nsacr = {
> +.name = "NSACR", .type = ARM_CP_CONST,
> +.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
> +.access = PL1_RW, .accessfn = nsacr_access,
> +.resetvalue = 0xc00
> +};
> +define_one_arm_cp_reg(cpu, );
> +} else {
> +ARMCPRegInfo nsacr = {
> +.name = "NSACR",
> +.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
> +.access = PL3_RW | PL1_R,
> +.resetvalue = 0,
> +.fieldoffset = offsetof(CPUARMState, cp15.nsacr)
> +};
> +define_one_arm_cp_reg(cpu, );
> +}
> +} else {
> +if (arm_feature(env, ARM_FEATURE_V8)) {
> +ARMCPRegInfo nsacr = {
> +.name = "NSACR", .type = ARM_CP_CONST,
> +.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
> +.access = PL1_R,
> +.resetvalue = 0xc00
> +};
> +define_one_arm_cp_reg(cpu, );
> +}
> +}
> +
>  if (arm_feature(env, ARM_FEATURE_MPU)) {
>  if (arm_feature(env, ARM_FEATURE_V6)) {
>  /* PMSAv6 not implemented */
> -- 
> 1.9.1
> 



[Qemu-devel] [PATCH 6/7] target-arm: Implement NSACR trapping behaviour

2016-02-03 Thread Peter Maydell
Implement some corner cases of the behaviour of the NSACR
register on ARMv8:
 * if EL3 is AArch64 then accessing the NSACR from Secure EL1
   with AArch32 should trap to EL3
 * if EL3 is not present or is AArch64 then reads from NS EL1 and
   NS EL2 return constant 0xc00

It would in theory be possible to implement all these with
a single reginfo definition, but for clarity we use three
separate definitions for the three cases and install the
right one based on the CPU feature flags.

Signed-off-by: Peter Maydell 
---
 target-arm/helper.c | 62 +
 1 file changed, 58 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8bc3ea3..34dc144 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3562,6 +3562,25 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
 REGINFO_SENTINEL
 };
 
+static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri,
+   bool isread)
+{
+/* The NSACR is RW at EL3, and RO for NS EL1 and NS EL2.
+ * At Secure EL1 it traps to EL3.
+ */
+if (arm_current_el(env) == 3) {
+return CP_ACCESS_OK;
+}
+if (arm_is_secure_below_el3(env)) {
+return CP_ACCESS_TRAP_EL3;
+}
+/* Accesses from EL1 NS and EL2 NS are UNDEF for write but allow reads. */
+if (isread) {
+return CP_ACCESS_OK;
+}
+return CP_ACCESS_TRAP_UNCATEGORIZED;
+}
+
 static const ARMCPRegInfo el3_cp_reginfo[] = {
 { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
@@ -3587,10 +3606,6 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
   .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 1,
   .access = PL3_RW, .resetvalue = 0,
   .fieldoffset = offsetoflow32(CPUARMState, cp15.sder) },
-  /* TODO: Implement NSACR trapping of secure EL1 accesses to EL3 */
-{ .name = "NSACR", .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
-  .access = PL3_W | PL1_R, .resetvalue = 0,
-  .fieldoffset = offsetof(CPUARMState, cp15.nsacr) },
 { .name = "MVBAR", .cp = 15, .opc1 = 0, .crn = 12, .crm = 0, .opc2 = 1,
   .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
   .writefn = vbar_write, .resetvalue = 0,
@@ -4361,6 +4376,45 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 };
 define_one_arm_cp_reg(cpu, );
 }
+/* The behaviour of NSACR is sufficiently various that we don't
+ * try to describe it in a single reginfo:
+ *  if EL3 is 64 bit, then trap to EL3 from S EL1,
+ * reads as constant 0xc00 from NS EL1 and NS EL2
+ *  if EL3 is 32 bit, then RW at EL3, RO at NS EL1 and NS EL2
+ *  if v7 without EL3, register doesn't exist
+ *  if v8 without EL3, reads as constant 0xc00 from NS EL1 and NS EL2
+ */
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+ARMCPRegInfo nsacr = {
+.name = "NSACR", .type = ARM_CP_CONST,
+.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
+.access = PL1_RW, .accessfn = nsacr_access,
+.resetvalue = 0xc00
+};
+define_one_arm_cp_reg(cpu, );
+} else {
+ARMCPRegInfo nsacr = {
+.name = "NSACR",
+.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
+.access = PL3_RW | PL1_R,
+.resetvalue = 0,
+.fieldoffset = offsetof(CPUARMState, cp15.nsacr)
+};
+define_one_arm_cp_reg(cpu, );
+}
+} else {
+if (arm_feature(env, ARM_FEATURE_V8)) {
+ARMCPRegInfo nsacr = {
+.name = "NSACR", .type = ARM_CP_CONST,
+.cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 2,
+.access = PL1_R,
+.resetvalue = 0xc00
+};
+define_one_arm_cp_reg(cpu, );
+}
+}
+
 if (arm_feature(env, ARM_FEATURE_MPU)) {
 if (arm_feature(env, ARM_FEATURE_V6)) {
 /* PMSAv6 not implemented */
-- 
1.9.1