qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()
@ 2025-05-21 19:02 Oliver Upton
  2025-05-23  0:06 ` Gustavo Romero
  2025-05-24 16:03 ` Richard Henderson
  0 siblings, 2 replies; 4+ messages in thread
From: Oliver Upton @ 2025-05-21 19:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm, Peter Maydell, Oliver Upton

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()
  2025-05-21 19:02 [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost() Oliver Upton
@ 2025-05-23  0:06 ` Gustavo Romero
  2025-05-23  7:11   ` Oliver Upton
  2025-05-24 16:03 ` Richard Henderson
  1 sibling, 1 reply; 4+ messages in thread
From: Gustavo Romero @ 2025-05-23  0:06 UTC (permalink / raw)
  To: Oliver Upton, qemu-devel; +Cc: qemu-arm, Peter Maydell

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()
  2025-05-23  0:06 ` Gustavo Romero
@ 2025-05-23  7:11   ` Oliver Upton
  0 siblings, 0 replies; 4+ messages in thread
From: Oliver Upton @ 2025-05-23  7:11 UTC (permalink / raw)
  To: Gustavo Romero; +Cc: qemu-devel, qemu-arm, Peter Maydell

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost()
  2025-05-21 19:02 [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost() Oliver Upton
  2025-05-23  0:06 ` Gustavo Romero
@ 2025-05-24 16:03 ` Richard Henderson
  1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2025-05-24 16:03 UTC (permalink / raw)
  To: qemu-devel

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~


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-05-24 16:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-21 19:02 [PATCH] target/arm: Ignore SCTLR_EL2.EnSCXT when !ELIsInHost() Oliver Upton
2025-05-23  0:06 ` Gustavo Romero
2025-05-23  7:11   ` Oliver Upton
2025-05-24 16:03 ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).