|
On 23/08/2013 15:02, Jan Beulich wrote:
... at once making conditional forward jumps, which are statically
predicted to be not taken, only used for the unlikely (error) cases.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
I presume it is intentional to create many different symbols for
different ud2 instructions right next to each other? It isn't clear
from the commit message, but would certainly be the logical choice
for debugging an issue arising from such a bug.
As each of these ud2 instructions will never result in continued
normal execution, is it necessary to have the 'likely' tag and jump
back? I cant see any non-buggy case where they would be executed,
at which point they are superfluous.
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -67,6 +67,30 @@ void ret_from_intr(void);
#define ASSERT_NOT_IN_ATOMIC
#endif
+#else
+
+#ifdef __clang__ /* clang's builtin assember can't do .subsection */
+
+#define UNLIKELY_START_SECTION ".pushsection .fixup,\"ax\""
+#define UNLIKELY_END_SECTION ".popsection"
+
+#else
+
+#define UNLIKELY_START_SECTION ".subsection 1"
+#define UNLIKELY_END_SECTION ".subsection 0"
+
+#endif
+
+#define UNLIKELY_START(cond, tag) \
+ "j" #cond " .Lunlikely%=.tag;\n\t" \
+ UNLIKELY_START_SECTION "\n" \
+ ".Lunlikely%=.tag:"
+
+#define UNLIKELY_END(tag) \
+ "jmp .Llikely%=.tag;\n\t" \
+ UNLIKELY_END_SECTION "\n" \
+ ".Llikely%=.tag:"
+
#endif
#endif /* __X86_ASM_DEFNS_H__ */
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -285,7 +285,9 @@ static inline void __vmptrld(u64 addr)
asm volatile ( VMPTRLD_OPCODE
MODRM_EAX_06
/* CF==1 or ZF==1 --> crash (ud2) */
- "ja 1f ; ud2 ; 1:\n"
+ UNLIKELY_START(be, vmptrld)
+ "\tud2\n"
+ UNLIKELY_END_SECTION
:
: "a" (&addr)
: "memory");
@@ -296,7 +298,9 @@ static inline void __vmpclear(u64 addr)
asm volatile ( VMCLEAR_OPCODE
MODRM_EAX_06
/* CF==1 or ZF==1 --> crash (ud2) */
- "ja 1f ; ud2 ; 1:\n"
+ UNLIKELY_START(be, vmclear)
+ "\tud2\n"
+ UNLIKELY_END_SECTION
:
: "a" (&addr)
: "memory");
@@ -309,7 +313,9 @@ static inline unsigned long __vmread(uns
asm volatile ( VMREAD_OPCODE
MODRM_EAX_ECX
/* CF==1 or ZF==1 --> crash (ud2) */
- "ja 1f ; ud2 ; 1:\n"
+ UNLIKELY_START(be, vmread)
+ "\tud2\n"
+ UNLIKELY_END_SECTION
: "=c" (ecx)
: "a" (field)
: "memory");
@@ -322,7 +328,9 @@ static inline void __vmwrite(unsigned lo
asm volatile ( VMWRITE_OPCODE
MODRM_EAX_ECX
/* CF==1 or ZF==1 --> crash (ud2) */
- "ja 1f ; ud2 ; 1:\n"
+ UNLIKELY_START(be, vmwrite)
+ "\tud2\n"
+ UNLIKELY_END_SECTION
:
: "a" (field) , "c" (value)
: "memory");
@@ -360,7 +368,9 @@ static inline void __invept(int type, u6
asm volatile ( INVEPT_OPCODE
MODRM_EAX_08
/* CF==1 or ZF==1 --> crash (ud2) */
- "ja 1f ; ud2 ; 1:\n"
+ UNLIKELY_START(be, invept)
+ "\tud2\n"
+ UNLIKELY_END_SECTION
:
: "a" (&operand), "c" (type)
: "memory" );
@@ -377,7 +387,9 @@ static inline void __invvpid(int type, u
/* Fix up #UD exceptions which occur when TLBs are flushed before VMXON. */
asm volatile ( "1: " INVVPID_OPCODE MODRM_EAX_08
/* CF==1 or ZF==1 --> crash (ud2) */
- "ja 2f ; ud2 ; 2:\n"
+ "2: " UNLIKELY_START(be, invvpid)
+ "\tud2\n"
+ UNLIKELY_END_SECTION "\n"
_ASM_EXTABLE(1b, 2b)
Is this logic still correct?
To my reading, the logic used to say "if there was an exception
while executing INVVPID, then jump to 2: to fix up." where 2:
happens to be the end of the inline assembly, bypassing the ud2.
After the change, the jbe (formed from UNLIKELY_START) is part of
the fixup section, which possibly includes the ud2. As the state of
the flags while executing the INVVPID as unknown, this leaves an
unknown chance of "fixing up" to a ud2 instruction.
~Andrew
:
: "a" (&operand), "c" (type)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
