Re: [PATCH v20 03/14] KVM: arm64: Manage GCS access and registers for guests

[Lorenzo Stoakes (ARM)]

On Fri, Sep 04, 2026 at 10:07:49PM +0100, Mark Brown wrote:
> On Fri, Sep 04, 2026 at 09:54:51AM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Sep 03, 2026 at 09:41:26PM +0100, Mark Brown wrote:
>
> > > originally written the code without expressing this dependency but on a
> > > previous version Marc asked for this nesting as an optimisation.  Since
> > > it's about optimisastion adding checks that don't otherwise exist on the
> > > restore path would doubtless get the similar complaints.  Exactly the
> > > same concerns were raised on v19.
>
> > Could you implement the nesting the same in the cases where there is a bare
> > ctx_has_gcs() as an alternative?
>
> > So then it'd always be tcrx -> s1pie -> gcs everywhere and that'd resolve things
> > also and make things symmetric.
>
> > You could also I think express the dependency if it makes sense to.
>
> Clearly we *can*, it's a question of what'd be acceptable - it'd result
> in the compiler emitting a bunch of extra alternatives and conditionals
> in the context switch path.

Right. And that's presumably a rather _sensitive_ bit of code.

>
> > > We could do checking of the ID registers at vCPU creation so we could
> > > avoid worrying about them in the fast path but there was also feedback
> > > about not doing that.  One idea I had was to generate feature
>
> > Could you possibly check in sanitise_id_aa64pfr1_el1(), something like:
>
> Sanitising the writes is a bit tricky since multiple ID registers are
> involved, TCRX and S1PIE are in ID_AA64MMFR3_EL1 while GCS is in
> ID_AA64PFR1_EL1 so you'd create an ordering constraint for userspace,
> and you'd also have to handle the case where TCR2 or S1PIE are turned
> off with GCS already enabled.  It feels like a bunch of complication,
> especially if you start applying the same approach with other features.

Yeah, you're right, it is a bit of a nightmare and feels like it could get
a bit combinatorial.

>
> If we're going to validate the ID registers it feels safer to check at
> the point where we finalise the ID registers and refuse to start the
> vCPU if there's an unsupportable configuration, that would mean the
> check would only need to be done in one place and userspace wouldn't
> trip over any ordering requirements with how it updates registers.  Only
> userspaces that set architecturally invalid configurations should see an
> error.  We could also handle things by fixing up the configuration at
> the same point, disabling features that are missing their dependencies,
> that's a bit more friendly in the short term but could lead to trouble
> later on as it means we hide issues in userspace.
>
> I think the best thing overall would be to leave the runtime paths as
> they are and refuse to run with an architecturally invalid setup, that
> would avoid bloating the fast path.

Ack, it'd be nice to not have to worry about that.

Presumably we definitely want an oops on a GCS violation in general?

--
Cheers, Lorenzo