Re: [PATCH v3 3/5] x86/mm: Optionally flush L1D on context switch

2020-12-04 Thread Singh, Balbir
On Fri, 2020-12-04 at 22:21 +0100, Thomas Gleixner wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you can confirm the sender and know the 
> content is safe.
> 
> 
> 
> On Fri, Nov 27 2020 at 17:59, Balbir Singh wrote:
> > 
> > + /*
> > +  * Flush only if SMT is disabled as per the contract, which is checked
> > +  * when the feature is enabled.
> > +  */
> > + if (sched_smt_active() && !this_cpu_read(cpu_info.smt_active) &&
> > + (prev_mm & LAST_USER_MM_L1D_FLUSH))
> > + l1d_flush_hw();
> 
> So if SMT is completely disabled then no flush? Shouldn't the logic be:
> 
> if ((!sched_smt_active() || !this_cpu_read(cpu_info.smt_active) &&
>  (prev_mm & LAST_USER_MM_L1D_FLUSH))
> 
> Hmm?
> 
> But that's bad, because it's lot's of conditions to evaluate for every
> switch_mm where most of them are not interested in it at all.
> 
> Let me read through the rest of the pile.
>


We don't need this anymore with the new checks for preempting killing
of the task, so it can be removed

Balbir 


Re: [PATCH v3 3/5] x86/mm: Optionally flush L1D on context switch

2020-12-04 Thread Thomas Gleixner
On Fri, Nov 27 2020 at 17:59, Balbir Singh wrote:
>  
> + /*
> +  * Flush only if SMT is disabled as per the contract, which is checked
> +  * when the feature is enabled.
> +  */
> + if (sched_smt_active() && !this_cpu_read(cpu_info.smt_active) &&
> + (prev_mm & LAST_USER_MM_L1D_FLUSH))
> + l1d_flush_hw();

So if SMT is completely disabled then no flush? Shouldn't the logic be:

if ((!sched_smt_active() || !this_cpu_read(cpu_info.smt_active) &&
 (prev_mm & LAST_USER_MM_L1D_FLUSH))

Hmm?

But that's bad, because it's lot's of conditions to evaluate for every
switch_mm where most of them are not interested in it at all.

Let me read through the rest of the pile.

Thanks,

tglx




[PATCH v3 3/5] x86/mm: Optionally flush L1D on context switch

2020-11-26 Thread Balbir Singh
Implement a mechanism to selectively flush the L1D cache. The goal is to
allow tasks that want to save sensitive information, found by the recent
snoop assisted data sampling vulnerabilites, to flush their L1D on being
switched out.  This protects their data from being snooped or leaked via
side channels after the task has context switched out.

There are two scenarios we might want to protect against, a task leaving
the CPU with data still in L1D (which is the main concern of this patch),
the second scenario is a malicious task coming in (not so well trusted)
for which we want to clean up the cache before it starts. Only the case
for the former is addressed.

A new thread_info flag TIF_SPEC_L1D_FLUSH is added to track tasks which
opt-into L1D flushing. cpu_tlbstate.last_user_mm_spec is used to convert
the TIF flags into mm state (per cpu via last_user_mm_spec) in
cond_mitigation(), which then used to do decide when to flush the
L1D cache.

A new helper inline function l1d_flush_hw() has been introduced.
Currently it returns an error code if hardware flushing is not
supported.  The caller currently does not check the return value, in the
context of these patches, the routine is called only when HW assisted
flushing is available.

Suggested-by: Thomas Gleixner 
Signed-off-by: Balbir Singh 
Signed-off-by: Thomas Gleixner 
Link: https://lore.kernel.org/r/20200729001103.6450-4-sbl...@amazon.com
---
 arch/x86/include/asm/cacheflush.h  |  8 
 arch/x86/include/asm/thread_info.h |  9 +++--
 arch/x86/mm/tlb.c  | 30 +++---
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/cacheflush.h 
b/arch/x86/include/asm/cacheflush.h
index b192d917a6d0..554eaf697f3f 100644
--- a/arch/x86/include/asm/cacheflush.h
+++ b/arch/x86/include/asm/cacheflush.h
@@ -10,4 +10,12 @@
 
 void clflush_cache_range(void *addr, unsigned int size);
 
+static inline int l1d_flush_hw(void)
+{
+   if (static_cpu_has(X86_FEATURE_FLUSH_L1D)) {
+   wrmsrl(MSR_IA32_FLUSH_CMD, L1D_FLUSH);
+   return 0;
+   }
+   return -EOPNOTSUPP;
+}
 #endif /* _ASM_X86_CACHEFLUSH_H */
diff --git a/arch/x86/include/asm/thread_info.h 
b/arch/x86/include/asm/thread_info.h
index 44733a4bfc42..36a11cfb1061 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -84,7 +84,7 @@ struct thread_info {
 #define TIF_SYSCALL_AUDIT  7   /* syscall auditing active */
 #define TIF_SECCOMP8   /* secure computing */
 #define TIF_SPEC_IB9   /* Indirect branch speculation 
mitigation */
-#define TIF_SPEC_FORCE_UPDATE  10  /* Force speculation MSR update in 
context switch */
+#define TIF_SPEC_L1D_FLUSH 10  /* Flush L1D on mm switches (processes) 
*/
 #define TIF_USER_RETURN_NOTIFY 11  /* notify kernel of userspace return */
 #define TIF_UPROBE 12  /* breakpointed or singlestepping */
 #define TIF_PATCH_PENDING  13  /* pending live patching update */
@@ -96,6 +96,7 @@ struct thread_info {
 #define TIF_MEMDIE 20  /* is terminating due to OOM killer */
 #define TIF_POLLING_NRFLAG 21  /* idle is polling for TIF_NEED_RESCHED 
*/
 #define TIF_IO_BITMAP  22  /* uses I/O bitmap */
+#define TIF_SPEC_FORCE_UPDATE  23  /* Force speculation MSR update in 
context switch */
 #define TIF_FORCED_TF  24  /* true if TF in eflags artificially */
 #define TIF_BLOCKSTEP  25  /* set when we want DEBUGCTLMSR_BTF */
 #define TIF_LAZY_MMU_UPDATES   27  /* task is updating the mmu lazily */
@@ -113,7 +114,7 @@ struct thread_info {
 #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP   (1 << TIF_SECCOMP)
 #define _TIF_SPEC_IB   (1 << TIF_SPEC_IB)
-#define _TIF_SPEC_FORCE_UPDATE (1 << TIF_SPEC_FORCE_UPDATE)
+#define _TIF_SPEC_L1D_FLUSH(1 << TIF_SPEC_L1D_FLUSH)
 #define _TIF_USER_RETURN_NOTIFY(1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_UPROBE(1 << TIF_UPROBE)
 #define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -124,6 +125,7 @@ struct thread_info {
 #define _TIF_SLD   (1 << TIF_SLD)
 #define _TIF_POLLING_NRFLAG(1 << TIF_POLLING_NRFLAG)
 #define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP)
+#define _TIF_SPEC_FORCE_UPDATE (1 << TIF_SPEC_FORCE_UPDATE)
 #define _TIF_FORCED_TF (1 << TIF_FORCED_TF)
 #define _TIF_BLOCKSTEP (1 << TIF_BLOCKSTEP)
 #define _TIF_LAZY_MMU_UPDATES  (1 << TIF_LAZY_MMU_UPDATES)
@@ -228,6 +230,9 @@ static inline int arch_within_stack_frames(const void * 
const stack,
   current_thread_info()->status & TS_COMPAT)
 #endif
 
+extern int enable_l1d_flush_for_task(struct task_struct *tsk);
+extern int disable_l1d_flush_for_task(struct task_struct *tsk);
+
 extern void arch_task_cache_init(void);
 extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct