[PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return

Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
@ 2026-06-17 14:49 Fuad Tabba
  2026-06-17 16:45 ` Marc Zyngier
  2026-06-17 17:36 ` Marc Zyngier
  0 siblings, 2 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-06-17 14:49 UTC (permalink / raw)
  To: Marc Zyngier, Oliver Upton, kvmarm, linux-arm-kernel
  Cc: Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, Jintack Lim, Ganapatrao Kulkarni,
	Christoffer Dall, linux-kernel, tabba

kvm_check_illegal_exception_return() sourced the flags {N,Z,C,V} and
masks {D,A,I,F} of the resulting PSTATE from the current PSTATE, but
R_VWJHB takes them from the SPSR being returned to and leaves
PSTATE.{EL,SP,nRW} (and EXLOCK when FEAT_GCS) unchanged. PAN, ALLINT
and PM were not applied at all.

Build the PSTATE by taking those fields from the SPSR while preserving
EL, SP, nRW and EXLOCK from the current PSTATE, then set IL.

Fixes: 47f3a2fc765a ("KVM: arm64: nv: Support virtual EL2 exceptions")
Suggested-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
Signed-off-by: Fuad Tabba <tabba@google.com>
---
This is a modified version of Marc's suggested diff [1]. That diff applied
a single mask to the incoming SPSR, which also takes PSTATE.{EL,SP,nRW}
(and EXLOCK) from the SPSR. The ARM ARM leaves those fields unchanged on an
illegal exception return. This path is reached precisely because SPSR.M is
illegal (EL3, M[1]=1, AArch32, EL1 under TGE), so this version preserves
EL/SP/nRW/EXLOCK from the current PSTATE and takes only the flags, masks
and PAN/ALLINT/PM from the SPSR.

[1] https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
---
 arch/arm64/kvm/emulate-nested.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index dba7ced74ca5..ace2b40cf875 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2738,17 +2738,30 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
 	    (spsr & PSR_MODE32_BIT) ||
 	    (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
 					   mode == PSR_MODE_EL1h))) {
-		/*
-		 * The guest is playing with our nerves. Preserve EL, SP,
-		 * masks, flags from the existing PSTATE, and set IL.
-		 * The HW will then generate an Illegal State Exception
-		 * immediately after ERET.
-		 */
-		spsr = *vcpu_cpsr(vcpu);
+		u64 cpsr = *vcpu_cpsr(vcpu);
+		u64 mask;
 
-		spsr &= (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
-			 PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT |
-			 PSR_MODE_MASK | PSR_MODE32_BIT);
+		/*
+		 * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
+		 * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
+		 * and masks are taken from the SPSR (R_VWJHB). Set IL so the
+		 * HW generates an Illegal State Exception right after ERET.
+		 */
+		mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
+		       PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
+
+		if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, PAN, IMP))
+			mask |= PSR_PAN_BIT;
+		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, NMI, IMP))
+			mask |= ALLINT_ALLINT;
+		/* FEAT_SPE_EXC and FEAT_TRBE_EXC also gate PSTATE.PM one day... */
+		if (kvm_has_feat(vcpu->kvm, ID_AA64DFR1_EL1, EBEP, IMP))
+			mask |= BIT_ULL(32);	/* PSTATE.PM */
+
+		spsr &= mask;
+		spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
+		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
+			spsr |= cpsr & BIT_ULL(34);	/* PSTATE.EXLOCK */
 		spsr |= PSR_IL_BIT;
 	}
 
-- 
2.54.0.1136.gdb2ca164c4-goog



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

* Re: [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
  2026-06-17 14:49 [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return Fuad Tabba
@ 2026-06-17 16:45 ` Marc Zyngier
  2026-06-17 17:19   ` Fuad Tabba
  2026-06-17 17:36 ` Marc Zyngier
  1 sibling, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2026-06-17 16:45 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Oliver Upton, kvmarm, linux-arm-kernel, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Jintack Lim, Ganapatrao Kulkarni, Christoffer Dall, linux-kernel

On Wed, 17 Jun 2026 15:49:07 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> kvm_check_illegal_exception_return() sourced the flags {N,Z,C,V} and
> masks {D,A,I,F} of the resulting PSTATE from the current PSTATE, but
> R_VWJHB takes them from the SPSR being returned to and leaves
> PSTATE.{EL,SP,nRW} (and EXLOCK when FEAT_GCS) unchanged. PAN, ALLINT
> and PM were not applied at all.
> 
> Build the PSTATE by taking those fields from the SPSR while preserving
> EL, SP, nRW and EXLOCK from the current PSTATE, then set IL.
> 
> Fixes: 47f3a2fc765a ("KVM: arm64: nv: Support virtual EL2 exceptions")
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> This is a modified version of Marc's suggested diff [1]. That diff applied
> a single mask to the incoming SPSR, which also takes PSTATE.{EL,SP,nRW}
> (and EXLOCK) from the SPSR. The ARM ARM leaves those fields unchanged on an
> illegal exception return. This path is reached precisely because SPSR.M is
> illegal (EL3, M[1]=1, AArch32, EL1 under TGE), so this version preserves
> EL/SP/nRW/EXLOCK from the current PSTATE and takes only the flags, masks
> and PAN/ALLINT/PM from the SPSR.
> 
> [1] https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> ---
>  arch/arm64/kvm/emulate-nested.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index dba7ced74ca5..ace2b40cf875 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2738,17 +2738,30 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
>  	    (spsr & PSR_MODE32_BIT) ||
>  	    (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
>  					   mode == PSR_MODE_EL1h))) {
> -		/*
> -		 * The guest is playing with our nerves. Preserve EL, SP,
> -		 * masks, flags from the existing PSTATE, and set IL.
> -		 * The HW will then generate an Illegal State Exception
> -		 * immediately after ERET.
> -		 */
> -		spsr = *vcpu_cpsr(vcpu);
> +		u64 cpsr = *vcpu_cpsr(vcpu);
> +		u64 mask;
>  
> -		spsr &= (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> -			 PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT |
> -			 PSR_MODE_MASK | PSR_MODE32_BIT);
> +		/*
> +		 * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
> +		 * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
> +		 * and masks are taken from the SPSR (R_VWJHB). Set IL so the
> +		 * HW generates an Illegal State Exception right after ERET.
> +		 */
> +		mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> +		       PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
> +
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, PAN, IMP))
> +			mask |= PSR_PAN_BIT;
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, NMI, IMP))
> +			mask |= ALLINT_ALLINT;
> +		/* FEAT_SPE_EXC and FEAT_TRBE_EXC also gate PSTATE.PM one day... */
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64DFR1_EL1, EBEP, IMP))
> +			mask |= BIT_ULL(32);	/* PSTATE.PM */
> +
> +		spsr &= mask;
> +		spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> +			spsr |= cpsr & BIT_ULL(34);	/* PSTATE.EXLOCK */
>  		spsr |= PSR_IL_BIT;
>  	}

While I'm happy that you caught the bugs I left for you to address,
the overall structure is a bit inconsistent. I'd like to have:

- a mask of the bits we preserve from SPSR, and apply that to SPSR
  itself

- a mask of the bits we preserve from PSTATE, and transfer them to
  SPSR

- the comment at the top to describe this in that particular order.

With that, I reworked your patch as follows:

diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index bb335fa16f7cc..243e5e26f7018 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2746,14 +2746,14 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
 	    (spsr & PSR_MODE32_BIT) ||
 	    (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
 					   mode == PSR_MODE_EL1h))) {
-		u64 cpsr = *vcpu_cpsr(vcpu);
 		u64 mask;
 
 		/*
-		 * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
-		 * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
-		 * and masks are taken from the SPSR (R_VWJHB). Set IL so the
-		 * HW generates an Illegal State Exception right after ERET.
+		 * On an illegal exception return, the flags and masks are
+		 * taken from the SPSR while PSTATE.{EL,SP,nRW} and, if
+		 * FEAT_GCS, PSTATE.EXLOCK are unchanged (R_VWJHB). Set IL
+		 * so the HW generates an Illegal State Exception right
+		 * after ERET.
 		 */
 		mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
 		       PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
@@ -2767,9 +2767,12 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
 			mask |= BIT_ULL(32);	/* PSTATE.PM */
 
 		spsr &= mask;
-		spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
+
+		mask = PSR_MODE_MASK | PSR_MODE32_BIT;
 		if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
-			spsr |= cpsr & BIT_ULL(34);	/* PSTATE.EXLOCK */
+			mask |= BIT_ULL(34);	/* PSTATE.EXLOCK */
+
+		spsr |= *vcpu_cpsr(vcpu) & mask;
 		spsr |= PSR_IL_BIT;
 	}
 
Does that work for you?

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
  2026-06-17 16:45 ` Marc Zyngier
@ 2026-06-17 17:19   ` Fuad Tabba
  2026-06-17 17:32     ` Marc Zyngier
  0 siblings, 1 reply; 5+ messages in thread
From: Fuad Tabba @ 2026-06-17 17:19 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Oliver Upton, kvmarm, linux-arm-kernel, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Jintack Lim, Ganapatrao Kulkarni, Christoffer Dall, linux-kernel

On Wed, 17 Jun 2026 at 17:45, Marc Zyngier <maz@kernel.org> wrote:
>
> On Wed, 17 Jun 2026 15:49:07 +0100,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > kvm_check_illegal_exception_return() sourced the flags {N,Z,C,V} and
> > masks {D,A,I,F} of the resulting PSTATE from the current PSTATE, but
> > R_VWJHB takes them from the SPSR being returned to and leaves
> > PSTATE.{EL,SP,nRW} (and EXLOCK when FEAT_GCS) unchanged. PAN, ALLINT
> > and PM were not applied at all.
> >
> > Build the PSTATE by taking those fields from the SPSR while preserving
> > EL, SP, nRW and EXLOCK from the current PSTATE, then set IL.
> >
> > Fixes: 47f3a2fc765a ("KVM: arm64: nv: Support virtual EL2 exceptions")
> > Suggested-by: Marc Zyngier <maz@kernel.org>
> > Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > This is a modified version of Marc's suggested diff [1]. That diff applied
> > a single mask to the incoming SPSR, which also takes PSTATE.{EL,SP,nRW}
> > (and EXLOCK) from the SPSR. The ARM ARM leaves those fields unchanged on an
> > illegal exception return. This path is reached precisely because SPSR.M is
> > illegal (EL3, M[1]=1, AArch32, EL1 under TGE), so this version preserves
> > EL/SP/nRW/EXLOCK from the current PSTATE and takes only the flags, masks
> > and PAN/ALLINT/PM from the SPSR.
> >
> > [1] https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> > ---
> >  arch/arm64/kvm/emulate-nested.c | 33 +++++++++++++++++++++++----------
> >  1 file changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index dba7ced74ca5..ace2b40cf875 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2738,17 +2738,30 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
> >           (spsr & PSR_MODE32_BIT) ||
> >           (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
> >                                          mode == PSR_MODE_EL1h))) {
> > -             /*
> > -              * The guest is playing with our nerves. Preserve EL, SP,
> > -              * masks, flags from the existing PSTATE, and set IL.
> > -              * The HW will then generate an Illegal State Exception
> > -              * immediately after ERET.
> > -              */
> > -             spsr = *vcpu_cpsr(vcpu);
> > +             u64 cpsr = *vcpu_cpsr(vcpu);
> > +             u64 mask;
> >
> > -             spsr &= (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> > -                      PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT |
> > -                      PSR_MODE_MASK | PSR_MODE32_BIT);
> > +             /*
> > +              * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
> > +              * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
> > +              * and masks are taken from the SPSR (R_VWJHB). Set IL so the
> > +              * HW generates an Illegal State Exception right after ERET.
> > +              */
> > +             mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> > +                    PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
> > +
> > +             if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, PAN, IMP))
> > +                     mask |= PSR_PAN_BIT;
> > +             if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, NMI, IMP))
> > +                     mask |= ALLINT_ALLINT;
> > +             /* FEAT_SPE_EXC and FEAT_TRBE_EXC also gate PSTATE.PM one day... */
> > +             if (kvm_has_feat(vcpu->kvm, ID_AA64DFR1_EL1, EBEP, IMP))
> > +                     mask |= BIT_ULL(32);    /* PSTATE.PM */
> > +
> > +             spsr &= mask;
> > +             spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> > +             if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> > +                     spsr |= cpsr & BIT_ULL(34);     /* PSTATE.EXLOCK */
> >               spsr |= PSR_IL_BIT;
> >       }
>
> While I'm happy that you caught the bugs I left for you to address,
> the overall structure is a bit inconsistent. I'd like to have:
>
> - a mask of the bits we preserve from SPSR, and apply that to SPSR
>   itself
>
> - a mask of the bits we preserve from PSTATE, and transfer them to
>   SPSR
>
> - the comment at the top to describe this in that particular order.
>
> With that, I reworked your patch as follows:
>
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index bb335fa16f7cc..243e5e26f7018 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -2746,14 +2746,14 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
>             (spsr & PSR_MODE32_BIT) ||
>             (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
>                                            mode == PSR_MODE_EL1h))) {
> -               u64 cpsr = *vcpu_cpsr(vcpu);
>                 u64 mask;
>
>                 /*
> -                * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
> -                * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
> -                * and masks are taken from the SPSR (R_VWJHB). Set IL so the
> -                * HW generates an Illegal State Exception right after ERET.
> +                * On an illegal exception return, the flags and masks are
> +                * taken from the SPSR while PSTATE.{EL,SP,nRW} and, if
> +                * FEAT_GCS, PSTATE.EXLOCK are unchanged (R_VWJHB). Set IL
> +                * so the HW generates an Illegal State Exception right
> +                * after ERET.
>                  */
>                 mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
>                        PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
> @@ -2767,9 +2767,12 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
>                         mask |= BIT_ULL(32);    /* PSTATE.PM */
>
>                 spsr &= mask;
> -               spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> +
> +               mask = PSR_MODE_MASK | PSR_MODE32_BIT;
>                 if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> -                       spsr |= cpsr & BIT_ULL(34);     /* PSTATE.EXLOCK */
> +                       mask |= BIT_ULL(34);    /* PSTATE.EXLOCK */
> +
> +               spsr |= *vcpu_cpsr(vcpu) & mask;
>                 spsr |= PSR_IL_BIT;
>         }
>
> Does that work for you?

Yes, that's cleaner, thanks. The two-mask form (take from SPSR, then
preserve from PSTATE) reads better than my interleaved version.

Would you like me to send a v2 (which would be identical to the above?
:) ), or would you fold it in?

Cheers,
/fuad

>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.


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

* Re: [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
  2026-06-17 17:19   ` Fuad Tabba
@ 2026-06-17 17:32     ` Marc Zyngier
  0 siblings, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2026-06-17 17:32 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Oliver Upton, kvmarm, linux-arm-kernel, Joey Gouly, Steffen Eiden,
	Suzuki K Poulose, Zenghui Yu, Catalin Marinas, Will Deacon,
	Jintack Lim, Ganapatrao Kulkarni, Christoffer Dall, linux-kernel

On Wed, 17 Jun 2026 18:19:20 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> On Wed, 17 Jun 2026 at 17:45, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Wed, 17 Jun 2026 15:49:07 +0100,
> > Fuad Tabba <tabba@google.com> wrote:
> > >
> > > kvm_check_illegal_exception_return() sourced the flags {N,Z,C,V} and
> > > masks {D,A,I,F} of the resulting PSTATE from the current PSTATE, but
> > > R_VWJHB takes them from the SPSR being returned to and leaves
> > > PSTATE.{EL,SP,nRW} (and EXLOCK when FEAT_GCS) unchanged. PAN, ALLINT
> > > and PM were not applied at all.
> > >
> > > Build the PSTATE by taking those fields from the SPSR while preserving
> > > EL, SP, nRW and EXLOCK from the current PSTATE, then set IL.
> > >
> > > Fixes: 47f3a2fc765a ("KVM: arm64: nv: Support virtual EL2 exceptions")
> > > Suggested-by: Marc Zyngier <maz@kernel.org>
> > > Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > > This is a modified version of Marc's suggested diff [1]. That diff applied
> > > a single mask to the incoming SPSR, which also takes PSTATE.{EL,SP,nRW}
> > > (and EXLOCK) from the SPSR. The ARM ARM leaves those fields unchanged on an
> > > illegal exception return. This path is reached precisely because SPSR.M is
> > > illegal (EL3, M[1]=1, AArch32, EL1 under TGE), so this version preserves
> > > EL/SP/nRW/EXLOCK from the current PSTATE and takes only the flags, masks
> > > and PAN/ALLINT/PM from the SPSR.
> > >
> > > [1] https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> > > ---
> > >  arch/arm64/kvm/emulate-nested.c | 33 +++++++++++++++++++++++----------
> > >  1 file changed, 23 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > > index dba7ced74ca5..ace2b40cf875 100644
> > > --- a/arch/arm64/kvm/emulate-nested.c
> > > +++ b/arch/arm64/kvm/emulate-nested.c
> > > @@ -2738,17 +2738,30 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
> > >           (spsr & PSR_MODE32_BIT) ||
> > >           (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
> > >                                          mode == PSR_MODE_EL1h))) {
> > > -             /*
> > > -              * The guest is playing with our nerves. Preserve EL, SP,
> > > -              * masks, flags from the existing PSTATE, and set IL.
> > > -              * The HW will then generate an Illegal State Exception
> > > -              * immediately after ERET.
> > > -              */
> > > -             spsr = *vcpu_cpsr(vcpu);
> > > +             u64 cpsr = *vcpu_cpsr(vcpu);
> > > +             u64 mask;
> > >
> > > -             spsr &= (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> > > -                      PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT |
> > > -                      PSR_MODE_MASK | PSR_MODE32_BIT);
> > > +             /*
> > > +              * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
> > > +              * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
> > > +              * and masks are taken from the SPSR (R_VWJHB). Set IL so the
> > > +              * HW generates an Illegal State Exception right after ERET.
> > > +              */
> > > +             mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> > > +                    PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
> > > +
> > > +             if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, PAN, IMP))
> > > +                     mask |= PSR_PAN_BIT;
> > > +             if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, NMI, IMP))
> > > +                     mask |= ALLINT_ALLINT;
> > > +             /* FEAT_SPE_EXC and FEAT_TRBE_EXC also gate PSTATE.PM one day... */
> > > +             if (kvm_has_feat(vcpu->kvm, ID_AA64DFR1_EL1, EBEP, IMP))
> > > +                     mask |= BIT_ULL(32);    /* PSTATE.PM */
> > > +
> > > +             spsr &= mask;
> > > +             spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> > > +             if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> > > +                     spsr |= cpsr & BIT_ULL(34);     /* PSTATE.EXLOCK */
> > >               spsr |= PSR_IL_BIT;
> > >       }
> >
> > While I'm happy that you caught the bugs I left for you to address,
> > the overall structure is a bit inconsistent. I'd like to have:
> >
> > - a mask of the bits we preserve from SPSR, and apply that to SPSR
> >   itself
> >
> > - a mask of the bits we preserve from PSTATE, and transfer them to
> >   SPSR
> >
> > - the comment at the top to describe this in that particular order.
> >
> > With that, I reworked your patch as follows:
> >
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index bb335fa16f7cc..243e5e26f7018 100644
> > --- a/arch/arm64/kvm/emulate-nested.c
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2746,14 +2746,14 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
> >             (spsr & PSR_MODE32_BIT) ||
> >             (vcpu_el2_tge_is_set(vcpu) && (mode == PSR_MODE_EL1t ||
> >                                            mode == PSR_MODE_EL1h))) {
> > -               u64 cpsr = *vcpu_cpsr(vcpu);
> >                 u64 mask;
> >
> >                 /*
> > -                * On an illegal exception return, PSTATE.{EL,SP,nRW} and,
> > -                * if FEAT_GCS, PSTATE.EXLOCK are unchanged, while the flags
> > -                * and masks are taken from the SPSR (R_VWJHB). Set IL so the
> > -                * HW generates an Illegal State Exception right after ERET.
> > +                * On an illegal exception return, the flags and masks are
> > +                * taken from the SPSR while PSTATE.{EL,SP,nRW} and, if
> > +                * FEAT_GCS, PSTATE.EXLOCK are unchanged (R_VWJHB). Set IL
> > +                * so the HW generates an Illegal State Exception right
> > +                * after ERET.
> >                  */
> >                 mask = PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> >                        PSR_N_BIT | PSR_Z_BIT | PSR_C_BIT | PSR_V_BIT;
> > @@ -2767,9 +2767,12 @@ static u64 kvm_check_illegal_exception_return(struct kvm_vcpu *vcpu, u64 spsr)
> >                         mask |= BIT_ULL(32);    /* PSTATE.PM */
> >
> >                 spsr &= mask;
> > -               spsr |= cpsr & (PSR_MODE_MASK | PSR_MODE32_BIT);
> > +
> > +               mask = PSR_MODE_MASK | PSR_MODE32_BIT;
> >                 if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> > -                       spsr |= cpsr & BIT_ULL(34);     /* PSTATE.EXLOCK */
> > +                       mask |= BIT_ULL(34);    /* PSTATE.EXLOCK */
> > +
> > +               spsr |= *vcpu_cpsr(vcpu) & mask;
> >                 spsr |= PSR_IL_BIT;
> >         }
> >
> > Does that work for you?
> 
> Yes, that's cleaner, thanks. The two-mask form (take from SPSR, then
> preserve from PSTATE) reads better than my interleaved version.
> 
> Would you like me to send a v2 (which would be identical to the above?
> :) ), or would you fold it in?

I've squashed that in.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
  2026-06-17 14:49 [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return Fuad Tabba
  2026-06-17 16:45 ` Marc Zyngier
@ 2026-06-17 17:36 ` Marc Zyngier
  1 sibling, 0 replies; 5+ messages in thread
From: Marc Zyngier @ 2026-06-17 17:36 UTC (permalink / raw)
  To: Oliver Upton, kvmarm, linux-arm-kernel, Fuad Tabba
  Cc: Joey Gouly, Steffen Eiden, Suzuki K Poulose, Zenghui Yu,
	Catalin Marinas, Will Deacon, Jintack Lim, Ganapatrao Kulkarni,
	Christoffer Dall, linux-kernel

On Wed, 17 Jun 2026 15:49:07 +0100, Fuad Tabba wrote:
> kvm_check_illegal_exception_return() sourced the flags {N,Z,C,V} and
> masks {D,A,I,F} of the resulting PSTATE from the current PSTATE, but
> R_VWJHB takes them from the SPSR being returned to and leaves
> PSTATE.{EL,SP,nRW} (and EXLOCK when FEAT_GCS) unchanged. PAN, ALLINT
> and PM were not applied at all.
> 
> Build the PSTATE by taking those fields from the SPSR while preserving
> EL, SP, nRW and EXLOCK from the current PSTATE, then set IL.
> 
> [...]

Applied to fixes, thanks!

[1/1] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
      commit: 0e8e955b9bcf84a70f20079390e19971fec1586d

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.




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

end of thread, other threads:[~2026-06-17 17:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 14:49 [PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return Fuad Tabba
2026-06-17 16:45 ` Marc Zyngier
2026-06-17 17:19   ` Fuad Tabba
2026-06-17 17:32     ` Marc Zyngier
2026-06-17 17:36 ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox