On Wed, Aug 19, 2026 at 05:46:00PM +0100, Mark Brown wrote:
> On Wed, Aug 19, 2026 at 05:32:25PM +0100, Leonardo Bras wrote:
> > On Wed, Aug 12, 2026 at 08:12:02PM +0100, Mark Brown wrote:
>
> > > In order to allow guests to use GCS we also need to configure
> > > HCRX_EL2.GCSEn, if this is not set GCS instructions will be noops and
> > > CHKFEAT will report GCS as disabled.
>
> > It is zero on reset, and keeping it in zero disables GCS in EL0&EL1, so
> > unless we are in EL2&0 (HCR_EL2.{E2H, TGE} is {1, 1}), we need to enable it
> > so EL1&0 (guests) can have access to it.
>
> Right.
>
> > > @@ -77,6 +80,8 @@ static void __sysreg_save_vel2_state(struct kvm_vcpu
> > > *vcpu)
> > > __vcpu_assign_sys_reg(vcpu, SP_EL2, read_sysreg(sp_el1));
> > > __vcpu_assign_sys_reg(vcpu, ELR_EL2, read_sysreg_el1(SYS_ELR));
> > > __vcpu_assign_sys_reg(vcpu, SPSR_EL2, read_sysreg_el1(SYS_SPSR));
> > > + if (ctxt_has_gcs(&vcpu->arch.ctxt))
> > > + __vcpu_assign_sys_reg(vcpu, GCSPR_EL2,
> > > read_sysreg_el1(SYS_GCSPR));
>
> > Out of curiosity here,
> > Why does the saving of GCSCR_EL2 depends on ctxt_has_tcrx() (and E2H set),
> > and saving SYS_GCSPR does not? ...
>
> This is just because there isn't a preexisting ctxt_has_tcrx() check
> already there, FEAT_GCS architecturally depends on FEAT_TCRX and there
> was a request to make this explicit in the code to try to optimise
> things a bit. The compiler should skip over both blocks at once if TCRX
> isn't there rather than having two separate tests or static branches. I
> didn't add new checks where there were none since I expect that to be
> unhelpful for code generation, you'd get the reverse situation and emit
> two checks.
Humm, but then why saving GCSPR_EL2 does not depend on TCRX/E2H? Or maybe a
better question, why are not GCSPR_EL2 and GCSCR_EL2 saved in the same 'if'
clause under ctxt_has_tcrx() (and E2H set), if they are restored in the
same 'if' clause?
As you mentioned, GCS depends on TCRX, so it should be fine, but just by
reading the code I see:
- GCSCR_EL2 :
Save if GCS=1, TCRX=1, E2H=1
Restore if GCS=1, TCRX=1
- GCSPR_EL2 :
Save if GCS=1,
Restore if GCS=1, TCRX=1
Which looks kind of confusing for the as a first time reader.
Does it make sense?
Thanks!
Leo