While supposedly safe via enforcing a control flow change when modifying
already prefetched code, it may not really be. Afaik a request is
pending to drop the first of the two options in the SDM's "Handling
Self- and Cross-Modifying Code" section (still present there as of
version 087). Insert a serializing instruction there, and remove the
"noinline" in exchange.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Strictly speaking with LIVEPATCH=n the if() (but not its body) is dead
code. Adding IS_ENABLED(CONFIG_LIVEPATCH) would feel quite a bit like
extra clutter, though. Plus Misra rule 2.2 is globally deviated anyway.
---
v6: Split off. Extend commentary. Load deterministic value into CR2.
Drop "noinline".
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -194,13 +194,18 @@ void *place_ret(void *ptr)
* You should run this with interrupts disabled or on code that is not
* executing.
*
- * "noinline" to cause control flow change and thus invalidate I$ and
- * cause refetch after modification.
+ * While the SDM continues to suggest using "noinline" would be sufficient, it
+ * may not be, e.g. due to errata. Issue a serializing insn afterwards, unless
+ * this is for live-patching, where we modify code before it goes live. Issue
+ * a serializing insn which is unlikely to be intercepted by a hypervisor, in
+ * case we run virtualized ourselves.
*/
-static void init_or_livepatch noinline
+static void init_or_livepatch
text_poke(void *addr, const void *opcode, size_t len)
{
memcpy(addr, opcode, len);
+ if ( system_state < SYS_STATE_active )
+ asm volatile ( "mov %0, %%cr2" :: "r" (0L) : "memory" );
}
extern void *const __initdata_cf_clobber_start[];