On Thu, Oct 21, 2021 at 03:39:35PM -0300, Marcelo Tosatti wrote:

Peter,

static __always_inline void arch_exit_to_user_mode(void)
{
        mds_user_clear_cpu_buffers();
}

/**
 * mds_user_clear_cpu_buffers - Mitigation for MDS and TAA vulnerability
 *
 * Clear CPU buffers if the corresponding static key is enabled
 */
static __always_inline void mds_user_clear_cpu_buffers(void)
{
        if (static_branch_likely(&mds_user_clear))
                mds_clear_cpu_buffers();
}

We were discussing how to perform objtool style validation 
that no code after the check for 

I'm not sure what the point of the above is... Were you trying to ask
for validation that nothing runs after the mds_user_clear_cpu_buffer()?

That isn't strictly true today, there's lockdep code after it. I can't
recall why that order is as it is though.

Pretty much everything in noinstr is magical, we just have to think
harder there (and possibly start writing more comments there).

quoted

+             /* NMI happens here and must still do/finish CT_WORK_n */
+             sync_core();

But after the discussion with you, it seems doing the TLB checking 
and (also sync_core) checking very late/very early on exit/entry 
makes things easier to review.

I don't know about late, it must happen *very* early in entry. The
sync_core() must happen before any self-modifying code gets called
(static_branch, static_call, etc..) with possible exception of the
context_tracking static_branch.

The TLBi must also happen super early, possibly while still on the
entry stack (since the task stack is vmap'ed). We currently don't run C
code on the entry stack, that needs quite a bit of careful work to make
happen.

Can then use a single atomic variable with USER/KERNEL state and cmpxchg
loops.

We're not going to add an atomic to context tracking. There is one, we
just got to extract/share it with RCU.