x86/vdso: drop implicit common-page-size linker flag
diff mbox series

Message ID 20181206191231.192355-1-ndesaulniers@google.com
State In Next
Commit ac3e233d29f7f77f28243af0132057d378d3ea58
Headers show
Series
  • x86/vdso: drop implicit common-page-size linker flag
Related show

Commit Message

Nick Desaulniers Dec. 6, 2018, 7:12 p.m. UTC
These are implied by the target architecture and for x86_64 match the
max-page-size. The default for non-NaCl x86_64 is 0x1000 (4096).

In bfd the common page size is defined as 0x1000 (4096) for non-NaCl
x86_64 targets:

bfd/elf64-x86-64.c:
4998:#define ELF_COMMONPAGESIZE             0x1000

For gold, the common page size is defined as 0x1000 (4096) for non-NaCl
x86_64 targets:

gold/x86_64.cc:
1413:  0x1000, // common_pagesize (overridable by -z common-page-size)
1442:  0x1000, // common_pagesize (overridable by -z common-page-size)

(ELF_COMMONPAGESIZE also defaults to ELF_MAXPAGESIZE when not set
explicitly for a target architecture in bfd/elfxx-target.h, but that's
not relevant for x86_64).

Because it's implied by the target architecture, it's of questionable
use to implement in LLD.  This patch resolves one of the issues towards
using LLD to link an x86_64 kernel.

Fixes commit 2aae950b21e4 ("x86_64: Add vDSO for x86-64 with
gettimeofday/clock_gettime/getcpu")

Link: https://bugs.llvm.org/show_bug.cgi?id=38774
Link: https://github.com/ClangBuiltLinux/linux/issues/31
Cc: Fangrui Song <maskray@google.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Reported-by: Dmitry Golovin <dima@golovin.in>
Reported-by: Bill Wendling <morbo@google.com>
Suggested-by: Dmitry Golovin <dima@golovin.in>
Suggested-by: Rui Ueyama <ruiu@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/x86/entry/vdso/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andy Lutomirski Dec. 6, 2018, 8:29 p.m. UTC | #1
On Thu, Dec 6, 2018 at 11:12 AM <ndesaulniers@google.com> wrote:
>
> These are implied by the target architecture and for x86_64 match the
> max-page-size. The default for non-NaCl x86_64 is 0x1000 (4096).
>
> In bfd the common page size is defined as 0x1000 (4096) for non-NaCl
> x86_64 targets:
>
> bfd/elf64-x86-64.c:
> 4998:#define ELF_COMMONPAGESIZE             0x1000
>
> For gold, the common page size is defined as 0x1000 (4096) for non-NaCl
> x86_64 targets:
>
> gold/x86_64.cc:
> 1413:  0x1000, // common_pagesize (overridable by -z common-page-size)
> 1442:  0x1000, // common_pagesize (overridable by -z common-page-size)
>
> (ELF_COMMONPAGESIZE also defaults to ELF_MAXPAGESIZE when not set
> explicitly for a target architecture in bfd/elfxx-target.h, but that's
> not relevant for x86_64).
>
> Because it's implied by the target architecture, it's of questionable
> use to implement in LLD.  This patch resolves one of the issues towards
> using LLD to link an x86_64 kernel.

Sure.

Acked-by: Andy Lutomirski <luto@kernel.org>
Borislav Petkov Dec. 7, 2018, 10:18 a.m. UTC | #2
On Thu, Dec 06, 2018 at 11:12:31AM -0800, ndesaulniers@google.com wrote:
> These are implied by the target architecture and for x86_64 match the
> max-page-size. The default for non-NaCl x86_64 is 0x1000 (4096).
> 
> In bfd the common page size is defined as 0x1000 (4096) for non-NaCl

Sodium Chloride?

> x86_64 targets:
> 
> bfd/elf64-x86-64.c:
> 4998:#define ELF_COMMONPAGESIZE             0x1000
> 
> For gold, the common page size is defined as 0x1000 (4096) for non-NaCl
> x86_64 targets:
> 
> gold/x86_64.cc:
> 1413:  0x1000, // common_pagesize (overridable by -z common-page-size)
> 1442:  0x1000, // common_pagesize (overridable by -z common-page-size)
> 
> (ELF_COMMONPAGESIZE also defaults to ELF_MAXPAGESIZE when not set
> explicitly for a target architecture in bfd/elfxx-target.h, but that's
> not relevant for x86_64).
> 
> Because it's implied by the target architecture, it's of questionable
> use to implement in LLD.  This patch resolves one of the issues towards
> using LLD to link an x86_64 kernel.

LLD?

I can only guess what this commit message is about and have to look at
the patch itself and then look at the LD(1) man page and rhyme up what
it is aiming to do.

How about rewriting it for mere mortals?

Thx.
Nick Desaulniers Dec. 7, 2018, 5:45 p.m. UTC | #3
On Fri, Dec 7, 2018 at 2:18 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Dec 06, 2018 at 11:12:31AM -0800, ndesaulniers@google.com wrote:
> > These are implied by the target architecture and for x86_64 match the
> > max-page-size. The default for non-NaCl x86_64 is 0x1000 (4096).
> >
> > In bfd the common page size is defined as 0x1000 (4096) for non-NaCl
>
> Sodium Chloride?

Google's Native Client, a technology for running native code in a web
browser.  It's since been superseded by Portable Native Client (pNaCl)
and to an extent Web Assembly.  It seems that BFD still contains code
for NaCl.

>
> > x86_64 targets:
> >
> > bfd/elf64-x86-64.c:
> > 4998:#define ELF_COMMONPAGESIZE             0x1000
> >
> > For gold, the common page size is defined as 0x1000 (4096) for non-NaCl
> > x86_64 targets:
> >
> > gold/x86_64.cc:
> > 1413:  0x1000, // common_pagesize (overridable by -z common-page-size)
> > 1442:  0x1000, // common_pagesize (overridable by -z common-page-size)
> >
> > (ELF_COMMONPAGESIZE also defaults to ELF_MAXPAGESIZE when not set
> > explicitly for a target architecture in bfd/elfxx-target.h, but that's
> > not relevant for x86_64).
> >
> > Because it's implied by the target architecture, it's of questionable
> > use to implement in LLD.  This patch resolves one of the issues towards
> > using LLD to link an x86_64 kernel.
>
> LLD?

LLD is LLVM's linker.
https://lld.llvm.org/

>
> I can only guess what this commit message is about and have to look at
> the patch itself and then look at the LD(1) man page and rhyme up what
> it is aiming to do.
>
> How about rewriting it for mere mortals?

How does this sound:
TL;DR
-z common-page-size's default value is based on the target
architecture.  arch/x86/entry/vdso/Makefile sets it to the
architecture default, which is implicit and redundant.  Drop it.
Borislav Petkov Dec. 7, 2018, 5:52 p.m. UTC | #4
On Fri, Dec 07, 2018 at 09:45:42AM -0800, Nick Desaulniers wrote:
> Google's Native Client, a technology for running native code in a web
> browser.  It's since been superseded by Portable Native Client (pNaCl)
> and to an extent Web Assembly.  It seems that BFD still contains code
> for NaCl.

Yeah, found the wikipedia page. :)

> How does this sound:
> TL;DR
> -z common-page-size's default value is based on the target
> architecture.  arch/x86/entry/vdso/Makefile sets it to the
> architecture default, which is implicit and redundant.  Drop it.

Sure. Although the longer explanation gives more insight into the whole
deal but that's fine too - the longer explanation is on the ML and we
have the Link: tags.

So no need to resend - I'll simply take the three-liner above.

Thx.
Nick Desaulniers Dec. 7, 2018, 6 p.m. UTC | #5
On Fri, Dec 7, 2018 at 9:53 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Dec 07, 2018 at 09:45:42AM -0800, Nick Desaulniers wrote:
> > Google's Native Client, a technology for running native code in a web
> > browser.  It's since been superseded by Portable Native Client (pNaCl)
> > and to an extent Web Assembly.  It seems that BFD still contains code
> > for NaCl.
>
> Yeah, found the wikipedia page. :)
>
> > How does this sound:
> > TL;DR
> > -z common-page-size's default value is based on the target
> > architecture.  arch/x86/entry/vdso/Makefile sets it to the
> > architecture default, which is implicit and redundant.  Drop it.
>
> Sure. Although the longer explanation gives more insight into the whole
> deal but that's fine too - the longer explanation is on the ML and we
> have the Link: tags.
>
> So no need to resend - I'll simply take the three-liner above.

Thanks, it's being set to the default value again for x86 usermode
linux, arm, and for sparc.  I'll send additional clean ups for those
three.

Patch
diff mbox series

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 141d415a8c80..0624bf2266fd 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -47,7 +47,7 @@  targets += $(vdso_img_sodbg) $(vdso_img-y:%=vdso%.so)
 CPPFLAGS_vdso.lds += -P -C
 
 VDSO_LDFLAGS_vdso.lds = -m elf_x86_64 -soname linux-vdso.so.1 --no-undefined \
-			-z max-page-size=4096 -z common-page-size=4096
+			-z max-page-size=4096
 
 $(obj)/vdso64.so.dbg: $(obj)/vdso.lds $(vobjs) FORCE
 	$(call if_changed,vdso)
@@ -98,7 +98,7 @@  CFLAGS_REMOVE_vvar.o = -pg
 
 CPPFLAGS_vdsox32.lds = $(CPPFLAGS_vdso.lds)
 VDSO_LDFLAGS_vdsox32.lds = -m elf32_x86_64 -soname linux-vdso.so.1 \
-			   -z max-page-size=4096 -z common-page-size=4096
+			   -z max-page-size=4096
 
 # x32-rebranded versions
 vobjx32s-y := $(vobjs-y:.o=-x32.o)