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

Leonardo Bras <[email protected]>
Newsgroups org.kernel.vger.linux-doc,dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest
Message-ID <aocOi3wjcliZqoe7@LeoBrasDK>
On Thu, Aug 20, 2026 at 03:07:14PM +0100, Mark Brown wrote:
> On Thu, Aug 20, 2026 at 11:16:59AM +0100, Leonardo Bras wrote:
> > On Wed, Aug 19, 2026 at 05:46:00PM +0100, Mark Brown wrote:
> 
> > > 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?
> 
> I agree that the current situation is a bit hard to follow, I'd actually
> originally written things without the explict dependency because of that
> but Marc wanted the optimisation.

Well, I am not against the explicit dependency thing, it just looks odd 
to me that some save/restore have a dependency and it's counterpart does 
not. As well as the dependencies for both registers being different.

>  There's similar things with other
> registers in the current code (eg, the PIE and POE registers are in the
> same situaton as GCSCR_EL2).  One thing I did miss here is some nesting
> inside the PIE checks for EL2, I'll update for that.
> 

Awesome!

> I think what would help here would be to move to generating more of the
> save/restore sequences rather than open coding them, we could have lists
> of registers and then use data about the feature dependencies that the
> architecture has (hopefully it's already in the MRS, I didn't check) to
> generate these optimised feature checks rather than having to open code
> them.  Possibly even list features and generate some of the register
> lists, though that would need far more special case handling and it's
> *much* less clear if there could be a benefit.

Yeah, that would automatize the feature dependency for register loading, 
but maybe code would not be that clear on how it works? I have questionable 
taste, though.

Thanks!
Leo
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.