On Mon, 05 Feb 2024 12:35:53 +0000,
Mark Brown [off-list ref] wrote:

On Mon, Feb 05, 2024 at 09:46:16AM +0000, Marc Zyngier wrote:

quoted

On Sat, 03 Feb 2024 12:25:39 +0000,
Mark Brown [off-list ref] wrote:

quoted

quoted

+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h

@@ -25,6 +25,8 @@ static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt)
 {
 	ctxt_sys_reg(ctxt, TPIDR_EL0)	= read_sysreg(tpidr_el0);
 	ctxt_sys_reg(ctxt, TPIDRRO_EL0)	= read_sysreg(tpidrro_el0);
+	if (has_gcs())
+		ctxt_sys_reg(ctxt, GCSPR_EL0) = read_sysreg_s(SYS_GCSPR_EL0);

quoted

We have had this discussion in the past. This must be based on the
VM's configuration. Guarding the check with the host capability is a
valuable optimisation, but that's nowhere near enough. See the series
that I have posted on this very subject (you're on Cc), but you are
welcome to invent your own mechanism in the meantime.

Right, which postdates the version you're replying to and isn't merged
yet - the current code was what you were asking for at the time.

v1 and v2 predate it. And if the above is what I did ask, then I must
have done a very poor job of explaining what was required. For which I
apologise profusely.

I'm
expecting to update all these feature series to work with that once it
gets finalised and merged but it's not there yet, I do see I forgot to
put a note in v9 about that like I did for dpISA - sorry about that, I
was too focused on the clone3() rework when rebasing onto the new
kernel.

This particular series isn't going to get merged for a while yet anyway
due to the time it'll take for userspace testing, I'm expecting your
series to be in by the time it becomes an issue.

Right. Then I'll ignore it for the foreseeable future.

quoted

quoted

+	if (has_gcs()) {
+		write_sysreg_el1(ctxt_sys_reg(ctxt, GCSPR_EL1),	SYS_GCSPR);
+		write_sysreg_el1(ctxt_sys_reg(ctxt, GCSCR_EL1),	SYS_GCSCR);
+		write_sysreg_s(ctxt_sys_reg(ctxt, GCSCRE0_EL1),
+			       SYS_GCSCRE0_EL1);
+	}

quoted

For the benefit of the unsuspecting reviewers, and in the absence of a
public specification (which the XML drop isn't), it would be good to
have the commit message explaining the rationale of what gets saved
when.

What are you looking for in terms of rationale here?  The KVM house
style is often very reliant on reader context so it would be good to
know what considerations you'd like to see explicitly addressed.

Nothing to do with style, everything to do with substance: if nothing
in the host kernel makes any use of these registers, why are they
eagerly saved/restored on nVHE/hVHE? I'm sure you have a reason for
it, but it isn't that obvious. Because these two modes need all the
help they can get in terms of overhead reduction.

These
registers shouldn't do anything when we aren't running the guest so
they're not terribly ordering sensitive, the EL2 ones will need a bit
more consideration in the face of nested virt.

The EL2 registers should follow the exact same pattern, specially once
you fix the VNCR bugs I pointed out.

	M.

-- 
Without deviation from the norm, progress is not possible.