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

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

[Fuad 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete PSTATE allowlist on illegal exception return causes guest state corruption
--

commit 93d5c5c2769fe9f58076ca66eb01cbe04fd87921
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: nv: Fix PSTATE construction on illegal exception return

This commit updates PSTATE construction during an illegal exception return
to correctly source flags and masks from the SPSR being returned to, as
per ARM rule R_VWJHB. It constructs the PSTATE by taking these fields from
the SPSR while preserving EL, SP, nRW, and EXLOCK from the current PSTATE.

Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/

> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index dba7ced74ca5e..ace2b40cf8756 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)

[ ... ]

> +		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;

[Severity: High]
This is a pre-existing issue, but this explicit allowlist approach seems to
omit several architecturally defined and KVM-supported PSTATE fields.

For example, PSR_SSBS_BIT, PSR_DIT_BIT, PSR_UAO_BIT, PSR_TCO_BIT, and
PSR_BTYPE_MASK are not included in the mask. Because spsr is masked with
this incomplete allowlist, won't these valid bits be silently zeroed out
instead of being preserved?

When the guest takes the resulting Illegal State Exception, its SPSR_ELx
will have lost these bits, which could corrupt the guest's architectural
state and break guest exception handlers that rely on them.

Would a denylist approach (masking out only the mode bits and EXLOCK from
SPSR and taking them from CPSR) be safer to ensure all relevant bits are
retained?

> +		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;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617144907.2972095-1-tabba@google.com?part=1

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

[Fuad Tabba]

On Wed, 17 Jun 2026 at 16:04, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incomplete PSTATE allowlist on illegal exception return causes guest state corruption
> --
>
> commit 93d5c5c2769fe9f58076ca66eb01cbe04fd87921
> Author: Fuad Tabba <tabba@google.com>
>
> KVM: arm64: nv: Fix PSTATE construction on illegal exception return
>
> This commit updates PSTATE construction during an illegal exception return
> to correctly source flags and masks from the SPSR being returned to, as
> per ARM rule R_VWJHB. It constructs the PSTATE by taking these fields from
> the SPSR while preserving EL, SP, nRW, and EXLOCK from the current PSTATE.
>
> Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
>
> > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > index dba7ced74ca5e..ace2b40cf8756 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)
>
> [ ... ]
>
> > +             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;
>
> [Severity: High]
> This is a pre-existing issue, but this explicit allowlist approach seems to
> omit several architecturally defined and KVM-supported PSTATE fields.
>
> For example, PSR_SSBS_BIT, PSR_DIT_BIT, PSR_UAO_BIT, PSR_TCO_BIT, and
> PSR_BTYPE_MASK are not included in the mask. Because spsr is masked with
> this incomplete allowlist, won't these valid bits be silently zeroed out
> instead of being preserved?
>
> When the guest takes the resulting Illegal State Exception, its SPSR_ELx
> will have lost these bits, which could corrupt the guest's architectural
> state and break guest exception handlers that rely on them.
>
> Would a denylist approach (masking out only the mode bits and EXLOCK from
> SPSR and taking them from CPSR) be safer to ensure all relevant bits are
> retained?

I don't think so, but Marc, please correct me if I'm wrong.

My reading of the ARM ARM is that these fields are UNKNOWN on an illegal
exception return, not preserved. R_VWJHB says "All of the following are
UNKNOWN" and lists PSTATE.{UAO, DIT, TCO, SSBS, BTYPE, PACM}. If that is
right, masking them to 0 is a valid realisation of UNKNOWN rather than a
loss of state, since a guest cannot rely on them surviving an illegal ERET.

Cheers,
/fuad

>
> > +             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;
> >       }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260617144907.2972095-1-tabba@google.com?part=1

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

[Marc Zyngier]

On Wed, 17 Jun 2026 16:29:09 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> On Wed, 17 Jun 2026 at 16:04, <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >
> > Pre-existing issues:
> > - [High] Incomplete PSTATE allowlist on illegal exception return causes guest state corruption
> > --
> >
> > commit 93d5c5c2769fe9f58076ca66eb01cbe04fd87921
> > Author: Fuad Tabba <tabba@google.com>
> >
> > KVM: arm64: nv: Fix PSTATE construction on illegal exception return
> >
> > This commit updates PSTATE construction during an illegal exception return
> > to correctly source flags and masks from the SPSR being returned to, as
> > per ARM rule R_VWJHB. It constructs the PSTATE by taking these fields from
> > the SPSR while preserving EL, SP, nRW, and EXLOCK from the current PSTATE.
> >
> > Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> >
> > > diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> > > index dba7ced74ca5e..ace2b40cf8756 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)
> >
> > [ ... ]
> >
> > > +             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;
> >
> > [Severity: High]
> > This is a pre-existing issue, but this explicit allowlist approach seems to
> > omit several architecturally defined and KVM-supported PSTATE fields.
> >
> > For example, PSR_SSBS_BIT, PSR_DIT_BIT, PSR_UAO_BIT, PSR_TCO_BIT, and
> > PSR_BTYPE_MASK are not included in the mask. Because spsr is masked with
> > this incomplete allowlist, won't these valid bits be silently zeroed out
> > instead of being preserved?
> >
> > When the guest takes the resulting Illegal State Exception, its SPSR_ELx
> > will have lost these bits, which could corrupt the guest's architectural
> > state and break guest exception handlers that rely on them.
> >
> > Would a denylist approach (masking out only the mode bits and EXLOCK from
> > SPSR and taking them from CPSR) be safer to ensure all relevant bits are
> > retained?
> 
> I don't think so, but Marc, please correct me if I'm wrong.
> 
> My reading of the ARM ARM is that these fields are UNKNOWN on an illegal
> exception return, not preserved. R_VWJHB says "All of the following are
> UNKNOWN" and lists PSTATE.{UAO, DIT, TCO, SSBS, BTYPE, PACM}. If that is
> right, masking them to 0 is a valid realisation of UNKNOWN rather than a
> loss of state, since a guest cannot rely on them surviving an illegal ERET.

Correct. R_VWJHB is pretty clear that these bits are UNKNOWN, and
therefore 0 is a perfect value for it.

I'm already getting tired of Sashiko.

	M.

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

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

[Marc Zyngier]

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.

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

[Fuad Tabba]

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.

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

[Marc Zyngier]

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.

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

[Marc Zyngier]

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.