Re: [PATCH v21 08/15] KVM: arm64: Enforce EXLOCK for SPSR and ELR

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Thu, Oct 01, 2026 at 10:11:49PM +0100, Mark Brown wrote:
> On Thu, Oct 01, 2026 at 05:26:41PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Wed, Sep 30, 2026 at 10:48:18PM +0100, Mark Brown wrote:
> > > As per I_CFFNS and the pseudocode for the SPSR_ELx and ELR_ELx registers
> > > a GCS exception with ExType 1 is generated for attempts to write to
> > > those registers when both GCSCR_ELx.EXLOCKEN and PSTATE.EXLOCK are set.
>
> > From [4] I see I_CFFNS is defined as:
>
> > "When an MSR instruction would write to the relevant ELR_ELx or SPSR_ELx
> > for the current Exception level ELy, the Effective value of
> > GCSCR_ELy.EXLOCKEN and PSTATE.EXLOCK may prevent the write."
>
> > So the lock applies to writes to the current EL's own ELR/SPSR rather than
> > to any ELR_ELx/SPSR_ELx the current EL can write to?
>
> VHE and NV complicate things so "own" isn't just the same ELx, but my
> text above definitely oversimplifies too much and so is wrong.

It's all incredibly tricky and I felt my brain melting out of my head going
through the psuedocode/definition yesterday so it's understandable :)

>
> For example refering to the pseudocode for ELR_EL1 and SPSR_EL1 we see
> in the MSR handling:
>
>   elsif PSTATE.EL == EL2 then
>       if IsFeatureImplemented(FEAT_GCS) && GetCurrentEXLOCKEN() && !Halted() && PSTATE.EXLOCK == '1' && ELIsInHost(EL2) then
>           EXLOCKException();
>
> and note the use of ELx for ELR/SPSR and ELy for the current exception
> level and GCSCR in I_CFFNS (ie, ELx vs ELy).  My interpretation here is
> that the use of "the relevant" rather than just using ELx throughout is
> an effort to cover the complications resulting from VHE and NV.  With
> the above pseudocode writes to the EL1 register from EL2 are also
> covered when we're in host mode - the fact that we're in host mode makes
> the EL1 access relevant.

Yeah agreed.

>
> I'll reword what I've written in the commit log, like I say it's wrong.

Ack thanks!

>
> > So... TL;DR is, shouldn't this function look like:
> >
> > static inline bool sysregs_exlocked(struct kvm_vcpu *vcpu)
> > {
> > 	if (!kvm_has_gcs(vcpu->kvm))
> > 		return false;
> >
> > 	if (!(vcpu->arch.ctxt.regs.pstate & PSR_EXLOCK_BIT))
> > 		return false;
> >
> > 	if (is_hyp_ctxt(vcpu))
> > 		return false;
> >
> > 	return vcpu_read_sys_reg(vcpu, GCSCR_EL1) & GCSCR_ELx_EXLOCKEN;
> > }
>
> I think so, but I'll check through again.

Thanks!

>
> The confusions you identified in the bit of your mail above this were
> the result of me doing some but on all of the simplifications.  I was
> trying to make things clearer by including some code that couldn't run
> so it was more obvious that things correspond to the pseudocode, but
> really that shouldn't have had any simplifications in it - we should
> either have all the simplifications or none of them.  I'll add more
> comments instead.

Yeah it's all very complicated, unfortunately I think, the psuedocode had me
stumped a lot when I was reading it...

Definitely agree on all-or-nothing. If we did go the 'all' route, then we'd
need to put in the ELIsInHost(EL2) conditional too
i.e. vcpu_el2_e2h_is_set() as well.

But I think simplifying that is also valid, as the resultant function above
is a lot easier to reason about and avoids people having to figure out that
certain bits are irrelevant (though comments could square that off too!)

>
> > > +	/*
> > > +	 * Note that the EXLOCKEN for the running EL is checked
> > > +	 * regardless of the register written to.
> > > +	 */
> >
> > This seems to contradict [4] - the register written to is what decides
> > whether the lock applies?
>
> [4] is section D11.4.1 of DDI0487 M.d, containing rule I_CFFNS discussed
> above.  My intent there is to express that if EXLOCK exceptions might be
> generated we check EXLOCKEN for the running EL, not one influenced by
> the written register.  Some combinations of register, EL and system
> state do not generate exceptions but those that do use the current EL's
> EXLOCKEN rather than an _ELx register using GCSCR_ELx.EXLOCKEN.

Yeah that's clear thanks!

IOW - the register/El/state decides _whether_ an exception can be raised,
the running EL's EXLOCKEN is what is consulted _when_ it can be raised.

--
Cheers, Lorenzo