[Xen-devel] [PATCH v5 3/4] VMX: use proper instruction mnemonics if assembler supports them

["Jan Beulich" <JBeulich@xxxxxxxx>]

With the hex byte emission we were taking away a good part of
flexibility from the compiler, as for simplicity reasons these were
built using fixed operands. All half way modern build environments
would allow using the mnemonics (but we can't disable the hex variants
yet, since the binutils around at the time gcc 4.1 got released didn't
support these yet).

I didn't convert __vmread() yet because that would, just like for
__vmread_safe(), imply converting to a macro so that the output operand
can be the caller supplied variable rather than an intermediate one. As
that would require touching all invocation points of __vmread() (of
which there are quite a few), I'd first like to be certain the approach
is acceptable; the main question being whether the now conditional code
might be considered to cause future maintenance issues, and the second
being that of parameter/argument ordering (here I made __vmread_safe()
match __vmwrite(), but one could also take the position that read and
write should use the inverse order of one another, in line with the
actual instruction operands).

Additionally I was quite puzzled to find that all the asm()-s involved
here have memory clobbers - what are they needed for? Or can they be
dropped at least in some cases?

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v5: drop "q" operand qualifiers from __invept() and __invvpid() inline
    assembly (pointed out as questionable by Andrew Cooper), changing
    the types of their "type" parameters to "unsigned long" (which is
    by itself a fix to the original code)

--- a/Config.mk
+++ b/Config.mk
@@ -4,6 +4,12 @@ ifeq ($(filter /%,$(XEN_ROOT)),)
 $(error XEN_ROOT must be absolute)
 endif
 
+# Convenient variables
+comma   := ,
+squote  := '
+empty   :=
+space   := $(empty) $(empty)
+
 # fallback for older make
 realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo 
"$$PWD/$(notdir $(file))")))
 
@@ -128,6 +134,21 @@ endef
 check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1")
 $(eval $(check-y))
 
+# as-insn: Check whether assembler supports an instruction.
+# Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
+as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
+                       | $(1) -c -x c -o /dev/null - 2>&1),$(4),$(3))
+
+# as-insn-check: Add an option to compilation flags, but only if insn is
+#                supported by assembler.
+# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP)
+as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4)))
+define as-insn-check-closure
+    ifeq ($$(call as-insn,$$($(2)),$(3),y,n),y)
+        $(1) += $(4)
+    endif
+endef
+
 define buildmakevars2shellvars
     export PREFIX="$(PREFIX)";                                            \
     export XEN_SCRIPT_DIR="$(XEN_SCRIPT_DIR)";                            \
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -28,6 +28,8 @@ CFLAGS += -msoft-float
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 $(call cc-option-add,CFLAGS,CC,-Wnested-externs)
+$(call as-insn-check,CFLAGS,CC,"vmcall",-DHAVE_GAS_VMX)
+$(call as-insn-check,CFLAGS,CC,"invept (%rax)$$(comma)%rax",-DHAVE_GAS_EPT)
 
 ifeq ($(supervisor_mode_kernel),y)
 CFLAGS += -DCONFIG_X86_SUPERVISOR_MODE_KERNEL=1
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1292,12 +1292,11 @@ void vmx_do_resume(struct vcpu *v)
     reset_stack_and_jump(vmx_asm_do_vmentry);
 }
 
-static unsigned long vmr(unsigned long field)
+static inline unsigned long vmr(unsigned long field)
 {
-    int rc;
     unsigned long val;
-    val = __vmread_safe(field, &rc);
-    return rc ? 0 : val;
+
+    return __vmread_safe(field, &val) ? val : 0;
 }
 
 static void vmx_dump_sel(char *name, uint32_t selector)
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -951,13 +951,11 @@ fallback:
         vvmcs_to_shadow(vvmcs, field[i]);
 }
 
-static void shadow_to_vvmcs(void *vvmcs, unsigned int field)
+static inline void shadow_to_vvmcs(void *vvmcs, unsigned int field)
 {
-    u64 value;
-    int rc;
+    unsigned long value;
 
-    value = __vmread_safe(field, &rc);
-    if ( !rc )
+    if ( __vmread_safe(field, &value) )
         __set_vvmcs(vvmcs, field, value);
 }
 
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -282,27 +282,43 @@ extern uint8_t posted_intr_vector;
 
 static inline void __vmptrld(u64 addr)
 {
-    asm volatile ( VMPTRLD_OPCODE
-                   MODRM_EAX_06
+    asm volatile (
+#ifdef HAVE_GAS_VMX
+                   "vmptrld %0\n"
+#else
+                   VMPTRLD_OPCODE MODRM_EAX_06
+#endif
                    /* CF==1 or ZF==1 --> crash (ud2) */
                    UNLIKELY_START(be, vmptrld)
                    "\tud2\n"
                    UNLIKELY_END_SECTION
                    :
+#ifdef HAVE_GAS_VMX
+                   : "m" (addr)
+#else
                    : "a" (&addr)
+#endif
                    : "memory");
 }
 
 static inline void __vmpclear(u64 addr)
 {
-    asm volatile ( VMCLEAR_OPCODE
-                   MODRM_EAX_06
+    asm volatile (
+#ifdef HAVE_GAS_VMX
+                   "vmclear %0\n"
+#else
+                   VMCLEAR_OPCODE MODRM_EAX_06
+#endif
                    /* CF==1 or ZF==1 --> crash (ud2) */
                    UNLIKELY_START(be, vmclear)
                    "\tud2\n"
                    UNLIKELY_END_SECTION
                    :
+#ifdef HAVE_GAS_VMX
+                   : "m" (addr)
+#else
                    : "a" (&addr)
+#endif
                    : "memory");
 }
 
@@ -325,33 +341,50 @@ static inline unsigned long __vmread(uns
 
 static inline void __vmwrite(unsigned long field, unsigned long value)
 {
-    asm volatile ( VMWRITE_OPCODE
-                   MODRM_EAX_ECX
+    asm volatile (
+#ifdef HAVE_GAS_VMX
+                   "vmwrite %1, %0\n"
+#else
+                   VMWRITE_OPCODE MODRM_EAX_ECX
+#endif
                    /* CF==1 or ZF==1 --> crash (ud2) */
                    UNLIKELY_START(be, vmwrite)
                    "\tud2\n"
                    UNLIKELY_END_SECTION
                    : 
+#ifdef HAVE_GAS_VMX
+                   : "r" (field) , "rm" (value)
+#else
                    : "a" (field) , "c" (value)
+#endif
                    : "memory");
 }
 
-static inline unsigned long __vmread_safe(unsigned long field, int *error)
+static inline bool_t __vmread_safe(unsigned long field, unsigned long *value)
 {
-    unsigned long ecx;
+    bool_t okay;
 
-    asm volatile ( VMREAD_OPCODE
-                   MODRM_EAX_ECX
-                   /* CF==1 or ZF==1 --> rc = -1 */
-                   "setna %b0 ; neg %0"
-                   : "=q" (*error), "=c" (ecx)
-                   : "0" (0), "a" (field)
+    asm volatile (
+#ifdef HAVE_GAS_VMX
+                   "vmread %2, %1\n\t"
+#else
+                   VMREAD_OPCODE MODRM_EAX_ECX
+#endif
+                   /* CF==1 or ZF==1 --> rc = 0 */
+                   "setnbe %0"
+#ifdef HAVE_GAS_VMX
+                   : "=qm" (okay), "=rm" (*value)
+                   : "r" (field)
+#else
+                   : "=qm" (okay), "=c" (*value)
+                   : "a" (field)
+#endif
                    : "memory");
 
-    return ecx;
+    return okay;
 }
 
-static inline void __invept(int type, u64 eptp, u64 gpa)
+static inline void __invept(unsigned long type, u64 eptp, u64 gpa)
 {
     struct {
         u64 eptp, gpa;
@@ -365,18 +398,26 @@ static inline void __invept(int type, u6
          !cpu_has_vmx_ept_invept_single_context )
         type = INVEPT_ALL_CONTEXT;
 
-    asm volatile ( INVEPT_OPCODE
-                   MODRM_EAX_08
+    asm volatile (
+#ifdef HAVE_GAS_EPT
+                   "invept %0, %1\n"
+#else
+                   INVEPT_OPCODE MODRM_EAX_08
+#endif
                    /* CF==1 or ZF==1 --> crash (ud2) */
                    UNLIKELY_START(be, invept)
                    "\tud2\n"
                    UNLIKELY_END_SECTION
                    :
+#ifdef HAVE_GAS_EPT
+                   : "m" (operand), "r" (type)
+#else
                    : "a" (&operand), "c" (type)
+#endif
                    : "memory" );
 }
 
-static inline void __invvpid(int type, u16 vpid, u64 gva)
+static inline void __invvpid(unsigned long type, u16 vpid, u64 gva)
 {
     struct {
         u64 vpid:16;
@@ -385,7 +426,12 @@ static inline void __invvpid(int type, u
     } __attribute__ ((packed)) operand = {vpid, 0, gva};
 
     /* Fix up #UD exceptions which occur when TLBs are flushed before VMXON. */
-    asm volatile ( "1: " INVVPID_OPCODE MODRM_EAX_08
+    asm volatile ( "1: "
+#ifdef HAVE_GAS_EPT
+                   "invvpid %0, %1\n"
+#else
+                   INVVPID_OPCODE MODRM_EAX_08
+#endif
                    /* CF==1 or ZF==1 --> crash (ud2) */
                    UNLIKELY_START(be, invvpid)
                    "\tud2\n"
@@ -393,7 +439,11 @@ static inline void __invvpid(int type, u
                    "2:"
                    _ASM_EXTABLE(1b, 2b)
                    :
+#ifdef HAVE_GAS_EPT
+                   : "m" (operand), "r" (type)
+#else
                    : "a" (&operand), "c" (type)
+#endif
                    : "memory" );
 }

Attachment: VMX-use-mnemonics.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel