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

[Marc Zyngier <maz@kernel.org>]

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.