Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181211005647.104927-1-trong@android.com> <CAKwvOdmSp8PHj+iK5KT1XLC89gzu4vD8tN2eRYMdD9Zw24a_ww@mail.gmail.com>
In-Reply-To: <CAKwvOdmSp8PHj+iK5KT1XLC89gzu4vD8tN2eRYMdD9Zw24a_ww@mail.gmail.com>
From:   Tri Vo <trong@android.com>
Date:   Wed, 12 Dec 2018 11:48:06 -0800
Message-ID: <CANA+-vC_5W1CZCe99nvM+3wXm=3riw980+3TyHMytP=7sZPEXA@mail.gmail.com>
Subject: Re: [PATCH] x86_64: Mark per-cpu symbols as absolute.
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     x86@kernel.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        Bill Wendling <morbo@google.com>,
        Rafael Avila de Espindola <rafael.espindola@gmail.com>,
        Dmitry Golovin <dima@golovin.in>, hpa@zytor.com,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Adding appropriate people, lists.
On Tue, Dec 11, 2018 at 11:39 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Dec 10, 2018 at 4:57 PM Tri Vo <trong@android.com> wrote:
> >
> > From: Rafael Avila de Espindola <rafael.espindola@gmail.com>
> >
> > The kernel has many variables that it wants to have a copy for each
> > cpu. It is similar to how each thread wants a copy of a thread local
> > variable.
> >
> > To access such variable, the code has to find the offset of that
> > variable in the per cpu block and add it to the address of the current
> > brock for that cpu.
> >
> > Finding the offset is a problem. There are no relocations (at least on
> > X86_64) that compute the offset of a symbol in a section. To solve this
> > problem, the kernel linker script puts the .data..percpu section at
> > address 0. By doing this, the fake address is actually the offset.
> >
> > This means that symbols defined in that section are effectively
> > absolute. There is no way for the linker to guess that. So LLD complains
> > when linking the kernel:
> > ```
> > ld.lld: error: ./arch/x86/kernel/vmlinux.lds:153: at least one side of the expression must be absolute
> > ld.lld: error: ./arch/x86/kernel/vmlinux.lds:154: at least one side of the expression must be absolute
> > Makefile:1040: recipe for target 'vmlinux' failed
> > ```
>
> This fixes the above error for me when linking with LLD with the
> previous patch you sent applied (I'm having trouble finding a public
> mailing list link though; 'x86_64: Add "-m elf_i386" when linking i386
> object files.').  Still looks like there's maybe 2 more unrelated
> issues to track down with LLD, but this gets us closer.
>
> To make sure this doesn't regress anything for ld.bfd, I triple
> checked a more standard build without LLD.
>
> ld.bfd (clang) + this patch:
> -rwxr-xr-x 1 ndesaulniers primarygroup 56931688 Dec 11 11:32 vmlinux
>
> ld.bfd (clang) w/o this patch:
> -rwxr-xr-x 1 ndesaulniers primarygroup 56931688 Dec 11 11:33 vmlinux
>
> ld.bfd (gcc) w/o this patch:
> -rwxr-xr-x 1 ndesaulniers primarygroup 54047232 Dec 11 11:34 vmlinux
>
> ld.bfd (gcc) w/ this patch:
> -rwxr-xr-x 1 ndesaulniers primarygroup 54047232 Dec 11 11:35 vmlinux
>
> so looks like no change in binary size w/ vs w/o this patch.
>
> Further, I did a boot test of ld.bfd (gcc) w/ this patch in QEMU. No
> issue there.  Thanks for sending.
>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>
> >
> > Tell the linker that per-cpu symbols are absolute.
> >
> > Reported-by: Dmitry Golovin <dima@golovin.in>
> > Tested-by: Dmitry Golovin <dima@golovin.in>
> > Signed-off-by: Tri Vo <trong@android.com>
> > ---
> >  arch/x86/kernel/vmlinux.lds.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > index 0d618ee634ac..ee3b5c7d662e 100644
> > --- a/arch/x86/kernel/vmlinux.lds.S
> > +++ b/arch/x86/kernel/vmlinux.lds.S
> > @@ -401,7 +401,7 @@ SECTIONS
> >   * Per-cpu symbols which need to be offset from __per_cpu_load
> >   * for the boot processor.
> >   */
> > -#define INIT_PER_CPU(x) init_per_cpu__##x = x + __per_cpu_load
> > +#define INIT_PER_CPU(x) init_per_cpu__##x = ABSOLUTE(x) + __per_cpu_load
> >  INIT_PER_CPU(gdt_page);
> >  INIT_PER_CPU(irq_stack_union);
> >
> > --
> > 2.20.0.rc2.403.gdbc3b29805-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers