On Tue, Oct 26, 2021 at 03:19:11PM -0300, Marcelo Tosatti wrote:
> On Thu, Oct 21, 2021 at 10:18:59PM +0200, Peter Zijlstra wrote:
> > On Thu, Oct 21, 2021 at 04:57:09PM -0300, Marcelo Tosatti wrote:
> > > > Pretty much everything in noinstr is magical, we just have to think
> > > > harder there (and possibly start writing more comments there).
> > >
> > > mds_user_clear_cpu_buffers happens after sync_core, in your patchset,
> > > if i am not mistaken.
> >
> > Of course it does, mds_user_clear_cpu_buffers() is on exit, the
> > sync_core() is on entry.
>
> static_key enable/disable
>
> __exit_to_user_mode -> context_tracking_set_cpu_work(cpu, work)
> user_enter_irqoff -> preempt_disable();
> __context_tracking_enter(CONTEXT_USER); seq = atomic_read(&ct->seq);
> ct_seq_user_enter(raw_cpu_ptr(&context_tracking)); if (__context_tracking_seq_in_user(seq)) {
> { /* ctrl-dep */
> arch_atomic_set(&ct->work, 0); atomic_or(work, &ct->work);
> return arch_atomic_add_return(CT_SEQ_USER, &ct->seq); ret = atomic_try_cmpxchg(&ct->seq, &seq, seq|CT_SEQ_WORK);
> }
> } preempt_enable();
> arch_exit_to_user_mode()
> mds_user_clear_cpu_buffers(); <--- sync_core work queued,
> but not executed.
> i-cache potentially stale?
>
> ct_seq_user_enter should happen _after_ all possible static_key users?
Right, so this one is actually okay, because that branch is *never*
changed after boot.
I'm not quite sure why it isn't an alternative(). At some point I
proposed static_call_lock() [1] and the corrolary is static_branch_lock(),
which I suppose could be employed here. But I'm not sure that actually
helps much with auditing all that.
[1] https://lkml.kernel.org/r/20210904105529.GA5106@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
