diff mbox

[3/7] target-arm: Use access_trap_aa32s_el1() for SCR and MVBAR

Message ID 1454506721-11843-4-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
The registers MVBAR and SCR should have the behaviour of trapping to
EL3 if accessed from Secure EL1, but we were incorrectly implementing
them to UNDEF (which would trap to EL1).  Fix this by using the new
access_trap_aa32s_el1() access function.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

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

> The registers MVBAR and SCR should have the behaviour of trapping to
> EL3 if accessed from Secure EL1, but we were incorrectly implementing
> them to UNDEF (which would trap to EL1).  Fix this by using the new
> access_trap_aa32s_el1() access function.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 8b96b80..d85b04f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3547,7 +3547,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .resetvalue = 0, .writefn = scr_write },
>      { .name = "SCR",  .type = ARM_CP_ALIAS,
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
> -      .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .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,
> @@ -3569,7 +3570,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .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 = PL3_RW, .writefn = vbar_write, .resetvalue = 0,
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .writefn = vbar_write, .resetvalue = 0,
>        .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
>      { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée
Edgar E. Iglesias Feb. 6, 2016, 12:17 p.m. UTC | #2
On Wed, Feb 03, 2016 at 01:38:37PM +0000, Peter Maydell wrote:
> The registers MVBAR and SCR should have the behaviour of trapping to
> EL3 if accessed from Secure EL1, but we were incorrectly implementing
> them to UNDEF (which would trap to EL1).  Fix this by using the new
> access_trap_aa32s_el1() access function.

Hi,

It seems to me like if EL3 is running in AArch32, then we shouldn't
trap accesses from Secure EL1 but I can't find that logic. Am I missing
something?

Cheers,
Edgar


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 8b96b80..d85b04f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3547,7 +3547,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .resetvalue = 0, .writefn = scr_write },
>      { .name = "SCR",  .type = ARM_CP_ALIAS,
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
> -      .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .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,
> @@ -3569,7 +3570,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .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 = PL3_RW, .writefn = vbar_write, .resetvalue = 0,
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .writefn = vbar_write, .resetvalue = 0,
>        .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
>      { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */
> -- 
> 1.9.1
>
Peter Maydell Feb. 6, 2016, 1:48 p.m. UTC | #3
On 6 February 2016 at 12:17, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> It seems to me like if EL3 is running in AArch32, then we shouldn't
> trap accesses from Secure EL1 but I can't find that logic. Am I missing
> something?

If EL3 is running in AArch32 then there is no Secure EL1 -- all
of SVC, IRQ, and the other PL1 modes run at EL3 privilege,
and arm_current_el() will return 3 in this situation.

thanks
-- PMM
Edgar E. Iglesias Feb. 6, 2016, 4:03 p.m. UTC | #4
On Sat, Feb 06, 2016 at 01:48:19PM +0000, Peter Maydell wrote:
> On 6 February 2016 at 12:17, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > It seems to me like if EL3 is running in AArch32, then we shouldn't
> > trap accesses from Secure EL1 but I can't find that logic. Am I missing
> > something?
> 
> If EL3 is running in AArch32 then there is no Secure EL1 -- all
> of SVC, IRQ, and the other PL1 modes run at EL3 privilege,
> and arm_current_el() will return 3 in this situation.

Yep, I keep forgetting that for AArch32...

Cheers,
Edgar
Edgar E. Iglesias Feb. 6, 2016, 4:10 p.m. UTC | #5
On Wed, Feb 03, 2016 at 01:38:37PM +0000, Peter Maydell wrote:
> The registers MVBAR and SCR should have the behaviour of trapping to
> EL3 if accessed from Secure EL1, but we were incorrectly implementing
> them to UNDEF (which would trap to EL1).  Fix this by using the new
> access_trap_aa32s_el1() access function.

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


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 8b96b80..d85b04f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3547,7 +3547,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .resetvalue = 0, .writefn = scr_write },
>      { .name = "SCR",  .type = ARM_CP_ALIAS,
>        .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
> -      .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .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,
> @@ -3569,7 +3570,8 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
>        .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 = PL3_RW, .writefn = vbar_write, .resetvalue = 0,
> +      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> +      .writefn = vbar_write, .resetvalue = 0,
>        .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
>      { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */
> -- 
> 1.9.1
>
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8b96b80..d85b04f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3547,7 +3547,8 @@  static const ARMCPRegInfo el3_cp_reginfo[] = {
       .resetvalue = 0, .writefn = scr_write },
     { .name = "SCR",  .type = ARM_CP_ALIAS,
       .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
-      .access = PL3_RW, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3),
+      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
+      .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,
@@ -3569,7 +3570,8 @@  static const ARMCPRegInfo el3_cp_reginfo[] = {
       .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 = PL3_RW, .writefn = vbar_write, .resetvalue = 0,
+      .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
+      .writefn = vbar_write, .resetvalue = 0,
       .fieldoffset = offsetof(CPUARMState, cp15.mvbar) },
     { .name = "SCTLR_EL3", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_ALIAS, /* reset handled by AArch32 view */