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

On Thu, Sep 03, 2026 at 09:41:26PM +0100, Mark Brown wrote:

On Thu, Sep 03, 2026 at 07:13:17PM +0100, Lorenzo Stoakes (ARM) wrote:

quoted

On Tue, Sep 01, 2026 at 10:47:01PM +0100, Mark Brown wrote:

quoted

quoted

GCS introduces a number of system registers, on systems with GCS we need
to context switch them and expose them to VMMs to allow guests to use
GCS.

quoted

quoted

+				ctxt_sys_reg(ctxt, GCSCR_EL1)	= read_sysreg_el1(SYS_GCSCR);
+			}

quoted

I had AI check this (and I think sashiko also hit on) it but this is a chain of:

quoted

	ctxt_has_tcrx() -> ctxt_has_s1pie() -> ctx_has_gcs()

quoted

Is it correct to make the gcs stuff conditional on tcrx + s1pie + gcs?

There are architectural dependencies which mean it is not valid to
configure GCS without S1PIE, and S1PIE depends on TCRX.  I had

Ack yeah I think I note that elsewhere, so it reduces only to the 'users doing
weird stuff' case :)

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.

quoted

And it seems like that feature depends on ctxt_has_tcrx() so _architecturally_
fine, but it doesn't seem like KVM enforces the dependency at all and so in
theory somebody could KVM_SET_ONE_REG a GCS, !S1PIE configuration.

quoted

And nicer to be consistent everywhere also I think (+ shut sashiko up! :)

FWIW I do agree but I'm not sure how else to implement Marc's feedback
here.

Ack sure of course.

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:

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index fb2c9fd42e24..ba8780745987 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c

@@ -2193,7 +2193,8 @@ static u64 sanitise_id_aa64pfr1_el1(const struct kvm_vcpu *vcpu, u64 val)
 	      SYS_FIELD_GET(ID_AA64PFR0_EL1, RAS, pfr0) == ID_AA64PFR0_EL1_RAS_IMP))
 		val &= ~ID_AA64PFR1_EL1_RAS_frac;

-	if (!system_supports_gcs())
+	if (!system_supports_gcs() || !kvm_has_tcr2(vcpu->kvm) ||
+	    !kvm_has_s1pie(vcpu->kvm))
 		val &= ~ID_AA64PFR1_EL1_GCS;

 	val &= ~ID_AA64PFR1_EL1_SME;
--

And that way it slots in next to the system check?

combination validation from the MRS, I think the main complaint was
about open coding things rather than having the validation per se but
ICBW.  Last I heard we were very near to having code for working with
the MRS released which will help a lot with uses like that.  There is

Yeah that sounds like a really neat way of solving it! And a useful idea in
general.

the possibility that people are relying on doing architecturally
invalid configurations though.

Yeah, I guess it's possible somebody could think they could just set a single
register and use it or something.

quoted

And it seems like it makes it possible for a silly VMM which sets up the
registers wrong + some unfortunate guest behaviour -> oops via:

quoted

el1h_64_sync_handler() -> el1_gcs() -> do_el1_gcs()

quoted

Because the host's restore is skipped for s1pie=0, gcs=1, so a naughty guest can
set gcsr_el1.pcrsel=1 and some value in gcspr_el1, then the host will get a
mismatch and trigger the kernel die().

Yes, that's possible.  GCSCR_EL1.EXLOCKEN would create similar issues.

Yeah :(

--
Cheers, Lorenzo