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

[PATCH] 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")
Signed-off-by: Oliver Upton <oupton@kernel.org>
---
 target/arm/ptw.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 1470de3010..4a7aeb140c 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1414,9 +1414,13 @@ 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] target/arm: Only evaluate SCR_EL3.PIEN if ARM_FEATURE_EL3 is present

Posted by Richard Henderson 4 weeks, 1 day ago

On 6/26/26 13:36, Oliver Upton 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")
> Signed-off-by: Oliver Upton <oupton@kernel.org>
> ---
>   target/arm/ptw.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 1470de3010..4a7aeb140c 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1414,9 +1414,13 @@ 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

Braces required for the IF.  Otherwise,

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


r~

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

Posted by Oliver Upton 4 weeks, 1 day ago

On Fri, Jun 26, 2026 at 03:55:22PM -0700, Richard Henderson wrote:
> On 6/26/26 13:36, Oliver Upton 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")
> > Signed-off-by: Oliver Upton <oupton@kernel.org>
> > ---
> >   target/arm/ptw.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> > index 1470de3010..4a7aeb140c 100644
> > --- a/target/arm/ptw.c
> > +++ b/target/arm/ptw.c
> > @@ -1414,9 +1414,13 @@ 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
> 
> Braces required for the IF.  Otherwise,

Kernel style, my bad.

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

thanks!

-- 
Best,
Oliver