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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2525 bytes --]

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.

> > 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.

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]