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

[Marc Zyngier <maz@kernel.org>]

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.