Back in July last year we started having a problem in building compat
vDSOs on arm64 [1] [2] that was not present when the arm64 porting to
the Unified vDSO was done. In particular when the compat vDSO on such
architecture is built with gcc it generates the warning below:
In file included from ./arch/arm64/include/asm/thread_info.h:17:0,
from ./include/linux/thread_info.h:38,
from ./arch/arm64/include/asm/preempt.h:5,
from ./include/linux/preempt.h:78,
from ./include/linux/spinlock.h:51,
from ./include/linux/seqlock.h:36,
from ./include/linux/time.h:6,
from ./lib/vdso/gettimeofday.c:7,
from <command-line>:0:
./arch/arm64/include/asm/memory.h: In function ‘__tag_set’:
./arch/arm64/include/asm/memory.h:233:15: warning: cast from pointer
to integer of different size [-Wpointer-to-int-cast]
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
^
In file included from ./arch/arm64/include/asm/pgtable-hwdef.h:8:0,
from ./arch/arm64/include/asm/processor.h:34,
from ./arch/arm64/include/asm/elf.h:118,
from ./include/linux/elf.h:5,
from ./include/linux/elfnote.h:62,
from arch/arm64/kernel/vdso32/note.c:11:
./arch/arm64/include/asm/memory.h: In function ‘__tag_set’:
./arch/arm64/include/asm/memory.h:233:15: warning: cast from pointer
to integer of different size [-Wpointer-to-int-cast]
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
The same porting does not build at all when the selected compiler is
clang.
I started an investigation to try to understand better the problem and
after various discussions at Plumbers and Recipes last year the
conclusion was that the vDSO library as it stands it is including more
headers that it needs. In particular, being a user-space library, it
should require only the UAPI and a minimal vDSO kernel interface instead
of all the kernel-related inline functions which are not directly used
and in some cases can have side effects.
To solve the problem, I decided to use the approach below:
* Extract from include/linux/ the vDSO required kernel interface
and place it in include/vdso/
* Make sure that where meaningful the kernel includes "vdso" headers.
* Limit the vDSO library to include headers coming only from UAPI
and "vdso" (with 2 exceptions compiler.h for barriers and param.h
for HZ).
* Adapt all the architectures that support the unified vDSO library
to use "vdso" headers.
According to me this approach allows up to exercise a better control on
what the vDSO library can include and to prevent potential issues in
future.
This patch series contains the implementation of the described approach.
The "vdso" headers have been verified on all the architectures that support
unified vDSO using the vdsotest [3] testsuite for what concerns the vDSO part
and randconfig to verify that they are included in the correct places.
To simplify the testing, a copy of the patchset on top of a recent linux
tree can be found at [4].
[1] https://github.com/ClangBuiltLinux/linux/issues/595
[2] https://lore.kernel.org/lkml/[email protected]
[3] https://github.com/nathanlynch/vdsotest
[4] git://linux-arm.org/linux-vf.git common-headers/v3
Changes:
--------
v3:
- Changed the namespace from common to vdso.
- Addressed an issue involving parisc modules compilation.
- Added vdso header for clocksource.h.
- Addressed review comments.
- Rebased on tip/timers/core.
v2:
- Addressed review comments for clang support.
- Rebased on 5.6-rc4.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Russell King <[email protected]>
Cc: Paul Burton <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: Mark Salyzyn <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Peter Collingbourne <[email protected]>
Cc: Dmitry Safonov <[email protected]>
Cc: Andrei Vagin <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Mark Rutland <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
Vincenzo Frascino (26):
linux/const.h: Extract common header for vDSO
linux/bits.h: Extract common header for vDSO
linux/limits.h: Extract common header for vDSO
x86:Introduce asm/vdso/clocksource.h
arm: Introduce asm/vdso/clocksource.h
arm64: Introduce asm/vdso/clocksource.h
mips: Introduce asm/vdso/clocksource.h
linux/clocksource.h: Extract common header for vDSO
linux/math64.h: Extract common header for vDSO
linux/time.h: Extract common header for vDSO
linux/time32.h: Extract common header for vDSO
linux/time64.h: Extract common header for vDSO
linux/jiffies.h: Extract common header for vDSO
linux/ktime.h: Extract common header for vDSO
common: Introduce processor.h
scripts: Fix the inclusion order in modpost
linux/elfnote.h: Replace elf.h with UAPI equivalent
arm64: Introduce asm/vdso/processor.h
arm64: vdso: Include common headers in the vdso library
arm64: vdso32: Include common headers in the vdso library
arm64: Introduce asm/vdso/arch_timer.h
mips: vdso: Enable mips to use common headers
x86: vdso: Enable x86 to use common headers
arm: vdso: Enable arm to use common headers
lib: vdso: Enable common headers
arm64: vdso32: Enable Clang Compilation
arch/arm/include/asm/clocksource.h | 6 +--
arch/arm/include/asm/cp15.h | 20 +---------
arch/arm/include/asm/processor.h | 11 +-----
arch/arm/include/asm/vdso/clocksource.h | 8 ++++
arch/arm/include/asm/vdso/cp15.h | 38 +++++++++++++++++++
arch/arm/include/asm/vdso/gettimeofday.h | 4 +-
arch/arm/include/asm/vdso/processor.h | 22 +++++++++++
arch/arm64/include/asm/arch_timer.h | 29 +++-----------
arch/arm64/include/asm/clocksource.h | 3 +-
arch/arm64/include/asm/processor.h | 16 +-------
arch/arm64/include/asm/vdso/arch_timer.h | 33 ++++++++++++++++
arch/arm64/include/asm/vdso/clocksource.h | 8 ++++
.../include/asm/vdso/compat_gettimeofday.h | 2 +-
arch/arm64/include/asm/vdso/gettimeofday.h | 8 ++--
arch/arm64/include/asm/vdso/processor.h | 31 +++++++++++++++
arch/arm64/kernel/vdso/vgettimeofday.c | 2 -
arch/arm64/kernel/vdso32/Makefile | 11 ++++++
arch/arm64/kernel/vdso32/vgettimeofday.c | 3 --
arch/mips/include/asm/clocksource.h | 4 +-
arch/mips/include/asm/processor.h | 16 +-------
arch/mips/include/asm/vdso/clocksource.h | 9 +++++
arch/mips/include/asm/vdso/gettimeofday.h | 4 --
arch/mips/include/asm/vdso/processor.h | 27 +++++++++++++
arch/x86/include/asm/clocksource.h | 5 +--
arch/x86/include/asm/processor.h | 12 +-----
arch/x86/include/asm/vdso/clocksource.h | 10 +++++
arch/x86/include/asm/vdso/processor.h | 23 +++++++++++
include/linux/bits.h | 2 +-
include/linux/clocksource.h | 11 +-----
include/linux/const.h | 5 +--
include/linux/elfnote.h | 2 +-
include/linux/jiffies.h | 4 +-
include/linux/ktime.h | 9 +----
include/linux/limits.h | 13 +------
include/linux/math64.h | 20 +---------
include/linux/time.h | 5 +--
include/linux/time32.h | 14 +------
include/linux/time64.h | 10 +----
include/vdso/bits.h | 9 +++++
include/vdso/clocksource.h | 23 +++++++++++
include/vdso/const.h | 10 +++++
include/vdso/datapage.h | 33 ++++++++++++++--
include/vdso/jiffies.h | 11 ++++++
include/vdso/ktime.h | 16 ++++++++
include/vdso/limits.h | 18 +++++++++
include/vdso/math64.h | 24 ++++++++++++
include/vdso/processor.h | 14 +++++++
include/vdso/time.h | 12 ++++++
include/vdso/time32.h | 17 +++++++++
include/vdso/time64.h | 14 +++++++
lib/vdso/gettimeofday.c | 22 -----------
scripts/mod/modpost.c | 6 ++-
52 files changed, 459 insertions(+), 230 deletions(-)
create mode 100644 arch/arm/include/asm/vdso/clocksource.h
create mode 100644 arch/arm/include/asm/vdso/cp15.h
create mode 100644 arch/arm/include/asm/vdso/processor.h
create mode 100644 arch/arm64/include/asm/vdso/arch_timer.h
create mode 100644 arch/arm64/include/asm/vdso/clocksource.h
create mode 100644 arch/arm64/include/asm/vdso/processor.h
create mode 100644 arch/mips/include/asm/vdso/clocksource.h
create mode 100644 arch/mips/include/asm/vdso/processor.h
create mode 100644 arch/x86/include/asm/vdso/clocksource.h
create mode 100644 arch/x86/include/asm/vdso/processor.h
create mode 100644 include/vdso/bits.h
create mode 100644 include/vdso/clocksource.h
create mode 100644 include/vdso/const.h
create mode 100644 include/vdso/jiffies.h
create mode 100644 include/vdso/ktime.h
create mode 100644 include/vdso/limits.h
create mode 100644 include/vdso/math64.h
create mode 100644 include/vdso/processor.h
create mode 100644 include/vdso/time.h
create mode 100644 include/vdso/time32.h
create mode 100644 include/vdso/time64.h
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split const.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/const.h | 5 +----
include/vdso/const.h | 10 ++++++++++
2 files changed, 11 insertions(+), 4 deletions(-)
create mode 100644 include/vdso/const.h
diff --git a/include/linux/const.h b/include/linux/const.h
index 7b55a55f5911..81b8aae5a855 100644
--- a/include/linux/const.h
+++ b/include/linux/const.h
@@ -1,9 +1,6 @@
#ifndef _LINUX_CONST_H
#define _LINUX_CONST_H
-#include <uapi/linux/const.h>
-
-#define UL(x) (_UL(x))
-#define ULL(x) (_ULL(x))
+#include <vdso/const.h>
#endif /* _LINUX_CONST_H */
diff --git a/include/vdso/const.h b/include/vdso/const.h
new file mode 100644
index 000000000000..94b385ad438d
--- /dev/null
+++ b/include/vdso/const.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_CONST_H
+#define __VDSO_CONST_H
+
+#include <uapi/linux/const.h>
+
+#define UL(x) (_UL(x))
+#define ULL(x) (_ULL(x))
+
+#endif /* __VDSO_CONST_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split bits.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/bits.h | 2 +-
include/vdso/bits.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 include/vdso/bits.h
diff --git a/include/linux/bits.h b/include/linux/bits.h
index 669d69441a62..a740bbcf3cd2 100644
--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -3,9 +3,9 @@
#define __LINUX_BITS_H
#include <linux/const.h>
+#include <vdso/bits.h>
#include <asm/bitsperlong.h>
-#define BIT(nr) (UL(1) << (nr))
#define BIT_ULL(nr) (ULL(1) << (nr))
#define BIT_MASK(nr) (UL(1) << ((nr) % BITS_PER_LONG))
#define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
diff --git a/include/vdso/bits.h b/include/vdso/bits.h
new file mode 100644
index 000000000000..6d005a1f5d94
--- /dev/null
+++ b/include/vdso/bits.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_BITS_H
+#define __VDSO_BITS_H
+
+#include <vdso/const.h>
+
+#define BIT(nr) (UL(1) << (nr))
+
+#endif /* __VDSO_BITS_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split limits.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/limits.h | 13 +------------
include/vdso/limits.h | 18 ++++++++++++++++++
2 files changed, 19 insertions(+), 12 deletions(-)
create mode 100644 include/vdso/limits.h
diff --git a/include/linux/limits.h b/include/linux/limits.h
index 76afcd24ff8c..7fc497ee1393 100644
--- a/include/linux/limits.h
+++ b/include/linux/limits.h
@@ -4,19 +4,8 @@
#include <uapi/linux/limits.h>
#include <linux/types.h>
+#include <vdso/limits.h>
-#define USHRT_MAX ((unsigned short)~0U)
-#define SHRT_MAX ((short)(USHRT_MAX >> 1))
-#define SHRT_MIN ((short)(-SHRT_MAX - 1))
-#define INT_MAX ((int)(~0U >> 1))
-#define INT_MIN (-INT_MAX - 1)
-#define UINT_MAX (~0U)
-#define LONG_MAX ((long)(~0UL >> 1))
-#define LONG_MIN (-LONG_MAX - 1)
-#define ULONG_MAX (~0UL)
-#define LLONG_MAX ((long long)(~0ULL >> 1))
-#define LLONG_MIN (-LLONG_MAX - 1)
-#define ULLONG_MAX (~0ULL)
#define SIZE_MAX (~(size_t)0)
#define PHYS_ADDR_MAX (~(phys_addr_t)0)
diff --git a/include/vdso/limits.h b/include/vdso/limits.h
new file mode 100644
index 000000000000..7d2cd75c7ac2
--- /dev/null
+++ b/include/vdso/limits.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_LIMITS_H
+#define __VDSO_LIMITS_H
+
+#define USHRT_MAX ((unsigned short)~0U)
+#define SHRT_MAX ((short)(USHRT_MAX >> 1))
+#define SHRT_MIN ((short)(-SHRT_MAX - 1))
+#define INT_MAX ((int)(~0U >> 1))
+#define INT_MIN (-INT_MAX - 1)
+#define UINT_MAX (~0U)
+#define LONG_MAX ((long)(~0UL >> 1))
+#define LONG_MIN (-LONG_MAX - 1)
+#define ULONG_MAX (~0UL)
+#define LLONG_MAX ((long long)(~0ULL >> 1))
+#define LLONG_MIN (-LLONG_MAX - 1)
+#define ULLONG_MAX (~0ULL)
+
+#endif /* __VDSO_LIMITS_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce asm/vdso/clocksource.h to contain all the arm64 specific
functions that are suitable for vDSO inclusion.
This header will be required by a future patch that will generalize
vdso/clocksource.h.
Cc: Russell King <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm/include/asm/clocksource.h | 6 +++---
arch/arm/include/asm/vdso/clocksource.h | 8 ++++++++
2 files changed, 11 insertions(+), 3 deletions(-)
create mode 100644 arch/arm/include/asm/vdso/clocksource.h
diff --git a/arch/arm/include/asm/clocksource.h b/arch/arm/include/asm/clocksource.h
index 73beb7f131de..13651c731a81 100644
--- a/arch/arm/include/asm/clocksource.h
+++ b/arch/arm/include/asm/clocksource.h
@@ -1,7 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
#ifndef _ASM_CLOCKSOURCE_H
#define _ASM_CLOCKSOURCE_H
-#define VDSO_ARCH_CLOCKMODES \
- VDSO_CLOCKMODE_ARCHTIMER
+#include <asm/vdso/clocksource.h>
-#endif
+#endif /* _ASM_CLOCKSOURCE_H */
diff --git a/arch/arm/include/asm/vdso/clocksource.h b/arch/arm/include/asm/vdso/clocksource.h
new file mode 100644
index 000000000000..50c0b19fb755
--- /dev/null
+++ b/arch/arm/include/asm/vdso/clocksource.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSOCLOCKSOURCE_H
+#define __ASM_VDSOCLOCKSOURCE_H
+
+#define VDSO_ARCH_CLOCKMODES \
+ VDSO_CLOCKMODE_ARCHTIMER
+
+#endif /* __ASM_VDSOCLOCKSOURCE_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce asm/vdso/clocksource.h to contain all the arm64 specific
functions that are suitable for vDSO inclusion.
This header will be required by a future patch that will generalize
vdso/clocksource.h.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/include/asm/clocksource.h | 3 +--
arch/arm64/include/asm/vdso/clocksource.h | 8 ++++++++
2 files changed, 9 insertions(+), 2 deletions(-)
create mode 100644 arch/arm64/include/asm/vdso/clocksource.h
diff --git a/arch/arm64/include/asm/clocksource.h b/arch/arm64/include/asm/clocksource.h
index eb82e9d95c5d..482185566b0c 100644
--- a/arch/arm64/include/asm/clocksource.h
+++ b/arch/arm64/include/asm/clocksource.h
@@ -2,7 +2,6 @@
#ifndef _ASM_CLOCKSOURCE_H
#define _ASM_CLOCKSOURCE_H
-#define VDSO_ARCH_CLOCKMODES \
- VDSO_CLOCKMODE_ARCHTIMER
+#include <asm/vdso/clocksource.h>
#endif
diff --git a/arch/arm64/include/asm/vdso/clocksource.h b/arch/arm64/include/asm/vdso/clocksource.h
new file mode 100644
index 000000000000..df6ea65c1dec
--- /dev/null
+++ b/arch/arm64/include/asm/vdso/clocksource.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSOCLOCKSOURCE_H
+#define __ASM_VDSOCLOCKSOURCE_H
+
+#define VDSO_ARCH_CLOCKMODES \
+ VDSO_CLOCKMODE_ARCHTIMER
+
+#endif
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce asm/vdso/clocksource.h to contain all the arm64 specific
functions that are suitable for vDSO inclusion.
This header will be required by a future patch that will generalize
vdso/clocksource.h.
Cc: Paul Burton <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/mips/include/asm/clocksource.h | 4 +---
arch/mips/include/asm/vdso/clocksource.h | 9 +++++++++
2 files changed, 10 insertions(+), 3 deletions(-)
create mode 100644 arch/mips/include/asm/vdso/clocksource.h
diff --git a/arch/mips/include/asm/clocksource.h b/arch/mips/include/asm/clocksource.h
index de659cae0d4e..2f1ebbea3d72 100644
--- a/arch/mips/include/asm/clocksource.h
+++ b/arch/mips/include/asm/clocksource.h
@@ -6,8 +6,6 @@
#ifndef __ASM_CLOCKSOURCE_H
#define __ASM_CLOCKSOURCE_H
-#define VDSO_ARCH_CLOCKMODES \
- VDSO_CLOCKMODE_R4K, \
- VDSO_CLOCKMODE_GIC
+#include <asm/vdso/clocksource.h>
#endif /* __ASM_CLOCKSOURCE_H */
diff --git a/arch/mips/include/asm/vdso/clocksource.h b/arch/mips/include/asm/vdso/clocksource.h
new file mode 100644
index 000000000000..510e1671d898
--- /dev/null
+++ b/arch/mips/include/asm/vdso/clocksource.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __ASM_VDSOCLOCKSOURCE_H
+#define __ASM_VDSOCLOCKSOURCE_H
+
+#define VDSO_ARCH_CLOCKMODES \
+ VDSO_CLOCKMODE_R4K, \
+ VDSO_CLOCKMODE_GIC
+
+#endif /* __ASM_VDSOCLOCKSOURCE_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split clocksource.h into linux and common headers to make the latter
suitable for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/clocksource.h | 11 +----------
include/vdso/clocksource.h | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+), 10 deletions(-)
create mode 100644 include/vdso/clocksource.h
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 02e3282719bd..86d143db6523 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -28,16 +28,7 @@ struct module;
#include <asm/clocksource.h>
#endif
-enum vdso_clock_mode {
- VDSO_CLOCKMODE_NONE,
-#ifdef CONFIG_GENERIC_GETTIMEOFDAY
- VDSO_ARCH_CLOCKMODES,
-#endif
- VDSO_CLOCKMODE_MAX,
-
- /* Indicator for time namespace VDSO */
- VDSO_CLOCKMODE_TIMENS = INT_MAX
-};
+#include <vdso/clocksource.h>
/**
* struct clocksource - hardware abstraction for a free running counter
diff --git a/include/vdso/clocksource.h b/include/vdso/clocksource.h
new file mode 100644
index 000000000000..ab58330e4e5d
--- /dev/null
+++ b/include/vdso/clocksource.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_CLOCKSOURCE_H
+#define __VDSO_CLOCKSOURCE_H
+
+#include <vdso/limits.h>
+
+#if defined(CONFIG_ARCH_CLOCKSOURCE_DATA) || \
+ defined(CONFIG_GENERIC_GETTIMEOFDAY)
+#include <asm/vdso/clocksource.h>
+#endif /* CONFIG_ARCH_CLOCKSOURCE_DATA || CONFIG_GENERIC_GETTIMEOFDAY */
+
+enum vdso_clock_mode {
+ VDSO_CLOCKMODE_NONE,
+#ifdef CONFIG_GENERIC_GETTIMEOFDAY
+ VDSO_ARCH_CLOCKMODES,
+#endif
+ VDSO_CLOCKMODE_MAX,
+
+ /* Indicator for time namespace VDSO */
+ VDSO_CLOCKMODE_TIMENS = INT_MAX
+};
+
+#endif /* __VDSO_CLOCKSOURCE_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split time32.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/time32.h | 14 ++------------
include/vdso/time32.h | 17 +++++++++++++++++
2 files changed, 19 insertions(+), 12 deletions(-)
create mode 100644 include/vdso/time32.h
diff --git a/include/linux/time32.h b/include/linux/time32.h
index cad4c3186002..0933f28214c0 100644
--- a/include/linux/time32.h
+++ b/include/linux/time32.h
@@ -12,19 +12,9 @@
#include <linux/time64.h>
#include <linux/timex.h>
-#define TIME_T_MAX (__kernel_old_time_t)((1UL << ((sizeof(__kernel_old_time_t) << 3) - 1)) - 1)
-
-typedef s32 old_time32_t;
-
-struct old_timespec32 {
- old_time32_t tv_sec;
- s32 tv_nsec;
-};
+#include <vdso/time32.h>
-struct old_timeval32 {
- old_time32_t tv_sec;
- s32 tv_usec;
-};
+#define TIME_T_MAX (__kernel_old_time_t)((1UL << ((sizeof(__kernel_old_time_t) << 3) - 1)) - 1)
struct old_itimerspec32 {
struct old_timespec32 it_interval;
diff --git a/include/vdso/time32.h b/include/vdso/time32.h
new file mode 100644
index 000000000000..fdf56f932f67
--- /dev/null
+++ b/include/vdso/time32.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_TIME32_H
+#define __VDSO_TIME32_H
+
+typedef s32 old_time32_t;
+
+struct old_timespec32 {
+ old_time32_t tv_sec;
+ s32 tv_nsec;
+};
+
+struct old_timeval32 {
+ old_time32_t tv_sec;
+ s32 tv_usec;
+};
+
+#endif /* __VDSO_TIME32_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split time64.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/time64.h | 10 +---------
include/vdso/time64.h | 14 ++++++++++++++
2 files changed, 15 insertions(+), 9 deletions(-)
create mode 100644 include/vdso/time64.h
diff --git a/include/linux/time64.h b/include/linux/time64.h
index 19125489ae94..c9dcb3e5781f 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -3,6 +3,7 @@
#define _LINUX_TIME64_H
#include <linux/math64.h>
+#include <vdso/time64.h>
typedef __s64 time64_t;
typedef __u64 timeu64_t;
@@ -19,15 +20,6 @@ struct itimerspec64 {
struct timespec64 it_value;
};
-/* Parameters used to convert the timespec values: */
-#define MSEC_PER_SEC 1000L
-#define USEC_PER_MSEC 1000L
-#define NSEC_PER_USEC 1000L
-#define NSEC_PER_MSEC 1000000L
-#define USEC_PER_SEC 1000000L
-#define NSEC_PER_SEC 1000000000L
-#define FSEC_PER_SEC 1000000000000000LL
-
/* Located here for timespec[64]_valid_strict */
#define TIME64_MAX ((s64)~((u64)1 << 63))
#define TIME64_MIN (-TIME64_MAX - 1)
diff --git a/include/vdso/time64.h b/include/vdso/time64.h
new file mode 100644
index 000000000000..9d43c3f5e89d
--- /dev/null
+++ b/include/vdso/time64.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_TIME64_H
+#define __VDSO_TIME64_H
+
+/* Parameters used to convert the timespec values: */
+#define MSEC_PER_SEC 1000L
+#define USEC_PER_MSEC 1000L
+#define NSEC_PER_USEC 1000L
+#define NSEC_PER_MSEC 1000000L
+#define USEC_PER_SEC 1000000L
+#define NSEC_PER_SEC 1000000000L
+#define FSEC_PER_SEC 1000000000000000LL
+
+#endif /* __VDSO_TIME64_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce processor.h to contain all the processor specific functions
that are suitable for vDSO inclusion.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/vdso/processor.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 include/vdso/processor.h
diff --git a/include/vdso/processor.h b/include/vdso/processor.h
new file mode 100644
index 000000000000..fbe8265ea3c4
--- /dev/null
+++ b/include/vdso/processor.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __VDSO_PROCESSOR_H
+#define __VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/vdso/processor.h>
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __VDSO_PROCESSOR_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Refactor the vdso32 implementation to include common headers.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/include/asm/vdso/compat_gettimeofday.h | 2 +-
arch/arm64/kernel/vdso32/vgettimeofday.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/vdso/compat_gettimeofday.h b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
index 81b0c394f1d8..8d8d1c006a68 100644
--- a/arch/arm64/include/asm/vdso/compat_gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/compat_gettimeofday.h
@@ -8,7 +8,7 @@
#ifndef __ASSEMBLY__
#include <asm/unistd.h>
-#include <uapi/linux/time.h>
+#include <asm/errno.h>
#include <asm/vdso/compat_barrier.h>
diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c b/arch/arm64/kernel/vdso32/vgettimeofday.c
index 54fc1c2ce93f..9366ceb635a1 100644
--- a/arch/arm64/kernel/vdso32/vgettimeofday.c
+++ b/arch/arm64/kernel/vdso32/vgettimeofday.c
@@ -5,9 +5,6 @@
* Copyright (C) 2018 ARM Limited
*
*/
-#include <linux/time.h>
-#include <linux/types.h>
-
int __vdso_clock_gettime(clockid_t clock,
struct old_timespec32 *ts)
{
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce asm/vdso/arch_timer.h to contain all the arm64 specific
code. This allows to replace the second isb() in __arch_get_hw_counter()
with a fake dependent stack read of the counter which improves the vdso
library peformances of ~4.5%. Below the results of vdsotest [1] ran for
100 iterations.
Before the patch:
=================
clock-gettime-monotonic: syscall: 771 nsec/call
clock-gettime-monotonic: libc: 130 nsec/call
clock-gettime-monotonic: vdso: 111 nsec/call
...
clock-gettime-realtime: syscall: 762 nsec/call
clock-gettime-realtime: libc: 130 nsec/call
clock-gettime-realtime: vdso: 111 nsec/call
After the patch:
================
clock-gettime-monotonic: syscall: 792 nsec/call
clock-gettime-monotonic: libc: 124 nsec/call
clock-gettime-monotonic: vdso: 106 nsec/call
...
clock-gettime-realtime: syscall: 776 nsec/call
clock-gettime-realtime: libc: 124 nsec/call
clock-gettime-realtime: vdso: 106 nsec/call
[1] https://github.com/nathanlynch/vdsotest
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Mark Rutland <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/include/asm/arch_timer.h | 29 ++++---------------
arch/arm64/include/asm/vdso/arch_timer.h | 33 ++++++++++++++++++++++
arch/arm64/include/asm/vdso/gettimeofday.h | 7 +++--
3 files changed, 42 insertions(+), 27 deletions(-)
create mode 100644 arch/arm64/include/asm/vdso/arch_timer.h
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 7ae54d7d333a..7f22cd00ad45 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -164,24 +164,7 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
isb();
}
-/*
- * Ensure that reads of the counter are treated the same as memory reads
- * for the purposes of ordering by subsequent memory barriers.
- *
- * This insanity brought to you by speculative system register reads,
- * out-of-order memory accesses, sequence locks and Thomas Gleixner.
- *
- * http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/631195.html
- */
-#define arch_counter_enforce_ordering(val) do { \
- u64 tmp, _val = (val); \
- \
- asm volatile( \
- " eor %0, %1, %1\n" \
- " add %0, sp, %0\n" \
- " ldr xzr, [%0]" \
- : "=r" (tmp) : "r" (_val)); \
-} while (0)
+#include <asm/vdso/arch_timer.h>
static __always_inline u64 __arch_counter_get_cntpct_stable(void)
{
@@ -189,7 +172,7 @@ static __always_inline u64 __arch_counter_get_cntpct_stable(void)
isb();
cnt = arch_timer_reg_read_stable(cntpct_el0);
- arch_counter_enforce_ordering(cnt);
+ cnt = arch_counter_enforce_ordering(cnt);
return cnt;
}
@@ -199,7 +182,7 @@ static __always_inline u64 __arch_counter_get_cntpct(void)
isb();
cnt = read_sysreg(cntpct_el0);
- arch_counter_enforce_ordering(cnt);
+ cnt = arch_counter_enforce_ordering(cnt);
return cnt;
}
@@ -209,7 +192,7 @@ static __always_inline u64 __arch_counter_get_cntvct_stable(void)
isb();
cnt = arch_timer_reg_read_stable(cntvct_el0);
- arch_counter_enforce_ordering(cnt);
+ cnt = arch_counter_enforce_ordering(cnt);
return cnt;
}
@@ -219,12 +202,10 @@ static __always_inline u64 __arch_counter_get_cntvct(void)
isb();
cnt = read_sysreg(cntvct_el0);
- arch_counter_enforce_ordering(cnt);
+ cnt = arch_counter_enforce_ordering(cnt);
return cnt;
}
-#undef arch_counter_enforce_ordering
-
static inline int arch_timer_arch_init(void)
{
return 0;
diff --git a/arch/arm64/include/asm/vdso/arch_timer.h b/arch/arm64/include/asm/vdso/arch_timer.h
new file mode 100644
index 000000000000..a71bc83232f5
--- /dev/null
+++ b/arch/arm64/include/asm/vdso/arch_timer.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_VDSO_ARCH_TIMER_H
+#define __ASM_VDSO_ARCH_TIMER_H
+
+#include <uapi/linux/types.h>
+
+/*
+ * Ensure that reads of the counter are treated the same as memory reads
+ * for the purposes of ordering by subsequent memory barriers.
+ *
+ * This insanity brought to you by speculative system register reads,
+ * out-of-order memory accesses, sequence locks and Thomas Gleixner.
+ *
+ * http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/631195.html
+ *
+ */
+static u64 arch_counter_enforce_ordering(u64 val)
+{
+ u64 tmp, _val = (val);
+
+ asm volatile(
+ " eor %0, %1, %1\n"
+ " add %0, sp, %0\n"
+ " ldr xzr, [%0]"
+ : "=r" (tmp) : "r" (_val));
+
+ return _val;
+}
+
+#endif /* __ASM_VDSO_ARCH_TIMER_H */
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index afba6ba332f8..319808106625 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -8,6 +8,7 @@
#ifndef __ASSEMBLY__
#include <asm/unistd.h>
+#include <asm/vdso/arch_timer.h>
#define VDSO_HAS_CLOCK_GETRES 1
@@ -82,10 +83,10 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode)
isb();
asm volatile("mrs %0, cntvct_el0" : "=r" (res) :: "memory");
/*
- * This isb() is required to prevent that the seq lock is
- * speculated.#
+ * arch_counter_enforce_ordering() is required to prevent that
+ * the seq lock is speculated.
*/
- isb();
+ res = arch_counter_enforce_ordering(res);
return res;
}
--
2.25.1
Enable x86 to use only the common headers in the implementation
of the vDSO library.
Cc: Thomas Gleixner <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/x86/include/asm/processor.h | 12 +-----------
arch/x86/include/asm/vdso/processor.h | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+), 11 deletions(-)
create mode 100644 arch/x86/include/asm/vdso/processor.h
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 09705ccc393c..94789db550df 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -26,6 +26,7 @@ struct vm86;
#include <asm/fpu/types.h>
#include <asm/unwind_hints.h>
#include <asm/vmxfeatures.h>
+#include <asm/vdso/processor.h>
#include <linux/personality.h>
#include <linux/cache.h>
@@ -677,17 +678,6 @@ static inline unsigned int cpuid_edx(unsigned int op)
return edx;
}
-/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */
-static __always_inline void rep_nop(void)
-{
- asm volatile("rep; nop" ::: "memory");
-}
-
-static __always_inline void cpu_relax(void)
-{
- rep_nop();
-}
-
/*
* This function forces the icache and prefetched instruction stream to
* catch up with reality in two very specific cases:
diff --git a/arch/x86/include/asm/vdso/processor.h b/arch/x86/include/asm/vdso/processor.h
new file mode 100644
index 000000000000..57b1a7034c64
--- /dev/null
+++ b/arch/x86/include/asm/vdso/processor.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_VDSO_PROCESSOR_H
+#define __ASM_VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+/* REP NOP (PAUSE) is a good thing to insert into busy-wait loops. */
+static __always_inline void rep_nop(void)
+{
+ asm volatile("rep; nop" ::: "memory");
+}
+
+static __always_inline void cpu_relax(void)
+{
+ rep_nop();
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_PROCESSOR_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Refactor the unified vdso code to use the common headers.
Cc: Andy Lutomirski <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/vdso/datapage.h | 33 ++++++++++++++++++++++++++++++---
lib/vdso/gettimeofday.c | 22 ----------------------
2 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/include/vdso/datapage.h b/include/vdso/datapage.h
index 30c4cb0428d1..5cbc9fcbfd45 100644
--- a/include/vdso/datapage.h
+++ b/include/vdso/datapage.h
@@ -4,9 +4,20 @@
#ifndef __ASSEMBLY__
-#include <linux/bits.h>
-#include <linux/time.h>
-#include <linux/types.h>
+#include <linux/compiler.h>
+#include <uapi/linux/time.h>
+#include <uapi/linux/types.h>
+#include <uapi/asm-generic/errno-base.h>
+
+#include <vdso/bits.h>
+#include <vdso/clocksource.h>
+#include <vdso/ktime.h>
+#include <vdso/limits.h>
+#include <vdso/math64.h>
+#include <vdso/processor.h>
+#include <vdso/time.h>
+#include <vdso/time32.h>
+#include <vdso/time64.h>
#define VDSO_BASES (CLOCK_TAI + 1)
#define VDSO_HRES (BIT(CLOCK_REALTIME) | \
@@ -99,6 +110,22 @@ struct vdso_data {
*/
extern struct vdso_data _vdso_data[CS_BASES] __attribute__((visibility("hidden")));
+/*
+ * The generic vDSO implementation requires that gettimeofday.h
+ * provides:
+ * - __arch_get_vdso_data(): to get the vdso datapage.
+ * - __arch_get_hw_counter(): to get the hw counter based on the
+ * clock_mode.
+ * - gettimeofday_fallback(): fallback for gettimeofday.
+ * - clock_gettime_fallback(): fallback for clock_gettime.
+ * - clock_getres_fallback(): fallback for clock_getres.
+ */
+#ifdef ENABLE_COMPAT_VDSO
+#include <asm/vdso/compat_gettimeofday.h>
+#else
+#include <asm/vdso/gettimeofday.h>
+#endif /* ENABLE_COMPAT_VDSO */
+
#endif /* !__ASSEMBLY__ */
#endif /* __VDSO_DATAPAGE_H */
diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 72d282ffd156..a2909af4b924 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -2,31 +2,9 @@
/*
* Generic userspace implementations of gettimeofday() and similar.
*/
-#include <linux/compiler.h>
-#include <linux/math64.h>
-#include <linux/time.h>
-#include <linux/kernel.h>
-#include <linux/hrtimer_defs.h>
-#include <linux/clocksource.h>
#include <vdso/datapage.h>
#include <vdso/helpers.h>
-/*
- * The generic vDSO implementation requires that gettimeofday.h
- * provides:
- * - __arch_get_vdso_data(): to get the vdso datapage.
- * - __arch_get_hw_counter(): to get the hw counter based on the
- * clock_mode.
- * - gettimeofday_fallback(): fallback for gettimeofday.
- * - clock_gettime_fallback(): fallback for clock_gettime.
- * - clock_getres_fallback(): fallback for clock_getres.
- */
-#ifdef ENABLE_COMPAT_VDSO
-#include <asm/vdso/compat_gettimeofday.h>
-#else
-#include <asm/vdso/gettimeofday.h>
-#endif /* ENABLE_COMPAT_VDSO */
-
#ifndef vdso_calc_delta
/*
* Default implementation which works for all sane clocksources. That
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split math64.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/math64.h | 20 +-------------------
include/vdso/math64.h | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+), 19 deletions(-)
create mode 100644 include/vdso/math64.h
diff --git a/include/linux/math64.h b/include/linux/math64.h
index 65bef21cdddb..11a267413e8e 100644
--- a/include/linux/math64.h
+++ b/include/linux/math64.h
@@ -3,6 +3,7 @@
#define _LINUX_MATH64_H
#include <linux/types.h>
+#include <vdso/math64.h>
#include <asm/div64.h>
#if BITS_PER_LONG == 64
@@ -142,25 +143,6 @@ static inline s64 div_s64(s64 dividend, s32 divisor)
u32 iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder);
-static __always_inline u32
-__iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
-{
- u32 ret = 0;
-
- while (dividend >= divisor) {
- /* The following asm() prevents the compiler from
- optimising this loop into a modulo operation. */
- asm("" : "+rm"(dividend));
-
- dividend -= divisor;
- ret++;
- }
-
- *remainder = dividend;
-
- return ret;
-}
-
#ifndef mul_u32_u32
/*
* Many a GCC version messes this up and generates a 64x64 mult :-(
diff --git a/include/vdso/math64.h b/include/vdso/math64.h
new file mode 100644
index 000000000000..7da703ee5561
--- /dev/null
+++ b/include/vdso/math64.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_MATH64_H
+#define __VDSO_MATH64_H
+
+static __always_inline u32
+__iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
+{
+ u32 ret = 0;
+
+ while (dividend >= divisor) {
+ /* The following asm() prevents the compiler from
+ optimising this loop into a modulo operation. */
+ asm("" : "+rm"(dividend));
+
+ dividend -= divisor;
+ ret++;
+ }
+
+ *remainder = dividend;
+
+ return ret;
+}
+
+#endif /* __VDSO_MATH64_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Refactor the vdso implementation to include common headers.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/include/asm/vdso/gettimeofday.h | 1 -
arch/arm64/kernel/vdso/vgettimeofday.c | 2 --
2 files changed, 3 deletions(-)
diff --git a/arch/arm64/include/asm/vdso/gettimeofday.h b/arch/arm64/include/asm/vdso/gettimeofday.h
index 5a534432aa5d..afba6ba332f8 100644
--- a/arch/arm64/include/asm/vdso/gettimeofday.h
+++ b/arch/arm64/include/asm/vdso/gettimeofday.h
@@ -8,7 +8,6 @@
#ifndef __ASSEMBLY__
#include <asm/unistd.h>
-#include <uapi/linux/time.h>
#define VDSO_HAS_CLOCK_GETRES 1
diff --git a/arch/arm64/kernel/vdso/vgettimeofday.c b/arch/arm64/kernel/vdso/vgettimeofday.c
index 747635501a14..4236cf34d7d9 100644
--- a/arch/arm64/kernel/vdso/vgettimeofday.c
+++ b/arch/arm64/kernel/vdso/vgettimeofday.c
@@ -5,8 +5,6 @@
* Copyright (C) 2018 ARM Limited
*
*/
-#include <linux/time.h>
-#include <linux/types.h>
int __kernel_clock_gettime(clockid_t clock,
struct __kernel_timespec *ts)
--
2.25.1
Enable arm to use only the common headers in the implementation
of the vDSO library.
Cc: Russell King <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm/include/asm/cp15.h | 20 +------------
arch/arm/include/asm/processor.h | 11 +------
arch/arm/include/asm/vdso/cp15.h | 38 ++++++++++++++++++++++++
arch/arm/include/asm/vdso/gettimeofday.h | 4 +--
arch/arm/include/asm/vdso/processor.h | 22 ++++++++++++++
5 files changed, 64 insertions(+), 31 deletions(-)
create mode 100644 arch/arm/include/asm/vdso/cp15.h
create mode 100644 arch/arm/include/asm/vdso/processor.h
diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
index d2453e2d3f1f..a54230e65647 100644
--- a/arch/arm/include/asm/cp15.h
+++ b/arch/arm/include/asm/cp15.h
@@ -50,25 +50,7 @@
#ifdef CONFIG_CPU_CP15
-#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
- "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
-#define __ACCESS_CP15_64(Op1, CRm) \
- "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
-
-#define __read_sysreg(r, w, c, t) ({ \
- t __val; \
- asm volatile(r " " c : "=r" (__val)); \
- __val; \
-})
-#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
-
-#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
-#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
-
-#define BPIALL __ACCESS_CP15(c7, 0, c5, 6)
-#define ICIALLU __ACCESS_CP15(c7, 0, c5, 0)
-
-#define CNTVCT __ACCESS_CP15_64(1, c14)
+#include <asm/vdso/cp15.h>
extern unsigned long cr_alignment; /* defined in entry-armv.S */
diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
index 614bf829e454..b9241051e5cb 100644
--- a/arch/arm/include/asm/processor.h
+++ b/arch/arm/include/asm/processor.h
@@ -14,6 +14,7 @@
#include <asm/ptrace.h>
#include <asm/types.h>
#include <asm/unified.h>
+#include <asm/vdso/processor.h>
#ifdef __KERNEL__
#define STACK_TOP ((current->personality & ADDR_LIMIT_32BIT) ? \
@@ -85,16 +86,6 @@ extern void release_thread(struct task_struct *);
unsigned long get_wchan(struct task_struct *p);
-#if __LINUX_ARM_ARCH__ == 6 || defined(CONFIG_ARM_ERRATA_754327)
-#define cpu_relax() \
- do { \
- smp_mb(); \
- __asm__ __volatile__("nop; nop; nop; nop; nop; nop; nop; nop; nop; nop;"); \
- } while (0)
-#else
-#define cpu_relax() barrier()
-#endif
-
#define task_pt_regs(p) \
((struct pt_regs *)(THREAD_START_SP + task_stack_page(p)) - 1)
diff --git a/arch/arm/include/asm/vdso/cp15.h b/arch/arm/include/asm/vdso/cp15.h
new file mode 100644
index 000000000000..bed16fa1865e
--- /dev/null
+++ b/arch/arm/include/asm/vdso/cp15.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_VDSO_CP15_H
+#define __ASM_VDSO_CP15_H
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_CPU_CP15
+
+#include <linux/stringify.h>
+
+#define __ACCESS_CP15(CRn, Op1, CRm, Op2) \
+ "mrc", "mcr", __stringify(p15, Op1, %0, CRn, CRm, Op2), u32
+#define __ACCESS_CP15_64(Op1, CRm) \
+ "mrrc", "mcrr", __stringify(p15, Op1, %Q0, %R0, CRm), u64
+
+#define __read_sysreg(r, w, c, t) ({ \
+ t __val; \
+ asm volatile(r " " c : "=r" (__val)); \
+ __val; \
+})
+#define read_sysreg(...) __read_sysreg(__VA_ARGS__)
+
+#define __write_sysreg(v, r, w, c, t) asm volatile(w " " c : : "r" ((t)(v)))
+#define write_sysreg(v, ...) __write_sysreg(v, __VA_ARGS__)
+
+#define BPIALL __ACCESS_CP15(c7, 0, c5, 6)
+#define ICIALLU __ACCESS_CP15(c7, 0, c5, 0)
+
+#define CNTVCT __ACCESS_CP15_64(1, c14)
+
+#endif /* CONFIG_CPU_CP15 */
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_CP15_H */
diff --git a/arch/arm/include/asm/vdso/gettimeofday.h b/arch/arm/include/asm/vdso/gettimeofday.h
index 07d791c65cf7..36dc18553ed8 100644
--- a/arch/arm/include/asm/vdso/gettimeofday.h
+++ b/arch/arm/include/asm/vdso/gettimeofday.h
@@ -7,9 +7,9 @@
#ifndef __ASSEMBLY__
-#include <asm/barrier.h>
-#include <asm/cp15.h>
+#include <asm/errno.h>
#include <asm/unistd.h>
+#include <asm/vdso/cp15.h>
#include <uapi/linux/time.h>
#define VDSO_HAS_CLOCK_GETRES 1
diff --git a/arch/arm/include/asm/vdso/processor.h b/arch/arm/include/asm/vdso/processor.h
new file mode 100644
index 000000000000..45efb3ff511c
--- /dev/null
+++ b/arch/arm/include/asm/vdso/processor.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_VDSO_PROCESSOR_H
+#define __ASM_VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+#if __LINUX_ARM_ARCH__ == 6 || defined(CONFIG_ARM_ERRATA_754327)
+#define cpu_relax() \
+ do { \
+ smp_mb(); \
+ __asm__ __volatile__("nop; nop; nop; nop; nop; nop; nop; nop; nop; nop;"); \
+ } while (0)
+#else
+#define cpu_relax() barrier()
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_PROCESSOR_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split ktime.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/ktime.h | 9 +--------
include/vdso/ktime.h | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 8 deletions(-)
create mode 100644 include/vdso/ktime.h
diff --git a/include/linux/ktime.h b/include/linux/ktime.h
index b2bb44f87f5a..1fcfce97a020 100644
--- a/include/linux/ktime.h
+++ b/include/linux/ktime.h
@@ -253,14 +253,7 @@ static inline __must_check bool ktime_to_timespec64_cond(const ktime_t kt,
}
}
-/*
- * The resolution of the clocks. The resolution value is returned in
- * the clock_getres() system call to give application programmers an
- * idea of the (in)accuracy of timers. Timer values are rounded up to
- * this resolution values.
- */
-#define LOW_RES_NSEC TICK_NSEC
-#define KTIME_LOW_RES (LOW_RES_NSEC)
+#include <vdso/ktime.h>
static inline ktime_t ns_to_ktime(u64 ns)
{
diff --git a/include/vdso/ktime.h b/include/vdso/ktime.h
new file mode 100644
index 000000000000..a0fd07239e0e
--- /dev/null
+++ b/include/vdso/ktime.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_KTIME_H
+#define __VDSO_KTIME_H
+
+#include <vdso/jiffies.h>
+
+/*
+ * The resolution of the clocks. The resolution value is returned in
+ * the clock_getres() system call to give application programmers an
+ * idea of the (in)accuracy of timers. Timer values are rounded up to
+ * this resolution values.
+ */
+#define LOW_RES_NSEC TICK_NSEC
+#define KTIME_LOW_RES (LOW_RES_NSEC)
+
+#endif /* __VDSO_KTIME_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split time.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/time.h | 5 +----
include/vdso/time.h | 12 ++++++++++++
2 files changed, 13 insertions(+), 4 deletions(-)
create mode 100644 include/vdso/time.h
diff --git a/include/linux/time.h b/include/linux/time.h
index 8ef5e5cc9f57..4c325bf44ce0 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -111,9 +111,6 @@ static inline bool itimerspec64_valid(const struct itimerspec64 *its)
*/
#define time_between32(t, l, h) ((u32)(h) - (u32)(l) >= (u32)(t) - (u32)(l))
-struct timens_offset {
- s64 sec;
- u64 nsec;
-};
+# include <vdso/time.h>
#endif
diff --git a/include/vdso/time.h b/include/vdso/time.h
new file mode 100644
index 000000000000..739f53cd2949
--- /dev/null
+++ b/include/vdso/time.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_TIME_H
+#define __VDSO_TIME_H
+
+#include <uapi/linux/types.h>
+
+struct timens_offset {
+ s64 sec;
+ u64 nsec;
+};
+
+#endif /* __VDSO_TIME_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce asm/vdso/processor.h to contain all the arm64 specific
functions that are suitable for vDSO inclusion.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/include/asm/processor.h | 16 ++-----------
arch/arm64/include/asm/vdso/processor.h | 31 +++++++++++++++++++++++++
2 files changed, 33 insertions(+), 14 deletions(-)
create mode 100644 arch/arm64/include/asm/vdso/processor.h
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 5ba63204d078..89ba2c5be504 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -28,6 +28,8 @@
#include <linux/string.h>
#include <linux/thread_info.h>
+#include <vdso/processor.h>
+
#include <asm/alternative.h>
#include <asm/cpufeature.h>
#include <asm/hw_breakpoint.h>
@@ -47,15 +49,6 @@
#define TASK_SIZE_64 (UL(1) << vabits_actual)
#ifdef CONFIG_COMPAT
-#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
-/*
- * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
- * by the compat vectors page.
- */
-#define TASK_SIZE_32 UL(0x100000000)
-#else
-#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
-#endif /* CONFIG_ARM64_64K_PAGES */
#define TASK_SIZE (test_thread_flag(TIF_32BIT) ? \
TASK_SIZE_32 : TASK_SIZE_64)
#define TASK_SIZE_OF(tsk) (test_tsk_thread_flag(tsk, TIF_32BIT) ? \
@@ -256,11 +249,6 @@ extern void release_thread(struct task_struct *);
unsigned long get_wchan(struct task_struct *p);
-static inline void cpu_relax(void)
-{
- asm volatile("yield" ::: "memory");
-}
-
/* Thread switching */
extern struct task_struct *cpu_switch_to(struct task_struct *prev,
struct task_struct *next);
diff --git a/arch/arm64/include/asm/vdso/processor.h b/arch/arm64/include/asm/vdso/processor.h
new file mode 100644
index 000000000000..fb4883212a2d
--- /dev/null
+++ b/arch/arm64/include/asm/vdso/processor.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_VDSO_PROCESSOR_H
+#define __ASM_VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/page-def.h>
+
+#ifdef CONFIG_COMPAT
+#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
+/*
+ * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
+ * by the compat vectors page.
+ */
+#define TASK_SIZE_32 UL(0x100000000)
+#else
+#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
+#endif /* CONFIG_ARM64_64K_PAGES */
+#endif /* CONFIG_COMPAT */
+
+static inline void cpu_relax(void)
+{
+ asm volatile("yield" ::: "memory");
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_PROCESSOR_H */
--
2.25.1
Enable mips to use only the common headers in the implementation of
the vDSO library.
Cc: Paul Burton <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/mips/include/asm/processor.h | 16 +-------------
arch/mips/include/asm/vdso/gettimeofday.h | 4 ----
arch/mips/include/asm/vdso/processor.h | 27 +++++++++++++++++++++++
3 files changed, 28 insertions(+), 19 deletions(-)
create mode 100644 arch/mips/include/asm/vdso/processor.h
diff --git a/arch/mips/include/asm/processor.h b/arch/mips/include/asm/processor.h
index 7619ad319400..4c9cc667f3ed 100644
--- a/arch/mips/include/asm/processor.h
+++ b/arch/mips/include/asm/processor.h
@@ -22,6 +22,7 @@
#include <asm/dsemul.h>
#include <asm/mipsregs.h>
#include <asm/prefetch.h>
+#include <asm/vdso/processor.h>
/*
* System setup and hardware flags..
@@ -385,21 +386,6 @@ unsigned long get_wchan(struct task_struct *p);
#define KSTK_ESP(tsk) (task_pt_regs(tsk)->regs[29])
#define KSTK_STATUS(tsk) (task_pt_regs(tsk)->cp0_status)
-#ifdef CONFIG_CPU_LOONGSON64
-/*
- * Loongson-3's SFB (Store-Fill-Buffer) may buffer writes indefinitely when a
- * tight read loop is executed, because reads take priority over writes & the
- * hardware (incorrectly) doesn't ensure that writes will eventually occur.
- *
- * Since spin loops of any kind should have a cpu_relax() in them, force an SFB
- * flush from cpu_relax() such that any pending writes will become visible as
- * expected.
- */
-#define cpu_relax() smp_mb()
-#else
-#define cpu_relax() barrier()
-#endif
-
/*
* Return_address is a replacement for __builtin_return_address(count)
* which on certain architectures cannot reasonably be implemented in GCC
diff --git a/arch/mips/include/asm/vdso/gettimeofday.h b/arch/mips/include/asm/vdso/gettimeofday.h
index 88c3de1bdf22..c63ddcaea54c 100644
--- a/arch/mips/include/asm/vdso/gettimeofday.h
+++ b/arch/mips/include/asm/vdso/gettimeofday.h
@@ -13,12 +13,8 @@
#ifndef __ASSEMBLY__
-#include <linux/compiler.h>
-#include <linux/time.h>
-
#include <asm/vdso/vdso.h>
#include <asm/clocksource.h>
-#include <asm/io.h>
#include <asm/unistd.h>
#include <asm/vdso.h>
diff --git a/arch/mips/include/asm/vdso/processor.h b/arch/mips/include/asm/vdso/processor.h
new file mode 100644
index 000000000000..511c95d735e6
--- /dev/null
+++ b/arch/mips/include/asm/vdso/processor.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+#ifndef __ASM_VDSO_PROCESSOR_H
+#define __ASM_VDSO_PROCESSOR_H
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_CPU_LOONGSON64
+/*
+ * Loongson-3's SFB (Store-Fill-Buffer) may buffer writes indefinitely when a
+ * tight read loop is executed, because reads take priority over writes & the
+ * hardware (incorrectly) doesn't ensure that writes will eventually occur.
+ *
+ * Since spin loops of any kind should have a cpu_relax() in them, force an SFB
+ * flush from cpu_relax() such that any pending writes will become visible as
+ * expected.
+ */
+#define cpu_relax() smp_mb()
+#else
+#define cpu_relax() barrier()
+#endif
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_VDSO_PROCESSOR_H */
--
2.25.1
In the process of creating the source file of a module modpost injects a
set of includes that are not required if the compilation unit is
statically built into the kernel.
The order of inclusion of the headers can cause redefinition problems
(e.g.):
In file included from include/linux/elf.h:5:0,
from include/linux/module.h:18,
from crypto/arc4.mod.c:2:
>> arch/parisc/include/asm/elf.h:324:0: warning: "ELF_OSABI" redefined
#define ELF_OSABI ELFOSABI_LINUX
In file included from include/linux/elfnote.h:62:0,
from include/linux/build-salt.h:4,
from crypto/arc4.mod.c:1:
include/uapi/linux/elf.h:363:0: note: this is the location of
the previous definition
#define ELF_OSABI ELFOSABI_NONE
The issue was exposed during the development of the series [1].
[1] https://lore.kernel.org/lkml/[email protected]/
Reported-by: kbuild test robot <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Michal Marek <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
scripts/mod/modpost.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7edfdb2f4497..0f354b1ee2aa 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2251,8 +2251,12 @@ static int check_modname_len(struct module *mod)
**/
static void add_header(struct buffer *b, struct module *mod)
{
- buf_printf(b, "#include <linux/build-salt.h>\n");
buf_printf(b, "#include <linux/module.h>\n");
+ /*
+ * Include build-salt.h after module.h in order to
+ * inherit the definitions.
+ */
+ buf_printf(b, "#include <linux/build-salt.h>\n");
buf_printf(b, "#include <linux/vermagic.h>\n");
buf_printf(b, "#include <linux/compiler.h>\n");
buf_printf(b, "\n");
--
2.25.1
Enable Clang Compilation for the vdso32 library.
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Tested-by: Nathan Chancellor <[email protected]> # build
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/arm64/kernel/vdso32/Makefile | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 04df57b43cb1..3964738ebbde 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -10,7 +10,18 @@ include $(srctree)/lib/vdso/Makefile
# Same as cc-*option, but using CC_COMPAT instead of CC
ifeq ($(CONFIG_CC_IS_CLANG), y)
+COMPAT_GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE_COMPAT)elfedit))
+COMPAT_GCC_TOOLCHAIN := $(realpath $(COMPAT_GCC_TOOLCHAIN_DIR)/..)
+
+CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%))
+CC_COMPAT_CLANG_FLAGS += --prefix=$(COMPAT_GCC_TOOLCHAIN_DIR)
+CC_COMPAT_CLANG_FLAGS += -no-integrated-as -Qunused-arguments
+ifneq ($(COMPAT_GCC_TOOLCHAIN),)
+CC_COMPAT_CLANG_FLAGS += --gcc-toolchain=$(COMPAT_GCC_TOOLCHAIN)
+endif
+
CC_COMPAT ?= $(CC)
+CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS)
else
CC_COMPAT ?= $(CROSS_COMPILE_COMPAT)gcc
endif
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Replace linux/elf.h with UAPI equivalent in elfnote.h to make the header
suitable for vDSO inclusion.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/elfnote.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/elfnote.h b/include/linux/elfnote.h
index f236f5b931b2..594d4e78654f 100644
--- a/include/linux/elfnote.h
+++ b/include/linux/elfnote.h
@@ -59,7 +59,7 @@
ELFNOTE_END
#else /* !__ASSEMBLER__ */
-#include <linux/elf.h>
+#include <uapi/linux/elf.h>
/*
* Use an anonymous structure which matches the shape of
* Elf{32,64}_Nhdr, but includes the name and desc data. The size and
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Split jiffies.h into linux and common headers to make the latter suitable
for inclusion in the vDSO library.
Signed-off-by: Vincenzo Frascino <[email protected]>
---
include/linux/jiffies.h | 4 +---
include/vdso/jiffies.h | 11 +++++++++++
2 files changed, 12 insertions(+), 3 deletions(-)
create mode 100644 include/vdso/jiffies.h
diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index e3279ef24d28..fed6ba96c527 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -8,6 +8,7 @@
#include <linux/types.h>
#include <linux/time.h>
#include <linux/timex.h>
+#include <vdso/jiffies.h>
#include <asm/param.h> /* for HZ */
#include <generated/timeconst.h>
@@ -59,9 +60,6 @@
extern int register_refined_jiffies(long clock_tick_rate);
-/* TICK_NSEC is the time between ticks in nsec assuming SHIFTED_HZ */
-#define TICK_NSEC ((NSEC_PER_SEC+HZ/2)/HZ)
-
/* TICK_USEC is the time between ticks in usec assuming SHIFTED_HZ */
#define TICK_USEC ((USEC_PER_SEC + HZ/2) / HZ)
diff --git a/include/vdso/jiffies.h b/include/vdso/jiffies.h
new file mode 100644
index 000000000000..2f9d596c8b29
--- /dev/null
+++ b/include/vdso/jiffies.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __VDSO_JIFFIES_H
+#define __VDSO_JIFFIES_H
+
+#include <asm/param.h> /* for HZ */
+#include <vdso/time64.h>
+
+/* TICK_NSEC is the time between ticks in nsec assuming SHIFTED_HZ */
+#define TICK_NSEC ((NSEC_PER_SEC+HZ/2)/HZ)
+
+#endif /* __VDSO_JIFFIES_H */
--
2.25.1
The vDSO library should only include the necessary headers required for
a userspace library (UAPI and a minimal set of kernel headers). To make
this possible it is necessary to isolate from the kernel headers the
common parts that are strictly necessary to build the library.
Introduce asm/vdso/clocksource.h to contain all the arm64 specific
functions that are suitable for vDSO inclusion.
This header will be required by a future patch that will generalize
vdso/clocksource.h.
Cc: Thomas Gleixner <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
arch/x86/include/asm/clocksource.h | 5 +----
arch/x86/include/asm/vdso/clocksource.h | 10 ++++++++++
2 files changed, 11 insertions(+), 4 deletions(-)
create mode 100644 arch/x86/include/asm/vdso/clocksource.h
diff --git a/arch/x86/include/asm/clocksource.h b/arch/x86/include/asm/clocksource.h
index d561db67f96d..dc9dc7b3911a 100644
--- a/arch/x86/include/asm/clocksource.h
+++ b/arch/x86/include/asm/clocksource.h
@@ -4,10 +4,7 @@
#ifndef _ASM_X86_CLOCKSOURCE_H
#define _ASM_X86_CLOCKSOURCE_H
-#define VDSO_ARCH_CLOCKMODES \
- VDSO_CLOCKMODE_TSC, \
- VDSO_CLOCKMODE_PVCLOCK, \
- VDSO_CLOCKMODE_HVCLOCK
+#include <asm/vdso/clocksource.h>
extern unsigned int vclocks_used;
diff --git a/arch/x86/include/asm/vdso/clocksource.h b/arch/x86/include/asm/vdso/clocksource.h
new file mode 100644
index 000000000000..119ac8612d89
--- /dev/null
+++ b/arch/x86/include/asm/vdso/clocksource.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSO_CLOCKSOURCE_H
+#define __ASM_VDSO_CLOCKSOURCE_H
+
+#define VDSO_ARCH_CLOCKMODES \
+ VDSO_CLOCKMODE_TSC, \
+ VDSO_CLOCKMODE_PVCLOCK, \
+ VDSO_CLOCKMODE_HVCLOCK
+
+#endif /* __ASM_VDSO_CLOCKSOURCE_H */
--
2.25.1
Hi Vincenzo, all
I like the idea, but I'm wondering if we could have less-grained
headers? Like, AFAICS the patches create headers < 10 lines and even
mostly < 5 lines.. I like that header's names perfectly describe what's
inside, but I'm not sure how effective to have a lot of extra-small
includes.
Or maybe there's a plan to grow the code in them?
On 3/13/20 3:43 PM, Vincenzo Frascino wrote:
[..]
> create mode 100644 arch/arm/include/asm/vdso/clocksource.h
> create mode 100644 arch/arm/include/asm/vdso/cp15.h
> create mode 100644 arch/arm/include/asm/vdso/processor.h
> create mode 100644 arch/arm64/include/asm/vdso/arch_timer.h
> create mode 100644 arch/arm64/include/asm/vdso/clocksource.h
> create mode 100644 arch/arm64/include/asm/vdso/processor.h
> create mode 100644 arch/mips/include/asm/vdso/clocksource.h
> create mode 100644 arch/mips/include/asm/vdso/processor.h
> create mode 100644 arch/x86/include/asm/vdso/clocksource.h
> create mode 100644 arch/x86/include/asm/vdso/processor.h
> create mode 100644 include/vdso/bits.h
> create mode 100644 include/vdso/clocksource.h
> create mode 100644 include/vdso/const.h
> create mode 100644 include/vdso/jiffies.h
> create mode 100644 include/vdso/ktime.h
> create mode 100644 include/vdso/limits.h
> create mode 100644 include/vdso/math64.h
> create mode 100644 include/vdso/processor.h
> create mode 100644 include/vdso/time.h
> create mode 100644 include/vdso/time32.h
> create mode 100644 include/vdso/time64.h
Maybe we could made them less-grained?
I.e, time32 + time64 + time.h => time.h?
Thanks for Cc,
Dmitry
Dmitry Safonov <[email protected]> writes:
> I like the idea, but I'm wondering if we could have less-grained
> headers? Like, AFAICS the patches create headers < 10 lines and even
> mostly < 5 lines.. I like that header's names perfectly describe what's
> inside, but I'm not sure how effective to have a lot of extra-small
> includes.
If that goes all into a big header then the headers from where the bits and
pieces are split out would have all to include this big header which
might result in other include dependency nightmares.
>> create mode 100644 include/vdso/time.h
>> create mode 100644 include/vdso/time32.h
>> create mode 100644 include/vdso/time64.h
>
> Maybe we could made them less-grained?
>
> I.e, time32 + time64 + time.h => time.h?
Then you end up with ifdeffery. I like the clear separation.
Thanks,
tglx
On Fri, Mar 13, 2020 at 03:43:25PM +0000, Vincenzo Frascino wrote:
> The vDSO library should only include the necessary headers required for
> a userspace library (UAPI and a minimal set of kernel headers). To make
> this possible it is necessary to isolate from the kernel headers the
> common parts that are strictly necessary to build the library.
>
> Introduce asm/vdso/clocksource.h to contain all the arm64 specific
> functions that are suitable for vDSO inclusion.
>
> This header will be required by a future patch that will generalize
> vdso/clocksource.h.
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
> --- /dev/null
> +++ b/arch/arm64/include/asm/vdso/processor.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 ARM Ltd.
> + */
> +#ifndef __ASM_VDSO_PROCESSOR_H
> +#define __ASM_VDSO_PROCESSOR_H
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <asm/page-def.h>
> +
> +#ifdef CONFIG_COMPAT
> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
> +/*
> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
> + * by the compat vectors page.
> + */
> +#define TASK_SIZE_32 UL(0x100000000)
> +#else
> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
> +#endif /* CONFIG_ARM64_64K_PAGES */
> +#endif /* CONFIG_COMPAT */
Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
seem to move TASK_SIZE.
--
Catalin
On Fri, Mar 13, 2020 at 03:43:38PM +0000, Vincenzo Frascino wrote:
> The vDSO library should only include the necessary headers required for
> a userspace library (UAPI and a minimal set of kernel headers). To make
> this possible it is necessary to isolate from the kernel headers the
> common parts that are strictly necessary to build the library.
>
> Refactor the vdso implementation to include common headers.
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
On Fri, Mar 13, 2020 at 03:43:39PM +0000, Vincenzo Frascino wrote:
> The vDSO library should only include the necessary headers required for
> a userspace library (UAPI and a minimal set of kernel headers). To make
> this possible it is necessary to isolate from the kernel headers the
> common parts that are strictly necessary to build the library.
>
> Refactor the vdso32 implementation to include common headers.
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
On Fri, Mar 13, 2020 at 03:43:40PM +0000, Vincenzo Frascino wrote:
> The vDSO library should only include the necessary headers required for
> a userspace library (UAPI and a minimal set of kernel headers). To make
> this possible it is necessary to isolate from the kernel headers the
> common parts that are strictly necessary to build the library.
>
> Introduce asm/vdso/arch_timer.h to contain all the arm64 specific
> code. This allows to replace the second isb() in __arch_get_hw_counter()
> with a fake dependent stack read of the counter which improves the vdso
> library peformances of ~4.5%. Below the results of vdsotest [1] ran for
> 100 iterations.
The subject seems to imply a non-functional change but as you read, it
gets a lot more complicated. Could you keep the functional change
separate from the header clean-up, maybe submit it as an independent
patch? And it shouldn't go in without Will's ack ;).
--
Catalin
On Fri, Mar 13, 2020 at 03:43:45PM +0000, Vincenzo Frascino wrote:
> Enable Clang Compilation for the vdso32 library.
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Reviewed-by: Nathan Chancellor <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]> # build
> Signed-off-by: Vincenzo Frascino <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
Hi Catalin,
you should not really work on Sunday ;-) Joking, thanks for reviewing my patches.
On 3/15/20 6:30 PM, Catalin Marinas wrote:
> On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/vdso/processor.h
>> @@ -0,0 +1,31 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2020 ARM Ltd.
>> + */
>> +#ifndef __ASM_VDSO_PROCESSOR_H
>> +#define __ASM_VDSO_PROCESSOR_H
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#include <asm/page-def.h>
>> +
>> +#ifdef CONFIG_COMPAT
>> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
>> +/*
>> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
>> + * by the compat vectors page.
>> + */
>> +#define TASK_SIZE_32 UL(0x100000000)
>> +#else
>> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
>> +#endif /* CONFIG_ARM64_64K_PAGES */
>> +#endif /* CONFIG_COMPAT */
>
> Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
> seem to move TASK_SIZE.
>
I tried to fine grain the headers as much as I could in order to avoid
unneeded/unwanted inclusions:
* TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
arch/arm64/kernel/vdso32/vgettimeofday.c).
* TASK_SIZE is not required by the vdso library hence it is not present in
these headers.
--
Regards,
Vincenzo
Hi Vincenzo,
On Mon, Mar 16, 2020 at 09:42:32AM +0000, Vincenzo Frascino wrote:
> On 3/15/20 6:30 PM, Catalin Marinas wrote:
> > On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
> >> --- /dev/null
> >> +++ b/arch/arm64/include/asm/vdso/processor.h
> >> @@ -0,0 +1,31 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-only */
> >> +/*
> >> + * Copyright (C) 2020 ARM Ltd.
> >> + */
> >> +#ifndef __ASM_VDSO_PROCESSOR_H
> >> +#define __ASM_VDSO_PROCESSOR_H
> >> +
> >> +#ifndef __ASSEMBLY__
> >> +
> >> +#include <asm/page-def.h>
> >> +
> >> +#ifdef CONFIG_COMPAT
> >> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
> >> +/*
> >> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
> >> + * by the compat vectors page.
> >> + */
> >> +#define TASK_SIZE_32 UL(0x100000000)
> >> +#else
> >> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
> >> +#endif /* CONFIG_ARM64_64K_PAGES */
> >> +#endif /* CONFIG_COMPAT */
> >
> > Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
> > seem to move TASK_SIZE.
> >
>
> I tried to fine grain the headers as much as I could in order to avoid
> unneeded/unwanted inclusions:
> * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
> arch/arm64/kernel/vdso32/vgettimeofday.c).
> * TASK_SIZE is not required by the vdso library hence it is not present in
> these headers.
It would be worth noting the former point in the commit message, since
it can be surprising.
I also think it's worth keeping the definitions together if that's easy,
as it makes it easier to navigate the codebase, even if TASK_SIZE isn't
necessary for the VDSO itself.
Thanks,
Mark.
On Mon, Mar 16, 2020 at 10:22:15AM +0000, Mark Rutland wrote:
> On Mon, Mar 16, 2020 at 09:42:32AM +0000, Vincenzo Frascino wrote:
> > On 3/15/20 6:30 PM, Catalin Marinas wrote:
> > > On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
> > >> --- /dev/null
> > >> +++ b/arch/arm64/include/asm/vdso/processor.h
> > >> @@ -0,0 +1,31 @@
> > >> +/* SPDX-License-Identifier: GPL-2.0-only */
> > >> +/*
> > >> + * Copyright (C) 2020 ARM Ltd.
> > >> + */
> > >> +#ifndef __ASM_VDSO_PROCESSOR_H
> > >> +#define __ASM_VDSO_PROCESSOR_H
> > >> +
> > >> +#ifndef __ASSEMBLY__
> > >> +
> > >> +#include <asm/page-def.h>
> > >> +
> > >> +#ifdef CONFIG_COMPAT
> > >> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
> > >> +/*
> > >> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
> > >> + * by the compat vectors page.
> > >> + */
> > >> +#define TASK_SIZE_32 UL(0x100000000)
> > >> +#else
> > >> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
> > >> +#endif /* CONFIG_ARM64_64K_PAGES */
> > >> +#endif /* CONFIG_COMPAT */
> > >
> > > Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
> > > seem to move TASK_SIZE.
> > >
> >
> > I tried to fine grain the headers as much as I could in order to avoid
> > unneeded/unwanted inclusions:
> > * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
> > arch/arm64/kernel/vdso32/vgettimeofday.c).
> > * TASK_SIZE is not required by the vdso library hence it is not present in
> > these headers.
>
> It would be worth noting the former point in the commit message, since
> it can be surprising.
>
> I also think it's worth keeping the definitions together if that's easy,
> as it makes it easier to navigate the codebase, even if TASK_SIZE isn't
> necessary for the VDSO itself.
It won't work as TASK_SIZE requires (on arm64) test_thread_flag() which
can't be made available to the vDSO.
--
Catalin
On 3/16/20 10:22 AM, Mark Rutland wrote:
> Hi Vincenzo,
>
> On Mon, Mar 16, 2020 at 09:42:32AM +0000, Vincenzo Frascino wrote:
>> On 3/15/20 6:30 PM, Catalin Marinas wrote:
>>> On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
>>>> --- /dev/null
>>>> +++ b/arch/arm64/include/asm/vdso/processor.h
>>>> @@ -0,0 +1,31 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +/*
>>>> + * Copyright (C) 2020 ARM Ltd.
>>>> + */
>>>> +#ifndef __ASM_VDSO_PROCESSOR_H
>>>> +#define __ASM_VDSO_PROCESSOR_H
>>>> +
>>>> +#ifndef __ASSEMBLY__
>>>> +
>>>> +#include <asm/page-def.h>
>>>> +
>>>> +#ifdef CONFIG_COMPAT
>>>> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
>>>> +/*
>>>> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
>>>> + * by the compat vectors page.
>>>> + */
>>>> +#define TASK_SIZE_32 UL(0x100000000)
>>>> +#else
>>>> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
>>>> +#endif /* CONFIG_ARM64_64K_PAGES */
>>>> +#endif /* CONFIG_COMPAT */
>>>
>>> Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
>>> seem to move TASK_SIZE.
>>>
>>
>> I tried to fine grain the headers as much as I could in order to avoid
>> unneeded/unwanted inclusions:
>> * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
>> arch/arm64/kernel/vdso32/vgettimeofday.c).
>> * TASK_SIZE is not required by the vdso library hence it is not present in
>> these headers.
>
> It would be worth noting the former point in the commit message, since
> it can be surprising.
>
Sure it is a good point, I will add this to the commit message.
> I also think it's worth keeping the definitions together if that's easy,
> as it makes it easier to navigate the codebase, even if TASK_SIZE isn't
> necessary for the VDSO itself.
This can't be done because TASK_SIZE on arm64 requires test_thread_flag() with
is not suited for vDSO. In other words can cause the same problem we are trying
to solve.
>
> Thanks,
> Mark.
>
--
Regards,
Vincenzo
On 3/16/20 10:26 AM, Catalin Marinas wrote:
> On Mon, Mar 16, 2020 at 10:22:15AM +0000, Mark Rutland wrote:
>> On Mon, Mar 16, 2020 at 09:42:32AM +0000, Vincenzo Frascino wrote:
>>> On 3/15/20 6:30 PM, Catalin Marinas wrote:
>>>> On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/include/asm/vdso/processor.h
>>>>> @@ -0,0 +1,31 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> +/*
>>>>> + * Copyright (C) 2020 ARM Ltd.
>>>>> + */
>>>>> +#ifndef __ASM_VDSO_PROCESSOR_H
>>>>> +#define __ASM_VDSO_PROCESSOR_H
>>>>> +
>>>>> +#ifndef __ASSEMBLY__
>>>>> +
>>>>> +#include <asm/page-def.h>
>>>>> +
>>>>> +#ifdef CONFIG_COMPAT
>>>>> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
>>>>> +/*
>>>>> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
>>>>> + * by the compat vectors page.
>>>>> + */
>>>>> +#define TASK_SIZE_32 UL(0x100000000)
>>>>> +#else
>>>>> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
>>>>> +#endif /* CONFIG_ARM64_64K_PAGES */
>>>>> +#endif /* CONFIG_COMPAT */
>>>>
>>>> Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
>>>> seem to move TASK_SIZE.
>>>>
>>>
>>> I tried to fine grain the headers as much as I could in order to avoid
>>> unneeded/unwanted inclusions:
>>> * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
>>> arch/arm64/kernel/vdso32/vgettimeofday.c).
>>> * TASK_SIZE is not required by the vdso library hence it is not present in
>>> these headers.
>>
>> It would be worth noting the former point in the commit message, since
>> it can be surprising.
>>
>> I also think it's worth keeping the definitions together if that's easy,
>> as it makes it easier to navigate the codebase, even if TASK_SIZE isn't
>> necessary for the VDSO itself.
>
> It won't work as TASK_SIZE requires (on arm64) test_thread_flag() which
> can't be made available to the vDSO.
>
Ups, I did not see this :) luckily I agree ;)
--
Regards,
Vincenzo
Hi Vincenzo,
On Fri, Mar 13, 2020 at 03:43:40PM +0000, Vincenzo Frascino wrote:
> The vDSO library should only include the necessary headers required for
> a userspace library (UAPI and a minimal set of kernel headers). To make
> this possible it is necessary to isolate from the kernel headers the
> common parts that are strictly necessary to build the library.
>
> Introduce asm/vdso/arch_timer.h to contain all the arm64 specific
> code. This allows to replace the second isb() in __arch_get_hw_counter()
> with a fake dependent stack read of the counter which improves the vdso
> library peformances of ~4.5%. Below the results of vdsotest [1] ran for
> 100 iterations.
>
> Before the patch:
> =================
> clock-gettime-monotonic: syscall: 771 nsec/call
> clock-gettime-monotonic: libc: 130 nsec/call
> clock-gettime-monotonic: vdso: 111 nsec/call
> ...
> clock-gettime-realtime: syscall: 762 nsec/call
> clock-gettime-realtime: libc: 130 nsec/call
> clock-gettime-realtime: vdso: 111 nsec/call
>
> After the patch:
> ================
> clock-gettime-monotonic: syscall: 792 nsec/call
> clock-gettime-monotonic: libc: 124 nsec/call
> clock-gettime-monotonic: vdso: 106 nsec/call
> ...
> clock-gettime-realtime: syscall: 776 nsec/call
> clock-gettime-realtime: libc: 124 nsec/call
> clock-gettime-realtime: vdso: 106 nsec/call
>
> [1] https://github.com/nathanlynch/vdsotest
>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
> ---
> arch/arm64/include/asm/arch_timer.h | 29 ++++---------------
> arch/arm64/include/asm/vdso/arch_timer.h | 33 ++++++++++++++++++++++
> arch/arm64/include/asm/vdso/gettimeofday.h | 7 +++--
> 3 files changed, 42 insertions(+), 27 deletions(-)
> create mode 100644 arch/arm64/include/asm/vdso/arch_timer.h
>
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 7ae54d7d333a..7f22cd00ad45 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -164,24 +164,7 @@ static inline void arch_timer_set_cntkctl(u32 cntkctl)
> isb();
> }
>
> -/*
> - * Ensure that reads of the counter are treated the same as memory reads
> - * for the purposes of ordering by subsequent memory barriers.
> - *
> - * This insanity brought to you by speculative system register reads,
> - * out-of-order memory accesses, sequence locks and Thomas Gleixner.
> - *
> - * http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/631195.html
> - */
> -#define arch_counter_enforce_ordering(val) do { \
> - u64 tmp, _val = (val); \
> - \
> - asm volatile( \
> - " eor %0, %1, %1\n" \
> - " add %0, sp, %0\n" \
> - " ldr xzr, [%0]" \
> - : "=r" (tmp) : "r" (_val)); \
> -} while (0)
> +#include <asm/vdso/arch_timer.h>
>
> static __always_inline u64 __arch_counter_get_cntpct_stable(void)
> {
> @@ -189,7 +172,7 @@ static __always_inline u64 __arch_counter_get_cntpct_stable(void)
>
> isb();
> cnt = arch_timer_reg_read_stable(cntpct_el0);
> - arch_counter_enforce_ordering(cnt);
> + cnt = arch_counter_enforce_ordering(cnt);
> return cnt;
Why have you changed the structure of arch_counter_enforce_ordering() to
return a value? The commit message has no rationale for that.
If there is a reason to change that, I'd prefer the driver change as one
patch, before moving the definition.
[...]
> +/*
> + * Ensure that reads of the counter are treated the same as memory reads
> + * for the purposes of ordering by subsequent memory barriers.
> + *
> + * This insanity brought to you by speculative system register reads,
> + * out-of-order memory accesses, sequence locks and Thomas Gleixner.
> + *
> + * http://lists.infradead.org/pipermail/linux-arm-kernel/2019-February/631195.html
> + *
> + */
> +static u64 arch_counter_enforce_ordering(u64 val)
> +{
> + u64 tmp, _val = (val);
> +
> + asm volatile(
> + " eor %0, %1, %1\n"
> + " add %0, sp, %0\n"
> + " ldr xzr, [%0]"
> + : "=r" (tmp) : "r" (_val));
> +
> + return _val;
> +}
This change has no functional effect. Since `_val` is only passed in as
an input parameter, the compiler can assume the assembly has no effect
on it.
As above, what is the rationale for changing this?
> @@ -82,10 +83,10 @@ static __always_inline u64 __arch_get_hw_counter(s32 clock_mode)
> isb();
> asm volatile("mrs %0, cntvct_el0" : "=r" (res) :: "memory");
> /*
> - * This isb() is required to prevent that the seq lock is
> - * speculated.#
> + * arch_counter_enforce_ordering() is required to prevent that
> + * the seq lock is speculated.
> */
> - isb();
> + res = arch_counter_enforce_ordering(res);
Can we delete the comment entirely? We don't bother in <asm/arch_timer.h>.
Even better, can we factor out __arch_counter_get_cntvct(), and use
that?
Thanks,
Mark.
On Mon, Mar 16, 2020 at 10:26:22AM +0000, Catalin Marinas wrote:
> On Mon, Mar 16, 2020 at 10:22:15AM +0000, Mark Rutland wrote:
> > On Mon, Mar 16, 2020 at 09:42:32AM +0000, Vincenzo Frascino wrote:
> > > On 3/15/20 6:30 PM, Catalin Marinas wrote:
> > > > On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
> > > >> +#ifdef CONFIG_COMPAT
> > > >> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
> > > >> +/*
> > > >> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
> > > >> + * by the compat vectors page.
> > > >> + */
> > > >> +#define TASK_SIZE_32 UL(0x100000000)
> > > >> +#else
> > > >> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
> > > >> +#endif /* CONFIG_ARM64_64K_PAGES */
> > > >> +#endif /* CONFIG_COMPAT */
> > > >
> > > > Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
> > > > seem to move TASK_SIZE.
> > >
> > > I tried to fine grain the headers as much as I could in order to avoid
> > > unneeded/unwanted inclusions:
> > > * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
> > > arch/arm64/kernel/vdso32/vgettimeofday.c).
> > > * TASK_SIZE is not required by the vdso library hence it is not present in
> > > these headers.
> >
> > It would be worth noting the former point in the commit message, since
> > it can be surprising.
> >
> > I also think it's worth keeping the definitions together if that's easy,
> > as it makes it easier to navigate the codebase, even if TASK_SIZE isn't
> > necessary for the VDSO itself.
>
> It won't work as TASK_SIZE requires (on arm64) test_thread_flag() which
> can't be made available to the vDSO.
Ah; fair enough.
Thanks,
Mark.
On Mon, Mar 16, 2020 at 09:42:32AM +0000, Vincenzo Frascino wrote:
> you should not really work on Sunday ;-)
I was getting bored ;).
> On 3/15/20 6:30 PM, Catalin Marinas wrote:
> > On Fri, Mar 13, 2020 at 03:43:37PM +0000, Vincenzo Frascino wrote:
> >> --- /dev/null
> >> +++ b/arch/arm64/include/asm/vdso/processor.h
> >> @@ -0,0 +1,31 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-only */
> >> +/*
> >> + * Copyright (C) 2020 ARM Ltd.
> >> + */
> >> +#ifndef __ASM_VDSO_PROCESSOR_H
> >> +#define __ASM_VDSO_PROCESSOR_H
> >> +
> >> +#ifndef __ASSEMBLY__
> >> +
> >> +#include <asm/page-def.h>
> >> +
> >> +#ifdef CONFIG_COMPAT
> >> +#if defined(CONFIG_ARM64_64K_PAGES) && defined(CONFIG_KUSER_HELPERS)
> >> +/*
> >> + * With CONFIG_ARM64_64K_PAGES enabled, the last page is occupied
> >> + * by the compat vectors page.
> >> + */
> >> +#define TASK_SIZE_32 UL(0x100000000)
> >> +#else
> >> +#define TASK_SIZE_32 (UL(0x100000000) - PAGE_SIZE)
> >> +#endif /* CONFIG_ARM64_64K_PAGES */
> >> +#endif /* CONFIG_COMPAT */
> >
> > Just curious, what's TASK_SIZE_32 used for in the vDSO code? You don't
> > seem to move TASK_SIZE.
> >
>
> I tried to fine grain the headers as much as I could in order to avoid
> unneeded/unwanted inclusions:
> * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
> arch/arm64/kernel/vdso32/vgettimeofday.c).
I see. But the test is probably useless. With 4K pages, TASK_SIZE_32 is
1UL << 32, so you can't have a u32 greater than this. So I'd argue that
the ABI compatibility here doesn't matter.
With 16K or 64K pages, TASK_SIZE_32 is slightly smaller but arm32 never
supported it.
What's the side-effect of dropping this check altogether?
--
Catalin
Hi Catalin,
On 3/16/20 10:34 AM, Catalin Marinas wrote:
[...]
>>
>> I tried to fine grain the headers as much as I could in order to avoid
>> unneeded/unwanted inclusions:
>> * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
>> arch/arm64/kernel/vdso32/vgettimeofday.c).
>
> I see. But the test is probably useless. With 4K pages, TASK_SIZE_32 is
> 1UL << 32, so you can't have a u32 greater than this. So I'd argue that
> the ABI compatibility here doesn't matter.
>
> With 16K or 64K pages, TASK_SIZE_32 is slightly smaller but arm32 never
> supported it.
>
> What's the side-effect of dropping this check altogether?
>
The main side-effect is that arm32 and arm64 compat have a different behavior,
that it is what we want to avoid.
The vdsotest [1] I am using, verifies all the side conditions with respect to
the ABI, which we are now compatible with. Removing those checks would break
this condition.
[1] https://github.com/nlynch-mentor/vdsotest
--
Regards,
Vincenzo
On Mon, Mar 16, 2020 at 10:55:00AM +0000, Vincenzo Frascino wrote:
> On 3/16/20 10:34 AM, Catalin Marinas wrote:
> >> I tried to fine grain the headers as much as I could in order to avoid
> >> unneeded/unwanted inclusions:
> >> * TASK_SIZE_32 is used to verify ABI consistency on vdso32 (please refer to
> >> arch/arm64/kernel/vdso32/vgettimeofday.c).
> >
> > I see. But the test is probably useless. With 4K pages, TASK_SIZE_32 is
> > 1UL << 32, so you can't have a u32 greater than this. So I'd argue that
> > the ABI compatibility here doesn't matter.
> >
> > With 16K or 64K pages, TASK_SIZE_32 is slightly smaller but arm32 never
> > supported it.
> >
> > What's the side-effect of dropping this check altogether?
>
> The main side-effect is that arm32 and arm64 compat have a different behavior,
> that it is what we want to avoid.
>
> The vdsotest [1] I am using, verifies all the side conditions with respect to
> the ABI, which we are now compatible with. Removing those checks would break
> this condition.
As I said above, I don't see how removing 'if ((u32)ts >= (1UL << 32))'
makes any difference. This check was likely removed by the compiler
already.
Also, userspace doesn't have a trivial way to figure out TASK_SIZE and I
can't see anything that tests this in the vdsotest (though I haven't
spent that much time looking). If it's hard-coded, note that arm32
TASK_SIZE is different from TASK_SIZE_32 on arm64.
Can you tell what actually is failing in vdsotest if you remove the
TASK_SIZE_32 checks in the arm64 compat vdso?
--
Catalin
Hi Catalin,
On 3/16/20 11:22 AM, Catalin Marinas wrote:
> On Mon, Mar 16, 2020 at 10:55:00AM +0000, Vincenzo Frascino wrote:
>> On 3/16/20 10:34 AM, Catalin Marinas wrote:
[...]
>
> As I said above, I don't see how removing 'if ((u32)ts >= (1UL << 32))'
> makes any difference. This check was likely removed by the compiler
> already.
>
> Also, userspace doesn't have a trivial way to figure out TASK_SIZE and I
> can't see anything that tests this in the vdsotest (though I haven't
> spent that much time looking). If it's hard-coded, note that arm32
> TASK_SIZE is different from TASK_SIZE_32 on arm64.
>
> Can you tell what actually is failing in vdsotest if you remove the
> TASK_SIZE_32 checks in the arm64 compat vdso?
>
To me does not seem optimized out. Which version of the compiler are you using?
Please find below the list of errors for clock_gettime (similar for the other):
passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
clock-gettime-monotonic/abi: 1 failures/inconsistencies encountered
passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
clock-gettime-monotonic-coarse/abi: 1 failures/inconsistencies encountered
passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
clock-gettime-realtime/abi: 1 failures/inconsistencies encountered
passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
clock-gettime-realtime-coarse/abi: 1 failures/inconsistencies encountered
Please refer to [1] for more details on the test.
[1]
https://github.com/nlynch-mentor/vdsotest/blob/master/src/clock_gettime_template.c
--
Regards,
Vincenzo
On Mon, Mar 16, 2020 at 01:35:17PM +0000, Vincenzo Frascino wrote:
> On 3/16/20 11:22 AM, Catalin Marinas wrote:
> > As I said above, I don't see how removing 'if ((u32)ts >= (1UL << 32))'
> > makes any difference. This check was likely removed by the compiler
> > already.
> >
> > Also, userspace doesn't have a trivial way to figure out TASK_SIZE and I
> > can't see anything that tests this in the vdsotest (though I haven't
> > spent that much time looking). If it's hard-coded, note that arm32
> > TASK_SIZE is different from TASK_SIZE_32 on arm64.
> >
> > Can you tell what actually is failing in vdsotest if you remove the
> > TASK_SIZE_32 checks in the arm64 compat vdso?
>
> To me does not seem optimized out. Which version of the compiler are you using?
I misread the #ifdef'ery in asm/processor.h. So with 4K pages,
TASK_SIZE_32 is (1UL<<32)-PAGE_SIZE. However, with 64K pages _and_
CONFIG_KUSER_HELPERS, TASK_SIZE_32 is 1UL<<32 and the check is removed
by the compiler.
With the 4K build, __vdso_clock_gettime starts as:
00000194 <__vdso_clock_gettime>:
194: f511 5f80 cmn.w r1, #4096 ; 0x1000
198: d214 bcs.n 1c4 <__vdso_clock_gettime+0x30>
19a: b5b0 push {r4, r5, r7, lr}
...
1c4: f06f 000d mvn.w r0, #13
1c8: 4770 bx lr
With 64K pages:
00000194 <__vdso_clock_gettime>:
194: b5b0 push {r4, r5, r7, lr}
...
1be: bdb0 pop {r4, r5, r7, pc}
I haven't tried but it's likely that the vdsotest fails with 64K pages
and compat enabled (requires EXPERT).
> Please find below the list of errors for clock_gettime (similar for the other):
>
> passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
> clock-gettime-monotonic/abi: 1 failures/inconsistencies encountered
Ah, so it uses UINTPTR_MAX in the test. Fair enough but I don't think
the arm64 check is entirely useful. On arm32, the check was meant to
return -EFAULT for addresses beyond TASK_SIZE that may enter into the
kernel or module space. On arm64 compat, the kernel space is well above
the reach of the 32-bit code.
If you want to preserve some compatibility for this specific test, what
about checking for wrapping around 0, I think it would make more sense.
Something like:
if ((u32)ts > UINTPTR_MAX - sizeof(*ts) + 1)
--
Catalin
On 3/16/20 2:43 PM, Catalin Marinas wrote[...]
>> To me does not seem optimized out. Which version of the compiler are you using?
>
> I misread the #ifdef'ery in asm/processor.h. So with 4K pages,
> TASK_SIZE_32 is (1UL<<32)-PAGE_SIZE. However, with 64K pages _and_
> CONFIG_KUSER_HELPERS, TASK_SIZE_32 is 1UL<<32 and the check is removed
> by the compiler.
>
> With the 4K build, __vdso_clock_gettime starts as:
>
> 00000194 <__vdso_clock_gettime>:
> 194: f511 5f80 cmn.w r1, #4096 ; 0x1000
> 198: d214 bcs.n 1c4 <__vdso_clock_gettime+0x30>
> 19a: b5b0 push {r4, r5, r7, lr}
> ...
> 1c4: f06f 000d mvn.w r0, #13
> 1c8: 4770 bx lr
>
> With 64K pages:
>
> 00000194 <__vdso_clock_gettime>:
> 194: b5b0 push {r4, r5, r7, lr}
> ...
> 1be: bdb0 pop {r4, r5, r7, pc}
>
> I haven't tried but it's likely that the vdsotest fails with 64K pages
> and compat enabled (requires EXPERT).
>
This makes more sense. Thanks for the clarification.
I agree on the behavior of 64K pages and I think as well that the
"compatibility" issue is still there. However as you correctly stated in your
first email arm32 never supported 16K or 64K pages, hence I think we should not
be concerned about compatibility in this cases.
To make it more explicit we could make COMPAT_VDSO on arm64 depend on
ARM64_4K_PAGES. What do you think?
>> Please find below the list of errors for clock_gettime (similar for the other):
>>
>> passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
>> clock-gettime-monotonic/abi: 1 failures/inconsistencies encountered
>
> Ah, so it uses UINTPTR_MAX in the test. Fair enough but I don't think
> the arm64 check is entirely useful. On arm32, the check was meant to
> return -EFAULT for addresses beyond TASK_SIZE that may enter into the
> kernel or module space. On arm64 compat, the kernel space is well above
> the reach of the 32-bit code.
>
> If you want to preserve some compatibility for this specific test, what
> about checking for wrapping around 0, I think it would make more sense.
> Something like:
>
> if ((u32)ts > UINTPTR_MAX - sizeof(*ts) + 1)
>
Ok, sounds good to me. But it is something that this patch series inherited,
hence I would prefer to send a separate patch that introduces what you are
proposing and removes TASK_SIZE_32 from the headers. How does it sound?
--
Regards,
Vincenzo
Hi Catalin,
On 3/15/20 6:32 PM, Catalin Marinas wrote:
> On Fri, Mar 13, 2020 at 03:43:40PM +0000, Vincenzo Frascino wrote:
>> The vDSO library should only include the necessary headers required for
>> a userspace library (UAPI and a minimal set of kernel headers). To make
>> this possible it is necessary to isolate from the kernel headers the
>> common parts that are strictly necessary to build the library.
>>
>> Introduce asm/vdso/arch_timer.h to contain all the arm64 specific
>> code. This allows to replace the second isb() in __arch_get_hw_counter()
>> with a fake dependent stack read of the counter which improves the vdso
>> library peformances of ~4.5%. Below the results of vdsotest [1] ran for
>> 100 iterations.
>
> The subject seems to imply a non-functional change but as you read, it
> gets a lot more complicated. Could you keep the functional change
> separate from the header clean-up, maybe submit it as an independent
> patch? And it shouldn't go in without Will's ack ;).
>
It is fine by me. I will repost the series with the required fixes and without
this patch. This will give to me enough time to address Mark's comments as well
and to Will to have a proper look.
--
Regards,
Vincenzo
On Mon, Mar 16, 2020 at 03:33:30PM +0000, Vincenzo Frascino wrote:
> On 3/16/20 2:43 PM, Catalin Marinas wrote[...]
> >> To me does not seem optimized out. Which version of the compiler are you using?
> >
> > I misread the #ifdef'ery in asm/processor.h. So with 4K pages,
> > TASK_SIZE_32 is (1UL<<32)-PAGE_SIZE. However, with 64K pages _and_
> > CONFIG_KUSER_HELPERS, TASK_SIZE_32 is 1UL<<32 and the check is removed
> > by the compiler.
> >
> > With the 4K build, __vdso_clock_gettime starts as:
> >
> > 00000194 <__vdso_clock_gettime>:
> > 194: f511 5f80 cmn.w r1, #4096 ; 0x1000
> > 198: d214 bcs.n 1c4 <__vdso_clock_gettime+0x30>
> > 19a: b5b0 push {r4, r5, r7, lr}
> > ...
> > 1c4: f06f 000d mvn.w r0, #13
> > 1c8: 4770 bx lr
> >
> > With 64K pages:
> >
> > 00000194 <__vdso_clock_gettime>:
> > 194: b5b0 push {r4, r5, r7, lr}
> > ...
> > 1be: bdb0 pop {r4, r5, r7, pc}
> >
> > I haven't tried but it's likely that the vdsotest fails with 64K pages
> > and compat enabled (requires EXPERT).
>
> This makes more sense. Thanks for the clarification.
>
> I agree on the behavior of 64K pages and I think as well that the
> "compatibility" issue is still there. However as you correctly stated in your
> first email arm32 never supported 16K or 64K pages, hence I think we should not
> be concerned about compatibility in this cases.
My point is that even with 4K pages it's not really compatibility. The
test uses UINTPTR_MAX but on arm32 it would also fail with 0xc0000000.
On arm64 compat, however, this value would pass just fine.
> To make it more explicit we could make COMPAT_VDSO on arm64 depend on
> ARM64_4K_PAGES. What do you think?
No, I don't see why we should add this limitation.
> >> Please find below the list of errors for clock_gettime (similar for the other):
> >>
> >> passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
> >> clock-gettime-monotonic/abi: 1 failures/inconsistencies encountered
> >
> > Ah, so it uses UINTPTR_MAX in the test. Fair enough but I don't think
> > the arm64 check is entirely useful. On arm32, the check was meant to
> > return -EFAULT for addresses beyond TASK_SIZE that may enter into the
> > kernel or module space. On arm64 compat, the kernel space is well above
> > the reach of the 32-bit code.
> >
> > If you want to preserve some compatibility for this specific test, what
> > about checking for wrapping around 0, I think it would make more sense.
> > Something like:
> >
> > if ((u32)ts > UINTPTR_MAX - sizeof(*ts) + 1)
>
> Ok, sounds good to me. But it is something that this patch series inherited,
> hence I would prefer to send a separate patch that introduces what you are
> proposing and removes TASK_SIZE_32 from the headers. How does it sound?
I'd rather avoid moving TASK_SIZE_32 unnecessarily. Just add a
preparatory patch to your series for arm64 compat vdso and follow with
the rest without moving TASK_SIZE_32 around.
--
Catalin
On 3/16/20 3:49 PM, Catalin Marinas wrote:
> On Mon, Mar 16, 2020 at 03:33:30PM +0000, Vincenzo Frascino wrote:
>> On 3/16/20 2:43 PM, Catalin Marinas wrote[...]
[...]
>
>> To make it more explicit we could make COMPAT_VDSO on arm64 depend on
>> ARM64_4K_PAGES. What do you think?
>
> No, I don't see why we should add this limitation.
>
Fine by me.
>>>> Please find below the list of errors for clock_gettime (similar for the other):
>>>>
>>>> passing UINTPTR_MAX to clock_gettime (VDSO): terminated by unexpected signal 7
>>>> clock-gettime-monotonic/abi: 1 failures/inconsistencies encountered
>>>
>>> Ah, so it uses UINTPTR_MAX in the test. Fair enough but I don't think
>>> the arm64 check is entirely useful. On arm32, the check was meant to
>>> return -EFAULT for addresses beyond TASK_SIZE that may enter into the
>>> kernel or module space. On arm64 compat, the kernel space is well above
>>> the reach of the 32-bit code.
>>>
>>> If you want to preserve some compatibility for this specific test, what
>>> about checking for wrapping around 0, I think it would make more sense.
>>> Something like:
>>>
>>> if ((u32)ts > UINTPTR_MAX - sizeof(*ts) + 1)
>>
>> Ok, sounds good to me. But it is something that this patch series inherited,
>> hence I would prefer to send a separate patch that introduces what you are
>> proposing and removes TASK_SIZE_32 from the headers. How does it sound?
>
> I'd rather avoid moving TASK_SIZE_32 unnecessarily. Just add a
> preparatory patch to your series for arm64 compat vdso and follow with
> the rest without moving TASK_SIZE_32 around.
>
Ok, sounds good. I will test it and repost.
--
Regards,
Vincenzo
Hi Vincenzo,
Sorry, I was on holiday when you posted this and it slipped through the
cracks.
On Mon, Mar 16, 2020 at 03:37:23PM +0000, Vincenzo Frascino wrote:
> > On Fri, Mar 13, 2020 at 03:43:40PM +0000, Vincenzo Frascino wrote:
> >> The vDSO library should only include the necessary headers required for
> >> a userspace library (UAPI and a minimal set of kernel headers). To make
> >> this possible it is necessary to isolate from the kernel headers the
> >> common parts that are strictly necessary to build the library.
> >>
> >> Introduce asm/vdso/arch_timer.h to contain all the arm64 specific
> >> code. This allows to replace the second isb() in __arch_get_hw_counter()
> >> with a fake dependent stack read of the counter which improves the vdso
> >> library peformances of ~4.5%. Below the results of vdsotest [1] ran for
> >> 100 iterations.
> >
> > The subject seems to imply a non-functional change but as you read, it
> > gets a lot more complicated. Could you keep the functional change
> > separate from the header clean-up, maybe submit it as an independent
> > patch? And it shouldn't go in without Will's ack ;).
> >
>
> It is fine by me. I will repost the series with the required fixes and without
> this patch. This will give to me enough time to address Mark's comments as well
> and to Will to have a proper look.
Please can you post whatever is left at -rc1? I'll have a look then, but
let's stick to just moving code around rather than randomly changing it
at the same time, ok?
Thanks,
Will
Hi Will,
On 4/9/20 2:26 PM, Will Deacon wrote:
> Hi Vincenzo,
>
> Sorry, I was on holiday when you posted this and it slipped through the
> cracks.
>
No issue at all. Thank you for getting back to me.
> On Mon, Mar 16, 2020 at 03:37:23PM +0000, Vincenzo Frascino wrote:
>>> On Fri, Mar 13, 2020 at 03:43:40PM +0000, Vincenzo Frascino wrote:
>>>> The vDSO library should only include the necessary headers required for
>>>> a userspace library (UAPI and a minimal set of kernel headers). To make
>>>> this possible it is necessary to isolate from the kernel headers the
>>>> common parts that are strictly necessary to build the library.
>>>>
>>>> Introduce asm/vdso/arch_timer.h to contain all the arm64 specific
>>>> code. This allows to replace the second isb() in __arch_get_hw_counter()
>>>> with a fake dependent stack read of the counter which improves the vdso
>>>> library peformances of ~4.5%. Below the results of vdsotest [1] ran for
>>>> 100 iterations.
>>>
>>> The subject seems to imply a non-functional change but as you read, it
>>> gets a lot more complicated. Could you keep the functional change
>>> separate from the header clean-up, maybe submit it as an independent
>>> patch? And it shouldn't go in without Will's ack ;).
>>>
>>
>> It is fine by me. I will repost the series with the required fixes and without
>> this patch. This will give to me enough time to address Mark's comments as well
>> and to Will to have a proper look.
>
> Please can you post whatever is left at -rc1? I'll have a look then, but
> let's stick to just moving code around rather than randomly changing it
> at the same time, ok?
>
Sure, I will try to re-post it by -rc1 and take on board your comments.
> Thanks,
>
> Will
>
--
Regards,
Vincenzo