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

[Leonardo Bras <leo.bras@arm.com>]

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