target/arm: Clean up some corner cases of sysreg traps

[PATCH 08/14] hw/intc/arm_gicv3_cpuif(): Remove redundant tests of is_a64()

Posted by Peter Maydell 2 months ago

In the gicv3_{irq,fiq,irqfiq}_access() functions, in the
arm_current_el(env) == 3 case we do the following test:
    if (!is_a64(env) && !arm_is_el3_or_mon(env)) {
        r = CP_ACCESS_TRAP_EL3;
    }

In this check, the "!is_a64(env)" is redundant, because if
we are at EL3 and in AArch64 then arm_is_el3_or_mon() will
return true and we will skip the if() body anyway.

Remove the unnecessary tests.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/arm_gicv3_cpuif.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 8a715b3510b..7f1d071c198 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -2291,7 +2291,7 @@ static CPAccessResult gicv3_irqfiq_access(CPUARMState *env,
             r = CP_ACCESS_TRAP_EL3;
             break;
         case 3:
-            if (!is_a64(env) && !arm_is_el3_or_mon(env)) {
+            if (!arm_is_el3_or_mon(env)) {
                 r = CP_ACCESS_TRAP_EL3;
             }
             break;
@@ -2353,7 +2353,7 @@ static CPAccessResult gicv3_fiq_access(CPUARMState *env,
             r = CP_ACCESS_TRAP_EL3;
             break;
         case 3:
-            if (!is_a64(env) && !arm_is_el3_or_mon(env)) {
+            if (!arm_is_el3_or_mon(env)) {
                 r = CP_ACCESS_TRAP_EL3;
             }
             break;
@@ -2389,7 +2389,7 @@ static CPAccessResult gicv3_irq_access(CPUARMState *env,
             r = CP_ACCESS_TRAP_EL3;
             break;
         case 3:
-            if (!is_a64(env) && !arm_is_el3_or_mon(env)) {
+            if (!arm_is_el3_or_mon(env)) {
                 r = CP_ACCESS_TRAP_EL3;
             }
             break;
-- 
2.34.1

Re: [PATCH 08/14] hw/intc/arm_gicv3_cpuif(): Remove redundant tests of is_a64()

Posted by Richard Henderson 1 month, 3 weeks ago

On 1/30/25 10:23, Peter Maydell wrote:
> In the gicv3_{irq,fiq,irqfiq}_access() functions, in the
> arm_current_el(env) == 3 case we do the following test:
>      if (!is_a64(env) && !arm_is_el3_or_mon(env)) {
>          r = CP_ACCESS_TRAP_EL3;
>      }
> 
> In this check, the "!is_a64(env)" is redundant, because if
> we are at EL3 and in AArch64 then arm_is_el3_or_mon() will
> return true and we will skip the if() body anyway.
> 
> Remove the unnecessary tests.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/intc/arm_gicv3_cpuif.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 08/14] hw/intc/arm_gicv3_cpuif(): Remove redundant tests of is_a64()

Posted by Alex Bennée 1 month, 4 weeks ago

Peter Maydell <peter.maydell@linaro.org> writes:

> In the gicv3_{irq,fiq,irqfiq}_access() functions, in the
> arm_current_el(env) == 3 case we do the following test:
>     if (!is_a64(env) && !arm_is_el3_or_mon(env)) {
>         r = CP_ACCESS_TRAP_EL3;
>     }
>
> In this check, the "!is_a64(env)" is redundant, because if
> we are at EL3 and in AArch64 then arm_is_el3_or_mon() will
> return true and we will skip the if() body anyway.
>
> Remove the unnecessary tests.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro