Re: [PATCH 4/7] KVM: arm64: Set IL for injected FPAC exceptions during ERET emulation

[Marc Zyngier <maz@kernel.org>]

On Wed, 17 Jun 2026 13:39:14 +0100,
Fuad Tabba <tabba@google.com> wrote:
> 
> On Wed, 17 Jun 2026 at 13:36, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Mon, 15 Jun 2026 13:42:23 +0100,
> > Fuad Tabba <tabba@google.com> wrote:
> > >
> > > On Sun, 14 Jun 2026 at 17:45, <sashiko-bot@kernel.org> wrote:
> > > >
> > > One thing I noticed while looking at this:
> > > kvm_check_illegal_exception_return() preserves {N,Z,C,V,D,A,I,F} from
> > > the current PSTATE, but the Arm ARM says these should come from
> > > SPSR_ELx on an illegal exception return (EL and SP are unchanged, but
> > > the flags and masks are taken from SPSR). PAN and ALLINT should also
> > > come from SPSR_ELx if their respective features are implemented.
> > >
> > > Marc, is the current behavior intentional, or should we be
> > > constructing PSTATE from the original SPSR_EL2 for those fields?
> >
> > This looks like a long standing bug for something we really never
> > tested. I reckon the patch below would do the trick.
> >
> >         M.
> >
> > +++ b/arch/arm64/kvm/emulate-nested.c
> > @@ -2746,17 +2746,29 @@ 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 mask;
> > +
> >                 /*
> >                  * 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.
> > +                * masks, flags from the existing SPSR, and set IL (see
> > +                * R_VWJHB). The HW will then generate an Illegal State
> > +                * Exception immediately after ERET.
> >                  */
> > -               spsr = *vcpu_cpsr(vcpu);
> > -
> > -               spsr &= (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT |
> > +               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 |
> >                          PSR_MODE_MASK | PSR_MODE32_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;
> > +               /* Account for FEAT_SPE_EXC and FEAT_TRBE_EXC one day... */
> > +               if (kvm_has_feat(vcpu->kvm, ID_AA64DFR1_EL1, EBEP, IMP))
> > +                       mask |= BIT_ULL(32);
> > +               if (kvm_has_feat(vcpu->kvm, ID_AA64PFR1_EL1, GCS, IMP))
> > +                       mask |= BIT_ULL(34);
> > +
> > +               spsr &= mask;
> 
> I'm on it...

Sorry, in what sense?

	M.

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