[PATCH 0/5] arm64: vdso: getcpu() support

[PATCH 0/5] arm64: vdso: getcpu() support

[Mark Brown]

Some applications, especially tracing ones, benefit from avoiding the
syscall overhead for getcpu() so it is common for architectures to have
vDSO implementations. Add one for arm64, using TPIDRRO_EL0 to pass a
pointer to per-CPU data rather than just store the immediate value in
order to allow for future extensibility.

It is questionable if something TPIDRRO_EL0 based is worthwhile at all
on current kernels, since v4.18 we have had support for restartable
sequences which can be used to provide a sched_getcpu() implementation
with generally better performance than the vDSO approach on
architectures which have that[1].  Work is ongoing to implement this for
glibc:

    https://lore.kernel.org/lkml/20200527185130.5604-3-mathieu.desnoyers@efficios.com/

but is not yet merged and will need similar work for other userspaces.
The main advantages for the vDSO implementation are the node parameter
(though this is a static mapping to CPU number so could be looked up
separately when processing data if it's needed, it shouldn't need to be
in the hot path) and ease of implementation for users.  

This is currently not compatible with KPTI due to what Will suggests is
a misunderstanding on my part about the use of TPIDRRO_EL0 by the KPTI
trampoline, since this posting is mainly for discussion of the approach
as a whole and Will only just mentioned this that's not been addressed
here.

Since we currently only have a single data page for the vDSO this will
also only currently work for lower numbered CPUs, this restriction will
be addressed separately.

There is some overlap with an in flight patch series from Andrei Vagin
supporting time namespaces in the vDSO, there shouldn't be a fundamental
issue integrating the two serieses.

This builds on work done by Kristina Martsenko some time ago but is a
new implementation.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d7822b1e24f2df5df98c76f0e94a5416349ff759

Mark Brown (5):
  arm64: vdso: Provide a define when building the vDSO
  arm64: vdso: Add per-CPU data
  arm64: vdso: Initialise the per-CPU vDSO data
  arm64: vdso: Add getcpu() implementation
  selftests: vdso: Support arm64 in getcpu() test

 arch/arm64/include/asm/processor.h            | 12 +----
 arch/arm64/include/asm/vdso/datapage.h        | 54 +++++++++++++++++++
 arch/arm64/kernel/process.c                   | 26 ++++++++-
 arch/arm64/kernel/vdso.c                      | 33 +++++++++++-
 arch/arm64/kernel/vdso/Makefile               |  4 +-
 arch/arm64/kernel/vdso/vdso.lds.S             |  1 +
 arch/arm64/kernel/vdso/vgetcpu.c              | 48 +++++++++++++++++
 .../testing/selftests/vDSO/vdso_test_getcpu.c | 10 ++++
 8 files changed, 172 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm64/include/asm/vdso/datapage.h
 create mode 100644 arch/arm64/kernel/vdso/vgetcpu.c

-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/5] arm64: vdso: Provide a define when building the vDSO

[Mark Brown]

Provide a define identifying if code is being built for the vDSO to help
with writing headers that are shared between the kernel and the vDSO.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/vdso/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index 556d424c6f52..279b1b9fb956 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -27,7 +27,7 @@ ldflags-y := -shared -nostdlib -soname=linux-vdso.so.1 --hash-style=sysv \
 		-Bsymbolic --eh-frame-hdr --build-id -n $(btildflags-y) -T
 
 ccflags-y := -fno-common -fno-builtin -fno-stack-protector -ffixed-x18
-ccflags-y += -DDISABLE_BRANCH_PROFILING
+ccflags-y += -DDISABLE_BRANCH_PROFILING -D__VDSO__
 
 CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS)
 KBUILD_CFLAGS			+= $(DISABLE_LTO)
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/5] arm64: vdso: Add per-CPU data

[Mark Brown]

In order to support a vDSO getcpu() implementation add per-CPU data to
the vDSO data page. Do this by wrapping the generic vdso_data struct in
an arm64 specific one with an array of per-CPU data. The offset of the
per-CPU data applying to a CPU will be stored in TPIDRRO_EL0, this
allows us to get to the per-CPU data without doing any multiplications.

Since we currently only map a single data page for the vDSO but support
very large numbers of CPUs TPIDRRO may be set to zero for CPUs which don't
fit in the data page. This will also happen when KPTI is active since
kernel_ventry uses TPIDRRO_EL0 as a scratch register in that case, add a
comment to the code explaining this.

Acessors for the data are provided in the header since they will be needed
in multiple files and it seems neater to keep things together.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/processor.h     | 12 +-----
 arch/arm64/include/asm/vdso/datapage.h | 54 ++++++++++++++++++++++++++
 arch/arm64/kernel/process.c            | 26 ++++++++++++-
 arch/arm64/kernel/vdso.c               |  5 ++-
 4 files changed, 83 insertions(+), 14 deletions(-)
 create mode 100644 arch/arm64/include/asm/vdso/datapage.h

diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index 240fe5e5b720..db7a804030b3 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -207,17 +207,7 @@ static inline void set_compat_ssbs_bit(struct pt_regs *regs)
 	regs->pstate |= PSR_AA32_SSBS_BIT;
 }
 
-static inline void start_thread(struct pt_regs *regs, unsigned long pc,
-				unsigned long sp)
-{
-	start_thread_common(regs, pc);
-	regs->pstate = PSR_MODE_EL0t;
-
-	if (arm64_get_ssbd_state() != ARM64_SSBD_FORCE_ENABLE)
-		set_ssbs_bit(regs);
-
-	regs->sp = sp;
-}
+void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp);
 
 static inline bool is_ttbr0_addr(unsigned long addr)
 {
diff --git a/arch/arm64/include/asm/vdso/datapage.h b/arch/arm64/include/asm/vdso/datapage.h
new file mode 100644
index 000000000000..e88d97238c52
--- /dev/null
+++ b/arch/arm64/include/asm/vdso/datapage.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 ARM Limited
+ */
+#ifndef __ASM_VDSO_DATAPAGE_H
+#define __ASM_VDSO_DATAPAGE_H
+
+#include <vdso/datapage.h>
+
+struct vdso_cpu_data {
+	unsigned int cpu;
+	unsigned int node;
+};
+
+struct arm64_vdso_data {
+	/* Must be first in struct, we cast to vdso_data */
+	struct vdso_data data[CS_BASES];
+	struct vdso_cpu_data cpu_data[];
+};
+
+#ifdef __VDSO__
+static inline struct vdso_cpu_data *__vdso_cpu_data(void)
+{
+	unsigned long offset;
+
+	asm volatile(
+		"mrs %0, tpidrro_el0\n"
+	: "=r" (offset)
+	:
+	: "cc");
+
+	if (offset)
+		return (void *)(_vdso_data) + offset;
+
+	return NULL;
+}
+#else
+static inline size_t vdso_cpu_offset(void)
+{
+	size_t offset, data_end;
+
+	offset = offsetof(struct arm64_vdso_data, cpu_data) +
+		smp_processor_id() * sizeof(struct vdso_cpu_data);
+	data_end = offset + sizeof(struct vdso_cpu_data) + 1;
+
+	/* We only map a single page for vDSO data currently */
+	if (data_end > PAGE_SIZE)
+		return 0;
+
+	return offset;
+}
+#endif
+
+#endif
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6089638c7d43..b37fe0ceb1c9 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -55,6 +55,7 @@
 #include <asm/processor.h>
 #include <asm/pointer_auth.h>
 #include <asm/stacktrace.h>
+#include <asm/vdso/datapage.h>
 
 #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR_PER_TASK)
 #include <linux/stackprotector.h>
@@ -309,6 +310,28 @@ void show_regs(struct pt_regs * regs)
 	dump_backtrace(regs, NULL, KERN_DEFAULT);
 }
 
+void start_thread(struct pt_regs *regs, unsigned long pc, unsigned long sp)
+{
+	start_thread_common(regs, pc);
+	regs->pstate = PSR_MODE_EL0t;
+
+	if (arm64_get_ssbd_state() != ARM64_SSBD_FORCE_ENABLE)
+		set_ssbs_bit(regs);
+
+	regs->sp = sp;
+
+	/*
+	 * Store the vDSO per-CPU offset if supported. Disable
+	 * preemption to make sure we read the CPU offset on the CPU
+	 * we write it on.
+	 */
+	if (!arm64_kernel_unmapped_at_el0()) {
+		preempt_disable();
+		write_sysreg(vdso_cpu_offset(), tpidrro_el0);
+		preempt_enable();
+	}
+}
+
 static void tls_thread_flush(void)
 {
 	write_sysreg(0, tpidr_el0);
@@ -452,7 +475,8 @@ static void tls_thread_switch(struct task_struct *next)
 	if (is_compat_thread(task_thread_info(next)))
 		write_sysreg(next->thread.uw.tp_value, tpidrro_el0);
 	else if (!arm64_kernel_unmapped_at_el0())
-		write_sysreg(0, tpidrro_el0);
+		/* Used as scratch in KPTI trampoline so don't set here. */
+		write_sysreg(vdso_cpu_offset(), tpidrro_el0);
 
 	write_sysreg(*task_user_tls(next), tpidr_el0);
 }
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 4e016574bd91..ea5e18e37371 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -27,6 +27,7 @@
 #include <asm/cacheflush.h>
 #include <asm/signal32.h>
 #include <asm/vdso.h>
+#include <asm/vdso/datapage.h>
 
 extern char vdso_start[], vdso_end[];
 #ifdef CONFIG_COMPAT_VDSO
@@ -70,10 +71,10 @@ static struct vdso_abi_info vdso_info[] __ro_after_init = {
  * The vDSO data page.
  */
 static union {
-	struct vdso_data	data[CS_BASES];
+	struct arm64_vdso_data	data;
 	u8			page[PAGE_SIZE];
 } vdso_data_store __page_aligned_data;
-struct vdso_data *vdso_data = vdso_data_store.data;
+struct vdso_data *vdso_data = vdso_data_store.data.data;
 
 static int __vdso_remap(enum vdso_abi abi,
 			const struct vm_special_mapping *sm,
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/5] arm64: vdso: Initialise the per-CPU vDSO data

[Mark Brown]

Register with the CPU hotplug system to initialise the per-CPU data for
getcpu().

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/vdso.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index ea5e18e37371..9e5d12ce7de9 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -9,6 +9,7 @@
 
 #include <linux/cache.h>
 #include <linux/clocksource.h>
+#include <linux/cpuhotplug.h>
 #include <linux/elf.h>
 #include <linux/err.h>
 #include <linux/errno.h>
@@ -18,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/signal.h>
 #include <linux/slab.h>
+#include <linux/smp.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/vmalloc.h>
 #include <vdso/datapage.h>
@@ -363,6 +365,26 @@ int aarch32_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
 }
 #endif /* CONFIG_COMPAT */
 
+static void vdso_cpu_init(void *p)
+{
+	struct arm64_vdso_data *data = (struct arm64_vdso_data *)vdso_data;
+	unsigned int cpu;
+
+	if (vdso_cpu_offset()) {
+		cpu = smp_processor_id();
+
+		data->cpu_data[cpu].cpu = cpu;
+		data->cpu_data[cpu].node = cpu_to_node(cpu);
+	}
+}
+
+static int vdso_cpu_online(unsigned int cpu)
+{
+	smp_call_function_single(cpu, vdso_cpu_init, NULL, 1);
+
+	return 0;
+}
+
 static int vdso_mremap(const struct vm_special_mapping *sm,
 		struct vm_area_struct *new_vma)
 {
@@ -389,6 +411,12 @@ static int __init vdso_init(void)
 	vdso_info[VDSO_ABI_AA64].dm = &aarch64_vdso_maps[AA64_MAP_VVAR];
 	vdso_info[VDSO_ABI_AA64].cm = &aarch64_vdso_maps[AA64_MAP_VDSO];
 
+	/*
+	 * Initialize per-CPU data, callback runs for all current and
+	 * future CPUs.
+	 */
+	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "vdso", vdso_cpu_online, NULL);
+
 	return __vdso_init(VDSO_ABI_AA64);
 }
 arch_initcall(vdso_init);
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/5] arm64: vdso: Add getcpu() implementation

[Mark Brown]

Some applications, especially trace ones, benefit from avoiding the syscall
overhead on getcpu() calls so provide a vDSO implementation of it.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kernel/vdso/Makefile   |  2 +-
 arch/arm64/kernel/vdso/vdso.lds.S |  1 +
 arch/arm64/kernel/vdso/vgetcpu.c  | 48 +++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kernel/vdso/vgetcpu.c

diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index 279b1b9fb956..4ba2a159dd7c 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -11,7 +11,7 @@
 ARCH_REL_TYPE_ABS := R_AARCH64_JUMP_SLOT|R_AARCH64_GLOB_DAT|R_AARCH64_ABS64
 include $(srctree)/lib/vdso/Makefile
 
-obj-vdso := vgettimeofday.o note.o sigreturn.o
+obj-vdso := vgettimeofday.o note.o sigreturn.o vgetcpu.o
 
 # Build rules
 targets := $(obj-vdso) vdso.so vdso.so.dbg
diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
index 7ad2d3a0cd48..c2fd94f14b94 100644
--- a/arch/arm64/kernel/vdso/vdso.lds.S
+++ b/arch/arm64/kernel/vdso/vdso.lds.S
@@ -77,6 +77,7 @@ VERSION
 		__kernel_gettimeofday;
 		__kernel_clock_gettime;
 		__kernel_clock_getres;
+		__kernel_getcpu;
 	local: *;
 	};
 }
diff --git a/arch/arm64/kernel/vdso/vgetcpu.c b/arch/arm64/kernel/vdso/vgetcpu.c
new file mode 100644
index 000000000000..e8972e561e08
--- /dev/null
+++ b/arch/arm64/kernel/vdso/vgetcpu.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ARM64 userspace implementations of getcpu()
+ *
+ * Copyright (C) 2020 ARM Limited
+ *
+ */
+
+#include <asm/unistd.h>
+#include <asm/vdso/datapage.h>
+
+struct getcpucache;
+
+static __always_inline
+int getcpu_fallback(unsigned int *_cpu, unsigned int *_node,
+		    struct getcpucache *_c)
+{
+	register unsigned int *cpu asm("x0") = _cpu;
+	register unsigned int *node asm("x1") = _node;
+	register struct getcpucache *c asm("x2") = _c;
+	register long ret asm ("x0");
+	register long nr asm("x8") = __NR_getcpu;
+
+	asm volatile(
+	"       svc #0\n"
+	: "=r" (ret)
+	: "r" (cpu), "r" (node), "r" (c), "r" (nr)
+	: "memory");
+
+	return ret;
+}
+
+int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
+		    struct getcpucache *c)
+{
+	struct vdso_cpu_data *cpu_data = __vdso_cpu_data();
+
+	if (cpu_data) {
+		if (cpu)
+			*cpu = cpu_data->cpu;
+		if (node)
+			*node = cpu_data->node;
+
+		return 0;
+	}
+
+	return getcpu_fallback(cpu, node, c);
+}
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] arm64: vdso: Add getcpu() implementation

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2465 bytes --]

Hi Mark,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20200605]
[cannot apply to arm64/for-next/core arm-perf/for-next/perf kselftest/next v5.7 v5.7-rc7 v5.7-rc6 v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/arm64-vdso-getcpu-support/20200605-212237
base:    af30725c132e2e5c5369b60369ff0771fde7d4ff
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
|     ^~~~~~~~~~~~~~~
--
>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
|     ^~~~~~~~~~~~~~~
arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for '__kernel_clock_gettime' [-Wmissing-prototypes]
9 | int __kernel_clock_gettime(clockid_t clock,
|     ^~~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for '__kernel_gettimeofday' [-Wmissing-prototypes]
15 | int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
|     ^~~~~~~~~~~~~~~~~~~~~
arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for '__kernel_clock_getres' [-Wmissing-prototypes]
21 | int __kernel_clock_getres(clockid_t clock_id,
|     ^~~~~~~~~~~~~~~~~~~~~

vim +/__kernel_getcpu +33 arch/arm64/kernel/vdso/vgetcpu.c

    32	
  > 33	int __kernel_getcpu(unsigned int *cpu, unsigned int *node,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72978 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] arm64: vdso: Add getcpu() implementation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 901 bytes --]

On Sat, Jun 06, 2020 at 12:11:08AM +0800, kernel test robot wrote:

> >> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
> 33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
> |     ^~~~~~~~~~~~~~~
> --
> >> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
> 33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
> |     ^~~~~~~~~~~~~~~
> arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for '__kernel_clock_gettime' [-Wmissing-prototypes]
> 9 | int __kernel_clock_gettime(clockid_t clock,
> |     ^~~~~~~~~~~~~~~~~~~~~~

I'm not seeing this here and what we're doing is in line with the
existing idiom as can be seen from the __kernel_clock_gettime() code
flagging the same thing.  Possibly an old/outdated toolchain?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kbuild-all] Re: [PATCH 4/5] arm64: vdso: Add getcpu() implementation

[Li Zhijian]

Hi Mark



On 6/6/20 12:35 AM, Mark Brown wrote:
> On Sat, Jun 06, 2020 at 12:11:08AM +0800, kernel test robot wrote:
>
>>>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
>> 33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
>> |     ^~~~~~~~~~~~~~~
>> --
>>>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
>> 33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
>> |     ^~~~~~~~~~~~~~~
>> arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for '__kernel_clock_gettime' [-Wmissing-prototypes]
>> 9 | int __kernel_clock_gettime(clockid_t clock,
>> |     ^~~~~~~~~~~~~~~~~~~~~~
> I'm not seeing this here and what we're doing is in line with the
> existing idiom as can be seen from the __kernel_clock_gettime() code
> flagging the same thing.  Possibly an old/outdated toolchain?
thanks for your input.


Actually, the origin mail contained 4 warnings,  and only one is *new*, 
as the mail mentioned that

> All warnings (new ones prefixed by >>, old ones prefixed by <<):

0Day/LKP had marked the *new* ones prefixed by '>>'


>
>>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for '__kernel_getcpu' [-Wmissing-prototypes]
> 33 | int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
> |     ^~~~~~~~~~~~~~~

the rest 3 warnings, indeed they are not introduced by this patch, 
0Day/LKP listed them here because they appeared near the *new* warning(s).
0Day/LKP hope the extra info/warnings could be also helpful for diagnosis.


Thanks





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kbuild-all] Re: [PATCH 4/5] arm64: vdso: Add getcpu() implementation

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1036 bytes --]

On Mon, Jun 08, 2020 at 03:46:57PM +0800, Li Zhijian wrote:
> On 6/6/20 12:35 AM, Mark Brown wrote:

> > > arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for '__kernel_clock_gettime' [-Wmissing-prototypes]
> > > 9 | int __kernel_clock_gettime(clockid_t clock,
> > > |     ^~~~~~~~~~~~~~~~~~~~~~

> > I'm not seeing this here and what we're doing is in line with the
> > existing idiom as can be seen from the __kernel_clock_gettime() code
> > flagging the same thing.  Possibly an old/outdated toolchain?

> Actually, the origin mail contained 4 warnings,  and only one is *new*, as
> the mail mentioned that

> > All warnings (new ones prefixed by >>, old ones prefixed by <<):

> 0Day/LKP had marked the *new* ones prefixed by '>>'

My point here is that all the warnings, both old and new, look spurious
and the new warning comes from following the pattern that generates the
existing warnings.  I'm not seeing any of those warnings locally or in
other build services like KernelCI.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [kbuild-all] Re: [PATCH 4/5] arm64: vdso: Add getcpu() implementation

[Philip Li]

On Mon, Jun 08, 2020 at 12:09:01PM +0100, Mark Brown wrote:
> On Mon, Jun 08, 2020 at 03:46:57PM +0800, Li Zhijian wrote:
> > On 6/6/20 12:35 AM, Mark Brown wrote:
> 
> > > > arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for '__kernel_clock_gettime' [-Wmissing-prototypes]
> > > > 9 | int __kernel_clock_gettime(clockid_t clock,
> > > > |     ^~~~~~~~~~~~~~~~~~~~~~
> 
> > > I'm not seeing this here and what we're doing is in line with the
> > > existing idiom as can be seen from the __kernel_clock_gettime() code
> > > flagging the same thing.  Possibly an old/outdated toolchain?
> 
> > Actually, the origin mail contained 4 warnings,  and only one is *new*, as
> > the mail mentioned that
> 
> > > All warnings (new ones prefixed by >>, old ones prefixed by <<):
> 
> > 0Day/LKP had marked the *new* ones prefixed by '>>'
> 
> My point here is that all the warnings, both old and new, look spurious
> and the new warning comes from following the pattern that generates the
> existing warnings.  I'm not seeing any of those warnings locally or in
> other build services like KernelCI.
Thanks Mark for the input. Here the 0-day kernel bot uses W=1 to build which
is mentioned as part of reproduce step.

> reproduce (this is a W=1 build):

And we just add FAQ at https://github.com/intel/lkp-tests/wiki/LKP-FAQ#how-is-one-meant-to-act-on-w1-reports-like--wmissing-prototypes
to explain the the missing-prototypes warning, hope this can provide
extra information.

Thanks

> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/5] arm64: vdso: Add getcpu() implementation

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3643 bytes --]

Hi Mark,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20200605]
[cannot apply to arm64/for-next/core arm-perf/for-next/perf kselftest/next v5.7 v5.7-rc7 v5.7-rc6 v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Mark-Brown/arm64-vdso-getcpu-support/20200605-212237
base:    af30725c132e2e5c5369b60369ff0771fde7d4ff
config: arm64-randconfig-r016-20200607 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for function '__kernel_getcpu' [-Wmissing-prototypes]
int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
^
arch/arm64/kernel/vdso/vgetcpu.c:33:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
^
static
1 warning generated.
--
>> arch/arm64/kernel/vdso/vgetcpu.c:33:5: warning: no previous prototype for function '__kernel_getcpu' [-Wmissing-prototypes]
int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
^
arch/arm64/kernel/vdso/vgetcpu.c:33:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __kernel_getcpu(unsigned int *cpu, unsigned int *node,
^
static
1 warning generated.
arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
int __kernel_clock_gettime(clockid_t clock,
^
arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __kernel_clock_gettime(clockid_t clock,
^
static
arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
^
arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
^
static
arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
int __kernel_clock_getres(clockid_t clock_id,
^
arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int __kernel_clock_getres(clockid_t clock_id,
^
static
3 warnings generated.

vim +/__kernel_getcpu +33 arch/arm64/kernel/vdso/vgetcpu.c

    32	
  > 33	int __kernel_getcpu(unsigned int *cpu, unsigned int *node,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33082 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/5] selftests: vdso: Support arm64 in getcpu() test

[Mark Brown]

arm64 exports the vDSO ABI with a version of LINUX_2.6.39 and symbols
prefixed with __kernel rather than __vdso. Update the getcpu() test to
handle this.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/vDSO/vdso_test_getcpu.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/vDSO/vdso_test_getcpu.c b/tools/testing/selftests/vDSO/vdso_test_getcpu.c
index fc25ede131b8..4aeb65012b81 100644
--- a/tools/testing/selftests/vDSO/vdso_test_getcpu.c
+++ b/tools/testing/selftests/vDSO/vdso_test_getcpu.c
@@ -14,8 +14,18 @@
 #include "../kselftest.h"
 #include "parse_vdso.h"
 
+/*
+ * ARM64's vDSO exports its getcpu() implementation with a different
+ * name and version from other architectures, so we need to handle it
+ * as a special case.
+ */
+#if defined(__aarch64__)
+const char *version = "LINUX_2.6.39";
+const char *name = "__kernel_getcpu";
+#else
 const char *version = "LINUX_2.6";
 const char *name = "__vdso_getcpu";
+#endif
 
 struct getcpu_cache;
 typedef long (*getcpu_t)(unsigned int *, unsigned int *,
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/5] arm64: vdso: getcpu() support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1561 bytes --]

On Fri, Jun 05, 2020 at 02:11:26PM +0100, Mark Brown wrote:

> It is questionable if something TPIDRRO_EL0 based is worthwhile at all
> on current kernels, since v4.18 we have had support for restartable
> sequences which can be used to provide a sched_getcpu() implementation
> with generally better performance than the vDSO approach on
> architectures which have that[1].  Work is ongoing to implement this for
> glibc:

...

> This is currently not compatible with KPTI due to what Will suggests is
> a misunderstanding on my part about the use of TPIDRRO_EL0 by the KPTI
> trampoline, since this posting is mainly for discussion of the approach
> as a whole and Will only just mentioned this that's not been addressed
> here.

So, I've been looking into this.  While it is true that we can do the
reinitialization I'm not sure that it's sensible to add this to what's
already a hot path where we're taking a substantial overhead for KPTI.
Given that once RSEQ sched_getcpu() support is merged into glibc (and
hopefully also other libcs) the majority of users will use that rather
than a vDSO implementation and the overwhelming impact will be overhead
until systems with E0PD start to be widely deployed which doesn't seem
great.

If we could find another scratch register for the trampoline that'd be
fine but otherwise I'm really questioning the value.  I've looked and
didn't see anything but it's entirely possible I'm missing something.
Otherwise do people think this is an idea worth pursuing, or should we
just suggest that people use RSEQ instead?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel