[PATCH] KVM: arm64: nv: Fix PSTATE construction on illegal exception return
Fuad Tabba
tabba at google.com
Wed Jun 17 10:19:20 PDT 2026
On Wed, 17 Jun 2026 at 17:45, Marc Zyngier <maz at kernel.org> wrote:
>
> On Wed, 17 Jun 2026 15:49:07 +0100,
> Fuad Tabba <tabba at 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 at kernel.org>
> > Link: https://lore.kernel.org/all/86wlvxs5r0.wl-maz@kernel.org/
> > Signed-off-by: Fuad Tabba <tabba at 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.
More information about the linux-arm-kernel
mailing list