Re: [PATCH 0/4] x86-64: fix vclock_gettime()

Re: [PATCH 0/4] x86-64: fix vclock_gettime()

[Andi Kleen]

> To rectify the situation, I saw the following options:
> 
>  1. Adjust pointers in the vDSO .altinstruction section during
>     vDSO setup, and then pass it to apply_alternatives()
>  2. List the relocations with objdump and add them again to vdso.S
>     using the .reloc GAS directive
>  3. Link the vDSO into the kernel with relocations, i.e. not just
>     as a blob

4. Just move the barriers back back into the main kernel.

-Andi

Re: [PATCH 0/4] x86-64: fix vclock_gettime()

[Petr Tesarik]

Andi Kleen píše v Čt 14. 05. 2009 v 14:58 +0200:
> > To rectify the situation, I saw the following options:
> > 
> >  1. Adjust pointers in the vDSO .altinstruction section during
> >     vDSO setup, and then pass it to apply_alternatives()
> >  2. List the relocations with objdump and add them again to vdso.S
> >     using the .reloc GAS directive
> >  3. Link the vDSO into the kernel with relocations, i.e. not just
> >     as a blob
> 
> 4. Just move the barriers back back into the main kernel.

Sorry, this doesn't work, because the function is inlined. And even if
it wasn't, the vDSO must be self-contained by definition. User-space
cannot simply call into kernel-space.

Cheers,
Petr Tesarik

[PATCH 0/4] x86-64: fix vclock_gettime()

[Petr Tesarik]

I noticed that x86_64 vDSOs are broken. More specifically, the
vclock_gettime() function may call vread_tsc(), which in turn
reads the TSC counter using vget_cycles().

Now, commit cb9e35dce94a1b9c59d46224e8a94377d673e204 removed
rdtsc_barrier() from that function, and moved it to
do_vgettimeofday(). AFAICS the same is also needed for
vgetns(). So far so good, a one-liner should do, shouldn't it?
No, it won't do.

Why? Because rdtsc_barrier() is defined as follows:

        alternative(ASM_NOP3, "mfence", X86_FEATURE_MFENCE_RDTSC);
        alternative(ASM_NOP3, "lfence", X86_FEATURE_LFENCE_RDTSC);

In other words, this expands to two NOP instructions, which are
patched by the kernel on startup. The location of the instruction
(and other information) is stored in the .altinstruction section.
And that's where the fun begins.

When building the vDSO, this section is saved into the vDSO. That
object will later be included as a blob, so the whole sections
ends outside the region delimited by __alt_instructions and
__alt_instructions_end, and alternative_instructions() will never
patch the NOPs in the vDSO.

To rectify the situation, I saw the following options:

 1. Adjust pointers in the vDSO .altinstruction section during
    vDSO setup, and then pass it to apply_alternatives()
 2. List the relocations with objdump and add them again to vdso.S
    using the .reloc GAS directive
 3. Link the vDSO into the kernel with relocations, i.e. not just
    as a blob

Each one has its drawbacks:

 1. a. All pointers in struct alt_instr are wrong, because they
       refer to the pre-linked location of the vDSO. If the layout
       of the structure is changed (or extended) in the future,
       further adjustments might be necessary.
    b. The .altinstruction is mapped into user-space, although
       user-space has no use for it. The .altinstr_replacement
       section must also be mapped, because .altinstruction
       contains relocations for that section.
       Consequently, in some cases the vDSO will need a few more
       bytes than fits into 1 page.
 2. Works pretty well, but needs binutils 2.18 or later.
 3. Requires linking the vDSO twice. First to produce the vDSO
    with all the ELF headers and linker-generated sections, and
    second to make the in-kernel structure with relocations.

I choose the third option, because it has some advantages, too:

 1. No run-time bloat. All that can be done at link time is done
    at link time.
 2. No user-space bloat. Only those sections which are needed by
    user-space are mapped into the process space.
 3. Possibility to re-use the technique for more general linking,
    e.g. get rid of VMAGIC and run-time vDSO "resolving" of vDSO
    variables in init_vdso_vars().

The following patchset implements the third option.

Beat me hard. ;)

Petr Tesarik (4):
  x86: Use vdso*-syms.h instead of vdso*-syms.lds
  x86: Cleanup vdso-layout.lds.S
  x86-64: link vDSO into the kernel with relocations
  x86: add rdtsc_barrier() to vgetns()

 arch/x86/include/asm/vdso.h     |   14 +-------
 arch/x86/vdso/.gitignore        |   11 +++---
 arch/x86/vdso/Makefile          |   39 +++++++++++++++--------
 arch/x86/vdso/vclock_gettime.c  |   11 ++++--
 arch/x86/vdso/vdso-layout.lds.S |   41 ++++++++++++++++++++----
 arch/x86/vdso/vdso-parts.S      |    7 ++++
 arch/x86/vdso/vdso-reloc.lds.S  |   66 +++++++++++++++++++++++++++++++++++++++
 arch/x86/vdso/vdso.S            |   10 ------
 arch/x86/vdso/vdso.lds.S        |    1 +
 arch/x86/vdso/vdso32-setup.c    |    2 +
 arch/x86/vdso/vma.c             |    1 +
 11 files changed, 151 insertions(+), 52 deletions(-)
 create mode 100644 arch/x86/vdso/vdso-parts.S
 create mode 100644 arch/x86/vdso/vdso-reloc.lds.S
 delete mode 100644 arch/x86/vdso/vdso.S

[PATCH 1/4] x86: Use vdso*-syms.h instead of vdso*-syms.lds

[Petr Tesarik]

Since we post-process the object file anyway, we can define the
constants with pre-processor directives instead of special symbols
at link time. This approach makes it possible to use the include
files with other tools than the linker.

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
---
 arch/x86/include/asm/vdso.h  |   14 ++------------
 arch/x86/vdso/.gitignore     |   10 +++++-----
 arch/x86/vdso/Makefile       |   16 ++++++++--------
 arch/x86/vdso/vdso32-setup.c |    2 ++
 arch/x86/vdso/vma.c          |    1 +
 5 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index 9064052..a959ac9 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h
@@ -2,31 +2,21 @@
 #define _ASM_X86_VDSO_H
 
 #ifdef CONFIG_X86_64
-extern const char VDSO64_PRELINK[];
-
 /*
  * Given a pointer to the vDSO image, find the pointer to VDSO64_name
  * as that symbol is defined in the vDSO sources or linker script.
  */
 #define VDSO64_SYMBOL(base, name)					\
-({									\
-	extern const char VDSO64_##name[];				\
-	(void *)(VDSO64_##name - VDSO64_PRELINK + (unsigned long)(base)); \
-})
+	((void *)(VDSO64_##name - VDSO64_PRELINK + (unsigned long)(base)))
 #endif
 
 #if defined CONFIG_X86_32 || defined CONFIG_COMPAT
-extern const char VDSO32_PRELINK[];
-
 /*
  * Given a pointer to the vDSO image, find the pointer to VDSO32_name
  * as that symbol is defined in the vDSO sources or linker script.
  */
 #define VDSO32_SYMBOL(base, name)					\
-({									\
-	extern const char VDSO32_##name[];				\
-	(void *)(VDSO32_##name - VDSO32_PRELINK + (unsigned long)(base)); \
-})
+	((void *)(VDSO32_##name - VDSO32_PRELINK + (unsigned long)(base)))
 #endif
 
 /*
diff --git a/arch/x86/vdso/.gitignore b/arch/x86/vdso/.gitignore
index 60274d5..6ac0a86 100644
--- a/arch/x86/vdso/.gitignore
+++ b/arch/x86/vdso/.gitignore
@@ -1,6 +1,6 @@
 vdso.lds
-vdso-syms.lds
-vdso32-syms.lds
-vdso32-syscall-syms.lds
-vdso32-sysenter-syms.lds
-vdso32-int80-syms.lds
+vdso-syms.h
+vdso32-syms.h
+vdso32-syscall-syms.h
+vdso32-sysenter-syms.h
+vdso32-int80-syms.h
diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 16a9020..550dca0 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -42,26 +42,25 @@ CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
 
 $(vobjs): KBUILD_CFLAGS += $(CFL)
 
-targets += vdso-syms.lds
-obj-$(VDSO64-y)			+= vdso-syms.lds
+targets += vdso-syms.h
+$(obj)/vma.o: $(obj)/vdso-syms.h
 
 #
 # Match symbols in the DSO that look like VDSO*; produce a file of constants.
 #
 sed-vdsosym := -e 's/^00*/0/' \
-	-e 's/^\([0-9a-fA-F]*\) . \(VDSO[a-zA-Z0-9_]*\)$$/\2 = 0x\1;/p'
+	-e 's/^\([0-9a-fA-F]*\) . \(VDSO[a-zA-Z0-9_]*\)$$/\#define \2 0x\1/p'
 quiet_cmd_vdsosym = VDSOSYM $@
 define cmd_vdsosym
 	$(NM) $< | LC_ALL=C sed -n $(sed-vdsosym) | LC_ALL=C sort > $@
 endef
 
-$(obj)/%-syms.lds: $(obj)/%.so.dbg FORCE
+$(obj)/%-syms.h: $(obj)/%.so.dbg FORCE
 	$(call if_changed,vdsosym)
 
 #
 # Build multiple 32-bit vDSO images to choose from at boot time.
 #
-obj-$(VDSO32-y)			+= vdso32-syms.lds
 vdso32.so-$(VDSO32-y)		+= int80
 vdso32.so-$(CONFIG_COMPAT)	+= syscall
 vdso32.so-$(VDSO32-y)		+= sysenter
@@ -93,10 +92,11 @@ $(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
 				 $(obj)/vdso32/%.o
 	$(call if_changed,vdso)
 
-# Make vdso32-*-syms.lds from each image, and then make sure they match.
+# Make vdso32-*-syms.h from each image, and then make sure they match.
 # The only difference should be that some do not define VDSO32_SYSENTER_RETURN.
 
-targets += vdso32-syms.lds $(vdso32.so-y:%=vdso32-%-syms.lds)
+targets += vdso32-syms.h $(vdso32.so-y:%=vdso32-%-syms.h)
+$(obj)/vdso32-setup.o: $(obj)/vdso32-syms.h
 
 quiet_cmd_vdso32sym = VDSOSYM $@
 define cmd_vdso32sym
@@ -111,7 +111,7 @@ define cmd_vdso32sym
 	fi
 endef
 
-$(obj)/vdso32-syms.lds: $(vdso32.so-y:%=$(obj)/vdso32-%-syms.lds) FORCE
+$(obj)/vdso32-syms.h: $(vdso32.so-y:%=$(obj)/vdso32-%-syms.h) FORCE
 	$(call if_changed,vdso32sym)
 
 #
diff --git a/arch/x86/vdso/vdso32-setup.c b/arch/x86/vdso/vdso32-setup.c
index 1241f11..7f39b89 100644
--- a/arch/x86/vdso/vdso32-setup.c
+++ b/arch/x86/vdso/vdso32-setup.c
@@ -26,6 +26,8 @@
 #include <asm/vdso.h>
 #include <asm/proto.h>
 
+#include "vdso32-syms.h"
+
 enum {
 	VDSO_DISABLED = 0,
 	VDSO_ENABLED = 1,
diff --git a/arch/x86/vdso/vma.c b/arch/x86/vdso/vma.c
index 7133cdf..289d410 100644
--- a/arch/x86/vdso/vma.c
+++ b/arch/x86/vdso/vma.c
@@ -13,6 +13,7 @@
 #include <asm/proto.h>
 #include <asm/vdso.h>
 
+#include "vdso-syms.h"
 #include "vextern.h"		/* Just for VMAGIC.  */
 #undef VEXTERN
 
-- 
1.6.0.2

[PATCH 2/4] x86: Cleanup vdso-layout.lds.S

[Petr Tesarik]

Make the following changes:
  - move linker-generated sections out of .data
  - add all (even theoretically) possible rodata, data and text input
    sections to their respective output sections
  - discard .altinstructions and .altinstr_replacements

Since the .altinstructions section is currently not used anyway, this
is a pure cleanup.

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
---
 arch/x86/vdso/vdso-layout.lds.S |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index 634a2cf..aaa3026 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -21,29 +21,40 @@ SECTIONS
 	.eh_frame_hdr	: { *(.eh_frame_hdr) }		:text	:eh_frame_hdr
 	.eh_frame	: { KEEP (*(.eh_frame)) }	:text
 
+	/* Linker-generated sections */
 	.dynamic	: { *(.dynamic) }		:text	:dynamic
+	.got		: { *(.got) }			:text
+	.got.plt	: { *(.got.plt) }
+	.dynbss		: { *(.dynbss) }
 
-	.rodata		: { *(.rodata*) }		:text
+	.rodata		: {
+	      *(.rodata*)
+	      *(.gnu.linkonce.r.*)
+	}
 	.data		: {
 	      *(.data*)
 	      *(.sdata*)
-	      *(.got.plt) *(.got)
 	      *(.gnu.linkonce.d.*)
 	      *(.bss*)
-	      *(.dynbss*)
+	      *(.sbss*)
 	      *(.gnu.linkonce.b.*)
 	}
 
-	.altinstructions	: { *(.altinstructions) }
-	.altinstr_replacement	: { *(.altinstr_replacement) }
-
 	/*
 	 * Align the actual code well away from the non-instruction data.
 	 * This is the best thing for the I-cache.
 	 */
 	. = ALIGN(0x100);
 
-	.text		: { *(.text*) }			:text	=0x90909090
+	.text		: {
+	      *(.text*)
+	      *(.gnu.linkonce.t.*)
+	}						:text	=0x90909090
+
+	/DISCARD/ : {
+	      *(.altinstructions)
+	      *(.altinstr_replacement)
+	}
 }
 
 /*
-- 
1.6.0.2

[PATCH 3/4] x86-64: link vDSO into the kernel with relocations

[Petr Tesarik]

Preserve relocation info about the sections in the vDSO.
This is achieved by linking the vDSO twice.

First link produces a complete vdso.so as seen by user-space.
This image is used to:

 - determine the final layout of the vDSO
   (extracted by nm to vdso-syms.h)
 - obtain the ELF headers and linker-generated sections
   (copied by the assembler .incbin directive to .vdso_head and
    .vdso_tail sections of vdso-parts.o)

The second link combines the head and tail portions with the
rodata, data and text sections of the object which make up the vDSO,
but doesn't produce a shared library, but a relocatable object for
further linking.

Advantages of this approach:

 1. No run-time bloat. All that can be done at link time is done
    at link time.
 2. No user-space bloat. Only those sections which are needed by
    user-space are mapped into the process space.
 3. Possibility to re-use the technique for more general linking,
    e.g. get rid of VMAGIC and run-time vDSO "resolving" of vDSO
    variables in init_vdso_vars().

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
---
 arch/x86/vdso/.gitignore        |    1 +
 arch/x86/vdso/Makefile          |   25 ++++++++++----
 arch/x86/vdso/vdso-layout.lds.S |   16 +++++++++
 arch/x86/vdso/vdso-parts.S      |    7 ++++
 arch/x86/vdso/vdso-reloc.lds.S  |   66 +++++++++++++++++++++++++++++++++++++++
 arch/x86/vdso/vdso.S            |   10 ------
 arch/x86/vdso/vdso.lds.S        |    1 +
 7 files changed, 109 insertions(+), 17 deletions(-)
 create mode 100644 arch/x86/vdso/vdso-parts.S
 create mode 100644 arch/x86/vdso/vdso-reloc.lds.S
 delete mode 100644 arch/x86/vdso/vdso.S

diff --git a/arch/x86/vdso/.gitignore b/arch/x86/vdso/.gitignore
index 6ac0a86..9d887a8 100644
--- a/arch/x86/vdso/.gitignore
+++ b/arch/x86/vdso/.gitignore
@@ -1,4 +1,5 @@
 vdso.lds
+vdso-reloc.lds
 vdso-syms.h
 vdso32-syms.h
 vdso32-syscall-syms.h
diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index 550dca0..194db49 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -19,17 +19,17 @@ obj-$(VDSO32-y)			+= vdso32.o vdso32-setup.o
 
 vobjs := $(foreach F,$(vobjs-y),$(obj)/$F)
 
-$(obj)/vdso.o: $(obj)/vdso.so
+#
+# Rules for the vDSO as visible to user-space
+#
 
 targets += vdso.so vdso.so.dbg vdso.lds $(vobjs-y)
 
 export CPPFLAGS_vdso.lds += -P -C
 
-VDSO_LDFLAGS_vdso.lds = -m elf_x86_64 -Wl,-soname=linux-vdso.so.1 \
+VDSO_LDFLAGS_vdso.lds = -shared -m elf_x86_64 -Wl,-soname=linux-vdso.so.1 \
 		      	-Wl,-z,max-page-size=4096 -Wl,-z,common-page-size=4096
 
-$(obj)/vdso.o: $(src)/vdso.S $(obj)/vdso.so
-
 $(obj)/vdso.so.dbg: $(src)/vdso.lds $(vobjs) FORCE
 	$(call if_changed,vdso)
 
@@ -42,9 +42,20 @@ CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
 
 $(vobjs): KBUILD_CFLAGS += $(CFL)
 
-targets += vdso-syms.h
+#
+# Rules for the vDSO as linked into the kernel
+#
+
+targets += vdso-syms.h vdso-parts.o vdso-reloc.lds
+
+$(obj)/vdso-parts.o: $(obj)/vdso-syms.h $(obj)/vdso.so
 $(obj)/vma.o: $(obj)/vdso-syms.h
 
+export CPPFLAGS_vdso-reloc.lds += -P -C
+VDSO_LDFLAGS_vdso.o = -r -m elf_x86_64
+$(obj)/vdso.o: $(src)/vdso-reloc.lds $(vobjs) $(obj)/vdso-parts.o
+	$(call if_changed,vdso)
+
 #
 # Match symbols in the DSO that look like VDSO*; produce a file of constants.
 #
@@ -68,7 +79,7 @@ vdso32.so-$(VDSO32-y)		+= sysenter
 vdso32-images			= $(vdso32.so-y:%=vdso32-%.so)
 
 CPPFLAGS_vdso32.lds = $(CPPFLAGS_vdso.lds)
-VDSO_LDFLAGS_vdso32.lds = -m elf_i386 -Wl,-soname=linux-gate.so.1
+VDSO_LDFLAGS_vdso32.lds = -shared -m elf_i386 -Wl,-soname=linux-gate.so.1
 
 # This makes sure the $(obj) subdirectory exists even though vdso32/
 # is not a kbuild sub-make subdirectory.
@@ -122,7 +133,7 @@ quiet_cmd_vdso = VDSO    $@
 		       $(VDSO_LDFLAGS) $(VDSO_LDFLAGS_$(filter %.lds,$(^F))) \
 		       -Wl,-T,$(filter %.lds,$^) $(filter %.o,$^)
 
-VDSO_LDFLAGS = -fPIC -shared $(call ld-option, -Wl$(comma)--hash-style=sysv)
+VDSO_LDFLAGS = -fPIC $(call ld-option, -Wl$(comma)--hash-style=sysv)
 
 #
 # Install the unstripped copy of vdso*.so listed in $(vdso-install-y).
diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S
index aaa3026..c2c6aef 100644
--- a/arch/x86/vdso/vdso-layout.lds.S
+++ b/arch/x86/vdso/vdso-layout.lds.S
@@ -4,6 +4,12 @@
  * This script controls its layout.
  */
 
+#ifdef VDSO_MARK_SECTIONS
+# define MARK(sym)	VDSO_ ## sym = .;
+#else
+# define MARK(sym)
+#endif
+
 SECTIONS
 {
 	. = VDSO_PRELINK + SIZEOF_HEADERS;
@@ -27,17 +33,25 @@ SECTIONS
 	.got.plt	: { *(.got.plt) }
 	.dynbss		: { *(.dynbss) }
 
+	/* IMPORTANT
+	 * If you change the link order here, make sure you also
+	 * reflect your changes in vdso-reloc.lds.S.
+	 */
 	.rodata		: {
+	      MARK(rodata_start)
 	      *(.rodata*)
 	      *(.gnu.linkonce.r.*)
+	      MARK(rodata_end)
 	}
 	.data		: {
+	      MARK(data_start)
 	      *(.data*)
 	      *(.sdata*)
 	      *(.gnu.linkonce.d.*)
 	      *(.bss*)
 	      *(.sbss*)
 	      *(.gnu.linkonce.b.*)
+	      MARK(data_end)
 	}
 
 	/*
@@ -47,8 +61,10 @@ SECTIONS
 	. = ALIGN(0x100);
 
 	.text		: {
+	      MARK(text_start)
 	      *(.text*)
 	      *(.gnu.linkonce.t.*)
+	      MARK(text_end)
 	}						:text	=0x90909090
 
 	/DISCARD/ : {
diff --git a/arch/x86/vdso/vdso-parts.S b/arch/x86/vdso/vdso-parts.S
new file mode 100644
index 0000000..6c34c5e
--- /dev/null
+++ b/arch/x86/vdso/vdso-parts.S
@@ -0,0 +1,7 @@
+#include "vdso-syms.h"
+
+	.section .vdso_head,"a"
+	.incbin "arch/x86/vdso/vdso.so", 0, VDSO_rodata_start-VDSO64_PRELINK
+
+	.section .vdso_tail,"a"
+	.incbin "arch/x86/vdso/vdso.so", VDSO_text_end-VDSO64_PRELINK
diff --git a/arch/x86/vdso/vdso-reloc.lds.S b/arch/x86/vdso/vdso-reloc.lds.S
new file mode 100644
index 0000000..44e0fb5
--- /dev/null
+++ b/arch/x86/vdso/vdso-reloc.lds.S
@@ -0,0 +1,66 @@
+#include "vdso-syms.h"
+
+#define STRINGIFY(x)	#x
+
+/* Choose a name which is not a valid C identifier to prevent conflicts */
+#define MARKER(sym)	STRINGIFY(.vdso. ## sym)
+
+/* Saved value from vdso-syms.h */
+#define VDSOSYM(sym)	(VDSO_ ## sym - VDSO64_PRELINK)
+
+/* Older binutils do not allow ASSERT() inside SECTIONS, so we must
+ * save the current pointer and check it later at file level.
+ */
+#define MARK(sym)	MARKER(sym) = .;
+#define CHECK(sym) \
+	ASSERT(VDSOSYM(sym) == MARKER(sym) - vdso_start, \
+		STRINGIFY(sym in kernel does not match the pre-linked image))
+
+SECTIONS {
+	.init.data	: {
+	      FILL(0);
+
+	      vdso_start = .;
+	      *(.vdso_head)
+
+	/* .rodata */
+	      MARK(rodata_start)
+	      *(.rodata*)
+	      *(.gnu.linkonce.r.*)
+	      MARK(rodata_end)
+
+	/* .data */
+	      MARK(data_start)
+	      *(.data*)
+	      *(.sdata*)
+	      *(.gnu.linkonce.d.*)
+	      *(.bss*)
+	      *(.sbss*)
+	      *(.gnu.linkonce.b.*)
+	      MARK(data_end)
+
+	/* .text */
+	      . = ALIGN(. - vdso_start,0x100);
+	      FILL(0x90909090);
+	      MARK(text_start)
+	      *(.text*)
+	      *(.gnu.linkonce.t.*)
+	      MARK(text_end)
+
+	      *(.vdso_tail)
+	      vdso_end = .;
+	}						=0x90909090
+
+	.altinstructions	: { *(.altinstructions) }
+	.altinstr_replacement	: { *(.altinstr_replacement) }
+
+	/* The custom vDSO note section is not understood in vmlinux */
+	/DISCARD/		: { *(.note.Linux) }
+}
+
+CHECK(rodata_start)
+CHECK(rodata_end)
+CHECK(data_start)
+CHECK(data_end)
+CHECK(text_start)
+CHECK(text_end)
diff --git a/arch/x86/vdso/vdso.S b/arch/x86/vdso/vdso.S
deleted file mode 100644
index 1d3aa6b..0000000
--- a/arch/x86/vdso/vdso.S
+++ /dev/null
@@ -1,10 +0,0 @@
-#include <linux/init.h>
-
-__INITDATA
-
-	.globl vdso_start, vdso_end
-vdso_start:
-	.incbin "arch/x86/vdso/vdso.so"
-vdso_end:
-
-__FINIT
diff --git a/arch/x86/vdso/vdso.lds.S b/arch/x86/vdso/vdso.lds.S
index 4e5dd3b..d5cf24f 100644
--- a/arch/x86/vdso/vdso.lds.S
+++ b/arch/x86/vdso/vdso.lds.S
@@ -9,6 +9,7 @@
  */
 
 #define VDSO_PRELINK 0xffffffffff700000
+#define VDSO_MARK_SECTIONS 1
 #include "vdso-layout.lds.S"
 
 /*
-- 
1.6.0.2

[PATCH 4/4] x86: add rdtsc_barrier() to vgetns()

[Petr Tesarik]

Commit cb9e35dce94a1b9c59d46224e8a94377d673e204 removed
rdtsc_barrier() from vget_cycles() and moved it to
do_vgettimeofday().

AFAICS the same is also needed for vgetns(), because it may
call vread_tsc(), which in turn reads the TSC counter
using vget_cycles().

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>
---
 arch/x86/vdso/vclock_gettime.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index 6a40b78..ab1ae6c 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -36,11 +36,14 @@ notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
 
 notrace static inline long vgetns(void)
 {
-	long v;
-	cycles_t (*vread)(void);
+	cycle_t now, cycle_delta;
+	cycle_t (*vread)(void);
 	vread = gtod->clock.vread;
-	v = (vread() - gtod->clock.cycle_last) & gtod->clock.mask;
-	return (v * gtod->clock.mult) >> gtod->clock.shift;
+	rdtsc_barrier();
+	now = vread();
+	rdtsc_barrier();
+	cycle_delta = (now - gtod->clock.cycle_last) & gtod->clock.mask;
+	return (cycle_delta * gtod->clock.mult) >> gtod->clock.shift;
 }
 
 notrace static noinline int do_realtime(struct timespec *ts)
-- 
1.6.0.2

Re: [PATCH 0/4] x86-64: fix vclock_gettime()

[Petr Tesarik]

Petr Tesarik píše v Čt 14. 05. 2009 v 15:06 +0200:
> Now, commit cb9e35dce94a1b9c59d46224e8a94377d673e204 removed
> rdtsc_barrier() from that function, and moved it to
> do_vgettimeofday(). AFAICS the same is also needed for
> vgetns(). So far so good, a one-liner should do, shouldn't it?
> No, it won't do.

Just to make things clear. The vDSO had been broken even before Ingo's
commit, but the commit made the .altinstructions section go away from
vDSO, which made the bug kind of disappear.

I could even argue that the synchronization is not even needed, because
this code path gets used rather often, and nobody has complained so far,
but that's probably not the point...

Petr Tesarik

Re: [PATCH 0/4] x86-64: fix vclock_gettime()

[Andi Kleen]

> Sorry, this doesn't work, because the function is inlined. And even if
> it wasn't, the vDSO must be self-contained by definition. User-space
> cannot simply call into kernel-space.

Wrong, it can call into vsyscall space.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 0/4] x86-64: fix vclock_gettime()

[Petr Tesarik]

Andi Kleen píše v Čt 14. 05. 2009 v 16:02 +0200:
> > Sorry, this doesn't work, because the function is inlined. And even if
> > it wasn't, the vDSO must be self-contained by definition. User-space
> > cannot simply call into kernel-space.
> 
> Wrong, it can call into vsyscall space.

Ah, yes, you're right. The vsyscall is always mapped and it is a special
case for the instruction replacement code already.

So, is there any reason not to put the rdtsc_barrier() around
vget_cycles() in vread_tsc() ?

Petr Tesarik