Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	 Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Joey Gouly <joey.gouly@arm.com>,
	 Suzuki K Poulose <suzuki.poulose@arm.com>,
	Shuah Khan <shuah@kernel.org>, Oliver Upton <oupton@kernel.org>,
	 Fuad Tabba <fuad.tabba@linux.dev>,
	Peter Maydell <peter.maydell@linaro.org>,
	 Leonardo Bras <leo.bras@arm.com>,
	Wei-Lin Chang <weilin.chang@arm.com>,
	 Yao Yuan <yaoyuan@linux.alibaba.com>,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
	 kvmarm@lists.linux.dev, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v20 03/14] KVM: arm64: Manage GCS access and registers for guests
Date: Fri, 4 Sep 2026 09:54:51 +0100	[thread overview]
Message-ID: <apqA_SuoA6-TyHsS@gremlin> (raw)
In-Reply-To: <d2659ac3-2f00-4fc4-8f9e-0585691a4ba2@sirena.org.uk>

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:
> > On Tue, Sep 01, 2026 at 10:47:01PM +0100, Mark Brown wrote:
>
> > > 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.
>
> > > +				ctxt_sys_reg(ctxt, GCSCR_EL1)	= read_sysreg_el1(SYS_GCSCR);
> > > +			}
>
> > I had AI check this (and I think sashiko also hit on) it but this is a chain of:
>
> > 	ctxt_has_tcrx() -> ctxt_has_s1pie() -> ctx_has_gcs()
>
> > 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.

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

>
> > And it seems like it makes it possible for a silly VMM which sets up the
> > registers wrong + some unfortunate guest behaviour -> oops via:
>
> > el1h_64_sync_handler() -> el1_gcs() -> do_el1_gcs()
>
> > 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

  reply	other threads:[~2026-09-04  8:54 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01 21:46 [PATCH v20 00/14] KVM: arm64: Provide guest support for GCS Mark Brown
2026-09-01 21:46 ` [PATCH v20 01/14] arm64/gcs: Ensure FGTs for EL1 GCS instructions are disabled Mark Brown
2026-09-03 16:10   ` Lorenzo Stoakes (ARM)
2026-09-01 21:47 ` [PATCH v20 02/14] KVM: arm64: Fix FGT mapping for HFGITR_EL2.nGCSEPP Mark Brown
2026-09-03 16:23   ` Lorenzo Stoakes (ARM)
2026-09-03 19:29     ` Mark Brown
2026-09-01 21:47 ` [PATCH v20 03/14] KVM: arm64: Manage GCS access and registers for guests Mark Brown
2026-09-02 16:44   ` Leonardo Bras
2026-09-03 20:52     ` Mark Brown
2026-09-03 18:13   ` Lorenzo Stoakes (ARM)
2026-09-03 20:41     ` Mark Brown
2026-09-04  8:54       ` Lorenzo Stoakes (ARM) [this message]
2026-09-01 21:47 ` [PATCH v20 04/14] KVM: arm64: Ensure GCS memory effects are visible Mark Brown
2026-09-02 16:30   ` Leonardo Bras
2026-09-04 12:16   ` Lorenzo Stoakes (ARM)
2026-09-01 21:47 ` [PATCH v20 05/14] KVM: arm64: Set PSTATE.EXLOCK when entering an exception Mark Brown
2026-09-03 14:25   ` Leonardo Bras
2026-09-03 16:20     ` Mark Brown
2026-09-03 16:40       ` Leonardo Bras
2026-09-04 13:04   ` Lorenzo Stoakes (ARM)
2026-09-01 21:47 ` [PATCH v20 06/14] KVM: arm64: Validate GCS exception lock when emulating ERET Mark Brown
2026-09-03 15:37   ` Leonardo Bras
2026-09-03 19:22     ` Mark Brown
2026-09-04 13:16       ` Leonardo Bras
2026-09-01 21:47 ` [PATCH v20 07/14] KVM: arm64: Forward GCS exceptions to nested guests Mark Brown
2026-09-01 21:47 ` [PATCH v20 08/14] KVM: arm64: Enforce EXLOCK for SPSR and ELR Mark Brown
2026-09-01 21:47 ` [PATCH v20 09/14] KVM: arm64: Allow GCS to be enabled for guests Mark Brown
2026-09-01 21:47 ` [PATCH v20 10/14] KVM: selftests: arm64: Add GCS registers to get-reg-list Mark Brown
2026-09-01 21:47 ` [PATCH v20 11/14] KVM: selftests: arm64: Add GCS to set_id_regs Mark Brown
2026-09-01 21:47 ` [PATCH v20 12/14] KVM: selftests: arm64: Only restore SPSR_EL1 and ELR_EL1 if they change Mark Brown
2026-09-01 21:47 ` [PATCH v20 13/14] tools: Synchronise the kernel esr.h Mark Brown
2026-09-01 21:47 ` [PATCH v20 14/14] KVM: selftests: arm64: Add GCS EXLOCK exception emulation test Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=apqA_SuoA6-TyHsS@gremlin \
    --to=ljs@kernel.org \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=fuad.tabba@linux.dev \
    --cc=joey.gouly@arm.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=leo.bras@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=peter.maydell@linaro.org \
    --cc=shuah@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=weilin.chang@arm.com \
    --cc=will@kernel.org \
    --cc=yaoyuan@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox