[kvm-unit-tests PATCH v3 0/7] x86: Fix duplicate symbols w/ clang

[kvm-unit-tests PATCH v3 0/7] x86: Fix duplicate symbols w/ clang

[Sean Christopherson]

Add a "noinline" macro to mirror the kernel's wrapping of the attribute         
and to save typing, and use it to fix a variety of duplicate symbol errors      
that pop up with some versions of clang due to clang aggressively inlining      
functions that define globally visible labels in inline asm blobs.

Bill Wendling (5):
  lib: define the "noinline" macro
  x86: realmode: mark exec_in_big_real_mode as noinline
  x86: svm: mark test_run as noinline
  x86: umip: mark do_ring3 as noinline
  x86: vmx: mark some test_* functions as noinline

Sean Christopherson (2):
  lib: Drop x86/processor.h's barrier() in favor of compiler.h version
  lib: Move __unused attribute macro to compiler.h

 lib/libcflat.h       | 3 +--
 lib/linux/compiler.h | 2 ++
 lib/x86/processor.h  | 5 -----
 x86/pmu_lbr.c        | 4 ++--
 x86/realmode.c       | 4 +++-
 x86/svm.c            | 2 +-
 x86/umip.c           | 2 +-
 x86/vmx.c            | 6 +++---
 8 files changed, 13 insertions(+), 15 deletions(-)

-- 
2.33.0.309.g3052b89438-goog

[kvm-unit-tests PATCH v3 1/7] lib: Drop x86/processor.h's barrier() in favor of compiler.h version

[Sean Christopherson]

Drop x86's duplicate version of barrier() in favor of the generic #define
provided by linux/compiler.h.  Include compiler.h in the all-encompassing
libcflat.h to pick up barrier() and other future goodies, e.g. new
attributes defines.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/libcflat.h      | 1 +
 lib/x86/processor.h | 5 -----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 97db9e3..e619de1 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -22,6 +22,7 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/compiler.h>
 #include <stdarg.h>
 #include <stddef.h>
 #include <stdint.h>
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index f380321..eaf24d4 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -216,11 +216,6 @@ struct descriptor_table_ptr {
     ulong base;
 } __attribute__((packed));
 
-static inline void barrier(void)
-{
-    asm volatile ("" : : : "memory");
-}
-
 static inline void clac(void)
 {
     asm volatile (".byte 0x0f, 0x01, 0xca" : : : "memory");
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 1/7] lib: Drop x86/processor.h's barrier() in favor of compiler.h version

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:33 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Drop x86's duplicate version of barrier() in favor of the generic #define
> provided by linux/compiler.h.  Include compiler.h in the all-encompassing
> libcflat.h to pick up barrier() and other future goodies, e.g. new
> attributes defines.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[kvm-unit-tests PATCH v3 2/7] lib: define the "noinline" macro

[Sean Christopherson]

From: Bill Wendling <morbo@google.com>

Define "noline" macro to reduce the amount of typing for functions using
the "noinline" attribute.  Opportunsitically convert existing users.

Signed-off-by: Bill Wendling <morbo@google.com>
[sean: put macro in compiler.h instead of libcflat.h]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/linux/compiler.h | 1 +
 x86/pmu_lbr.c        | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
index 5d9552a..5937b7b 100644
--- a/lib/linux/compiler.h
+++ b/lib/linux/compiler.h
@@ -46,6 +46,7 @@
 #define barrier()	asm volatile("" : : : "memory")
 
 #define __always_inline	inline __attribute__((always_inline))
+#define noinline __attribute__((noinline))
 
 static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
 {
diff --git a/x86/pmu_lbr.c b/x86/pmu_lbr.c
index 3bd9e9f..5ff805a 100644
--- a/x86/pmu_lbr.c
+++ b/x86/pmu_lbr.c
@@ -16,14 +16,14 @@
 
 volatile int count;
 
-static __attribute__((noinline)) int compute_flag(int i)
+static noinline int compute_flag(int i)
 {
 	if (i % 10 < 4)
 		return i + 1;
 	return 0;
 }
 
-static __attribute__((noinline)) int lbr_test(void)
+static noinline int lbr_test(void)
 {
 	int i;
 	int flag;
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 2/7] lib: define the "noinline" macro

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:32 AM Sean Christopherson <seanjc@google.com> wrote:
>
> From: Bill Wendling <morbo@google.com>
>
> Define "noline" macro to reduce the amount of typing for functions using
> the "noinline" attribute.  Opportunsitically convert existing users.

Nit: Opportunistically.

> Signed-off-by: Bill Wendling <morbo@google.com>
> [sean: put macro in compiler.h instead of libcflat.h]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[kvm-unit-tests PATCH v3 3/7] lib: Move __unused attribute macro to compiler.h

[Sean Christopherson]

Move the __unused macro to linux/compiler.h to co-locate it with the
other macros that provide syntactic sugar around __attribute__.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/libcflat.h       | 2 --
 lib/linux/compiler.h | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index e619de1..39f4552 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -29,8 +29,6 @@
 #include <string.h>
 #include <stdbool.h>
 
-#define __unused __attribute__((__unused__))
-
 #define xstr(s...) xxstr(s)
 #define xxstr(s...) #s
 
diff --git a/lib/linux/compiler.h b/lib/linux/compiler.h
index 5937b7b..c7fc0cf 100644
--- a/lib/linux/compiler.h
+++ b/lib/linux/compiler.h
@@ -47,6 +47,7 @@
 
 #define __always_inline	inline __attribute__((always_inline))
 #define noinline __attribute__((noinline))
+#define __unused __attribute__((__unused__))
 
 static __always_inline void __read_once_size(const volatile void *p, void *res, int size)
 {
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 3/7] lib: Move __unused attribute macro to compiler.h

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:32 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Move the __unused macro to linux/compiler.h to co-locate it with the
> other macros that provide syntactic sugar around __attribute__.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[kvm-unit-tests PATCH v3 4/7] x86: realmode: mark exec_in_big_real_mode as noinline

[Sean Christopherson]

From: Bill Wendling <morbo@google.com>

exec_in_big_real_mode() uses inline asm that defines labels that are
globally. Clang decides that it can inline this function, which causes the
assembler to complain about duplicate symbols. Mark the function as
"noinline" to prevent this.

Signed-off-by: Bill Wendling <morbo@google.com>
[sean: use noinline from compiler.h, call out the globally visible aspect]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/realmode.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/x86/realmode.c b/x86/realmode.c
index b4fa603..7a4423e 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -1,3 +1,5 @@
+#include <linux/compiler.h>
+
 #ifndef USE_SERIAL
 #define USE_SERIAL
 #endif
@@ -178,7 +180,7 @@ static inline void init_inregs(struct regs *regs)
 		inregs.esp = (unsigned long)&tmp_stack.top;
 }
 
-static void exec_in_big_real_mode(struct insn_desc *insn)
+static noinline void exec_in_big_real_mode(struct insn_desc *insn)
 {
 	unsigned long tmp;
 	static struct regs save;
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 4/7] x86: realmode: mark exec_in_big_real_mode as noinline

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:32 AM Sean Christopherson <seanjc@google.com> wrote:
>
> From: Bill Wendling <morbo@google.com>
>
> exec_in_big_real_mode() uses inline asm that defines labels that are
> globally. Clang decides that it can inline this function, which causes the
globally *visible*?
> assembler to complain about duplicate symbols. Mark the function as
> "noinline" to prevent this.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> [sean: use noinline from compiler.h, call out the globally visible aspect]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[kvm-unit-tests PATCH v3 5/7] x86: svm: mark test_run as noinline

[Sean Christopherson]

From: Bill Wendling <morbo@google.com>

test_run uses inline asm that defines globally visible labels. Clang
decides that it can inline this function, which causes the assembler to
complain about duplicate symbols. Mark the function as "noinline" to
prevent this.

Signed-off-by: Bill Wendling <morbo@google.com>
[sean: call out the globally visible aspect]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/svm.c b/x86/svm.c
index beb40b7..3f94b2a 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -256,7 +256,7 @@ int svm_vmrun(void)
 
 extern u8 vmrun_rip;
 
-static void test_run(struct svm_test *test)
+static noinline void test_run(struct svm_test *test)
 {
 	u64 vmcb_phys = virt_to_phys(vmcb);
 
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 5/7] x86: svm: mark test_run as noinline

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:33 AM Sean Christopherson <seanjc@google.com> wrote:
>
> From: Bill Wendling <morbo@google.com>
>
> test_run uses inline asm that defines globally visible labels. Clang
> decides that it can inline this function, which causes the assembler to
> complain about duplicate symbols. Mark the function as "noinline" to
> prevent this.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> [sean: call out the globally visible aspect]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[kvm-unit-tests PATCH v3 6/7] x86: umip: mark do_ring3 as noinline

[Sean Christopherson]

From: Bill Wendling <morbo@google.com>

do_ring3 uses inline asm that defines globally visible labels. Clang
decides that it can inline this function, which causes the assembler to
complain about duplicate symbols. Mark the function as "noinline" to
prevent this.

Signed-off-by: Bill Wendling <morbo@google.com>
[sean: call out the globally visible aspect]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/umip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/umip.c b/x86/umip.c
index c5700b3..0fc1f65 100644
--- a/x86/umip.c
+++ b/x86/umip.c
@@ -114,7 +114,7 @@ static void test_umip_gp(const char *msg)
 
 /* The ugly mode switching code */
 
-static int do_ring3(void (*fn)(const char *), const char *arg)
+static noinline int do_ring3(void (*fn)(const char *), const char *arg)
 {
     static unsigned char user_stack[4096];
     int ret;
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 6/7] x86: umip: mark do_ring3 as noinline

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:32 AM Sean Christopherson <seanjc@google.com> wrote:
>
> From: Bill Wendling <morbo@google.com>
>
> do_ring3 uses inline asm that defines globally visible labels. Clang
> decides that it can inline this function, which causes the assembler to
> complain about duplicate symbols. Mark the function as "noinline" to
> prevent this.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> [sean: call out the globally visible aspect]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

[kvm-unit-tests PATCH v3 7/7] x86: vmx: mark some test_* functions as noinline

[Sean Christopherson]

From: Bill Wendling <morbo@google.com>

Some test_* functions use inline asm that define globally visible labels.
Clang decides that it can inline these functions, which causes the
assembler to complain about duplicate symbols. Mark the functions as
"noinline" to prevent this.

Signed-off-by: Bill Wendling <morbo@google.com>
[sean: call out the globally visible aspect]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index f0b853a..2a32aa2 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -426,7 +426,7 @@ static void prep_flags_test_env(void **vpage, struct vmcs **vmcs, handler *old)
 	*old = handle_exception(PF_VECTOR, &pf_handler);
 }
 
-static void test_read_sentinel(void)
+static noinline void test_read_sentinel(void)
 {
 	void *vpage;
 	struct vmcs *vmcs;
@@ -474,7 +474,7 @@ static void test_vmread_flags_touch(void)
 	test_read_sentinel();
 }
 
-static void test_write_sentinel(void)
+static noinline void test_write_sentinel(void)
 {
 	void *vpage;
 	struct vmcs *vmcs;
@@ -1786,7 +1786,7 @@ static int exit_handler(union exit_reason exit_reason)
  * Tries to enter the guest, populates @result with VM-Fail, VM-Exit, entered,
  * etc...
  */
-static void vmx_enter_guest(struct vmentry_result *result)
+static noinline void vmx_enter_guest(struct vmentry_result *result)
 {
 	memset(result, 0, sizeof(*result));
 
-- 
2.33.0.309.g3052b89438-goog

Re: [kvm-unit-tests PATCH v3 7/7] x86: vmx: mark some test_* functions as noinline

[Jim Mattson]

On Thu, Sep 9, 2021 at 11:32 AM Sean Christopherson <seanjc@google.com> wrote:
>
> From: Bill Wendling <morbo@google.com>
>
> Some test_* functions use inline asm that define globally visible labels.
> Clang decides that it can inline these functions, which causes the
> assembler to complain about duplicate symbols. Mark the functions as
> "noinline" to prevent this.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> [sean: call out the globally visible aspect]
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [kvm-unit-tests PATCH v3 0/7] x86: Fix duplicate symbols w/ clang

[Paolo Bonzini]

On 09/09/21 20:32, Sean Christopherson wrote:
> Add a "noinline" macro to mirror the kernel's wrapping of the attribute
> and to save typing, and use it to fix a variety of duplicate symbol errors
> that pop up with some versions of clang due to clang aggressively inlining
> functions that define globally visible labels in inline asm blobs.
> 
> Bill Wendling (5):
>    lib: define the "noinline" macro
>    x86: realmode: mark exec_in_big_real_mode as noinline
>    x86: svm: mark test_run as noinline
>    x86: umip: mark do_ring3 as noinline
>    x86: vmx: mark some test_* functions as noinline
> 
> Sean Christopherson (2):
>    lib: Drop x86/processor.h's barrier() in favor of compiler.h version
>    lib: Move __unused attribute macro to compiler.h
> 
>   lib/libcflat.h       | 3 +--
>   lib/linux/compiler.h | 2 ++
>   lib/x86/processor.h  | 5 -----
>   x86/pmu_lbr.c        | 4 ++--
>   x86/realmode.c       | 4 +++-
>   x86/svm.c            | 2 +-
>   x86/umip.c           | 2 +-
>   x86/vmx.c            | 6 +++---
>   8 files changed, 13 insertions(+), 15 deletions(-)
> 

Queued, thanks.

Paolo