[PATCH -tip v3 0/2] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes

[PATCH -tip v3 0/2] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes

[Masami Hiramatsu]

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] [PATCH -tip v3 0/2] x86: kprobes: Prohibit kprobes on Xen/KVM emulate prefixes

[Masami Hiramatsu]

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

[PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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

[PATCH -tip v3 2/2] x86: kprobes: Prohibit probing on instruction which has emulate prefix

[Masami Hiramatsu]

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] [PATCH -tip v3 2/2] x86: kprobes: Prohibit probing on instruction which has emulate prefix

[Masami Hiramatsu]

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

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Peter Zijlstra]

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 ?

Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Peter Zijlstra]

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

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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>

Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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>

Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Peter Zijlstra]

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.

Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Peter Zijlstra]

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

Re: [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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>

Re: [Xen-devel] [PATCH -tip v3 1/2] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Masami Hiramatsu]

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