Re: [Patch v4 17/18] x86/speculation: Update SPEC_CTRL MSRs of remote CPUs

[Tim Chen]

Thomas,

>> +	if (tifp & _TIF_UPDATE_SPEC_CTRL)
>> +		clear_tsk_thread_flag(prev_p, TIF_UPDATE_SPEC_CTRL);
>> +
>> +	if (tifn & _TIF_UPDATE_SPEC_CTRL) {
>> +		clear_tsk_thread_flag(next_p, TIF_UPDATE_SPEC_CTRL);
>> +		tifn &= ~_TIF_UPDATE_SPEC_CTRL;
>> +	}
> 
> I'm really unhappy about adding yet more conditionals into this code
> path. We really need to find some better solution for that.
> 
> There are basically two options:
> 
>  1) Restrict the PRCTL control so it is only possible to modify it at the
>     point where the application is still single threaded.
> 
>  2) Add _TIF_UPDATE_SPEC_CTRL to the SYSCALL_EXIT_WORK_FLAGS and handle it
>     in the slow work path.
> 
>     The KVM side can be handled in x86_virt_spec_ctrl().
> 

How about sending an IPI if a remote CPU needs to have its SPEC_CTRL MSR
updated?

Something like the following to replace this patch?

Tim

---

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index b8103991..7505925 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -772,9 +772,15 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
 	return 0;
 }
 
+static void spec_ctrl_update_func(void *info)
+{
+	speculation_ctrl_update(task_thread_info(current)->flags);
+}
+
 static void set_task_stibp(struct task_struct *tsk, bool stibp_on)
 {
 	bool update = false;
+	int cpu;
 
 	if (!static_branch_unlikely(&spectre_v2_app_lite))
 		return;
@@ -789,6 +795,12 @@ static void set_task_stibp(struct task_struct *tsk, bool stibp_on)
 
 	if (tsk == current)
 		speculation_ctrl_update_current();
+	else {
+		cpu = task_cpu(tsk);
+		if (cpu != smp_processor_id())
+			smp_call_function_single(cpu, spec_ctrl_update_func,
+						 NULL, false); 
+	}
 }
 
 void arch_set_security(struct task_struct *tsk, unsigned int value)