On Mon, Sep 13 2021 at 13:01, Sohil Mehta wrote:
User interrupt state is saved and restored using xstate supervisorAnd this special handling is?
feature support. This includes the MSR state and the User Interrupt Flag
(UIF) value.
During context switch update the UPID for a uintr task to reflect the
current state of the task; namely whether the task should receive
interrupt notifications and which cpu the task is currently running on.
XSAVES clears the notification vector (UINV) in the MISC MSR to prevent
interrupts from being recognized in the UIRR MSR while the task is being
context switched. The UINV is restored back when the kernel does an
XRSTORS.
However, this conflicts with the kernel's lazy restore optimization
which skips an XRSTORS if the kernel is scheduling the same user task
back and the underlying MSR state hasn't been modified. Special handling
is needed for a uintr task in the context switch path to keep using this
optimization.
+ * cleared.cpu_feature_enabled() please.
*/
void save_fpregs_to_fpstate(struct fpu *fpu)
{
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index ec0d836a13b1..62b82137db9c 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -53,6 +53,7 @@
#include <asm/xen/hypervisor.h>
#include <asm/vdso.h>
#include <asm/resctrl.h>
+#include <asm/uintr.h>
#include <asm/unistd.h>
#include <asm/fsgsbase.h>
#ifdef CONFIG_IA32_EMULATION
@@ -565,6 +566,9 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
WARN_ON_ONCE(IS_ENABLED(CONFIG_DEBUG_ENTRY) &&
this_cpu_read(hardirq_stack_inuse));
+ if (static_cpu_has(X86_FEATURE_UINTR))
Ok, will do. The SN bit could be read concurrently on another CPU executing SENDUIPI.+ switch_uintr_prepare(prev_p);Please add a comment why this needs to be a locked instruction.
+
if (!test_thread_flag(TIF_NEED_FPU_LOAD))
switch_fpu_prepare(prev_fpu, cpu);
diff --git a/arch/x86/kernel/uintr_core.c b/arch/x86/kernel/uintr_core.c
index 2c6042a6840a..7a29888050ad 100644
--- a/arch/x86/kernel/uintr_core.c
+++ b/arch/x86/kernel/uintr_core.c
@@ -238,3 +238,78 @@ int do_uintr_register_handler(u64 handler)
return 0;
}
+
+/* Suppress notifications since this task is being context switched out */
+void switch_uintr_prepare(struct task_struct *prev)
+{
+ struct uintr_upid *upid;
+
+ if (is_uintr_receiver(prev)) {
+ upid = prev->thread.ui_recv->upid_ctx->upid;
+ set_bit(UPID_SN, (unsigned long *)&upid->nc.status);
Of course this is invoked unconditionally when the CPU hasThe pseudo code in patch 5 covers this. I'll fix the code based on that.
X86_FEATURE_UINTR:
+ if (static_cpu_has(X86_FEATURE_UINTR))Why?
+ switch_uintr_return();
If the sequence is:
syscall()
do_stuff()
return_to_user()
then what on earth has modified that MSR state? Nothing at all, but you
still run this code. What for?