[Xen-devel] [PATCH v3 2/5] x86/clang: fix build with indirect thunks

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

The build with clang is currently broken because clang integrated
assembler requires asm macros to be declared inside the same inline
asm declaration where they are used.

In order to fix this always include indirect_thunk_asm.h in the same
asm declaration where it's being used.

This has been reported to upstream clang at:

https://bugs.llvm.org/show_bug.cgi?id=36110

This change has been tested with clang 3.5, clang 6.0 and gcc 6.4.0.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
---
Changes since v1:
 - Use as-insn to check if the assembler supports .include.
 - Open code a check for whether the assembler forgets .macro-s
   between asm()-s.
---
 Config.mk                              |  7 +++----
 xen/Rules.mk                           |  6 ++++--
 xen/arch/x86/Rules.mk                  | 17 ++++++++++++++---
 xen/arch/x86/extable.c                 |  3 ++-
 xen/arch/x86/x86_emulate/x86_emulate.c |  3 ++-
 xen/common/wait.c                      |  1 +
 xen/include/asm-x86/asm_defns.h        | 28 +++++++++++++++++++++++++---
 7 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/Config.mk b/Config.mk
index 51adc27d83..c55f023f35 100644
--- a/Config.mk
+++ b/Config.mk
@@ -157,17 +157,16 @@ ifndef XEN_HAS_CHECKPOLICY
 endif
 
 # as-insn: Check whether assembler supports an instruction.
-# Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
+# Usage: cflags-y += $(call as-insn cc,"insn",option-yes,option-no,FLAGS)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
-                       | $(1) $(filter-out -M% %.d -include 
%/include/xen/config.h,$(AFLAGS)) \
+                       | $(1) $(filter-out -M% %.d -include 
%/include/xen/config.h,$(5)) \
                               -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)
+    ifeq ($$(call as-insn,$$($(2)),$(3),y,n,$$(AFLAGS)),y)
         $(1) += $(4)
     endif
 endef
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3cf40754a6..205f0aff30 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -66,8 +66,10 @@ endif
 
 AFLAGS-y                += -D__ASSEMBLY__
 
-# Clang's built-in assembler can't handle embedded .include's
-CFLAGS-$(clang)         += -no-integrated-as
+# Clang's built-in assembler doesn't understand .skip or .rept assembler
+# directives without an absolute value:
+# https://bugs.llvm.org/show_bug.cgi?id=27369
+AFLAGS-$(clang)         += -no-integrated-as
 
 ALL_OBJS := $(ALL_OBJS-y)
 
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 56b2ea8356..aeae01cd97 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -42,8 +42,19 @@ CFLAGS += -DCONFIG_INDIRECT_THUNK
 export CONFIG_INDIRECT_THUNK=y
 endif
 
-# Set up the assembler include path properly for older GCC toolchains.  Clang
-# objects to the agument being passed however.
-ifneq ($(clang),y)
+# Set up the assembler include path properly for older toolchains.
 CFLAGS += -Wa,-I$(BASEDIR)/include
+
+ifeq ($(clang),y)
+    # Check whether clang asm()-s support .include.
+    ifeq ($(call as-insn,$(CC),".include 
\"asm/indirect_thunk_asm.h\"",y,n,$(CFLAGS)),n)
+        CFLAGS += -no-integrated-as
+    endif
+    # Check if the assembler keeps .macro-s between asm()-s.
+    ifeq ($(if $(shell echo 'void _(void) { asm volatile ( ".macro FOO\n.endm" 
); \
+                                            asm volatile ( ".macro FOO\n.endm" 
); }' \
+                       | $(CC) $(filter-out -M% %.d -include 
%/include/xen/config.h,$(CFLAGS)) \
+                              -c -x c -o /dev/null - 2>&1),n,y),y)
+        CFLAGS += -DCONFIG_INCLUDE_THUNK_INLINE
+    endif
 endif
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 72f30d9060..30893c3770 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -158,7 +158,8 @@ static int __init stub_selftest(void)
         memcpy(ptr, tests[i].opc, ARRAY_SIZE(tests[i].opc));
         unmap_domain_page(ptr);
 
-        asm volatile ( "INDIRECT_CALL %[stb]\n"
+        asm volatile ( INCLUDE_INDIRECT_THUNK
+                       "INDIRECT_CALL %[stb]\n"
                        ".Lret%=:\n\t"
                        ".pushsection .fixup,\"ax\"\n"
                        ".Lfix%=:\n\t"
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index ff0a003902..e525e6bb0d 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -867,7 +867,8 @@ static inline int mkec(uint8_t e, int32_t ec, ...)
 #ifdef __XEN__
 # define invoke_stub(pre, post, constraints...) do {                    \
     union stub_exception_token res_ = { .raw = ~0 };                    \
-    asm volatile ( pre "\n\tINDIRECT_CALL %[stub]\n\t" post "\n"        \
+    asm volatile ( INCLUDE_INDIRECT_THUNK                               \
+                   pre "\n\tINDIRECT_CALL %[stub]\n\t" post "\n"        \
                    ".Lret%=:\n\t"                                       \
                    ".pushsection .fixup,\"ax\"\n"                       \
                    ".Lfix%=:\n\t"                                       \
diff --git a/xen/common/wait.c b/xen/common/wait.c
index a57bc10d61..91dcd46342 100644
--- a/xen/common/wait.c
+++ b/xen/common/wait.c
@@ -207,6 +207,7 @@ void check_wakeup_from_wait(void)
      * restored from the stack, so are available for use here.
      */
     asm volatile (
+        INCLUDE_INDIRECT_THUNK
         "mov %1,%%"__OP"sp; INDIRECT_JMP %[ip]"
         : : "S" (wqv->stack), "D" (wqv->esp),
           "c" ((char *)get_cpu_info() - (char *)wqv->esp),
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index d2d91ca1fa..447bf67aed 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -16,9 +16,31 @@
 #ifdef __ASSEMBLY__
 # include <asm/indirect_thunk_asm.h>
 #else
-asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
-      __stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) );
-asm ( "\t.include \"asm/indirect_thunk_asm.h\"" );
+/*
+ * NB: for clang's integrated assembler the macro must be included in every
+ * asm() instance where it's used. For gcc/gas it only needs to be included
+ * once per file. Note that even guarding the indirect_thunk_asm.h with
+ * something like:
+ *
+ * .ifndef _X86_INDIRECT_THUNK_ASM_H_
+ * .equ _X86_INDIRECT_THUNK_ASM_H_, 1
+ * ...
+ * .endif
+ *
+ * Doesn't work with clang because it will keep the .equ defined between asm()
+ * instances, but will forget the .macro. This has been reported upstream:
+ * https://bugs.llvm.org/show_bug.cgi?id=36110
+ */
+# define INDIRECT_THUNK_ASM                                     \
+    "\t.equ CONFIG_INDIRECT_THUNK, "                            \
+    __stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) "\n"         \
+    "\t.include \"asm/indirect_thunk_asm.h\"\n"
+# ifdef CONFIG_INCLUDE_THUNK_INLINE
+#  define INCLUDE_INDIRECT_THUNK INDIRECT_THUNK_ASM
+# else
+   asm ( INDIRECT_THUNK_ASM );
+#  define INCLUDE_INDIRECT_THUNK
+# endif
 #endif
 
 #ifndef __ASSEMBLY__
-- 
2.15.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel