diff mbox

[2/7] target-arm: Implement MDCR_EL3 and SDCR

Message ID 1454506721-11843-3-git-send-email-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Maydell Feb. 3, 2016, 1:38 p.m. UTC
Implement the MDCR_EL3 register (which is SDCR for AArch32).
For the moment we implement it as reads-as-written.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

Comments

Alex Bennée Feb. 5, 2016, 11:13 a.m. UTC | #1
Peter Maydell <peter.maydell@linaro.org> writes:

> Implement the MDCR_EL3 register (which is SDCR for AArch32).
> For the moment we implement it as reads-as-written.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 52284e9..cf2df50 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -382,6 +382,7 @@ typedef struct CPUARMState {
>          uint64_t mdscr_el1;
>          uint64_t oslsr_el1; /* OS Lock Status */
>          uint64_t mdcr_el2;
> +        uint64_t mdcr_el3;
>          /* If the counter is enabled, this stores the last time the counter
>           * was reset. Otherwise it stores the counter value
>           */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b631b83..8b96b80 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -364,6 +364,23 @@ static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
>      return CP_ACCESS_OK;
>  }
>
> +/* Some secure-only AArch32 registers trap to EL3 if used from
> + * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts).
> + * We assume that the .access field is set to PL1_RW.
> + */
> +static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
> +                                            const ARMCPRegInfo *ri)
> +{

I wonder if we should assert the fact we are in AArch32 here in case the
wrong access function gets added to a AArch64 register?

> +    if (arm_current_el(env) == 3) {
> +        return CP_ACCESS_OK;
> +    }
> +    if (arm_is_secure_below_el3(env)) {
> +        return CP_ACCESS_TRAP_EL3;
> +    }
> +    /* This will be EL1 NS and EL2 NS, which just UNDEF */
> +    return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +
>  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>  {
>      ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -3532,6 +3549,13 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
>        .writefn = scr_write },
> +    { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1,
> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el3) },
> +    { .name = "SDCR", .type = ARM_CP_ALIAS,
> +      .cp = 15, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 1,
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },

Does anything ensure the fields are reset to 0 on a warm reset?

>      { .name = "SDER32_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 1,
>        .access = PL3_RW, .resetvalue = 0,


--
Alex Bennée
Peter Maydell Feb. 5, 2016, 11:28 a.m. UTC | #2
On 5 February 2016 at 11:13, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> Implement the MDCR_EL3 register (which is SDCR for AArch32).
>> For the moment we implement it as reads-as-written.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> +/* Some secure-only AArch32 registers trap to EL3 if used from
>> + * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts).
>> + * We assume that the .access field is set to PL1_RW.
>> + */
>> +static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
>> +                                            const ARMCPRegInfo *ri)
>> +{
>
> I wonder if we should assert the fact we are in AArch32 here in case the
> wrong access function gets added to a AArch64 register?

We mostly don't do that kind of checking in these access
functions. If you add the wrong access function to your regdef
then your register misbehaves anyway :-)

>> +    if (arm_current_el(env) == 3) {
>> +        return CP_ACCESS_OK;
>> +    }
>> +    if (arm_is_secure_below_el3(env)) {
>> +        return CP_ACCESS_TRAP_EL3;
>> +    }
>> +    /* This will be EL1 NS and EL2 NS, which just UNDEF */
>> +    return CP_ACCESS_TRAP_UNCATEGORIZED;
>> +}
>> +
>>  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>>  {
>>      ARMCPU *cpu = arm_env_get_cpu(env);
>> @@ -3532,6 +3549,13 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
>>        .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
>>        .writefn = scr_write },
>> +    { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1,
>> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el3) },
>> +    { .name = "SDCR", .type = ARM_CP_ALIAS,
>> +      .cp = 15, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 1,
>> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
>> +      .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },
>
> Does anything ensure the fields are reset to 0 on a warm reset?

Yes, the cpreg framework resets things. arm_cpu_reset() calls
cp_reg_reset() on every register the CPU knows about. Note that
.resetvalue is 0 as usual for unspecified fields in structure
initializers, but it would be clearer to specifically state it
(ie ".resetvalue = 0") as we do in most cases.

thanks
-- PMM
Edgar E. Iglesias Feb. 6, 2016, 12:04 p.m. UTC | #3
On Wed, Feb 03, 2016 at 01:38:36PM +0000, Peter Maydell wrote:
> Implement the MDCR_EL3 register (which is SDCR for AArch32).
> For the moment we implement it as reads-as-written.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 52284e9..cf2df50 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -382,6 +382,7 @@ typedef struct CPUARMState {
>          uint64_t mdscr_el1;
>          uint64_t oslsr_el1; /* OS Lock Status */
>          uint64_t mdcr_el2;
> +        uint64_t mdcr_el3;


Should we maybe arrayify these even if we waste space?

Anyway:

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



>          /* If the counter is enabled, this stores the last time the counter
>           * was reset. Otherwise it stores the counter value
>           */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b631b83..8b96b80 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -364,6 +364,23 @@ static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
>      return CP_ACCESS_OK;
>  }
>  
> +/* Some secure-only AArch32 registers trap to EL3 if used from
> + * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts).
> + * We assume that the .access field is set to PL1_RW.
> + */
> +static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
> +                                            const ARMCPRegInfo *ri)
> +{
> +    if (arm_current_el(env) == 3) {
> +        return CP_ACCESS_OK;
> +    }
> +    if (arm_is_secure_below_el3(env)) {
> +        return CP_ACCESS_TRAP_EL3;
> +    }
> +    /* This will be EL1 NS and EL2 NS, which just UNDEF */
> +    return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +
>  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>  {
>      ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -3532,6 +3549,13 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
>        .writefn = scr_write },
> +    { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1,
> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el3) },
> +    { .name = "SDCR", .type = ARM_CP_ALIAS,
> +      .cp = 15, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 1,
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },
>      { .name = "SDER32_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 1,
>        .access = PL3_RW, .resetvalue = 0,
> -- 
> 1.9.1
>
Sergey Fedorov Feb. 6, 2016, 6:42 p.m. UTC | #4
On 03.02.2016 16:38, Peter Maydell wrote:
> Implement the MDCR_EL3 register (which is SDCR for AArch32).
> For the moment we implement it as reads-as-written.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 52284e9..cf2df50 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -382,6 +382,7 @@ typedef struct CPUARMState {
>          uint64_t mdscr_el1;
>          uint64_t oslsr_el1; /* OS Lock Status */
>          uint64_t mdcr_el2;
> +        uint64_t mdcr_el3;
>          /* If the counter is enabled, this stores the last time the counter
>           * was reset. Otherwise it stores the counter value
>           */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b631b83..8b96b80 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -364,6 +364,23 @@ static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
>      return CP_ACCESS_OK;
>  }
>  
> +/* Some secure-only AArch32 registers trap to EL3 if used from
> + * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts).
> + * We assume that the .access field is set to PL1_RW.
> + */

Maybe we should also make a note that there is no secure EL1 if EL3 is
using AArch32 as it is done for ats_access()?

Kind regards,
Sergey

> +static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
> +                                            const ARMCPRegInfo *ri)
> +{
> +    if (arm_current_el(env) == 3) {
> +        return CP_ACCESS_OK;
> +    }
> +    if (arm_is_secure_below_el3(env)) {
> +        return CP_ACCESS_TRAP_EL3;
> +    }
> +    /* This will be EL1 NS and EL2 NS, which just UNDEF */
> +    return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +
>  static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>  {
>      ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -3532,6 +3549,13 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
>        .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
>        .writefn = scr_write },
> +    { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1,
> +      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el3) },
> +    { .name = "SDCR", .type = ARM_CP_ALIAS,
> +      .cp = 15, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 1,
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },
>      { .name = "SDER32_EL3", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 1,
>        .access = PL3_RW, .resetvalue = 0,
Peter Maydell Feb. 8, 2016, 1:11 p.m. UTC | #5
On 6 February 2016 at 18:42, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> On 03.02.2016 16:38, Peter Maydell wrote:
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -364,6 +364,23 @@ static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
>>      return CP_ACCESS_OK;
>>  }
>>
>> +/* Some secure-only AArch32 registers trap to EL3 if used from
>> + * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts).
>> + * We assume that the .access field is set to PL1_RW.
>> + */
>
> Maybe we should also make a note that there is no secure EL1 if EL3 is
> using AArch32 as it is done for ats_access()?

I added a line to the comment:
 * Note that an access from Secure EL1 can only happen if EL3 is AArch64.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 52284e9..cf2df50 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -382,6 +382,7 @@  typedef struct CPUARMState {
         uint64_t mdscr_el1;
         uint64_t oslsr_el1; /* OS Lock Status */
         uint64_t mdcr_el2;
+        uint64_t mdcr_el3;
         /* If the counter is enabled, this stores the last time the counter
          * was reset. Otherwise it stores the counter value
          */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index b631b83..8b96b80 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -364,6 +364,23 @@  static CPAccessResult access_el3_aa32ns_aa64any(CPUARMState *env,
     return CP_ACCESS_OK;
 }
 
+/* Some secure-only AArch32 registers trap to EL3 if used from
+ * Secure EL1 (but are just ordinary UNDEF in other non-EL3 contexts).
+ * We assume that the .access field is set to PL1_RW.
+ */
+static CPAccessResult access_trap_aa32s_el1(CPUARMState *env,
+                                            const ARMCPRegInfo *ri)
+{
+    if (arm_current_el(env) == 3) {
+        return CP_ACCESS_OK;
+    }
+    if (arm_is_secure_below_el3(env)) {
+        return CP_ACCESS_TRAP_EL3;
+    }
+    /* This will be EL1 NS and EL2 NS, which just UNDEF */
+    return CP_ACCESS_TRAP_UNCATEGORIZED;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
@@ -3532,6 +3549,13 @@  static const ARMCPRegInfo el3_cp_reginfo[] = {
       .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
       .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
       .writefn = scr_write },
+    { .name = "MDCR_EL3", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 3, .opc2 = 1,
+      .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el3) },
+    { .name = "SDCR", .type = ARM_CP_ALIAS,
+      .cp = 15, .opc1 = 0, .crn = 1, .crm = 3, .opc2 = 1,
+      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },
     { .name = "SDER32_EL3", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 1,
       .access = PL3_RW, .resetvalue = 0,