On Wed, Aug 16, 2023 at 07:15:53PM +0100, Mark Brown wrote:

On Fri, Aug 11, 2023 at 04:32:10PM +0100, Catalin Marinas wrote:

quoted

On Mon, Aug 07, 2023 at 11:00:23PM +0100, Mark Brown wrote:

quoted

quoted

+		gcs_free(current);
+		current->thread.gcs_el0_mode = 0;
+		write_sysreg_s(0, SYS_GCSCRE0_EL1);
+		write_sysreg_s(0, SYS_GCSPR_EL0);
+	}
+}

quoted

Do we need and isb() or there's one on this path? If it's only EL0
making use of this register, we should be fine with the ERET before
returning to user. Not sure whether the kernel uses this, GCSSTTR
doesn't need it.

They're only used by EL0, at EL1 we do read GCSPR for signal handling
but AIUI that shouldn't be any more of an issue than it is for the
TPIDRs which we don't have a barrier for.  It's possible I'm
misunderstanding though.

We should be alright without since we'll eventually have an ERET to EL0.

quoted

quoted

+	/*
+	 * Ensure that GCS changes are observable by/from other PEs in
+	 * case of migration.
+	 */
+	if (task_gcs_el0_enabled(current) || task_gcs_el0_enabled(next))
+		gcsb_dsync();

quoted

What's this barrier for? The spec (at least the version I have) only
talks about accesses, nothing to do with the registers that we context
switch here.

Right, it's for the GCS memory rather than the registers.  I'm fairly
sure it's excessive but but was erring on the side of caution until I
have convinced myself that the interactions between GCS barriers and
regular barriers were doing the right thing, until we have physical
implementations to contend with I'd guess the practical impact will be
minimal.

Well, I'd say either we are clear about why it's (not) needed or we ask
the architects to clarify the spec. I haven't checked your latest
series but in principle I don't like adding barriers just because we are
not sure they are needed (and I don't think having hardware eventually
changes this).

-- 
Catalin