Re: [PATCH 1/2] powerpc/64s: Fix crashes when toggling stf barrier

Nathan Lynch [off-list ref] writes:

Michael Ellerman [off-list ref] writes:

quoted

-void do_stf_barrier_fixups(enum stf_barrier_type types)
+static int __do_stf_barrier_fixups(void *data)
 {
+	enum stf_barrier_type types = (enum stf_barrier_type)data;
+
 	do_stf_entry_barrier_fixups(types);
 	do_stf_exit_barrier_fixups(types);
+
+	return 0;
+}
+
+void do_stf_barrier_fixups(enum stf_barrier_type types)
+{
+	/*
+	 * The call to the fallback entry flush, and the fallback/sync-ori exit
+	 * flush can not be safely patched in/out while other CPUs are executing
+	 * them. So call __do_stf_barrier_fixups() on one CPU while all other CPUs
+	 * spin in the stop machine core with interrupts hard disabled.
+	 */
+	stop_machine_cpuslocked(__do_stf_barrier_fixups, (void *)types, NULL);

Would it be preferable to avoid the explicit casts:

	stop_machine_cpuslocked(__do_stf_barrier_fixups, &types, NULL);

...

static int __do_stf_barrier_fixups(void *data)
{
	enum stf_barrier_type *types = data;

 	do_stf_entry_barrier_fixups(*types);
 	do_stf_exit_barrier_fixups(*types);

?

Yes.

That will also avoid the pesky issue of undefined behaviour :facepalm:

post_mobility_fixup() does cpus_read_unlock() before calling
pseries_setup_security_mitigations(), I think that will need to be
changed?

I don't think so.

I'm using stop_machine_cpuslocked() but that's because I'm a goose and
forgot to switch to stop_machine() after I reworked the code to not take
cpus_read_lock() by hand. I really shouldn't send patches after 11pm.

I don't think it's important to keep the cpus lock held from where we
take it in post_mobility_fixup(). If some CPUs come or go between there
and here that's fine.

I'll send a v2.

cheers