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

2016-02-06 Thread Edgar E. Iglesias
On Sat, Feb 06, 2016 at 01:48:19PM +, Peter Maydell wrote:
> On 6 February 2016 at 12:17, Edgar E. Iglesias  
> 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



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

2016-02-06 Thread Edgar E. Iglesias
On Wed, Feb 03, 2016 at 01:38:37PM +, 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 


> 
> Signed-off-by: Peter Maydell 
> ---
>  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
> 



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

2016-02-06 Thread Edgar E. Iglesias
On Wed, Feb 03, 2016 at 01:38:37PM +, 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 
> ---
>  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
> 



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

2016-02-06 Thread Peter Maydell
On 6 February 2016 at 12:17, Edgar E. Iglesias  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