On Fri, 16 Aug 2024 15:40:33 +0100,
Mark Brown [off-list ref] wrote:

[1  <text/plain; us-ascii (7bit)>]
On Fri, Aug 16, 2024 at 03:15:19PM +0100, Marc Zyngier wrote:

quoted

Mark Brown [off-list ref] wrote:

quoted

quoted

+	{ SYS_DESC(SYS_GCSCR_EL1), NULL, reset_val, GCSCR_EL1, 0 },
+	{ SYS_DESC(SYS_GCSPR_EL1), NULL, reset_unknown, GCSPR_EL1 },
+	{ SYS_DESC(SYS_GCSCRE0_EL1), NULL, reset_val, GCSCRE0_EL1, 0 },

quoted

Global visibility for these registers? Why should we expose them to
userspace if the feature is neither present nor configured?

...

quoted

quoted

+	if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, GCS, IMP))
+		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nGCS_EL0 |
+						HFGxTR_EL2_nGCS_EL1);

quoted

How can this work if you don't handle ID_AA64PFR_EL1 being written to?
You are exposing GCS to all guests without giving the VMM an
opportunity to turn it off. This breaks A->B->A migration, which is
not acceptable.

This was done based on your positive review of the POE series which
follows the same pattern:

   https://lore.kernel.org/linux-arm-kernel/20240503130147.1154804-8-joey.gouly@arm.com/ (local)
   https://lore.kernel.org/linux-arm-kernel/864jagmxn7.wl-maz@kernel.org/ (local)

in which you didn't note any concerns about the handling for the
sysregs.

If your decisions have changed then you'll need to withdraw your review
there, I'd figured that given the current incompleteness of the
writability conversions and there being a bunch of existing registers
exposed unconditionally you'd decided to defer until some more general
cleanup of the situation.

Thanks for pointing out that I missed this crucial detail in the POE
series. I'll immediately go and point that out.

	M.

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