Re: [kvm-unit-tests PATCH v2 2/5] x86: realmode: mark exec_in_big_real_mode as noinline

[Sean Christopherson <seanjc@google.com>]

On Wed, Sep 08, 2021, Bill Wendling wrote:
> On Wed, Sep 8, 2021 at 2:49 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Wed, Sep 08, 2021, Bill Wendling wrote:
> > > exec_in_big_real_mode() uses inline asm that has labels. Clang decides
> >
> > _global_ labels.  Inlining functions with local labels, including asm goto labels,
> > is not problematic, the issue is specific to labels that must be unique for a
> > given compilation unit.
> >
> > > 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>
> > > ---
> > >  x86/realmode.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/x86/realmode.c b/x86/realmode.c
> > > index b4fa603..07a477f 100644
> > > --- a/x86/realmode.c
> > > +++ b/x86/realmode.c
> > > @@ -178,7 +178,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 __attribute__((noinline)) void exec_in_big_real_mode(struct insn_desc *insn)
> >
> > Forgot to use the new define in this patch :-)
> >
> This was intentional. realmode.c doesn't #include any header files,
> and adding '#include "libflat.h" causes a lot of warnings and errors.
> We could do that, but I feel it's beyond the scope of this series of
> patches.

Ah, right, realmode is compiled for real mode and can't use any of the libcflat
stuff.

A better option would be to put the #define in linux/compiler.h and include that
in libcflat.h and directly in realmode.h.  It only requires a small prep patch to
avoid a duplicate barrier() definition.

From 6e6971ef22c335732a9597409a45fee8a3be6fb7 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Wed, 8 Sep 2021 15:41:12 -0700
Subject: [PATCH] lib: Drop x86/processor.h's barrier() in favor of compiler.h
 version

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");
--

and then your patch 1 can become:

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))