| Message ID | 1454506721-11843-4-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
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
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 >
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
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
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 --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 */
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(-)