On Fri, May 19, 2017 at 07:49:06PM +0100, Andrew Cooper wrote:
> From: Borislav Petkov <b...@suse.de>
>
> 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 <torva...@linux-foundation.org>
> Signed-off-by: Borislav Petkov <b...@suse.de>
> Reviewed-by: Andy Lutomirski <l...@kernel.org>
> [Linux commit 34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466]
>
> Ported to Xen.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
> ---
> CC: Jan Beulich <jbeul...@suse.com>
> ---
> xen/arch/x86/alternative.c | 11 ++++++-----
> xen/arch/x86/livepatch.c | 20 +++++++++++++++-----
Weird that the CC list didn't pick me up.
But Acked-by on the livepatch part.
> 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@lists.xen.org
> https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
