* [PATCH -tip v3 0/2] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes
@ 2019-09-06 1:45 ` Masami Hiramatsu
0 siblings, 0 replies; 16+ messages in thread
From: Masami Hiramatsu @ 2019-09-06 1:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: Josh Poimboeuf, Andrew Cooper, Peter Zijlstra, Randy Dunlap,
Borislav Petkov, Juergen Gross, Boris Ostrovsky,
Stefano Stabellini, x86, linux-kernel, xen-devel
Hi,
Here is the 3rd version of patches to handle Xen/KVM emulate
prefix by x86 instruction decoder.
These patches allow x86 instruction decoder to decode
Xen and KVM emulate prefix correctly, and prohibit kprobes to
probe on it.
Josh reported that the objtool can not decode such special
prefixed instructions, and I found that we also have to
prohibit kprobes to probe on such instruction.
This series can be applied on -tip master branch which
has merged Josh's objtool/perf sharing common x86 insn
decoder series.
In the 2nd version, I added KVM emulate prefix support and generalized
the interface. (insn_has_xen_prefix -> insn_has_emulate_prefix)
Also, I added insn.emulate_prefix_size for those prefixes because
that prefix is NOT an x86 instruction prefix, and the next instruction
of those emulate prefixes can have x86 instruction prefix. So we
can not use insn.prefix for it.
In this 3rd version, I just fixed tools/perf/check-headers.sh so
that it can ignore the difference of xen/prefix header path.
Thank you,
---
Masami Hiramatsu (2):
x86: xen: insn: Decode Xen and KVM emulate-prefix signature
x86: kprobes: Prohibit probing on instruction which has emulate prefix
arch/x86/include/asm/insn.h | 6 +++++
arch/x86/include/asm/xen/interface.h | 7 ++++--
arch/x86/include/asm/xen/prefix.h | 10 +++++++++
arch/x86/kernel/kprobes/core.c | 4 +++
arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++
tools/arch/x86/include/asm/insn.h | 6 +++++
tools/arch/x86/include/asm/xen/prefix.h | 10 +++++++++
tools/arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++
tools/objtool/sync-check.sh | 3 ++-
tools/perf/check-headers.sh | 2 +-
10 files changed, 116 insertions(+), 4 deletions(-)
create mode 100644 arch/x86/include/asm/xen/prefix.h
create mode 100644 tools/arch/x86/include/asm/xen/prefix.h
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 16+ messages in thread* [Xen-devel] [PATCH -tip v3 0/2] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes @ 2019-09-06 1:45 ` Masami Hiramatsu 0 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 1:45 UTC (permalink / raw) To: Ingo Molnar Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky Hi, Here is the 3rd version of patches to handle Xen/KVM emulate prefix by x86 instruction decoder. These patches allow x86 instruction decoder to decode Xen and KVM emulate prefix correctly, and prohibit kprobes to probe on it. Josh reported that the objtool can not decode such special prefixed instructions, and I found that we also have to prohibit kprobes to probe on such instruction. This series can be applied on -tip master branch which has merged Josh's objtool/perf sharing common x86 insn decoder series. In the 2nd version, I added KVM emulate prefix support and generalized the interface. (insn_has_xen_prefix -> insn_has_emulate_prefix) Also, I added insn.emulate_prefix_size for those prefixes because that prefix is NOT an x86 instruction prefix, and the next instruction of those emulate prefixes can have x86 instruction prefix. So we can not use insn.prefix for it. In this 3rd version, I just fixed tools/perf/check-headers.sh so that it can ignore the difference of xen/prefix header path. Thank you, --- Masami Hiramatsu (2): x86: xen: insn: Decode Xen and KVM emulate-prefix signature x86: kprobes: Prohibit probing on instruction which has emulate prefix arch/x86/include/asm/insn.h | 6 +++++ arch/x86/include/asm/xen/interface.h | 7 ++++-- arch/x86/include/asm/xen/prefix.h | 10 +++++++++ arch/x86/kernel/kprobes/core.c | 4 +++ arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++ tools/arch/x86/include/asm/insn.h | 6 +++++ tools/arch/x86/include/asm/xen/prefix.h | 10 +++++++++ tools/arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++ tools/objtool/sync-check.sh | 3 ++- tools/perf/check-headers.sh | 2 +- 10 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 arch/x86/include/asm/xen/prefix.h create mode 100644 tools/arch/x86/include/asm/xen/prefix.h -- Masami Hiramatsu (Linaro) <mhiramat@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature 2019-09-06 1:45 ` [Xen-devel] " Masami Hiramatsu @ 2019-09-06 1:45 ` Masami Hiramatsu -1 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 1:45 UTC (permalink / raw) To: Ingo Molnar Cc: Josh Poimboeuf, Andrew Cooper, Peter Zijlstra, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel Decode Xen and KVM's emulate-prefix signature by x86 insn decoder. It is called "prefix" but actually not x86 instruction prefix, so this adds insn.emulate_prefix_size field instead of reusing insn.prefixes. If x86 decoder finds a special sequence of instructions of XEN_EMULATE_PREFIX and 'ud2a; .ascii "kvm"', it just counts the length, set insn.emulate_prefix_size and fold it with the next instruction. In other words, the signature and the next instruction is treated as a single instruction. Reported-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> --- Changes in v3: - Fix perf's check script too. Changes in v2: - Generalize the emulate-prefix handling not only for Xen but KVM. - Introduce insn.emulate_prefix_size instead of using insn.prefixes. --- arch/x86/include/asm/insn.h | 6 +++++ arch/x86/include/asm/xen/interface.h | 7 ++++-- arch/x86/include/asm/xen/prefix.h | 10 +++++++++ arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++ tools/arch/x86/include/asm/insn.h | 6 +++++ tools/arch/x86/include/asm/xen/prefix.h | 10 +++++++++ tools/arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++ tools/objtool/sync-check.sh | 3 ++- tools/perf/check-headers.sh | 2 +- 9 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 arch/x86/include/asm/xen/prefix.h create mode 100644 tools/arch/x86/include/asm/xen/prefix.h diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h index 154f27be8bfc..5c1ae3eff9d4 100644 --- a/arch/x86/include/asm/insn.h +++ b/arch/x86/include/asm/insn.h @@ -45,6 +45,7 @@ struct insn { struct insn_field immediate2; /* for 64bit imm or seg16 */ }; + int emulate_prefix_size; insn_attr_t attr; unsigned char opnd_bytes; unsigned char addr_bytes; @@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn) return (insn->vex_prefix.nbytes == 4); } +static inline int insn_has_emulate_prefix(struct insn *insn) +{ + return !!insn->emulate_prefix_size; +} + /* Ensure this instruction is decoded completely */ static inline int insn_complete(struct insn *insn) { diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h index 62ca03ef5c65..fe33a9798708 100644 --- a/arch/x86/include/asm/xen/interface.h +++ b/arch/x86/include/asm/xen/interface.h @@ -379,12 +379,15 @@ struct xen_pmu_arch { * Prefix forces emulation of some non-trapping instructions. * Currently only CPUID. */ +#include <asm/xen/prefix.h> + #ifdef __ASSEMBLY__ -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; #define XEN_CPUID XEN_EMULATE_PREFIX cpuid #else -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" + #endif #endif /* _ASM_X86_XEN_INTERFACE_H */ diff --git a/arch/x86/include/asm/xen/prefix.h b/arch/x86/include/asm/xen/prefix.h new file mode 100644 index 000000000000..f901be0d7a95 --- /dev/null +++ b/arch/x86/include/asm/xen/prefix.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H +#define _TOOLS_ASM_X86_XEN_PREFIX_H + +#include <linux/stringify.h> + +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) + +#endif diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c index 0b5862ba6a75..b7eb50187db9 100644 --- a/arch/x86/lib/insn.c +++ b/arch/x86/lib/insn.c @@ -13,6 +13,9 @@ #include <asm/inat.h> #include <asm/insn.h> +/* For special Xen prefix */ +#include <asm/xen/prefix.h> + /* Verify next sizeof(t) bytes can be on the same instruction */ #define validate_next(t, insn, n) \ ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) insn->addr_bytes = 4; } +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; +/* See handle_ud()@arch/x86/kvm/x86.c */ +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; + +static int __insn_get_emulate_prefix(struct insn *insn, + const insn_byte_t *prefix, size_t len) +{ + size_t i; + + for (i = 0; i < len; i++) { + if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i]) + goto err_out; + } + + insn->emulate_prefix_size = len; + insn->next_byte += len; + + return 1; + +err_out: + return 0; +} + +static void insn_get_emulate_prefix(struct insn *insn) +{ + if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix))) + return; + + __insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix)); +} + /** * insn_get_prefixes - scan x86 instruction prefix bytes * @insn: &struct insn containing instruction @@ -76,6 +110,8 @@ void insn_get_prefixes(struct insn *insn) if (prefixes->got) return; + insn_get_emulate_prefix(insn); + nb = 0; lb = 0; b = peek_next(insn_byte_t, insn); diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h index 37a4c390750b..568854b14d0a 100644 --- a/tools/arch/x86/include/asm/insn.h +++ b/tools/arch/x86/include/asm/insn.h @@ -45,6 +45,7 @@ struct insn { struct insn_field immediate2; /* for 64bit imm or seg16 */ }; + int emulate_prefix_size; insn_attr_t attr; unsigned char opnd_bytes; unsigned char addr_bytes; @@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn) return (insn->vex_prefix.nbytes == 4); } +static inline int insn_has_emulate_prefix(struct insn *insn) +{ + return !!insn->emulate_prefix_size; +} + /* Ensure this instruction is decoded completely */ static inline int insn_complete(struct insn *insn) { diff --git a/tools/arch/x86/include/asm/xen/prefix.h b/tools/arch/x86/include/asm/xen/prefix.h new file mode 100644 index 000000000000..f901be0d7a95 --- /dev/null +++ b/tools/arch/x86/include/asm/xen/prefix.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H +#define _TOOLS_ASM_X86_XEN_PREFIX_H + +#include <linux/stringify.h> + +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) + +#endif diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c index 79e048f1d902..ce04e43e0749 100644 --- a/tools/arch/x86/lib/insn.c +++ b/tools/arch/x86/lib/insn.c @@ -13,6 +13,9 @@ #include "../include/asm/inat.h" #include "../include/asm/insn.h" +/* For special Xen prefix */ +#include "../include/asm/xen/prefix.h" + /* Verify next sizeof(t) bytes can be on the same instruction */ #define validate_next(t, insn, n) \ ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) insn->addr_bytes = 4; } +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; +/* See handle_ud()@arch/x86/kvm/x86.c */ +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; + +static int __insn_get_emulate_prefix(struct insn *insn, + const insn_byte_t *prefix, size_t len) +{ + size_t i; + + for (i = 0; i < len; i++) { + if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i]) + goto err_out; + } + + insn->emulate_prefix_size = len; + insn->next_byte += len; + + return 1; + +err_out: + return 0; +} + +static void insn_get_emulate_prefix(struct insn *insn) +{ + if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix))) + return; + + __insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix)); +} + /** * insn_get_prefixes - scan x86 instruction prefix bytes * @insn: &struct insn containing instruction @@ -76,6 +110,8 @@ void insn_get_prefixes(struct insn *insn) if (prefixes->got) return; + insn_get_emulate_prefix(insn); + nb = 0; lb = 0; b = peek_next(insn_byte_t, insn); diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh index 0a832e265a50..34143ea3d477 100755 --- a/tools/objtool/sync-check.sh +++ b/tools/objtool/sync-check.sh @@ -4,6 +4,7 @@ FILES=' arch/x86/include/asm/inat_types.h arch/x86/include/asm/orc_types.h +arch/x86/include/asm/xen/prefix.h arch/x86/lib/x86-opcode-map.txt arch/x86/tools/gen-insn-attr-x86.awk ' @@ -46,6 +47,6 @@ done check arch/x86/include/asm/inat.h '-I "^#include [\"<]\(asm/\)*inat_types.h[\">]"' check arch/x86/include/asm/insn.h '-I "^#include [\"<]\(asm/\)*inat.h[\">]"' check arch/x86/lib/inat.c '-I "^#include [\"<]\(../include/\)*asm/insn.h[\">]"' -check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]"' +check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/xen/prefix.h[\">]"' cd - diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh index e2e0f06c97d0..edcffa55a826 100755 --- a/tools/perf/check-headers.sh +++ b/tools/perf/check-headers.sh @@ -115,7 +115,7 @@ check lib/ctype.c '-I "^EXPORT_SYMBOL" -I "^#include <linux/export.h>" -B check arch/x86/include/asm/inat.h '-I "^#include [\"<]\(asm/\)*inat_types.h[\">]"' check arch/x86/include/asm/insn.h '-I "^#include [\"<]\(asm/\)*inat.h[\">]"' check arch/x86/lib/inat.c '-I "^#include [\"<]\(../include/\)*asm/insn.h[\">]"' -check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]"' +check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/xen/prefix.h[\">]"' # diff non-symmetric files check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature @ 2019-09-06 1:45 ` Masami Hiramatsu 0 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 1:45 UTC (permalink / raw) To: Ingo Molnar Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky Decode Xen and KVM's emulate-prefix signature by x86 insn decoder. It is called "prefix" but actually not x86 instruction prefix, so this adds insn.emulate_prefix_size field instead of reusing insn.prefixes. If x86 decoder finds a special sequence of instructions of XEN_EMULATE_PREFIX and 'ud2a; .ascii "kvm"', it just counts the length, set insn.emulate_prefix_size and fold it with the next instruction. In other words, the signature and the next instruction is treated as a single instruction. Reported-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Acked-by: Josh Poimboeuf <jpoimboe@redhat.com> --- Changes in v3: - Fix perf's check script too. Changes in v2: - Generalize the emulate-prefix handling not only for Xen but KVM. - Introduce insn.emulate_prefix_size instead of using insn.prefixes. --- arch/x86/include/asm/insn.h | 6 +++++ arch/x86/include/asm/xen/interface.h | 7 ++++-- arch/x86/include/asm/xen/prefix.h | 10 +++++++++ arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++ tools/arch/x86/include/asm/insn.h | 6 +++++ tools/arch/x86/include/asm/xen/prefix.h | 10 +++++++++ tools/arch/x86/lib/insn.c | 36 +++++++++++++++++++++++++++++++ tools/objtool/sync-check.sh | 3 ++- tools/perf/check-headers.sh | 2 +- 9 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 arch/x86/include/asm/xen/prefix.h create mode 100644 tools/arch/x86/include/asm/xen/prefix.h diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h index 154f27be8bfc..5c1ae3eff9d4 100644 --- a/arch/x86/include/asm/insn.h +++ b/arch/x86/include/asm/insn.h @@ -45,6 +45,7 @@ struct insn { struct insn_field immediate2; /* for 64bit imm or seg16 */ }; + int emulate_prefix_size; insn_attr_t attr; unsigned char opnd_bytes; unsigned char addr_bytes; @@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn) return (insn->vex_prefix.nbytes == 4); } +static inline int insn_has_emulate_prefix(struct insn *insn) +{ + return !!insn->emulate_prefix_size; +} + /* Ensure this instruction is decoded completely */ static inline int insn_complete(struct insn *insn) { diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h index 62ca03ef5c65..fe33a9798708 100644 --- a/arch/x86/include/asm/xen/interface.h +++ b/arch/x86/include/asm/xen/interface.h @@ -379,12 +379,15 @@ struct xen_pmu_arch { * Prefix forces emulation of some non-trapping instructions. * Currently only CPUID. */ +#include <asm/xen/prefix.h> + #ifdef __ASSEMBLY__ -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; #define XEN_CPUID XEN_EMULATE_PREFIX cpuid #else -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" + #endif #endif /* _ASM_X86_XEN_INTERFACE_H */ diff --git a/arch/x86/include/asm/xen/prefix.h b/arch/x86/include/asm/xen/prefix.h new file mode 100644 index 000000000000..f901be0d7a95 --- /dev/null +++ b/arch/x86/include/asm/xen/prefix.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H +#define _TOOLS_ASM_X86_XEN_PREFIX_H + +#include <linux/stringify.h> + +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) + +#endif diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c index 0b5862ba6a75..b7eb50187db9 100644 --- a/arch/x86/lib/insn.c +++ b/arch/x86/lib/insn.c @@ -13,6 +13,9 @@ #include <asm/inat.h> #include <asm/insn.h> +/* For special Xen prefix */ +#include <asm/xen/prefix.h> + /* Verify next sizeof(t) bytes can be on the same instruction */ #define validate_next(t, insn, n) \ ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) insn->addr_bytes = 4; } +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; +/* See handle_ud()@arch/x86/kvm/x86.c */ +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; + +static int __insn_get_emulate_prefix(struct insn *insn, + const insn_byte_t *prefix, size_t len) +{ + size_t i; + + for (i = 0; i < len; i++) { + if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i]) + goto err_out; + } + + insn->emulate_prefix_size = len; + insn->next_byte += len; + + return 1; + +err_out: + return 0; +} + +static void insn_get_emulate_prefix(struct insn *insn) +{ + if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix))) + return; + + __insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix)); +} + /** * insn_get_prefixes - scan x86 instruction prefix bytes * @insn: &struct insn containing instruction @@ -76,6 +110,8 @@ void insn_get_prefixes(struct insn *insn) if (prefixes->got) return; + insn_get_emulate_prefix(insn); + nb = 0; lb = 0; b = peek_next(insn_byte_t, insn); diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h index 37a4c390750b..568854b14d0a 100644 --- a/tools/arch/x86/include/asm/insn.h +++ b/tools/arch/x86/include/asm/insn.h @@ -45,6 +45,7 @@ struct insn { struct insn_field immediate2; /* for 64bit imm or seg16 */ }; + int emulate_prefix_size; insn_attr_t attr; unsigned char opnd_bytes; unsigned char addr_bytes; @@ -128,6 +129,11 @@ static inline int insn_is_evex(struct insn *insn) return (insn->vex_prefix.nbytes == 4); } +static inline int insn_has_emulate_prefix(struct insn *insn) +{ + return !!insn->emulate_prefix_size; +} + /* Ensure this instruction is decoded completely */ static inline int insn_complete(struct insn *insn) { diff --git a/tools/arch/x86/include/asm/xen/prefix.h b/tools/arch/x86/include/asm/xen/prefix.h new file mode 100644 index 000000000000..f901be0d7a95 --- /dev/null +++ b/tools/arch/x86/include/asm/xen/prefix.h @@ -0,0 +1,10 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H +#define _TOOLS_ASM_X86_XEN_PREFIX_H + +#include <linux/stringify.h> + +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) + +#endif diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c index 79e048f1d902..ce04e43e0749 100644 --- a/tools/arch/x86/lib/insn.c +++ b/tools/arch/x86/lib/insn.c @@ -13,6 +13,9 @@ #include "../include/asm/inat.h" #include "../include/asm/insn.h" +/* For special Xen prefix */ +#include "../include/asm/xen/prefix.h" + /* Verify next sizeof(t) bytes can be on the same instruction */ #define validate_next(t, insn, n) \ ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) insn->addr_bytes = 4; } +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; +/* See handle_ud()@arch/x86/kvm/x86.c */ +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; + +static int __insn_get_emulate_prefix(struct insn *insn, + const insn_byte_t *prefix, size_t len) +{ + size_t i; + + for (i = 0; i < len; i++) { + if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i]) + goto err_out; + } + + insn->emulate_prefix_size = len; + insn->next_byte += len; + + return 1; + +err_out: + return 0; +} + +static void insn_get_emulate_prefix(struct insn *insn) +{ + if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix))) + return; + + __insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix)); +} + /** * insn_get_prefixes - scan x86 instruction prefix bytes * @insn: &struct insn containing instruction @@ -76,6 +110,8 @@ void insn_get_prefixes(struct insn *insn) if (prefixes->got) return; + insn_get_emulate_prefix(insn); + nb = 0; lb = 0; b = peek_next(insn_byte_t, insn); diff --git a/tools/objtool/sync-check.sh b/tools/objtool/sync-check.sh index 0a832e265a50..34143ea3d477 100755 --- a/tools/objtool/sync-check.sh +++ b/tools/objtool/sync-check.sh @@ -4,6 +4,7 @@ FILES=' arch/x86/include/asm/inat_types.h arch/x86/include/asm/orc_types.h +arch/x86/include/asm/xen/prefix.h arch/x86/lib/x86-opcode-map.txt arch/x86/tools/gen-insn-attr-x86.awk ' @@ -46,6 +47,6 @@ done check arch/x86/include/asm/inat.h '-I "^#include [\"<]\(asm/\)*inat_types.h[\">]"' check arch/x86/include/asm/insn.h '-I "^#include [\"<]\(asm/\)*inat.h[\">]"' check arch/x86/lib/inat.c '-I "^#include [\"<]\(../include/\)*asm/insn.h[\">]"' -check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]"' +check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/xen/prefix.h[\">]"' cd - diff --git a/tools/perf/check-headers.sh b/tools/perf/check-headers.sh index e2e0f06c97d0..edcffa55a826 100755 --- a/tools/perf/check-headers.sh +++ b/tools/perf/check-headers.sh @@ -115,7 +115,7 @@ check lib/ctype.c '-I "^EXPORT_SYMBOL" -I "^#include <linux/export.h>" -B check arch/x86/include/asm/inat.h '-I "^#include [\"<]\(asm/\)*inat_types.h[\">]"' check arch/x86/include/asm/insn.h '-I "^#include [\"<]\(asm/\)*inat.h[\">]"' check arch/x86/lib/inat.c '-I "^#include [\"<]\(../include/\)*asm/insn.h[\">]"' -check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]"' +check arch/x86/lib/insn.c '-I "^#include [\"<]\(../include/\)*asm/in\(at\|sn\).h[\">]" -I "^#include [\"<]\(../include/\)*asm/xen/prefix.h[\">]"' # diff non-symmetric files check_2 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl arch/x86/entry/syscalls/syscall_64.tbl _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature 2019-09-06 1:45 ` [Xen-devel] " Masami Hiramatsu @ 2019-09-06 7:34 ` Peter Zijlstra -1 siblings, 0 replies; 16+ messages in thread From: Peter Zijlstra @ 2019-09-06 7:34 UTC (permalink / raw) To: Masami Hiramatsu Cc: Ingo Molnar, Josh Poimboeuf, Andrew Cooper, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel On Fri, Sep 06, 2019 at 10:45:48AM +0900, Masami Hiramatsu wrote: > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h > index 62ca03ef5c65..fe33a9798708 100644 > --- a/arch/x86/include/asm/xen/interface.h > +++ b/arch/x86/include/asm/xen/interface.h > @@ -379,12 +379,15 @@ struct xen_pmu_arch { > * Prefix forces emulation of some non-trapping instructions. > * Currently only CPUID. > */ > +#include <asm/xen/prefix.h> > + > #ifdef __ASSEMBLY__ > -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; > +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; > #define XEN_CPUID XEN_EMULATE_PREFIX cpuid > #else > -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " > +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " > #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" > + > #endif Possibly you can do something like: #define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) #define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) > #endif /* _ASM_X86_XEN_INTERFACE_H */ > diff --git a/arch/x86/include/asm/xen/prefix.h b/arch/x86/include/asm/xen/prefix.h > new file mode 100644 > index 000000000000..f901be0d7a95 > --- /dev/null > +++ b/arch/x86/include/asm/xen/prefix.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H > +#define _TOOLS_ASM_X86_XEN_PREFIX_H > + > +#include <linux/stringify.h> > + > +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e > +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) > + > +#endif How about we make this asm/virt_prefix.h or something and include: /* * Virt escape sequences to trigger instruction emulation; * ideally these would decode to 'whole' instruction and not destroy * the instruction stream; sadly this is not true for the 'kvm' one :/ */ #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ > diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c > index 0b5862ba6a75..b7eb50187db9 100644 > --- a/arch/x86/lib/insn.c > +++ b/arch/x86/lib/insn.c > @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) > insn->addr_bytes = 4; > } > > +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; > +/* See handle_ud()@arch/x86/kvm/x86.c */ > +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; Then you can make this consistent; maybe even something like: static const insn_byte_t *virt_prefix[] = { { __XEN_EMULATE_PREFIX }, { __KVM_EMULATE_PREFIX }, { NULL }, }; And then change emulate_prefix_size to emulate_prefix_index ? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature @ 2019-09-06 7:34 ` Peter Zijlstra 0 siblings, 0 replies; 16+ messages in thread From: Peter Zijlstra @ 2019-09-06 7:34 UTC (permalink / raw) To: Masami Hiramatsu Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky, Ingo Molnar On Fri, Sep 06, 2019 at 10:45:48AM +0900, Masami Hiramatsu wrote: > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h > index 62ca03ef5c65..fe33a9798708 100644 > --- a/arch/x86/include/asm/xen/interface.h > +++ b/arch/x86/include/asm/xen/interface.h > @@ -379,12 +379,15 @@ struct xen_pmu_arch { > * Prefix forces emulation of some non-trapping instructions. > * Currently only CPUID. > */ > +#include <asm/xen/prefix.h> > + > #ifdef __ASSEMBLY__ > -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; > +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; > #define XEN_CPUID XEN_EMULATE_PREFIX cpuid > #else > -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " > +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " > #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" > + > #endif Possibly you can do something like: #define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) #define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) > #endif /* _ASM_X86_XEN_INTERFACE_H */ > diff --git a/arch/x86/include/asm/xen/prefix.h b/arch/x86/include/asm/xen/prefix.h > new file mode 100644 > index 000000000000..f901be0d7a95 > --- /dev/null > +++ b/arch/x86/include/asm/xen/prefix.h > @@ -0,0 +1,10 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H > +#define _TOOLS_ASM_X86_XEN_PREFIX_H > + > +#include <linux/stringify.h> > + > +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e > +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) > + > +#endif How about we make this asm/virt_prefix.h or something and include: /* * Virt escape sequences to trigger instruction emulation; * ideally these would decode to 'whole' instruction and not destroy * the instruction stream; sadly this is not true for the 'kvm' one :/ */ #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ > diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c > index 0b5862ba6a75..b7eb50187db9 100644 > --- a/arch/x86/lib/insn.c > +++ b/arch/x86/lib/insn.c > @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) > insn->addr_bytes = 4; > } > > +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; > +/* See handle_ud()@arch/x86/kvm/x86.c */ > +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; Then you can make this consistent; maybe even something like: static const insn_byte_t *virt_prefix[] = { { __XEN_EMULATE_PREFIX }, { __KVM_EMULATE_PREFIX }, { NULL }, }; And then change emulate_prefix_size to emulate_prefix_index ? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature 2019-09-06 7:34 ` [Xen-devel] " Peter Zijlstra @ 2019-09-06 8:45 ` Masami Hiramatsu -1 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 8:45 UTC (permalink / raw) To: Peter Zijlstra Cc: Ingo Molnar, Josh Poimboeuf, Andrew Cooper, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel On Fri, 6 Sep 2019 09:34:36 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Sep 06, 2019 at 10:45:48AM +0900, Masami Hiramatsu wrote: > > > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h > > index 62ca03ef5c65..fe33a9798708 100644 > > --- a/arch/x86/include/asm/xen/interface.h > > +++ b/arch/x86/include/asm/xen/interface.h > > @@ -379,12 +379,15 @@ struct xen_pmu_arch { > > * Prefix forces emulation of some non-trapping instructions. > > * Currently only CPUID. > > */ > > +#include <asm/xen/prefix.h> > > + > > #ifdef __ASSEMBLY__ > > -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; > > +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; > > #define XEN_CPUID XEN_EMULATE_PREFIX cpuid > > #else > > -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " > > +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " > > #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" > > + > > #endif > > Possibly you can do something like: > > #define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) > #define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) Hmm, OK. But should I split this change from insn decoder change? > > > #endif /* _ASM_X86_XEN_INTERFACE_H */ > > diff --git a/arch/x86/include/asm/xen/prefix.h b/arch/x86/include/asm/xen/prefix.h > > new file mode 100644 > > index 000000000000..f901be0d7a95 > > --- /dev/null > > +++ b/arch/x86/include/asm/xen/prefix.h > > @@ -0,0 +1,10 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H > > +#define _TOOLS_ASM_X86_XEN_PREFIX_H > > + > > +#include <linux/stringify.h> > > + > > +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e > > +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) > > + > > +#endif > > How about we make this asm/virt_prefix.h or something and include: > > /* > * Virt escape sequences to trigger instruction emulation; > * ideally these would decode to 'whole' instruction and not destroy > * the instruction stream; sadly this is not true for the 'kvm' one :/ > */ > > #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ > #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ OK, and in that case I think we should do this in separated patch from this change. > > > diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c > > index 0b5862ba6a75..b7eb50187db9 100644 > > --- a/arch/x86/lib/insn.c > > +++ b/arch/x86/lib/insn.c > > > @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) > > insn->addr_bytes = 4; > > } > > > > +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; > > +/* See handle_ud()@arch/x86/kvm/x86.c */ > > +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; > > Then you can make this consistent; maybe even something like: > > static const insn_byte_t *virt_prefix[] = { > { __XEN_EMULATE_PREFIX }, > { __KVM_EMULATE_PREFIX }, > { NULL }, > }; > > And then change emulate_prefix_size to emulate_prefix_index ? Hmm, how we can get the length of those emulate prefixes? For struct insn, since the size information is important, other sub fields have "nbyte" field so that caller can find the actual bytes from original byte stream. Maybe we can have struct emulate_prefix { .nbyte, .type }; and define enum etc.. but for me, it seems a bit over engineering. (since no one use that feature) Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature @ 2019-09-06 8:45 ` Masami Hiramatsu 0 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 8:45 UTC (permalink / raw) To: Peter Zijlstra Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky, Ingo Molnar On Fri, 6 Sep 2019 09:34:36 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Sep 06, 2019 at 10:45:48AM +0900, Masami Hiramatsu wrote: > > > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h > > index 62ca03ef5c65..fe33a9798708 100644 > > --- a/arch/x86/include/asm/xen/interface.h > > +++ b/arch/x86/include/asm/xen/interface.h > > @@ -379,12 +379,15 @@ struct xen_pmu_arch { > > * Prefix forces emulation of some non-trapping instructions. > > * Currently only CPUID. > > */ > > +#include <asm/xen/prefix.h> > > + > > #ifdef __ASSEMBLY__ > > -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; > > +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; > > #define XEN_CPUID XEN_EMULATE_PREFIX cpuid > > #else > > -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " > > +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " > > #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" > > + > > #endif > > Possibly you can do something like: > > #define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) > #define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) Hmm, OK. But should I split this change from insn decoder change? > > > #endif /* _ASM_X86_XEN_INTERFACE_H */ > > diff --git a/arch/x86/include/asm/xen/prefix.h b/arch/x86/include/asm/xen/prefix.h > > new file mode 100644 > > index 000000000000..f901be0d7a95 > > --- /dev/null > > +++ b/arch/x86/include/asm/xen/prefix.h > > @@ -0,0 +1,10 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _TOOLS_ASM_X86_XEN_PREFIX_H > > +#define _TOOLS_ASM_X86_XEN_PREFIX_H > > + > > +#include <linux/stringify.h> > > + > > +#define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e > > +#define __XEN_EMULATE_PREFIX_STR __stringify(__XEN_EMULATE_PREFIX) > > + > > +#endif > > How about we make this asm/virt_prefix.h or something and include: > > /* > * Virt escape sequences to trigger instruction emulation; > * ideally these would decode to 'whole' instruction and not destroy > * the instruction stream; sadly this is not true for the 'kvm' one :/ > */ > > #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ > #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ OK, and in that case I think we should do this in separated patch from this change. > > > diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c > > index 0b5862ba6a75..b7eb50187db9 100644 > > --- a/arch/x86/lib/insn.c > > +++ b/arch/x86/lib/insn.c > > > @@ -58,6 +61,37 @@ void insn_init(struct insn *insn, const void *kaddr, int buf_len, int x86_64) > > insn->addr_bytes = 4; > > } > > > > +static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX }; > > +/* See handle_ud()@arch/x86/kvm/x86.c */ > > +static const insn_byte_t kvm_prefix[] = "\xf\xbkvm"; > > Then you can make this consistent; maybe even something like: > > static const insn_byte_t *virt_prefix[] = { > { __XEN_EMULATE_PREFIX }, > { __KVM_EMULATE_PREFIX }, > { NULL }, > }; > > And then change emulate_prefix_size to emulate_prefix_index ? Hmm, how we can get the length of those emulate prefixes? For struct insn, since the size information is important, other sub fields have "nbyte" field so that caller can find the actual bytes from original byte stream. Maybe we can have struct emulate_prefix { .nbyte, .type }; and define enum etc.. but for me, it seems a bit over engineering. (since no one use that feature) Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature 2019-09-06 8:45 ` [Xen-devel] " Masami Hiramatsu @ 2019-09-06 8:51 ` Masami Hiramatsu -1 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 8:51 UTC (permalink / raw) To: Masami Hiramatsu Cc: Peter Zijlstra, Ingo Molnar, Josh Poimboeuf, Andrew Cooper, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel On Fri, 6 Sep 2019 17:45:19 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > How about we make this asm/virt_prefix.h or something and include: > > > > /* > > * Virt escape sequences to trigger instruction emulation; > > * ideally these would decode to 'whole' instruction and not destroy > > * the instruction stream; sadly this is not true for the 'kvm' one :/ > > */ > > > > #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ > > #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ BTW, what should we call it, "emulate prefix" or "virt prefix"? "virt prefix" sounds too generic to me. So I rather like emulate_prefix.h. Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature @ 2019-09-06 8:51 ` Masami Hiramatsu 0 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 8:51 UTC (permalink / raw) To: Masami Hiramatsu Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky, Ingo Molnar On Fri, 6 Sep 2019 17:45:19 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > How about we make this asm/virt_prefix.h or something and include: > > > > /* > > * Virt escape sequences to trigger instruction emulation; > > * ideally these would decode to 'whole' instruction and not destroy > > * the instruction stream; sadly this is not true for the 'kvm' one :/ > > */ > > > > #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ > > #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ BTW, what should we call it, "emulate prefix" or "virt prefix"? "virt prefix" sounds too generic to me. So I rather like emulate_prefix.h. Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature 2019-09-06 8:51 ` [Xen-devel] " Masami Hiramatsu @ 2019-09-06 9:15 ` Peter Zijlstra -1 siblings, 0 replies; 16+ messages in thread From: Peter Zijlstra @ 2019-09-06 9:15 UTC (permalink / raw) To: Masami Hiramatsu Cc: Ingo Molnar, Josh Poimboeuf, Andrew Cooper, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel On Fri, Sep 06, 2019 at 05:51:43PM +0900, Masami Hiramatsu wrote: > On Fri, 6 Sep 2019 17:45:19 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > > > > How about we make this asm/virt_prefix.h or something and include: > > > > > > /* > > > * Virt escape sequences to trigger instruction emulation; > > > * ideally these would decode to 'whole' instruction and not destroy > > > * the instruction stream; sadly this is not true for the 'kvm' one :/ > > > */ > > > > > > #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ > > > #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ > > BTW, what should we call it, "emulate prefix" or "virt prefix"? > "virt prefix" sounds too generic to me. So I rather like emulate_prefix.h. Works for me; and yeah, just see what is best for the other things. I only started down that road because the Xen and KVM 'prefixes' were initialized so inconsistently. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature @ 2019-09-06 9:15 ` Peter Zijlstra 0 siblings, 0 replies; 16+ messages in thread From: Peter Zijlstra @ 2019-09-06 9:15 UTC (permalink / raw) To: Masami Hiramatsu Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky, Ingo Molnar On Fri, Sep 06, 2019 at 05:51:43PM +0900, Masami Hiramatsu wrote: > On Fri, 6 Sep 2019 17:45:19 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > > > > How about we make this asm/virt_prefix.h or something and include: > > > > > > /* > > > * Virt escape sequences to trigger instruction emulation; > > > * ideally these would decode to 'whole' instruction and not destroy > > > * the instruction stream; sadly this is not true for the 'kvm' one :/ > > > */ > > > > > > #define __XEN_EMULATE_PREFIX 0x0f,0x0b,0x78,0x65,0x6e /* ud2 ; .ascii "xen" */ > > > #define __KVM_EMULATE_PREFIX 0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */ > > BTW, what should we call it, "emulate prefix" or "virt prefix"? > "virt prefix" sounds too generic to me. So I rather like emulate_prefix.h. Works for me; and yeah, just see what is best for the other things. I only started down that road because the Xen and KVM 'prefixes' were initialized so inconsistently. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature 2019-09-06 7:34 ` [Xen-devel] " Peter Zijlstra @ 2019-09-06 10:28 ` Masami Hiramatsu -1 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 10:28 UTC (permalink / raw) To: Peter Zijlstra Cc: Ingo Molnar, Josh Poimboeuf, Andrew Cooper, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel On Fri, 6 Sep 2019 09:34:36 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Sep 06, 2019 at 10:45:48AM +0900, Masami Hiramatsu wrote: > > > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h > > index 62ca03ef5c65..fe33a9798708 100644 > > --- a/arch/x86/include/asm/xen/interface.h > > +++ b/arch/x86/include/asm/xen/interface.h > > @@ -379,12 +379,15 @@ struct xen_pmu_arch { > > * Prefix forces emulation of some non-trapping instructions. > > * Currently only CPUID. > > */ > > +#include <asm/xen/prefix.h> > > + > > #ifdef __ASSEMBLY__ > > -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; > > +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; > > #define XEN_CPUID XEN_EMULATE_PREFIX cpuid > > #else > > -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " > > +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " > > #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" > > + > > #endif > > Possibly you can do something like: > > #define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) > #define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) Oops, this doesn't work, since __ASM_FORM(x) uses #x directly # define __ASM_FORM(x) " " #x " " which doesn't expand "x" if x is a macro. We have to use __stringify like # include <linux/stringify.h> # define __ASM_FORM(x) " " __stringify(x) " " So this needs aonther patch in the series :) Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature @ 2019-09-06 10:28 ` Masami Hiramatsu 0 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 10:28 UTC (permalink / raw) To: Peter Zijlstra Cc: Juergen Gross, Stefano Stabellini, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky, Ingo Molnar On Fri, 6 Sep 2019 09:34:36 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Sep 06, 2019 at 10:45:48AM +0900, Masami Hiramatsu wrote: > > > diff --git a/arch/x86/include/asm/xen/interface.h b/arch/x86/include/asm/xen/interface.h > > index 62ca03ef5c65..fe33a9798708 100644 > > --- a/arch/x86/include/asm/xen/interface.h > > +++ b/arch/x86/include/asm/xen/interface.h > > @@ -379,12 +379,15 @@ struct xen_pmu_arch { > > * Prefix forces emulation of some non-trapping instructions. > > * Currently only CPUID. > > */ > > +#include <asm/xen/prefix.h> > > + > > #ifdef __ASSEMBLY__ > > -#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; > > +#define XEN_EMULATE_PREFIX .byte __XEN_EMULATE_PREFIX ; > > #define XEN_CPUID XEN_EMULATE_PREFIX cpuid > > #else > > -#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " > > +#define XEN_EMULATE_PREFIX ".byte " __XEN_EMULATE_PREFIX_STR " ; " > > #define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" > > + > > #endif > > Possibly you can do something like: > > #define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;) > #define XEN_CPUID XEN_EMULATE_PREFIX __ASM_FORM(cpuid) Oops, this doesn't work, since __ASM_FORM(x) uses #x directly # define __ASM_FORM(x) " " #x " " which doesn't expand "x" if x is a macro. We have to use __stringify like # include <linux/stringify.h> # define __ASM_FORM(x) " " __stringify(x) " " So this needs aonther patch in the series :) Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH -tip v3 2/2] x86: kprobes: Prohibit probing on instruction which has emulate prefix 2019-09-06 1:45 ` [Xen-devel] " Masami Hiramatsu @ 2019-09-06 1:45 ` Masami Hiramatsu -1 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 1:45 UTC (permalink / raw) To: Ingo Molnar Cc: Josh Poimboeuf, Andrew Cooper, Peter Zijlstra, Randy Dunlap, Borislav Petkov, Juergen Gross, Boris Ostrovsky, Stefano Stabellini, x86, linux-kernel, xen-devel Prohibit probing on instruction which has XEN_EMULATE_PREFIX or KVM's emulate prefix. Since that prefix is a marker for Xen and KVM, if we modify the marker by kprobe's int3, that doesn't work as expected. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/kernel/kprobes/core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index 43fc13c831af..4f13af7cbcdb 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -351,6 +351,10 @@ int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn) kernel_insn_init(insn, dest, MAX_INSN_SIZE); insn_get_length(insn); + /* We can not probe force emulate prefixed instruction */ + if (insn_has_emulate_prefix(insn)) + return 0; + /* Another subsystem puts a breakpoint, failed to recover */ if (insn->opcode.bytes[0] == BREAKPOINT_INSTRUCTION) return 0; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Xen-devel] [PATCH -tip v3 2/2] x86: kprobes: Prohibit probing on instruction which has emulate prefix @ 2019-09-06 1:45 ` Masami Hiramatsu 0 siblings, 0 replies; 16+ messages in thread From: Masami Hiramatsu @ 2019-09-06 1:45 UTC (permalink / raw) To: Ingo Molnar Cc: Juergen Gross, Stefano Stabellini, Peter Zijlstra, Andrew Cooper, Randy Dunlap, x86, linux-kernel, Borislav Petkov, Josh Poimboeuf, xen-devel, Boris Ostrovsky Prohibit probing on instruction which has XEN_EMULATE_PREFIX or KVM's emulate prefix. Since that prefix is a marker for Xen and KVM, if we modify the marker by kprobe's int3, that doesn't work as expected. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- arch/x86/kernel/kprobes/core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index 43fc13c831af..4f13af7cbcdb 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -351,6 +351,10 @@ int __copy_instruction(u8 *dest, u8 *src, u8 *real, struct insn *insn) kernel_insn_init(insn, dest, MAX_INSN_SIZE); insn_get_length(insn); + /* We can not probe force emulate prefixed instruction */ + if (insn_has_emulate_prefix(insn)) + return 0; + /* Another subsystem puts a breakpoint, failed to recover */ if (insn->opcode.bytes[0] == BREAKPOINT_INSTRUCTION) return 0; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-09-06 10:29 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-06 1:45 [PATCH -tip v3 0/2] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes Masami Hiramatsu 2019-09-06 1:45 ` [Xen-devel] " Masami Hiramatsu 2019-09-06 1:45 ` [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature Masami Hiramatsu 2019-09-06 1:45 ` [Xen-devel] " Masami Hiramatsu 2019-09-06 7:34 ` Peter Zijlstra 2019-09-06 7:34 ` [Xen-devel] " Peter Zijlstra 2019-09-06 8:45 ` Masami Hiramatsu 2019-09-06 8:45 ` [Xen-devel] " Masami Hiramatsu 2019-09-06 8:51 ` Masami Hiramatsu 2019-09-06 8:51 ` [Xen-devel] " Masami Hiramatsu 2019-09-06 9:15 ` Peter Zijlstra 2019-09-06 9:15 ` [Xen-devel] " Peter Zijlstra 2019-09-06 10:28 ` Masami Hiramatsu 2019-09-06 10:28 ` [Xen-devel] " Masami Hiramatsu 2019-09-06 1:45 ` [PATCH -tip v3 2/2] x86: kprobes: Prohibit probing on instruction which has emulate prefix Masami Hiramatsu 2019-09-06 1:45 ` [Xen-devel] " Masami Hiramatsu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.