Re: [Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Sergey Fedorov]

On 14.01.2016 21:34, Peter Maydell wrote:
> We already implement almost all the checks for the illegal
> return events from AArch64 state described in the ARM ARM section
> D1.11.2. Add the two missing ones:
>  * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
>  * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1
>
> (We don't implement external debug, so the case of "debug state exit
> from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
> for QEMU.)

Reviewed-by: Sergey Fedorov 

> Signed-off-by: Peter Maydell 
> ---
>  target-arm/op_helper.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 38d46d8..5789ccb 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
>  goto illegal_return;
>  }
>  
> +if (new_el == 2 && arm_is_secure_below_el3(env)) {
> +/* Return to the non-existent secure-EL2 */
> +goto illegal_return;
> +}
> +
> +if (new_el == 1 &&
> +arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)
> +&& !arm_is_secure_below_el3(env)) {
> +goto illegal_return;
> +}
> +
>  if (!return_to_aa64) {
>  env->aarch64 = 0;
>  env->uncached_cpsr = spsr & CPSR_M;

Re: [Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Edgar E. Iglesias]

On Thu, Jan 14, 2016 at 06:34:10PM +, Peter Maydell wrote:
> We already implement almost all the checks for the illegal
> return events from AArch64 state described in the ARM ARM section
> D1.11.2. Add the two missing ones:
>  * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
>  * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1
> 
> (We don't implement external debug, so the case of "debug state exit
> from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
> for QEMU.)
> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/op_helper.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 38d46d8..5789ccb 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
>  goto illegal_return;
>  }
>  
> +if (new_el == 2 && arm_is_secure_below_el3(env)) {
> +/* Return to the non-existent secure-EL2 */
> +goto illegal_return;
> +}
> +
> +if (new_el == 1 &&
> +arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)

I'm not sure you need to check for EL2 as hcr_el2.tge cannot be reached/set 
otherwise.

Anyway:
Reviewed-by: Edgar E. Iglesias 



> +&& !arm_is_secure_below_el3(env)) {
> +goto illegal_return;
> +}
> +
>  if (!return_to_aa64) {
>  env->aarch64 = 0;
>  env->uncached_cpsr = spsr & CPSR_M;
> -- 
> 1.9.1
> 

Re: [Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Peter Maydell]

On 19 January 2016 at 16:53, Edgar E. Iglesias  wrote:
> On Thu, Jan 14, 2016 at 06:34:10PM +, Peter Maydell wrote:
>> We already implement almost all the checks for the illegal
>> return events from AArch64 state described in the ARM ARM section
>> D1.11.2. Add the two missing ones:
>>  * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
>>  * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1
>>
>> (We don't implement external debug, so the case of "debug state exit
>> from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
>> for QEMU.)
>>
>> Signed-off-by: Peter Maydell 
>> ---
>>  target-arm/op_helper.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
>> index 38d46d8..5789ccb 100644
>> --- a/target-arm/op_helper.c
>> +++ b/target-arm/op_helper.c
>> @@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
>>  goto illegal_return;
>>  }
>>
>> +if (new_el == 2 && arm_is_secure_below_el3(env)) {
>> +/* Return to the non-existent secure-EL2 */
>> +goto illegal_return;
>> +}
>> +
>> +if (new_el == 1 &&
>> +arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)
>
> I'm not sure you need to check for EL2 as hcr_el2.tge cannot be reached/set 
> otherwise.

Mmm. I think we have existing code that takes both approaches here
but mostly we're going for the "no explicit EL2 feature check".
I'll take that bit of the condition out (but won't bother resending
the patchset unless there's something else that needs fixing).

> Anyway:
> Reviewed-by: Edgar E. Iglesias 

thanks
-- PMM

[Qemu-devel] [PATCH 7/8] target-arm: Implement remaining illegal return event checks

[Peter Maydell]

We already implement almost all the checks for the illegal
return events from AArch64 state described in the ARM ARM section
D1.11.2. Add the two missing ones:
 * return to EL2 when EL3 is implemented and SCR_EL3.NS is 0
 * return to Non-secure EL1 when EL2 is implemented and HCR_EL2.TGE is 1

(We don't implement external debug, so the case of "debug state exit
from EL0 using AArch64 state to EL0 using AArch32 state" doesn't apply
for QEMU.)

Signed-off-by: Peter Maydell 
---
 target-arm/op_helper.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 38d46d8..5789ccb 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -718,6 +718,17 @@ void HELPER(exception_return)(CPUARMState *env)
 goto illegal_return;
 }
 
+if (new_el == 2 && arm_is_secure_below_el3(env)) {
+/* Return to the non-existent secure-EL2 */
+goto illegal_return;
+}
+
+if (new_el == 1 &&
+arm_feature(env, ARM_FEATURE_EL2) && (env->cp15.hcr_el2 & HCR_TGE)
+&& !arm_is_secure_below_el3(env)) {
+goto illegal_return;
+}
+
 if (!return_to_aa64) {
 env->aarch64 = 0;
 env->uncached_cpsr = spsr & CPSR_M;
-- 
1.9.1