[PATCH v6 2/7] x86/alternatives: serialize after (self-)modifying code

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>
---
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[];