On Mon, Aug 19, 2024 at 04:44:42PM +0100, Mark Brown wrote:

On Mon, Aug 19, 2024 at 12:46:13PM +0100, Catalin Marinas wrote:

quoted

On Thu, Aug 01, 2024 at 01:06:46PM +0100, Mark Brown wrote:

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 the GCSB DSYNC supposed to do here? The Arm ARM talks about
ordering between GCS memory effects and other memory effects. I haven't
looked at the memory model in detail yet (D11.9.1) but AFAICT it has
nothing to do with the system registers. We'll need this barrier when
ordering is needed between explicit or implicit (e.g. BL) GCS accesses
and the explicit classic memory accesses. Paging comes to mind, so maybe
flush_dcache_page() would need this barrier. ptrace() is another case if
the memory accessed is a GCS page. I can see you added it in other
places, I'll have a look as I go through the rest. But I don't think one
is needed here.

It's not particuarly for the system registers, is there's so that
anything else that looks at the task's GCS sees the current state.

Ah, so that's the to ensure that any writes on the CPU to the GCS stack
would be observable if the task appears on a different CPU (together
with the additional classic ordering/spinlocks used for the run queues).
Maybe update the comment to say "GCS memory effects" instead of "GCS
changes". I read the latter as GCS sysreg changes. Something like below
would make it clearer:

	/*
	 * Ensure that GCS memory effects of the 'prev' thread are
	 * ordered before other memory accesses with release semantics
	 * (or preceded by a DMB) on the current PE. In addition, any
	 * memory accesses with acquire semantics (or succeeded by a
	 * DMB) are ordered before GCS memory effects of the 'next'
	 * thread. This will ensure that the GCS memory effects are
	 * visible to other PEs in case of migration.
	 */

Feel free to rephrase as you see fit.

I'm pretty confident this excessive, the goal was to err on the side
of correctness and then relax later.

I think we are missing some. Paging should be ok as we have a pte change
and TLBI and IIRC the same rules as for standard memory accesses apply.
ptrace() memory accesses may need something though I'm fine with
considering this a best effort (we can't guarantee anyway if any
accesses are on different CPUs). I haven't got to the signal handling
patch yet.

-- 
Catalin