target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()

[PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()

Posted by Oliver Upton 5 months, 4 weeks ago

Using an EL2 that enables SCXTNUM_ELx for guests while disabling the
feature for the host generates erroneous traps to EL2 when running under
TCG.

Fix the issue by only evaluating SCTLR_EL2.EnSCXT when ELIsInHost().

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 target/arm/helper.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7631210287..83d4236417 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7389,16 +7389,16 @@ static CPAccessResult access_scxtnum(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     uint64_t hcr = arm_hcr_el2_eff(env);
     int el = arm_current_el(env);
+    uint64_t sctlr;
 
-    if (el == 0 && !((hcr & HCR_E2H) && (hcr & HCR_TGE))) {
-        if (env->cp15.sctlr_el[1] & SCTLR_TSCXT) {
-            if (hcr & HCR_TGE) {
-                return CP_ACCESS_TRAP_EL2;
-            }
-            return CP_ACCESS_TRAP_EL1;
+    sctlr = el_is_in_host(env, el) ? env->cp15.sctlr_el[2] :
+            env->cp15.sctlr_el[1];
+
+    if (el == 0 && (sctlr & SCTLR_TSCXT)) {
+        if (hcr & HCR_TGE) {
+            return CP_ACCESS_TRAP_EL2;
         }
-    } else if (el < 2 && (env->cp15.sctlr_el[2] & SCTLR_TSCXT)) {
-        return CP_ACCESS_TRAP_EL2;
+        return CP_ACCESS_TRAP_EL1;
     }
     if (el < 2 && arm_is_el2_enabled(env) && !(hcr & HCR_ENSCXT)) {
         return CP_ACCESS_TRAP_EL2;
-- 
2.39.5

Re: [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()

Posted by Richard Henderson 5 months, 3 weeks ago

On 5/21/25 20:02, Oliver Upton wrote:
> Using an EL2 that enables SCXTNUM_ELx for guests while disabling the
> feature for the host generates erroneous traps to EL2 when running under
> TCG.
> 
> Fix the issue by only evaluating SCTLR_EL2.EnSCXT when ELIsInHost().
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>   target/arm/helper.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 7631210287..83d4236417 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -7389,16 +7389,16 @@ static CPAccessResult access_scxtnum(CPUARMState *env, const ARMCPRegInfo *ri,
>   {
>       uint64_t hcr = arm_hcr_el2_eff(env);
>       int el = arm_current_el(env);
> +    uint64_t sctlr;
>   
> -    if (el == 0 && !((hcr & HCR_E2H) && (hcr & HCR_TGE))) {
> -        if (env->cp15.sctlr_el[1] & SCTLR_TSCXT) {
> -            if (hcr & HCR_TGE) {
> -                return CP_ACCESS_TRAP_EL2;
> -            }
> -            return CP_ACCESS_TRAP_EL1;
> +    sctlr = el_is_in_host(env, el) ? env->cp15.sctlr_el[2] :
> +            env->cp15.sctlr_el[1];

Use arm_sctlr() instead.


r~

Re: [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()

Posted by Gustavo Romero 5 months, 3 weeks ago

Hi Oliver,

Thanks for patch.

On 5/21/25 16:02, Oliver Upton wrote:
> Using an EL2 that enables SCXTNUM_ELx for guests while disabling the
> feature for the host generates erroneous traps to EL2 when running under
> TCG.
> 
> Fix the issue by only evaluating SCTLR_EL2.EnSCXT when ELIsInHost().
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>   target/arm/helper.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 7631210287..83d4236417 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -7389,16 +7389,16 @@ static CPAccessResult access_scxtnum(CPUARMState *env, const ARMCPRegInfo *ri,
>   {
>       uint64_t hcr = arm_hcr_el2_eff(env);
>       int el = arm_current_el(env);
> +    uint64_t sctlr;
>   
> -    if (el == 0 && !((hcr & HCR_E2H) && (hcr & HCR_TGE))) {
> -        if (env->cp15.sctlr_el[1] & SCTLR_TSCXT) {
> -            if (hcr & HCR_TGE) {
> -                return CP_ACCESS_TRAP_EL2;
> -            }
> -            return CP_ACCESS_TRAP_EL1;
> +    sctlr = el_is_in_host(env, el) ? env->cp15.sctlr_el[2] :
> +            env->cp15.sctlr_el[1];
> +
> +    if (el == 0 && (sctlr & SCTLR_TSCXT)) {
> +        if (hcr & HCR_TGE) {
> +            return CP_ACCESS_TRAP_EL2;
>           }
> -    } else if (el < 2 && (env->cp15.sctlr_el[2] & SCTLR_TSCXT)) {
> -        return CP_ACCESS_TRAP_EL2;
> +        return CP_ACCESS_TRAP_EL1;
>       }
>       if (el < 2 && arm_is_el2_enabled(env) && !(hcr & HCR_ENSCXT)) {
>           return CP_ACCESS_TRAP_EL2;

Do you mind providing a bit more of context when these erroneous traps happen?

Do we have an issue in QEMU's gitlab about it? What are the QEMU options for a
VM where this issue can be reproduced and, is there an easy way we can reproduce it?


Thanks,
Gustavo

Re: [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()

Posted by Oliver Upton 5 months, 3 weeks ago

Hi Gustavo,

On Thu, May 22, 2025 at 09:06:06PM -0300, Gustavo Romero wrote:
> Hi Oliver,
> 
> Thanks for patch.
> 
> On 5/21/25 16:02, Oliver Upton wrote:
> > Using an EL2 that enables SCXTNUM_ELx for guests while disabling the
> > feature for the host generates erroneous traps to EL2 when running under
> > TCG.
> > 
> > Fix the issue by only evaluating SCTLR_EL2.EnSCXT when ELIsInHost().
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

%s/EnSCXT/TSCXT/

My bad.

> > ---
> >   target/arm/helper.c | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index 7631210287..83d4236417 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -7389,16 +7389,16 @@ static CPAccessResult access_scxtnum(CPUARMState *env, const ARMCPRegInfo *ri,
> >   {
> >       uint64_t hcr = arm_hcr_el2_eff(env);
> >       int el = arm_current_el(env);
> > +    uint64_t sctlr;
> > -    if (el == 0 && !((hcr & HCR_E2H) && (hcr & HCR_TGE))) {
> > -        if (env->cp15.sctlr_el[1] & SCTLR_TSCXT) {
> > -            if (hcr & HCR_TGE) {
> > -                return CP_ACCESS_TRAP_EL2;
> > -            }
> > -            return CP_ACCESS_TRAP_EL1;
> > +    sctlr = el_is_in_host(env, el) ? env->cp15.sctlr_el[2] :
> > +            env->cp15.sctlr_el[1];
> > +
> > +    if (el == 0 && (sctlr & SCTLR_TSCXT)) {
> > +        if (hcr & HCR_TGE) {
> > +            return CP_ACCESS_TRAP_EL2;
> >           }
> > -    } else if (el < 2 && (env->cp15.sctlr_el[2] & SCTLR_TSCXT)) {
> > -        return CP_ACCESS_TRAP_EL2;
> > +        return CP_ACCESS_TRAP_EL1;
> >       }
> >       if (el < 2 && arm_is_el2_enabled(env) && !(hcr & HCR_ENSCXT)) {
> >           return CP_ACCESS_TRAP_EL2;
> 
> Do you mind providing a bit more of context when these erroneous traps happen?

Sure. I was looking at updating our CSV2 limit in KVM [*] and needed to
implement SCXTNUM_ELx as part of that. Accessing SCXTNUM_ELx from a KVM
guest under TCG leads to an unexpected trap taken to EL2 in spite of
the fact that HCR_EL2.EnSCXT=1.

The host kernel still has SCTLR_EL2.TSCXT=1 which appears to be the
source of the trap.

> Do we have an issue in QEMU's gitlab about it? What are the QEMU options for a
> VM where this issue can be reproduced and, is there an easy way we can reproduce it?

You could try reproducing with the linked KVM patches but it is worth
noting the current trap routing is rather obviously wrong when compared
to the pseudocode in the ARM ARM. More generally, using the host's
SCTLR to compute traps while in a guest EL is unlikely to ever be right.

[*]: https://git.kernel.org/pub/scm/linux/kernel/git/oupton/linux.git/log/?h=kvm-arm64/csv2_3

Thanks,
Oliver