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:
quoted
On Thu, Oct 21, 2021 at 04:57:09PM -0300, Marcelo Tosatti wrote:
quoted
quoted
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@worktop.programming.kicks-ass.net (local)