On Thu, Aug 20, 2026 at 03:28:39PM +0100, Leonardo Bras wrote: > 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.
Hey I feel the question wasn't answered completely :) Here is the reason for the asymmetric save restore (besides the tcr2 part): GCSCR If guest E2H == 1: - L1 accesses to GCSCR_EL2 and GCSCR_EL1 have the same intent -> they both mean access the vCPU GCSCR_EL2. - L1 access to GCSCR_EL2 (e.g. msr GCSCR_EL2, x0) traps, KVM updates both the in-memory vCPU GCSCR_EL2, and the hardware GCSCR_EL1 for the the guest. In-memory copy stays up to date. - L1 access to GCSCR_EL1 does not trap -> in-memory vCPU GCSCR_EL2 becomes stale if it's a write. - From above, must save on exit. If guest E2H == 0: - Only L1 accesses to GCSCR_EL2 mean access the vCPU GCSCR_EL2. - L1 accesses to GCSCR_EL2 always traps. - In-memory vCPU GCSCR_EL2 always stay up to date. - From above, no need to save on exit. GCSPR - Regardless of the guest E2H value, hardware could write to the hardware GCSPR_EL1, making the in-memory vCPU GCSPR_EL2 stale. - From above, must save on exit. Restore is trivial, vCPU GCSCR_EL2/GCSPR_EL2 must be written to the hardware EL1 registers because someone else could be using it. Quite a few other registers are saved only if guest E2H == 1 because of the same reason. Thanks, Wei-Lin Chang > > > 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
