[v1,11/16] target-arm: Don't take interrupts targeting lower ELs

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h | 7 +++++++
 1 file changed, 7 insertions(+)

On 30 May 2014, at 09:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
> target-arm/cpu.h | 7 +++++++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9eddcc1..66c58bd 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1133,6 +1133,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
> static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> {
>     CPUARMState *env = cs->env_ptr;
> +    unsigned int cur_el = arm_current_pl(env);
> +    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> +
> +    /* Don't take exceptions if they target a lower EL.  */
> +    if (cur_el > target_el) {
> +        return false;
> +    }

Hi Edgar

When making arm_excp_unmasked() reflect tables D1-13, D1-14, D1-15 
and G1-18, G1-19 in ARM ARMv8 this should not be necessary if I am 
not mistaken. Cases in which target_el is lower than cur_el are marked with 
a P (pending) in the table. Or am I missing something interpreting the
tables?

I extended your arm_excp_unmasked() and arm_excp_target_el() to reflect 
the behaviour shown in the tables in ARM ARMv8 and ARM ARMv7. I will 
send them with the TZ patches.

Best,
Fabian

> 
>     switch (excp_idx) {
>     case EXCP_FIQ:
> -- 
> 1.8.3.2
>

On Sun, Jun 08, 2014 at 03:51:24PM +0000, Aggeler  Fabian wrote:
> 
> On 30 May 2014, at 09:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> 
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> > 
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> > target-arm/cpu.h | 7 +++++++
> > 1 file changed, 7 insertions(+)
> > 
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 9eddcc1..66c58bd 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -1133,6 +1133,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
> > static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> > {
> >     CPUARMState *env = cs->env_ptr;
> > +    unsigned int cur_el = arm_current_pl(env);
> > +    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> > +
> > +    /* Don't take exceptions if they target a lower EL.  */
> > +    if (cur_el > target_el) {
> > +        return false;
> > +    }
> 
> Hi Edgar

Hi Fabian,

> 
> When making arm_excp_unmasked() reflect tables D1-13, D1-14, D1-15 
> and G1-18, G1-19 in ARM ARMv8 this should not be necessary if I am 
> not mistaken. Cases in which target_el is lower than cur_el are marked with 
> a P (pending) in the table. Or am I missing something interpreting the
> tables?

This function is called to check if we can take a pending interrupt or
exception with current CPU state. It does not clear pending exceptions.
In this case, if the target_el is lower than the current EL we return false
and leave the exception pending (to be taken later).

> 
> I extended your arm_excp_unmasked() and arm_excp_target_el() to reflect 
> the behaviour shown in the tables in ARM ARMv8 and ARM ARMv7. I will 
> send them with the TZ patches.

Great, thanks.

Cheers,
Edgar


> 
> Best,
> Fabian
> 
> > 
> >     switch (excp_idx) {
> >     case EXCP_FIQ:
> > -- 
> > 1.8.3.2
> > 
>

On 09 Jun 2014, at 01:43, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:

> On Sun, Jun 08, 2014 at 03:51:24PM +0000, Aggeler  Fabian wrote:
>> 
>> On 30 May 2014, at 09:28, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> 
>>> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>>> 
>>> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>>> ---
>>> target-arm/cpu.h | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>> 
>>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> index 9eddcc1..66c58bd 100644
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -1133,6 +1133,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>>> static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
>>> {
>>>    CPUARMState *env = cs->env_ptr;
>>> +    unsigned int cur_el = arm_current_pl(env);
>>> +    unsigned int target_el = arm_excp_target_el(cs, excp_idx);
>>> +
>>> +    /* Don't take exceptions if they target a lower EL.  */
>>> +    if (cur_el > target_el) {
>>> +        return false;
>>> +    }
>> 
>> Hi Edgar
> 
> Hi Fabian,
> 
>> 
>> When making arm_excp_unmasked() reflect tables D1-13, D1-14, D1-15 
>> and G1-18, G1-19 in ARM ARMv8 this should not be necessary if I am 
>> not mistaken. Cases in which target_el is lower than cur_el are marked with 
>> a P (pending) in the table. Or am I missing something interpreting the
>> tables?
> 
> This function is called to check if we can take a pending interrupt or
> exception with current CPU state. It does not clear pending exceptions.
> In this case, if the target_el is lower than the current EL we return false
> and leave the exception pending (to be taken later).

I know, but what I want to say is, doesn’t the table which I mentioned reflect this
by marking it with a P for pending for cases where target_el would be lower than
the current EL it is taken from?

So if we handle these cases in arm_excp_unmasked() we don’t need to call 
arm_excp_target_el() to check.

> 
>> 
>> I extended your arm_excp_unmasked() and arm_excp_target_el() to reflect 
>> the behaviour shown in the tables in ARM ARMv8 and ARM ARMv7. I will 
>> send them with the TZ patches.
> 
> Great, thanks.
> 
> Cheers,
> Edgar
> 
> 
>> 
>> Best,
>> Fabian
>> 
>>> 
>>>    switch (excp_idx) {
>>>    case EXCP_FIQ:
>>> -- 
>>> 1.8.3.2