[v2] target/arm: Only evaluate SCR_EL3.PIEN if ARM_FEATURE_EL3 is present

[PATCH v2] target/arm: Only evaluate SCR_EL3.PIEN if ARM_FEATURE_EL3 is present

Posted by Oliver Upton 4 weeks, 1 day ago

Running KVM with (as of writing, out-of-tree) support for FEAT_S2PIE
on -cpu max gets stuck in an infinite loop of stage-2 permission faults
due to the PTW incorrectly using an effective value of 0 for S2PIR_EL2.

Similar to how S1PIE is handled, only use the IMPLEMENTATION SPECIFIC
value of 0 for S2PIR_EL2 if EL3 is implemented and PIEN=0.

Fixes: a811c5dafb ("target/arm: Implement get_S2prot_indirect")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 target/arm/ptw.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 1470de3010..51affba0bf 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1414,9 +1414,14 @@ static int get_S2prot_indirect(CPUARMState *env, GetPhysAddrResult *result,
                   PAGE_READ | PAGE_WRITE },
     };
 
-    uint64_t pir = (env->cp15.scr_el3 & SCR_PIEN ? env->cp15.s2pir_el2 : 0);
-    int s2pi = extract64(pir, pi_index * 4, 4);
+    uint64_t pir = env->cp15.s2pir_el2;
+    int s2pi;
 
+    if (arm_feature(env, ARM_FEATURE_EL3) && !(env->cp15.scr_el3 & SCR_PIEN)) {
+	pir = 0;
+    }
+
+    s2pi = extract64(pir, pi_index * 4, 4);
     result->f.prot = perm_table[s2pi][2];
     return perm_table[s2pi][s1_is_el0];
 }

base-commit: 8f1d3b586f1265023f75ea9c227c35d463321aef
-- 
2.47.3

Re: [PATCH v2] target/arm: Only evaluate SCR_EL3.PIEN if ARM_FEATURE_EL3 is present

Posted by Peter Maydell 3 weeks, 2 days ago

On Sat, 27 Jun 2026 at 00:17, Oliver Upton <oupton@kernel.org> wrote:
>
> Running KVM with (as of writing, out-of-tree) support for FEAT_S2PIE
> on -cpu max gets stuck in an infinite loop of stage-2 permission faults
> due to the PTW incorrectly using an effective value of 0 for S2PIR_EL2.
>
> Similar to how S1PIE is handled, only use the IMPLEMENTATION SPECIFIC
> value of 0 for S2PIR_EL2 if EL3 is implemented and PIEN=0.
>
> Fixes: a811c5dafb ("target/arm: Implement get_S2prot_indirect")
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>  target/arm/ptw.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 1470de3010..51affba0bf 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1414,9 +1414,14 @@ static int get_S2prot_indirect(CPUARMState *env, GetPhysAddrResult *result,
>                    PAGE_READ | PAGE_WRITE },
>      };
>
> -    uint64_t pir = (env->cp15.scr_el3 & SCR_PIEN ? env->cp15.s2pir_el2 : 0);
> -    int s2pi = extract64(pir, pi_index * 4, 4);
> +    uint64_t pir = env->cp15.s2pir_el2;
> +    int s2pi;
>
> +    if (arm_feature(env, ARM_FEATURE_EL3) && !(env->cp15.scr_el3 & SCR_PIEN)) {
> +       pir = 0;

There's a hardcoded tab lurking here. checkpatch.pl will
warn you about those.

> +    }
> +
> +    s2pi = extract64(pir, pi_index * 4, 4);
>      result->f.prot = perm_table[s2pi][2];
>      return perm_table[s2pi][s1_is_el0];
>  }


Applied to target-arm.next, thanks. I added a Cc:stable tag
so we backport it to the stable branches later.

-- PMM