[Xen-devel] [PATCH for-next 1/2] xen/x86/alternatives: Do not use sync_core() to serialize I$

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

From: Borislav Petkov <bp@xxxxxxx>

We use sync_core() in the alternatives code to stop speculative
execution of prefetched instructions because we are potentially changing
them and don't want to execute stale bytes.

What it does on most machines is call CPUID which is a serializing
instruction. And that's expensive.

However, the instruction cache is serialized when we're on the local CPU
and are changing the data through the same virtual address. So then, we
don't need the serializing CPUID but a simple control flow change. Last
being accomplished with a CALL/RET which the noinline causes.

Suggested-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Borislav Petkov <bp@xxxxxxx>
Reviewed-by: Andy Lutomirski <luto@xxxxxxxxxx>
[Linux commit 34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466]

Ported to Xen.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
---
 xen/arch/x86/alternative.c | 11 ++++++-----
 xen/arch/x86/livepatch.c   | 20 +++++++++++++++-----
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 6eaa10f..65062c2 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -128,13 +128,14 @@ void init_or_livepatch add_nops(void *insns, unsigned int 
len)
  *
  * 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.
  */
-static void *init_or_livepatch text_poke(void *addr, const void *opcode, 
size_t len)
+static void *init_or_livepatch noinline
+text_poke(void *addr, const void *opcode, size_t len)
 {
-    memcpy(addr, opcode, len);
-    sync_core();
-
-    return addr;
+    return memcpy(addr, opcode, len);
 }
 
 /*
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 9663ef6..dd50dd1 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -46,7 +46,11 @@ int arch_livepatch_verify_func(const struct livepatch_func 
*func)
     return 0;
 }
 
-void arch_livepatch_apply(struct livepatch_func *func)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+void noinline arch_livepatch_apply(struct livepatch_func *func)
 {
     uint8_t *old_ptr;
     uint8_t insn[sizeof(func->opaque)];
@@ -75,15 +79,21 @@ void arch_livepatch_apply(struct livepatch_func *func)
     memcpy(old_ptr, insn, len);
 }
 
-void arch_livepatch_revert(const struct livepatch_func *func)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+void noinline arch_livepatch_revert(const struct livepatch_func *func)
 {
     memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
 }
 
-/* Serialise the CPU pipeline. */
-void arch_livepatch_post_action(void)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+void noinline arch_livepatch_post_action(void)
 {
-    cpuid_eax(0);
 }
 
 static nmi_callback_t *saved_nmi_callback;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel