Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 2 Dec 2016 11:03:50 -0800 | Subject | Re: [PATCH v2 5/6] x86/xen: Add a Xen-specific sync_core() implementation |
| |
On Fri, Dec 2, 2016 at 10:50 AM, Borislav Petkov <bp@kernel.org> wrote: > > Right, we can try to do something like invalidate_icache() or so in > there with the JMP so that the BSP refetches modified code and see where > it gets us.
I'd really rather rjust mark it noinline with a comment. That way the return from the function acts as the control flow change.
> The good thing is, the early patching paths run before SMP is > up but from looking at load_module(), for example, which does > post_relocation()->module_finalize()->apply_alternatives(), this can > happen late. > > Now there I'd like to avoid other cores walking into that address being > patched. Or are we "safe" there in the sense that load_module() happens > on one CPU only sequentially? (I haven't looked at that code to see > what's going on there, actually).
'sync_core()' doesn't help for other CPU's anyway, you need to do the cross-call IPI. So worrying about other CPU's is *not* a valid reason to keep a "sync_core()" call.
Seriously, the only reason I can see for "sync_core()" really is:
- some deep non-serialized MSR access or similar (ie things like firmware loading etc really might want it, and a mchine check might want it)
- writing to one virtual address, and then executing from another. We do this for (a) user mode (of course) and (b) text_poke(), but text_poke() is a whole different dance.
but I may have forgotten some other case.
The issues with modifying code while another CPU may be just about to access it is a separate issue. And as noted, "sync_core()" is not sufficient for that, you have to do a whole careful dance with single-byte debug instruction writes and then a final cross-call.
See the whole "text_poke_bp()" and "text_poke()" for *that* whole dance. That's a much more complex thing from the normal apply_alternatives().
Linus
| |