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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 8, 2021 at 3:51 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Sep 08, 2021, Bill Wendling wrote:
> > On Wed, Sep 8, 2021 at 2:49 PM Sean Christopherson <seanjc@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > > ---
> > > >  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.
>
Sounds good to me. Please go ahead and submit this for inclusion and I
can revamp my stuff.

-bw

> From 6e6971ef22c335732a9597409a45fee8a3be6fb7 Mon Sep 17 00:00:00 2001
> From: Sean Christopherson <seanjc@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> ---
>  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))
>



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux