* [PATCH 1/6] x86/vmx: Improvements to vmentry failure handling
2015-10-30 19:59 [PATCH 0/6] Remove unqalified ud2 instructions Andrew Cooper
@ 2015-10-30 19:59 ` Andrew Cooper
2015-10-30 19:59 ` [PATCH v2 2/6] xen/x86: Replace unqualified ud2 instructions with BUG frames Andrew Cooper
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jun Nakajima, Jan Beulich
Combine the almost identical vm_launch_fail() and vm_resume_fail() into a
single vmx_vmentry_failure().
Re-save all GPRs so that domain_crash() prints the real register values,
rather than the stack frame of the vmx_vmentry_failure() call.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
v2:
* Swap printk for gprintk
---
xen/arch/x86/hvm/vmx/entry.S | 9 +++++----
xen/arch/x86/hvm/vmx/vmcs.c | 15 ++++-----------
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index 2a4ed57..a5438a4 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -101,14 +101,15 @@ UNLIKELY_END(realmode)
/*.Lvmx_resume:*/
VMRESUME
- sti
- call vm_resume_fail
- ud2
+ jmp .Lvmx_vmentry_fail
.Lvmx_launch:
VMLAUNCH
+
+.Lvmx_vmentry_fail:
sti
- call vm_launch_fail
+ SAVE_ALL
+ call vmx_vmentry_failure
ud2
ENTRY(vmx_asm_do_vmentry)
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 4ea1ad1..53207e6 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1612,21 +1612,14 @@ void vmx_destroy_vmcs(struct vcpu *v)
free_xenheap_page(v->arch.hvm_vmx.msr_bitmap);
}
-void vm_launch_fail(void)
-{
- unsigned long error;
-
- __vmread(VM_INSTRUCTION_ERROR, &error);
- printk("<vm_launch_fail> error code %lx\n", error);
- domain_crash_synchronous();
-}
-
-void vm_resume_fail(void)
+void vmx_vmentry_failure(void)
{
+ struct vcpu *curr = current;
unsigned long error;
__vmread(VM_INSTRUCTION_ERROR, &error);
- printk("<vm_resume_fail> error code %lx\n", error);
+ gprintk(XENLOG_ERR, "VM%s error: %#lx\n",
+ curr->arch.hvm_vmx.launched ? "RESUME" : "LAUNCH", error);
domain_crash_synchronous();
}
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/6] xen/x86: Replace unqualified ud2 instructions with BUG frames
2015-10-30 19:59 [PATCH 0/6] Remove unqalified ud2 instructions Andrew Cooper
2015-10-30 19:59 ` [PATCH 1/6] x86/vmx: Improvements to vmentry failure handling Andrew Cooper
@ 2015-10-30 19:59 ` Andrew Cooper
2015-11-03 7:02 ` Tian, Kevin
2015-10-30 19:59 ` [PATCH 3/6] x86/bug: Break out the internals of BUG_FRAME() Andrew Cooper
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, Jan Beulich
No functional change, other than the failure cases, which now produce a
far more clear error message.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/arch/x86/boot/x86_64.S | 2 +-
xen/arch/x86/hvm/vmx/entry.S | 2 +-
xen/arch/x86/x86_64/entry.S | 4 ++--
xen/include/asm-x86/asm_defns.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index c8bf9d0..94cf089 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -34,7 +34,7 @@ ENTRY(__high_start)
/* Pass off the Multiboot info structure to C land. */
mov multiboot_ptr(%rip),%edi
call __start_xen
- ud2 /* Force a panic (invalid opcode). */
+ BUG /* __start_xen() shouldn't return. */
/*** DESCRIPTOR TABLES ***/
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index a5438a4..5a1757e 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -110,7 +110,7 @@ UNLIKELY_END(realmode)
sti
SAVE_ALL
call vmx_vmentry_failure
- ud2
+ BUG /* vmx_vmentry_failure() shouldn't return. */
ENTRY(vmx_asm_do_vmentry)
GET_CURRENT(%rbx)
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 28c3214..d4dd8e8 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -547,7 +547,7 @@ exception_with_ints_disabled:
FATAL_exception_with_ints_disabled:
movq %rsp,%rdi
call fatal_trap
- ud2
+ BUG /* fatal_trap() shouldn't return. */
ENTRY(divide_error)
pushq $0
@@ -620,7 +620,7 @@ ENTRY(double_fault)
SAVE_ALL STAC
movq %rsp,%rdi
call do_double_fault
- ud2
+ BUG /* do_double_fault() shouldn't return. */
.pushsection .init.text, "ax", @progbits
ENTRY(early_page_fault)
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index d750f03..95ea21d 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -324,7 +324,7 @@ static always_inline void stac(void)
jne 789f
cmpq UREGS_r12(%rsp),%r12
je 987f
-789: ud2
+789: BUG /* Corruption of partial register state. */
.subsection 0
#endif
.endif
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 2/6] xen/x86: Replace unqualified ud2 instructions with BUG frames
2015-10-30 19:59 ` [PATCH v2 2/6] xen/x86: Replace unqualified ud2 instructions with BUG frames Andrew Cooper
@ 2015-11-03 7:02 ` Tian, Kevin
0 siblings, 0 replies; 12+ messages in thread
From: Tian, Kevin @ 2015-11-03 7:02 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Nakajima, Jun, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, October 31, 2015 3:59 AM
>
> No functional change, other than the failure cases, which now produce a
> far more clear error message.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/6] x86/bug: Break out the internals of BUG_FRAME()
2015-10-30 19:59 [PATCH 0/6] Remove unqalified ud2 instructions Andrew Cooper
2015-10-30 19:59 ` [PATCH 1/6] x86/vmx: Improvements to vmentry failure handling Andrew Cooper
2015-10-30 19:59 ` [PATCH v2 2/6] xen/x86: Replace unqualified ud2 instructions with BUG frames Andrew Cooper
@ 2015-10-30 19:59 ` Andrew Cooper
2015-10-30 19:59 ` [PATCH 4/6] x86/vmx: Replace unqualified ud2 instructions with BUG frames Andrew Cooper
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
To allow bug frames can be created inside existing asm() statements. In
order to do so, the current bugframe positional parameters are altered
to be named parameters, to avoid interactions with the parameters of the
existing asm() statement.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/include/asm-x86/bug.h | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/xen/include/asm-x86/bug.h b/xen/include/asm-x86/bug.h
index cec6bce..e868e85 100644
--- a/xen/include/asm-x86/bug.h
+++ b/xen/include/asm-x86/bug.h
@@ -29,23 +29,30 @@ struct bug_frame {
((1 << BUG_LINE_LO_WIDTH) - 1)))
#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
+#define _ASM_BUGFRAME_TEXT(second_frame) \
+ ".Lbug%=: ud2\n" \
+ ".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
+ ".p2align 2\n" \
+ ".Lfrm%=:\n" \
+ ".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n" \
+ ".long (%c[bf_ptr] - .Lfrm%=) + %c[bf_line_lo]\n" \
+ ".if " #second_frame "\n" \
+ ".long 0, %c[bf_msg] - .Lfrm%=\n" \
+ ".endif\n" \
+ ".popsection\n" \
+
+#define _ASM_BUGFRAME_INFO(type, line, ptr, msg) \
+ [bf_type] "i" (type), \
+ [bf_ptr] "i" (ptr), \
+ [bf_msg] "i" (msg), \
+ [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1)) \
+ << BUG_DISP_WIDTH), \
+ [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
+
#define BUG_FRAME(type, line, ptr, second_frame, msg) do { \
BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH)); \
- asm volatile ( ".Lbug%=: ud2\n" \
- ".pushsection .bug_frames.%c0, \"a\", @progbits\n" \
- ".p2align 2\n" \
- ".Lfrm%=:\n" \
- ".long (.Lbug%= - .Lfrm%=) + %c4\n" \
- ".long (%c1 - .Lfrm%=) + %c3\n" \
- ".if " #second_frame "\n" \
- ".long 0, %c2 - .Lfrm%=\n" \
- ".endif\n" \
- ".popsection" \
- : \
- : "i" (type), "i" (ptr), "i" (msg), \
- "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1)) \
- << BUG_DISP_WIDTH), \
- "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)); \
+ asm volatile ( _ASM_BUGFRAME_TEXT(second_frame) \
+ :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
} while (0)
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/6] x86/vmx: Replace unqualified ud2 instructions with BUG frames
2015-10-30 19:59 [PATCH 0/6] Remove unqalified ud2 instructions Andrew Cooper
` (2 preceding siblings ...)
2015-10-30 19:59 ` [PATCH 3/6] x86/bug: Break out the internals of BUG_FRAME() Andrew Cooper
@ 2015-10-30 19:59 ` Andrew Cooper
2015-11-03 7:03 ` Tian, Kevin
2015-10-30 19:59 ` [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions Andrew Cooper
2015-10-30 19:59 ` [PATCH 6/6] EXAMPLE TEST CODE Andrew Cooper
5 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Kevin Tian, Jun Nakajima, Jan Beulich
Using new _ASM_BUGFRAME* internals.
A side effect of complicating the ASM statements is that GCC now chooses to
out-of-line the stub functions, resulting in identical copies being present in
all translation units. As with the stac()/clac() stubs, force them always
inline.
No functional change, other than the failure cases, which now produce a
far more clear error message.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/include/asm-x86/hvm/vmx/vmx.h | 70 ++++++++++++++++++++++-----------------
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index e750a76..f16a306 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -281,7 +281,7 @@ extern uint8_t posted_intr_vector;
#define INVVPID_ALL_CONTEXT 2
#define INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 3
-static inline void __vmptrld(u64 addr)
+static always_inline void __vmptrld(u64 addr)
{
asm volatile (
#ifdef HAVE_GAS_VMX
@@ -289,20 +289,21 @@ static inline void __vmptrld(u64 addr)
#else
VMPTRLD_OPCODE MODRM_EAX_06
#endif
- /* CF==1 or ZF==1 --> crash (ud2) */
+ /* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, vmptrld)
- "\tud2\n"
+ _ASM_BUGFRAME_TEXT(0)
UNLIKELY_END_SECTION
:
#ifdef HAVE_GAS_VMX
- : "m" (addr)
+ : "m" (addr),
#else
- : "a" (&addr)
+ : "a" (&addr),
#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
: "memory");
}
-static inline void __vmpclear(u64 addr)
+static always_inline void __vmpclear(u64 addr)
{
asm volatile (
#ifdef HAVE_GAS_VMX
@@ -310,20 +311,21 @@ static inline void __vmpclear(u64 addr)
#else
VMCLEAR_OPCODE MODRM_EAX_06
#endif
- /* CF==1 or ZF==1 --> crash (ud2) */
+ /* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, vmclear)
- "\tud2\n"
+ _ASM_BUGFRAME_TEXT(0)
UNLIKELY_END_SECTION
:
#ifdef HAVE_GAS_VMX
- : "m" (addr)
+ : "m" (addr),
#else
- : "a" (&addr)
+ : "a" (&addr),
#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
: "memory");
}
-static inline void __vmread(unsigned long field, unsigned long *value)
+static always_inline void __vmread(unsigned long field, unsigned long *value)
{
asm volatile (
#ifdef HAVE_GAS_VMX
@@ -331,20 +333,22 @@ static inline void __vmread(unsigned long field, unsigned long *value)
#else
VMREAD_OPCODE MODRM_EAX_ECX
#endif
- /* CF==1 or ZF==1 --> crash (ud2) */
+ /* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, vmread)
- "\tud2\n"
+ _ASM_BUGFRAME_TEXT(0)
UNLIKELY_END_SECTION
#ifdef HAVE_GAS_VMX
: "=rm" (*value)
- : "r" (field));
+ : "r" (field),
#else
: "=c" (*value)
- : "a" (field));
+ : "a" (field),
#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ );
}
-static inline void __vmwrite(unsigned long field, unsigned long value)
+static always_inline void __vmwrite(unsigned long field, unsigned long value)
{
asm volatile (
#ifdef HAVE_GAS_VMX
@@ -352,16 +356,18 @@ static inline void __vmwrite(unsigned long field, unsigned long value)
#else
VMWRITE_OPCODE MODRM_EAX_ECX
#endif
- /* CF==1 or ZF==1 --> crash (ud2) */
+ /* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, vmwrite)
- "\tud2\n"
+ _ASM_BUGFRAME_TEXT(0)
UNLIKELY_END_SECTION
- :
+ :
#ifdef HAVE_GAS_VMX
- : "r" (field) , "rm" (value));
+ : "r" (field) , "rm" (value),
#else
- : "a" (field) , "c" (value));
+ : "a" (field) , "c" (value),
#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ );
}
static inline bool_t __vmread_safe(unsigned long field, unsigned long *value)
@@ -387,7 +393,7 @@ static inline bool_t __vmread_safe(unsigned long field, unsigned long *value)
return okay;
}
-static inline void __invept(unsigned long type, u64 eptp, u64 gpa)
+static always_inline void __invept(unsigned long type, u64 eptp, u64 gpa)
{
struct {
u64 eptp, gpa;
@@ -407,20 +413,21 @@ static inline void __invept(unsigned long type, u64 eptp, u64 gpa)
#else
INVEPT_OPCODE MODRM_EAX_08
#endif
- /* CF==1 or ZF==1 --> crash (ud2) */
+ /* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, invept)
- "\tud2\n"
+ _ASM_BUGFRAME_TEXT(0)
UNLIKELY_END_SECTION
:
#ifdef HAVE_GAS_EPT
- : "m" (operand), "r" (type)
+ : "m" (operand), "r" (type),
#else
- : "a" (&operand), "c" (type)
+ : "a" (&operand), "c" (type),
#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
: "memory" );
}
-static inline void __invvpid(unsigned long type, u16 vpid, u64 gva)
+static always_inline void __invvpid(unsigned long type, u16 vpid, u64 gva)
{
struct __packed {
u64 vpid:16;
@@ -435,18 +442,19 @@ static inline void __invvpid(unsigned long type, u16 vpid, u64 gva)
#else
INVVPID_OPCODE MODRM_EAX_08
#endif
- /* CF==1 or ZF==1 --> crash (ud2) */
+ /* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, invvpid)
- "\tud2\n"
+ _ASM_BUGFRAME_TEXT(0)
UNLIKELY_END_SECTION "\n"
"2:"
_ASM_EXTABLE(1b, 2b)
:
#ifdef HAVE_GAS_EPT
- : "m" (operand), "r" (type)
+ : "m" (operand), "r" (type),
#else
- : "a" (&operand), "c" (type)
+ : "a" (&operand), "c" (type),
#endif
+ _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
: "memory" );
}
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/6] x86/vmx: Replace unqualified ud2 instructions with BUG frames
2015-10-30 19:59 ` [PATCH 4/6] x86/vmx: Replace unqualified ud2 instructions with BUG frames Andrew Cooper
@ 2015-11-03 7:03 ` Tian, Kevin
0 siblings, 0 replies; 12+ messages in thread
From: Tian, Kevin @ 2015-11-03 7:03 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Nakajima, Jun, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Saturday, October 31, 2015 3:59 AM
>
> Using new _ASM_BUGFRAME* internals.
>
> A side effect of complicating the ASM statements is that GCC now chooses to
> out-of-line the stub functions, resulting in identical copies being present in
> all translation units. As with the stac()/clac() stubs, force them always
> inline.
>
> No functional change, other than the failure cases, which now produce a
> far more clear error message.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
2015-10-30 19:59 [PATCH 0/6] Remove unqalified ud2 instructions Andrew Cooper
` (3 preceding siblings ...)
2015-10-30 19:59 ` [PATCH 4/6] x86/vmx: Replace unqualified ud2 instructions with BUG frames Andrew Cooper
@ 2015-10-30 19:59 ` Andrew Cooper
2015-11-04 14:45 ` Jan Beulich
2015-10-30 19:59 ` [PATCH 6/6] EXAMPLE TEST CODE Andrew Cooper
5 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Using local labels causes the stack trace to use the last non-local
label emitted by the compiler in the translation unit, which is almost
always unrelated.
e.g. A (contrived debug) example switches from:
(XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
(XEN) CPU: 0
(XEN) RIP: e008:[<ffff82d0801961e2>] asm_domain_crash_synchronous+0x44/0x4c
...
(XEN) Xen call trace:
(XEN) [<ffff82d0801961e2>] asm_domain_crash_synchronous+0x44/0x4c
(XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
to:
(XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
(XEN) CPU: 0
(XEN) RIP: e008:[<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
...
(XEN) Xen call trace:
(XEN) [<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
(XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
which is far more relevant when identifying %eip.
Additionally, correct the inclusion of the tag parameter in the C UNLIKELY
blocks, to use the passed tag, rather than a literal ".tag".
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
It is not obvious from the documentation whether an expansion of %= is unique
to the current translation unit, or entire linked binary; I suspect the
former. There are no duplicate symbols introduced by this change in my
current compile, but I realise that this is hardly a guarantee.
---
xen/include/asm-x86/asm_defns.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index 95ea21d..1028375 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -95,9 +95,9 @@ void ret_from_intr(void);
#define UNLIKELY_START(cond, tag) \
.Ldispatch.tag: \
- j##cond .Lunlikely.tag; \
+ j##cond unlikely.tag; \
.subsection 1; \
- .Lunlikely.tag:
+ unlikely.tag:
#define UNLIKELY_DISPATCH_LABEL(tag) \
.Ldispatch.tag
@@ -153,15 +153,15 @@ void ret_from_intr(void);
#endif
-#define UNLIKELY_START(cond, tag) \
- "j" #cond " .Lunlikely%=.tag;\n\t" \
- UNLIKELY_START_SECTION "\n" \
- ".Lunlikely%=.tag:"
+#define UNLIKELY_START(cond, tag) \
+ "j" #cond " unlikely." #tag ".%=;\n\t" \
+ UNLIKELY_START_SECTION "\n" \
+ "unlikely." #tag ".%=:"
#define UNLIKELY_END(tag) \
- "jmp .Llikely%=.tag;\n\t" \
+ "jmp .Llikely." #tag ".%=;\n\t" \
UNLIKELY_END_SECTION "\n" \
- ".Llikely%=.tag:"
+ ".Llikely." #tag ".%=:"
#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
2015-10-30 19:59 ` [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions Andrew Cooper
@ 2015-11-04 14:45 ` Jan Beulich
2015-11-04 15:07 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2015-11-04 14:45 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 30.10.15 at 20:59, <andrew.cooper3@citrix.com> wrote:
> Using local labels causes the stack trace to use the last non-local
> label emitted by the compiler in the translation unit, which is almost
> always unrelated.
>
> e.g. A (contrived debug) example switches from:
>
> (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
> (XEN) CPU: 0
> (XEN) RIP: e008:[<ffff82d0801961e2>]
> asm_domain_crash_synchronous+0x44/0x4c
> ...
> (XEN) Xen call trace:
> (XEN) [<ffff82d0801961e2>] asm_domain_crash_synchronous+0x44/0x4c
> (XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
>
> to:
>
> (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
> (XEN) CPU: 0
> (XEN) RIP: e008:[<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
> ...
> (XEN) Xen call trace:
> (XEN) [<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
> (XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
>
> which is far more relevant when identifying %eip.
I disagree; I specifically used local labels here to avoid cluttering the
symbol table. If anything I'd agree to adding a label to the start of
subsection 1 (or .fixup), but it's not clear how emitting such labels
could be avoided when that section is empty (such labels would clash
with whatever label is first in the next compilation unit). Maybe we
could discard them in the symbols processing tool if their address
matches another symbol.
> Additionally, correct the inclusion of the tag parameter in the C UNLIKELY
> blocks, to use the passed tag, rather than a literal ".tag".
This part I agree with of course, and would therefore like to as for this
to be split off.
Jan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
2015-11-04 14:45 ` Jan Beulich
@ 2015-11-04 15:07 ` Andrew Cooper
2015-11-04 15:16 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2015-11-04 15:07 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 04/11/15 14:45, Jan Beulich wrote:
>>>> On 30.10.15 at 20:59, <andrew.cooper3@citrix.com> wrote:
>> Using local labels causes the stack trace to use the last non-local
>> label emitted by the compiler in the translation unit, which is almost
>> always unrelated.
>>
>> e.g. A (contrived debug) example switches from:
>>
>> (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
>> (XEN) CPU: 0
>> (XEN) RIP: e008:[<ffff82d0801961e2>]
>> asm_domain_crash_synchronous+0x44/0x4c
>> ...
>> (XEN) Xen call trace:
>> (XEN) [<ffff82d0801961e2>] asm_domain_crash_synchronous+0x44/0x4c
>> (XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
>>
>> to:
>>
>> (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
>> (XEN) CPU: 0
>> (XEN) RIP: e008:[<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
>> ...
>> (XEN) Xen call trace:
>> (XEN) [<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
>> (XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
>>
>> which is far more relevant when identifying %eip.
> I disagree; I specifically used local labels here to avoid cluttering the
> symbol table.
Needless cluttering is indeed bad. However, the effect here is a
misleading stack trace.
As the entire point of the symbol table is to generate usable
information in situations like this, I don't see this as an issue.
> If anything I'd agree to adding a label to the start of
> subsection 1 (or .fixup),
How is this suggestion any different to what I have presented? Each
unlikely section still gets a non-local label.
> but it's not clear how emitting such labels
> could be avoided when that section is empty (such labels would clash
> with whatever label is first in the next compilation unit).
All UNLIKELY() sections have content, as they are manually created. Or
have I missed something?
> Maybe we
> could discard them in the symbols processing tool if their address
> matches another symbol.
>
>> Additionally, correct the inclusion of the tag parameter in the C UNLIKELY
>> blocks, to use the passed tag, rather than a literal ".tag".
> This part I agree with of course, and would therefore like to as for this
> to be split off.
I will see about this, after we have agreed on the other bits.
~Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions
2015-11-04 15:07 ` Andrew Cooper
@ 2015-11-04 15:16 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2015-11-04 15:16 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 04.11.15 at 16:07, <andrew.cooper3@citrix.com> wrote:
> On 04/11/15 14:45, Jan Beulich wrote:
>>>>> On 30.10.15 at 20:59, <andrew.cooper3@citrix.com> wrote:
>>> Using local labels causes the stack trace to use the last non-local
>>> label emitted by the compiler in the translation unit, which is almost
>>> always unrelated.
>>>
>>> e.g. A (contrived debug) example switches from:
>>>
>>> (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
>>> (XEN) CPU: 0
>>> (XEN) RIP: e008:[<ffff82d0801961e2>]
>>> asm_domain_crash_synchronous+0x44/0x4c
>>> ...
>>> (XEN) Xen call trace:
>>> (XEN) [<ffff82d0801961e2>] asm_domain_crash_synchronous+0x44/0x4c
>>> (XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
>>>
>>> to:
>>>
>>> (XEN) ----[ Xen-4.7-unstable x86_64 debug=y Not tainted ]----
>>> (XEN) CPU: 0
>>> (XEN) RIP: e008:[<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
>>> ...
>>> (XEN) Xen call trace:
>>> (XEN) [<ffff82d0801961e2>] unlikely.vmptrld.921+0/0x8
>>> (XEN) [<ffff82d080114bdf>] handle_keypress+0xa4/0xd9
>>>
>>> which is far more relevant when identifying %eip.
>> I disagree; I specifically used local labels here to avoid cluttering the
>> symbol table.
>
> Needless cluttering is indeed bad. However, the effect here is a
> misleading stack trace.
>
> As the entire point of the symbol table is to generate usable
> information in situations like this, I don't see this as an issue.
>
>> If anything I'd agree to adding a label to the start of
>> subsection 1 (or .fixup),
>
> How is this suggestion any different to what I have presented? Each
> unlikely section still gets a non-local label.
Your proposal produces one label per code addition to the unlikely
section. My proposal generates just one label, no matter how many
contributions to the section get done in a compilation unit.
>> but it's not clear how emitting such labels
>> could be avoided when that section is empty (such labels would clash
>> with whatever label is first in the next compilation unit).
>
> All UNLIKELY() sections have content, as they are manually created. Or
> have I missed something?
I guess you take each .subsection as representation of a separate
section, while I take only their set per compilation unit as one (and
this being a sub-section means even that is still to wide a term).
Jan
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 6/6] EXAMPLE TEST CODE
2015-10-30 19:59 [PATCH 0/6] Remove unqalified ud2 instructions Andrew Cooper
` (4 preceding siblings ...)
2015-10-30 19:59 ` [PATCH 5/6] xen/x86: Avoid using local labels for UNLIKELY() regions Andrew Cooper
@ 2015-10-30 19:59 ` Andrew Cooper
5 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2015-10-30 19:59 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper
---
xen/arch/x86/traps.c | 17 +++++++++++++++++
xen/include/asm-x86/hvm/vmx/vmx.h | 4 +++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index b32f696..bc34d56 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -4045,6 +4045,23 @@ void asm_domain_crash_synchronous(unsigned long addr)
__domain_crash_synchronous();
}
+#include <xen/keyhandler.h>
+#include <asm/hvm/vmx/vmx.h>
+
+static void do_extreme_debug(unsigned char key, struct cpu_user_regs *regs)
+{
+ printk("'%c' pressed -> Extreme debugging in progress...\n", key);
+
+ __vmptrld(0x2);
+}
+
+static int __init extreme_debug_keyhandler_init(void)
+{
+ register_irq_keyhandler('1', &do_extreme_debug, "Extreme debugging", 0);
+ return 0;
+}
+__initcall(extreme_debug_keyhandler_init);
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h
index f16a306..42f904e 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -292,14 +292,16 @@ static always_inline void __vmptrld(u64 addr)
/* CF==1 or ZF==1 --> BUG() */
UNLIKELY_START(be, vmptrld)
_ASM_BUGFRAME_TEXT(0)
+ "jmp 1f;"
UNLIKELY_END_SECTION
+ "\n1:"
:
#ifdef HAVE_GAS_VMX
: "m" (addr),
#else
: "a" (&addr),
#endif
- _ASM_BUGFRAME_INFO(BUGFRAME_bug, __LINE__, __FILE__, 0)
+ _ASM_BUGFRAME_INFO(BUGFRAME_warn, __LINE__, __FILE__, 0)
: "memory");
}
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread