[PATCH v5 0/4] RISC-V: ifunced memcpy using new kernel hwprobe interface

[PATCH v5 0/4] RISC-V: ifunced memcpy using new kernel hwprobe interface

[Evan Green]

This series illustrates the use of a recently accepted Linux syscall that
enumerates architectural information about the RISC-V cores the system
is running on. In this series we expose a small wrapper function around
the syscall. An ifunc selector for memcpy queries it to see if unaligned
access is "fast" on this hardware. If it is, it selects a newly provided
implementation of memcpy that doesn't work hard at aligning the src and
destination buffers.

For applications and libraries outside of glibc that want to use
__riscv_hwprobe() in ifunc selectors, this series also introduces
__riscv_hwprobe_early(), which works correctly even before all symbols
have been resolved.

The memcpy implementation is independent enough from the rest of the
series that it can be omitted safely if desired.

Performance numbers were compared using a small test program [1], run on
a D1 Nezha board, which supports fast unaligned access. "Fast" here
means copying unaligned words is faster than copying byte-wise, but
still slower than copying aligned words. Here's the speed of various
memcpy()s with the generic implementation. The numbers before are using
v4's memcpy implementation, with the "copy last byte via overlapping
misaligned word" fix this should get even better, though I'm having
trouble with my setup right now and wasn't able to re-run the numbers
on the same hardware. I'll keep working on that.

memcpy size 1 count 1000000 offset 0 took 109564 us
memcpy size 3 count 1000000 offset 0 took 138425 us
memcpy size 4 count 1000000 offset 0 took 148374 us
memcpy size 7 count 1000000 offset 0 took 178433 us
memcpy size 8 count 1000000 offset 0 took 188430 us
memcpy size f count 1000000 offset 0 took 266118 us
memcpy size f count 1000000 offset 1 took 265940 us
memcpy size f count 1000000 offset 3 took 265934 us
memcpy size f count 1000000 offset 7 took 266215 us
memcpy size f count 1000000 offset 8 took 265954 us
memcpy size f count 1000000 offset 9 took 265886 us
memcpy size 10 count 1000000 offset 0 took 195308 us
memcpy size 11 count 1000000 offset 0 took 205161 us
memcpy size 17 count 1000000 offset 0 took 274376 us
memcpy size 18 count 1000000 offset 0 took 199188 us
memcpy size 19 count 1000000 offset 0 took 209258 us
memcpy size 1f count 1000000 offset 0 took 278263 us
memcpy size 20 count 1000000 offset 0 took 207364 us
memcpy size 21 count 1000000 offset 0 took 217143 us
memcpy size 3f count 1000000 offset 0 took 300023 us
memcpy size 40 count 1000000 offset 0 took 231063 us
memcpy size 41 count 1000000 offset 0 took 241259 us
memcpy size 7c count 100000 offset 0 took 32807 us
memcpy size 7f count 100000 offset 0 took 36274 us
memcpy size ff count 100000 offset 0 took 47818 us
memcpy size ff count 100000 offset 0 took 47932 us
memcpy size 100 count 100000 offset 0 took 40468 us
memcpy size 200 count 100000 offset 0 took 64245 us
memcpy size 27f count 100000 offset 0 took 82549 us
memcpy size 400 count 100000 offset 0 took 111254 us
memcpy size 407 count 100000 offset 0 took 119364 us
memcpy size 800 count 100000 offset 0 took 203899 us
memcpy size 87f count 100000 offset 0 took 222465 us
memcpy size 87f count 100000 offset 3 took 222289 us
memcpy size 1000 count 100000 offset 0 took 388846 us
memcpy size 1000 count 100000 offset 1 took 468827 us
memcpy size 1000 count 100000 offset 3 took 397098 us
memcpy size 1000 count 100000 offset 4 took 397379 us
memcpy size 1000 count 100000 offset 5 took 397368 us
memcpy size 1000 count 100000 offset 7 took 396867 us
memcpy size 1000 count 100000 offset 8 took 389227 us
memcpy size 1000 count 100000 offset 9 took 395949 us
memcpy size 3000 count 50000 offset 0 took 674837 us
memcpy size 3000 count 50000 offset 1 took 676944 us
memcpy size 3000 count 50000 offset 3 took 679709 us
memcpy size 3000 count 50000 offset 4 took 680829 us
memcpy size 3000 count 50000 offset 5 took 678024 us
memcpy size 3000 count 50000 offset 7 took 681097 us
memcpy size 3000 count 50000 offset 8 took 670004 us
memcpy size 3000 count 50000 offset 9 took 674553 us

Here is that same test run with the assembly memcpy() in this series:
memcpy size 1 count 1000000 offset 0 took 92703 us
memcpy size 3 count 1000000 offset 0 took 112527 us
memcpy size 4 count 1000000 offset 0 took 120481 us
memcpy size 7 count 1000000 offset 0 took 149558 us
memcpy size 8 count 1000000 offset 0 took 90617 us
memcpy size f count 1000000 offset 0 took 174373 us
memcpy size f count 1000000 offset 1 took 178615 us
memcpy size f count 1000000 offset 3 took 178845 us
memcpy size f count 1000000 offset 7 took 178636 us
memcpy size f count 1000000 offset 8 took 174442 us
memcpy size f count 1000000 offset 9 took 178660 us
memcpy size 10 count 1000000 offset 0 took 99845 us
memcpy size 11 count 1000000 offset 0 took 112522 us
memcpy size 17 count 1000000 offset 0 took 179735 us
memcpy size 18 count 1000000 offset 0 took 110870 us
memcpy size 19 count 1000000 offset 0 took 121472 us
memcpy size 1f count 1000000 offset 0 took 188231 us
memcpy size 20 count 1000000 offset 0 took 119571 us
memcpy size 21 count 1000000 offset 0 took 132429 us
memcpy size 3f count 1000000 offset 0 took 227021 us
memcpy size 40 count 1000000 offset 0 took 166416 us
memcpy size 41 count 1000000 offset 0 took 180206 us
memcpy size 7c count 100000 offset 0 took 28602 us
memcpy size 7f count 100000 offset 0 took 31676 us
memcpy size ff count 100000 offset 0 took 39257 us
memcpy size ff count 100000 offset 0 took 39176 us
memcpy size 100 count 100000 offset 0 took 21928 us
memcpy size 200 count 100000 offset 0 took 35814 us
memcpy size 27f count 100000 offset 0 took 60315 us
memcpy size 400 count 100000 offset 0 took 63652 us
memcpy size 407 count 100000 offset 0 took 73160 us
memcpy size 800 count 100000 offset 0 took 121532 us
memcpy size 87f count 100000 offset 0 took 147269 us
memcpy size 87f count 100000 offset 3 took 144744 us
memcpy size 1000 count 100000 offset 0 took 232057 us
memcpy size 1000 count 100000 offset 1 took 254319 us
memcpy size 1000 count 100000 offset 3 took 256973 us
memcpy size 1000 count 100000 offset 4 took 257655 us
memcpy size 1000 count 100000 offset 5 took 259456 us
memcpy size 1000 count 100000 offset 7 took 260849 us
memcpy size 1000 count 100000 offset 8 took 232347 us
memcpy size 1000 count 100000 offset 9 took 254330 us
memcpy size 3000 count 50000 offset 0 took 382376 us
memcpy size 3000 count 50000 offset 1 took 389872 us
memcpy size 3000 count 50000 offset 3 took 385310 us
memcpy size 3000 count 50000 offset 4 took 389748 us
memcpy size 3000 count 50000 offset 5 took 391707 us
memcpy size 3000 count 50000 offset 7 took 386778 us
memcpy size 3000 count 50000 offset 8 took 385691 us
memcpy size 3000 count 50000 offset 9 took 392030 us

The assembly routine is measurably better.

[1] https://pastebin.com/DRyECNQW


Changes in v5:
 - Introduced __riscv_hwprobe_early()
 - Do unaligned word access for final trailing bytes (Richard)

Changes in v4:
 - Remove __USE_GNU (Florian)
 - __nonnull, __wur, __THROW, and  __fortified_attr_access decorations
  (Florian)
 - change long to long int (Florian)
 - Fix comment formatting (Florian)
 - Update backup kernel header content copy.
 - Fix function declaration formatting (Florian)
 - Changed export versions to 2.38
 - Fixed comment style (Florian)

Changes in v3:
 - Update argument types to match v4 kernel interface
 - Add the "return" to the vsyscall
 - Fix up vdso arg types to match kernel v4 version
 - Remove ifdef around INLINE_VSYSCALL (Adhemerval)
 - Word align dest for large memcpy()s.
 - Add tags
 - Remove spurious blank line from sysdeps/riscv/memcpy.c

Changes in v2:
 - hwprobe.h: Use __has_include and duplicate Linux content to make
   compilation work when Linux headers are absent (Adhemerval)
 - hwprobe.h: Put declaration under __USE_GNU (Adhemerval)
 - Use INLINE_SYSCALL_CALL (Adhemerval)
 - Update versions
 - Update UNALIGNED_MASK to match kernel v3 series.
 - Add vDSO interface
 - Used _MASK instead of _FAST value itself.

Evan Green (4):
  riscv: Add Linux hwprobe syscall support
  riscv: Add hwprobe vdso call support
  riscv: Add ifunc-compatible hwprobe function
  riscv: Add and use alignment-ignorant memcpy

 sysdeps/riscv/memcopy.h                       |  26 ++++
 sysdeps/riscv/memcpy.c                        |  64 +++++++++
 sysdeps/riscv/memcpy_noalignment.S            | 134 ++++++++++++++++++
 sysdeps/unix/sysv/linux/dl-vdso-setup.c       |  10 ++
 sysdeps/unix/sysv/linux/dl-vdso-setup.h       |   3 +
 sysdeps/unix/sysv/linux/riscv/Makefile        |   9 +-
 sysdeps/unix/sysv/linux/riscv/Versions        |   4 +
 sysdeps/unix/sysv/linux/riscv/hwprobe.c       |  33 +++++
 .../unix/sysv/linux/riscv/hwprobe_static.c    |  36 +++++
 .../unix/sysv/linux/riscv/memcpy-generic.c    |  24 ++++
 .../unix/sysv/linux/riscv/rv32/libc.abilist   |   2 +
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |   2 +
 sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h   |  93 ++++++++++++
 sysdeps/unix/sysv/linux/riscv/sysdep.h        |   1 +
 14 files changed, 439 insertions(+), 2 deletions(-)
 create mode 100644 sysdeps/riscv/memcopy.h
 create mode 100644 sysdeps/riscv/memcpy.c
 create mode 100644 sysdeps/riscv/memcpy_noalignment.S
 create mode 100644 sysdeps/unix/sysv/linux/riscv/hwprobe.c
 create mode 100644 sysdeps/unix/sysv/linux/riscv/hwprobe_static.c
 create mode 100644 sysdeps/unix/sysv/linux/riscv/memcpy-generic.c
 create mode 100644 sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h

-- 
2.34.1

[PATCH v5 1/4] riscv: Add Linux hwprobe syscall support

[Evan Green]

Add awareness and a thin wrapper function around a new Linux system call
that allows callers to get architecture and microarchitecture
information about the CPUs from the kernel. This can be used to
do things like dynamically choose a memcpy implementation.

Signed-off-by: Evan Green <evan@rivosinc.com>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
---

(no changes since v4)

Changes in v4:
 - Remove __USE_GNU (Florian)
 - __nonnull, __wur, __THROW, and  __fortified_attr_access decorations
  (Florian)
 - change long to long int (Florian)
 - Fix comment formatting (Florian)
 - Update backup kernel header content copy.
 - Fix function declaration formatting (Florian)
 - Changed export versions to 2.38

Changes in v3:
 - Update argument types to match v4 kernel interface

Changes in v2:
 - hwprobe.h: Use __has_include and duplicate Linux content to make
   compilation work when Linux headers are absent (Adhemerval)
 - hwprobe.h: Put declaration under __USE_GNU (Adhemerval)
 - Use INLINE_SYSCALL_CALL (Adhemerval)
 - Update versions
 - Update UNALIGNED_MASK to match kernel v3 series.

 sysdeps/unix/sysv/linux/riscv/Makefile        |  4 +-
 sysdeps/unix/sysv/linux/riscv/Versions        |  3 +
 sysdeps/unix/sysv/linux/riscv/hwprobe.c       | 30 ++++++++
 .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 +
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |  1 +
 sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h   | 72 +++++++++++++++++++
 6 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/riscv/hwprobe.c
 create mode 100644 sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h

diff --git a/sysdeps/unix/sysv/linux/riscv/Makefile b/sysdeps/unix/sysv/linux/riscv/Makefile
index 4b6eacb32f..45cc29e40d 100644
--- a/sysdeps/unix/sysv/linux/riscv/Makefile
+++ b/sysdeps/unix/sysv/linux/riscv/Makefile
@@ -1,6 +1,6 @@
 ifeq ($(subdir),misc)
-sysdep_headers += sys/cachectl.h
-sysdep_routines += flush-icache
+sysdep_headers += sys/cachectl.h sys/hwprobe.h
+sysdep_routines += flush-icache hwprobe
 endif
 
 ifeq ($(subdir),stdlib)
diff --git a/sysdeps/unix/sysv/linux/riscv/Versions b/sysdeps/unix/sysv/linux/riscv/Versions
index 5625d2a0b8..0c4016382d 100644
--- a/sysdeps/unix/sysv/linux/riscv/Versions
+++ b/sysdeps/unix/sysv/linux/riscv/Versions
@@ -8,4 +8,7 @@ libc {
   GLIBC_2.27 {
     __riscv_flush_icache;
   }
+  GLIBC_2.38 {
+    __riscv_hwprobe;
+  }
 }
diff --git a/sysdeps/unix/sysv/linux/riscv/hwprobe.c b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
new file mode 100644
index 0000000000..a8a14d29a5
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
@@ -0,0 +1,30 @@
+/* RISC-V hardware feature probing support on Linux
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sys/syscall.h>
+#include <sys/hwprobe.h>
+#include <sysdep.h>
+
+int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
+		     size_t cpu_count, unsigned long int *cpus,
+		     unsigned int flags)
+{
+  return INLINE_SYSCALL_CALL (riscv_hwprobe, pairs, pair_count,
+                         cpu_count, cpus, flags);
+}
diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
index b9740a1afc..8fab4a606f 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
@@ -2436,3 +2436,4 @@ GLIBC_2.38 strlcat F
 GLIBC_2.38 strlcpy F
 GLIBC_2.38 wcslcat F
 GLIBC_2.38 wcslcpy F
+GLIBC_2.38 __riscv_hwprobe F
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
index e3b4656aa2..1ebb91deed 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
@@ -2636,3 +2636,4 @@ GLIBC_2.38 strlcat F
 GLIBC_2.38 strlcpy F
 GLIBC_2.38 wcslcat F
 GLIBC_2.38 wcslcpy F
+GLIBC_2.38 __riscv_hwprobe F
diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
new file mode 100644
index 0000000000..b27af5cb07
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
@@ -0,0 +1,72 @@
+/* RISC-V architecture probe interface
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_HWPROBE_H
+#define _SYS_HWPROBE_H 1
+
+#include <features.h>
+#include <stddef.h>
+#ifdef __has_include
+# if __has_include (<asm/hwprobe.h>)
+#  include <asm/hwprobe.h>
+# endif
+#endif
+
+/* Define a (probably stale) version of the interface if the Linux headers
+   aren't present.  */
+#ifndef RISCV_HWPROBE_KEY_MVENDORID
+struct riscv_hwprobe {
+	signed long long int key;
+	unsigned long long int value;
+};
+
+#define RISCV_HWPROBE_KEY_MVENDORID	0
+#define RISCV_HWPROBE_KEY_MARCHID	1
+#define RISCV_HWPROBE_KEY_MIMPID	2
+#define RISCV_HWPROBE_KEY_BASE_BEHAVIOR	3
+#define		RISCV_HWPROBE_BASE_BEHAVIOR_IMA	(1 << 0)
+#define RISCV_HWPROBE_KEY_IMA_EXT_0	4
+#define		RISCV_HWPROBE_IMA_FD		(1 << 0)
+#define		RISCV_HWPROBE_IMA_C		(1 << 1)
+#define		RISCV_HWPROBE_IMA_V		(1 << 2)
+#define		RISCV_HWPROBE_EXT_ZBA		(1 << 3)
+#define		RISCV_HWPROBE_EXT_ZBB		(1 << 4)
+#define		RISCV_HWPROBE_EXT_ZBS		(1 << 5)
+#define RISCV_HWPROBE_KEY_CPUPERF_0	5
+#define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
+#define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
+#define		RISCV_HWPROBE_MISALIGNED_SLOW		(2 << 0)
+#define		RISCV_HWPROBE_MISALIGNED_FAST		(3 << 0)
+#define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
+#define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
+
+#endif /* RISCV_HWPROBE_KEY_MVENDORID */
+
+__BEGIN_DECLS
+
+extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
+			    size_t cpu_count, unsigned long int *cpus,
+			    unsigned int flags)
+     __THROW __nonnull ((1)) __wur
+     __fortified_attr_access (__read_write__, 1, 2)
+     __fortified_attr_access (__read_only__, 4, 3);
+
+__END_DECLS
+
+#endif /* sys/hwprobe.h */
-- 
2.34.1

[PATCH v5 2/4] riscv: Add hwprobe vdso call support

[Evan Green]

The new riscv_hwprobe syscall also comes with a vDSO for faster answers
to your most common questions. Call in today to speak with a kernel
representative near you!

Signed-off-by: Evan Green <evan@rivosinc.com>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
---

(no changes since v3)

Changes in v3:
 - Add the "return" to the vsyscall
 - Fix up vdso arg types to match kernel v4 version
 - Remove ifdef around INLINE_VSYSCALL (Adhemerval)

Changes in v2:
 - Add vDSO interface

 sysdeps/unix/sysv/linux/dl-vdso-setup.c | 10 ++++++++++
 sysdeps/unix/sysv/linux/dl-vdso-setup.h |  3 +++
 sysdeps/unix/sysv/linux/riscv/hwprobe.c |  5 +++--
 sysdeps/unix/sysv/linux/riscv/sysdep.h  |  1 +
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/dl-vdso-setup.c b/sysdeps/unix/sysv/linux/dl-vdso-setup.c
index 97eaaeac37..ed8b1ef426 100644
--- a/sysdeps/unix/sysv/linux/dl-vdso-setup.c
+++ b/sysdeps/unix/sysv/linux/dl-vdso-setup.c
@@ -71,6 +71,16 @@ PROCINFO_CLASS int (*_dl_vdso_clock_getres_time64) (clockid_t,
 # ifdef HAVE_GET_TBFREQ
 PROCINFO_CLASS uint64_t (*_dl_vdso_get_tbfreq)(void) RELRO;
 # endif
+
+/* RISC-V specific ones.  */
+# ifdef HAVE_RISCV_HWPROBE
+PROCINFO_CLASS int (*_dl_vdso_riscv_hwprobe)(void *,
+                                             size_t,
+                                             size_t,
+                                             unsigned long *,
+                                             unsigned int) RELRO;
+# endif
+
 #endif
 
 #undef RELRO
diff --git a/sysdeps/unix/sysv/linux/dl-vdso-setup.h b/sysdeps/unix/sysv/linux/dl-vdso-setup.h
index 867072b897..39eafd5316 100644
--- a/sysdeps/unix/sysv/linux/dl-vdso-setup.h
+++ b/sysdeps/unix/sysv/linux/dl-vdso-setup.h
@@ -47,6 +47,9 @@ setup_vdso_pointers (void)
 #ifdef HAVE_GET_TBFREQ
   GLRO(dl_vdso_get_tbfreq) = dl_vdso_vsym (HAVE_GET_TBFREQ);
 #endif
+#ifdef HAVE_RISCV_HWPROBE
+  GLRO(dl_vdso_riscv_hwprobe) = dl_vdso_vsym (HAVE_RISCV_HWPROBE);
+#endif
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/riscv/hwprobe.c b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
index a8a14d29a5..14f7136998 100644
--- a/sysdeps/unix/sysv/linux/riscv/hwprobe.c
+++ b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
@@ -20,11 +20,12 @@
 #include <sys/syscall.h>
 #include <sys/hwprobe.h>
 #include <sysdep.h>
+#include <sysdep-vdso.h>
 
 int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
 		     size_t cpu_count, unsigned long int *cpus,
 		     unsigned int flags)
 {
-  return INLINE_SYSCALL_CALL (riscv_hwprobe, pairs, pair_count,
-                         cpu_count, cpus, flags);
+ /* The vDSO may be able to provide the answer without a syscall. */
+  return INLINE_VSYSCALL(riscv_hwprobe, 5, pairs, pair_count, cpu_count, cpus, flags);
 }
diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h
index 5583b96d23..ee015dfeb6 100644
--- a/sysdeps/unix/sysv/linux/riscv/sysdep.h
+++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h
@@ -156,6 +156,7 @@
 /* List of system calls which are supported as vsyscalls (for RV32 and
    RV64).  */
 # define HAVE_GETCPU_VSYSCALL		"__vdso_getcpu"
+# define HAVE_RISCV_HWPROBE		"__vdso_riscv_hwprobe"
 
 # undef HAVE_INTERNAL_BRK_ADDR_SYMBOL
 # define HAVE_INTERNAL_BRK_ADDR_SYMBOL 1
-- 
2.34.1

[PATCH v5 3/4] riscv: Add ifunc-compatible hwprobe function

[Evan Green]

ifunc selector functions are called very early, and are not able to call
external functions that require relocation. This presents a problem for
applications and libraries outside of glibc that may want to use
__riscv_hwprobe() in their ifunc selectors.

Add a new function, __riscv_hwprobe_early(), that is compiled into
libc_nonshared.a and is safe to use in application and library ifunc
selectors. The _early() variant checks a weak alias of __riscv_hwprobe,
__riscv_hwprobe_weak, to see if the symbol is available, and simply
calls that if so. However if it's called before the symbol is available,
it makes the system call directly. The early call has some caveats,
primarily that it does not take advantage of the cached vDSO data, and
does not set errno (another external symbol) upon failing.

The original dynamic __riscv_hwprobe() function is still available for
users that don't need the overhead of handling early init cases.

Signed-off-by: Evan Green <evan@rivosinc.com>

---

Changes in v5:
 - Introduced __riscv_hwprobe_early()

 sysdeps/unix/sysv/linux/riscv/Makefile        |  3 +-
 sysdeps/unix/sysv/linux/riscv/Versions        |  1 +
 sysdeps/unix/sysv/linux/riscv/hwprobe.c       |  2 ++
 .../unix/sysv/linux/riscv/hwprobe_static.c    | 36 +++++++++++++++++++
 .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 +
 .../unix/sysv/linux/riscv/rv64/libc.abilist   |  1 +
 sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h   | 21 +++++++++++
 7 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 sysdeps/unix/sysv/linux/riscv/hwprobe_static.c

diff --git a/sysdeps/unix/sysv/linux/riscv/Makefile b/sysdeps/unix/sysv/linux/riscv/Makefile
index 45cc29e40d..fd8fd48ade 100644
--- a/sysdeps/unix/sysv/linux/riscv/Makefile
+++ b/sysdeps/unix/sysv/linux/riscv/Makefile
@@ -1,6 +1,7 @@
 ifeq ($(subdir),misc)
 sysdep_headers += sys/cachectl.h sys/hwprobe.h
-sysdep_routines += flush-icache hwprobe
+sysdep_routines += flush-icache hwprobe hwprobe_static
+static-only-routines += hwprobe_static
 endif
 
 ifeq ($(subdir),stdlib)
diff --git a/sysdeps/unix/sysv/linux/riscv/Versions b/sysdeps/unix/sysv/linux/riscv/Versions
index 0c4016382d..571d4c2046 100644
--- a/sysdeps/unix/sysv/linux/riscv/Versions
+++ b/sysdeps/unix/sysv/linux/riscv/Versions
@@ -10,5 +10,6 @@ libc {
   }
   GLIBC_2.38 {
     __riscv_hwprobe;
+    __riscv_hwprobe_weak;
   }
 }
diff --git a/sysdeps/unix/sysv/linux/riscv/hwprobe.c b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
index 14f7136998..a8bd63fca4 100644
--- a/sysdeps/unix/sysv/linux/riscv/hwprobe.c
+++ b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
@@ -29,3 +29,5 @@ int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
  /* The vDSO may be able to provide the answer without a syscall. */
   return INLINE_VSYSCALL(riscv_hwprobe, 5, pairs, pair_count, cpu_count, cpus, flags);
 }
+
+weak_alias(__riscv_hwprobe, __riscv_hwprobe_weak)
diff --git a/sysdeps/unix/sysv/linux/riscv/hwprobe_static.c b/sysdeps/unix/sysv/linux/riscv/hwprobe_static.c
new file mode 100644
index 0000000000..b1c0f4089f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/riscv/hwprobe_static.c
@@ -0,0 +1,36 @@
+/* RISC-V hardware feature probing support on Linux, ifunc-compatible
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sys/syscall.h>
+#include <sys/hwprobe.h>
+#include <sysdep.h>
+#include <sysdep-vdso.h>
+
+attribute_hidden
+int __riscv_hwprobe_early (struct riscv_hwprobe *pairs, size_t pair_count,
+			   size_t cpu_count, unsigned long int *cpus,
+			   unsigned int flags)
+{
+  if (!__riscv_hwprobe_weak) {
+    /* Use INTERNAL_SYSCALL since errno is not available. */
+    return INTERNAL_SYSCALL(riscv_hwprobe, 5, pairs, pair_count, cpu_count, cpus, flags);
+  }
+
+  return __riscv_hwprobe_weak(pairs, pair_count, cpu_count, cpus, flags);
+}
diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
index 8fab4a606f..942e397be0 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
@@ -2437,3 +2437,4 @@ GLIBC_2.38 strlcpy F
 GLIBC_2.38 wcslcat F
 GLIBC_2.38 wcslcpy F
 GLIBC_2.38 __riscv_hwprobe F
+GLIBC_2.38 __riscv_hwprobe_weak F
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
index 1ebb91deed..bc140fa7df 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
@@ -2637,3 +2637,4 @@ GLIBC_2.38 strlcpy F
 GLIBC_2.38 wcslcat F
 GLIBC_2.38 wcslcpy F
 GLIBC_2.38 __riscv_hwprobe F
+GLIBC_2.38 __riscv_hwprobe_weak F
diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
index b27af5cb07..d739371806 100644
--- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
+++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
@@ -67,6 +67,27 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
      __fortified_attr_access (__read_write__, 1, 2)
      __fortified_attr_access (__read_only__, 4, 3);
 
+extern int __riscv_hwprobe_weak (struct riscv_hwprobe *pairs, size_t pair_count,
+				 size_t cpu_count, unsigned long int *cpus,
+				 unsigned int flags)
+     __THROW __nonnull ((1)) __wur
+     __fortified_attr_access (__read_write__, 1, 2)
+     __fortified_attr_access (__read_only__, 4, 3);
+
+# ifdef weak_extern
+weak_extern (__riscv_hwprobe_weak)
+# else
+#  pragma weak __riscv_hwprobe_weak
+# endif
+
+extern int __riscv_hwprobe_early (struct riscv_hwprobe *pairs, size_t pair_count,
+				  size_t cpu_count, unsigned long int *cpus,
+				  unsigned int flags)
+     __THROW __nonnull ((1)) __wur
+     __fortified_attr_access (__read_write__, 1, 2)
+     __fortified_attr_access (__read_only__, 4, 3)
+     __attribute__ ((__visibility__ ("hidden")));
+
 __END_DECLS
 
 #endif /* sys/hwprobe.h */
-- 
2.34.1

[PATCH v5 4/4] riscv: Add and use alignment-ignorant memcpy

[Evan Green]

For CPU implementations that can perform unaligned accesses with little
or no performance penalty, create a memcpy implementation that does not
bother aligning buffers. It will use a block of integer registers, a
single integer register, and fall back to bytewise copy for the
remainder.

Signed-off-by: Evan Green <evan@rivosinc.com>
Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

---

Changes in v5:
 - Do unaligned word access for final trailing bytes (Richard)

Changes in v4:
 - Fixed comment style (Florian)

Changes in v3:
 - Word align dest for large memcpy()s.
 - Add tags
 - Remove spurious blank line from sysdeps/riscv/memcpy.c

Changes in v2:
 - Used _MASK instead of _FAST value itself.


---
 sysdeps/riscv/memcopy.h                       |  26 ++++
 sysdeps/riscv/memcpy.c                        |  64 +++++++++
 sysdeps/riscv/memcpy_noalignment.S            | 134 ++++++++++++++++++
 sysdeps/unix/sysv/linux/riscv/Makefile        |   4 +
 .../unix/sysv/linux/riscv/memcpy-generic.c    |  24 ++++
 5 files changed, 252 insertions(+)
 create mode 100644 sysdeps/riscv/memcopy.h
 create mode 100644 sysdeps/riscv/memcpy.c
 create mode 100644 sysdeps/riscv/memcpy_noalignment.S
 create mode 100644 sysdeps/unix/sysv/linux/riscv/memcpy-generic.c

diff --git a/sysdeps/riscv/memcopy.h b/sysdeps/riscv/memcopy.h
new file mode 100644
index 0000000000..2b685c8aa0
--- /dev/null
+++ b/sysdeps/riscv/memcopy.h
@@ -0,0 +1,26 @@
+/* memcopy.h -- definitions for memory copy functions. RISC-V version.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sysdeps/generic/memcopy.h>
+
+/* Redefine the generic memcpy implementation to __memcpy_generic, so
+   the memcpy ifunc can select between generic and special versions.
+   In rtld, don't bother with all the ifunciness. */
+#if IS_IN (libc)
+#define MEMCPY __memcpy_generic
+#endif
diff --git a/sysdeps/riscv/memcpy.c b/sysdeps/riscv/memcpy.c
new file mode 100644
index 0000000000..fdb8dc3208
--- /dev/null
+++ b/sysdeps/riscv/memcpy.c
@@ -0,0 +1,64 @@
+/* Multiple versions of memcpy.
+   All versions must be listed in ifunc-impl-list.c.
+   Copyright (C) 2017-2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#if IS_IN (libc)
+/* Redefine memcpy so that the compiler won't complain about the type
+   mismatch with the IFUNC selector in strong_alias, below.  */
+# undef memcpy
+# define memcpy __redirect_memcpy
+# include <string.h>
+#include <ifunc-init.h>
+#include <sys/hwprobe.h>
+
+#define INIT_ARCH()
+
+extern __typeof (__redirect_memcpy) __libc_memcpy;
+
+extern __typeof (__redirect_memcpy) __memcpy_generic attribute_hidden;
+extern __typeof (__redirect_memcpy) __memcpy_noalignment attribute_hidden;
+
+static inline __typeof (__redirect_memcpy) *
+select_memcpy_ifunc (void)
+{
+  INIT_ARCH ();
+
+  struct riscv_hwprobe pair;
+
+  pair.key = RISCV_HWPROBE_KEY_CPUPERF_0;
+  if (__riscv_hwprobe(&pair, 1, 0, NULL, 0) != 0)
+    return __memcpy_generic;
+
+  if ((pair.key > 0) &&
+      (pair.value & RISCV_HWPROBE_MISALIGNED_MASK) ==
+       RISCV_HWPROBE_MISALIGNED_FAST)
+    return __memcpy_noalignment;
+
+  return __memcpy_generic;
+}
+
+libc_ifunc (__libc_memcpy, select_memcpy_ifunc ());
+
+# undef memcpy
+strong_alias (__libc_memcpy, memcpy);
+# ifdef SHARED
+__hidden_ver1 (memcpy, __GI_memcpy, __redirect_memcpy)
+  __attribute__ ((visibility ("hidden"))) __attribute_copy__ (memcpy);
+# endif
+
+#endif
diff --git a/sysdeps/riscv/memcpy_noalignment.S b/sysdeps/riscv/memcpy_noalignment.S
new file mode 100644
index 0000000000..907b858b18
--- /dev/null
+++ b/sysdeps/riscv/memcpy_noalignment.S
@@ -0,0 +1,134 @@
+/* memcpy for RISC-V, ignoring buffer alignment
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <sysdep.h>
+#include <sys/asm.h>
+
+/* void *memcpy(void *, const void *, size_t) */
+ENTRY (__memcpy_noalignment)
+	move t6, a0  /* Preserve return value */
+
+	/* Bail if 0 */
+	beqz a2, 6f
+
+	/* Jump to byte copy if size < SZREG */
+	li a4, SZREG
+	bltu a2, a4, 5f
+
+	/* Round down to the nearest "page" size */
+	andi a4, a2, ~((16*SZREG)-1)
+	beqz a4, 2f
+	add a3, a1, a4
+
+	/* Copy the first word to get dest word aligned */
+	andi a5, t6, SZREG-1
+	beqz a5, 1f
+	REG_L a6, (a1)
+	REG_S a6, (t6)
+
+	/* Align dst up to a word, move src and size as well. */
+	addi t6, t6, SZREG-1
+	andi t6, t6, ~(SZREG-1)
+	sub a5, t6, a0
+	add a1, a1, a5
+	sub a2, a2, a5
+
+	/* Recompute page count */
+	andi a4, a2, ~((16*SZREG)-1)
+	beqz a4, 2f
+
+1:
+	/* Copy "pages" (chunks of 16 registers) */
+	REG_L a4,       0(a1)
+	REG_L a5,   SZREG(a1)
+	REG_L a6, 2*SZREG(a1)
+	REG_L a7, 3*SZREG(a1)
+	REG_L t0, 4*SZREG(a1)
+	REG_L t1, 5*SZREG(a1)
+	REG_L t2, 6*SZREG(a1)
+	REG_L t3, 7*SZREG(a1)
+	REG_L t4, 8*SZREG(a1)
+	REG_L t5, 9*SZREG(a1)
+	REG_S a4,       0(t6)
+	REG_S a5,   SZREG(t6)
+	REG_S a6, 2*SZREG(t6)
+	REG_S a7, 3*SZREG(t6)
+	REG_S t0, 4*SZREG(t6)
+	REG_S t1, 5*SZREG(t6)
+	REG_S t2, 6*SZREG(t6)
+	REG_S t3, 7*SZREG(t6)
+	REG_S t4, 8*SZREG(t6)
+	REG_S t5, 9*SZREG(t6)
+	REG_L a4, 10*SZREG(a1)
+	REG_L a5, 11*SZREG(a1)
+	REG_L a6, 12*SZREG(a1)
+	REG_L a7, 13*SZREG(a1)
+	REG_L t0, 14*SZREG(a1)
+	REG_L t1, 15*SZREG(a1)
+	addi a1, a1, 16*SZREG
+	REG_S a4, 10*SZREG(t6)
+	REG_S a5, 11*SZREG(t6)
+	REG_S a6, 12*SZREG(t6)
+	REG_S a7, 13*SZREG(t6)
+	REG_S t0, 14*SZREG(t6)
+	REG_S t1, 15*SZREG(t6)
+	addi t6, t6, 16*SZREG
+	bltu a1, a3, 1b
+	andi a2, a2, (16*SZREG)-1  /* Update count */
+
+2:
+	/* Remainder is smaller than a page, compute native word count */
+	beqz a2, 6f
+	andi a5, a2, ~(SZREG-1)
+	andi a2, a2, (SZREG-1)
+	add a3, a1, a5
+	/* Jump directly to last word if no words. */
+	beqz a5, 4f
+
+3:
+	/* Use single native register copy */
+	REG_L a4, 0(a1)
+	addi a1, a1, SZREG
+	REG_S a4, 0(t6)
+	addi t6, t6, SZREG
+	bltu a1, a3, 3b
+
+	/* Jump directly out if no more bytes */
+	beqz a2, 6f
+
+4:
+	/* Copy the last word unaligned */
+	add a3, a1, a2
+	add a4, t6, a2
+	REG_L a4, -SZREG(a4)
+	REG_S a4, -SZREG(a3)
+	ret
+
+5:
+	/* Copy bytes when the total copy is <SZREG */
+	lb a4, 0(a1)
+	addi a1, a1, 1
+	sb a4, 0(t6)
+	addi t6, t6, 1
+	bltu a1, a3, 5b
+6:
+	ret
+
+END (__memcpy_noalignment)
+
+hidden_def (__memcpy_noalignment)
diff --git a/sysdeps/unix/sysv/linux/riscv/Makefile b/sysdeps/unix/sysv/linux/riscv/Makefile
index fd8fd48ade..ef4888b295 100644
--- a/sysdeps/unix/sysv/linux/riscv/Makefile
+++ b/sysdeps/unix/sysv/linux/riscv/Makefile
@@ -8,6 +8,10 @@ ifeq ($(subdir),stdlib)
 gen-as-const-headers += ucontext_i.sym
 endif
 
+ifeq ($(subdir),string)
+sysdep_routines += memcpy memcpy-generic memcpy_noalignment
+endif
+
 abi-variants := ilp32 ilp32d lp64 lp64d
 
 ifeq (,$(filter $(default-abi),$(abi-variants)))
diff --git a/sysdeps/unix/sysv/linux/riscv/memcpy-generic.c b/sysdeps/unix/sysv/linux/riscv/memcpy-generic.c
new file mode 100644
index 0000000000..0abe03f7f5
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/riscv/memcpy-generic.c
@@ -0,0 +1,24 @@
+/* Re-include the default memcpy implementation.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <string.h>
+
+extern __typeof (memcpy) __memcpy_generic;
+hidden_proto(__memcpy_generic)
+
+#include <string/memcpy.c>
-- 
2.34.1

Re: [PATCH v5 0/4] RISC-V: ifunced memcpy using new kernel hwprobe interface

[Evan Green]

On Wed, Jul 12, 2023 at 12:36 PM Evan Green <evan@rivosinc.com> wrote:
>
>
> This series illustrates the use of a recently accepted Linux syscall that
> enumerates architectural information about the RISC-V cores the system
> is running on. In this series we expose a small wrapper function around
> the syscall. An ifunc selector for memcpy queries it to see if unaligned
> access is "fast" on this hardware. If it is, it selects a newly provided
> implementation of memcpy that doesn't work hard at aligning the src and
> destination buffers.
>
> For applications and libraries outside of glibc that want to use
> __riscv_hwprobe() in ifunc selectors, this series also introduces
> __riscv_hwprobe_early(), which works correctly even before all symbols
> have been resolved.
>
> The memcpy implementation is independent enough from the rest of the
> series that it can be omitted safely if desired.
>
> Performance numbers were compared using a small test program [1], run on
> a D1 Nezha board, which supports fast unaligned access. "Fast" here
> means copying unaligned words is faster than copying byte-wise, but
> still slower than copying aligned words. Here's the speed of various
> memcpy()s with the generic implementation. The numbers before are using
> v4's memcpy implementation, with the "copy last byte via overlapping
> misaligned word" fix this should get even better, though I'm having
> trouble with my setup right now and wasn't able to re-run the numbers
> on the same hardware. I'll keep working on that.

Sigh, so the mysterious "issue" I was facing was in fact that my
assembly code was totally broken. I have an actually-tested version of
the assembly I can send the next spin anytime, but I figure I'll wait
for any comments Florian might have about my ifunc-friendly hwprobe
patch. Sorry for the noise.

Revised performance numbers with v5 are improved over v4 for odd
numbered sizes. Copies with already aligned buffers and sizes take a
small penalty over v4, which is expected due to the extra conditional.
But both v4 and v5 are an improvement over what's in-tree now:

memcpy size 1 count 1000000 offset 0 took 82887 us
memcpy size 3 count 1000000 offset 0 took 102711 us
memcpy size 4 count 1000000 offset 0 took 111857 us
memcpy size 7 count 1000000 offset 0 took 142587 us
memcpy size 8 count 1000000 offset 0 took 96730 us
memcpy size f count 1000000 offset 0 took 107694 us
memcpy size f count 1000000 offset 1 took 110674 us
memcpy size f count 1000000 offset 3 took 111728 us
memcpy size f count 1000000 offset 7 took 111767 us
memcpy size f count 1000000 offset 8 took 107698 us
memcpy size f count 1000000 offset 9 took 110699 us
memcpy size 10 count 1000000 offset 0 took 108755 us
memcpy size 11 count 1000000 offset 0 took 128412 us
memcpy size 17 count 1000000 offset 0 took 127772 us
memcpy size 18 count 1000000 offset 0 took 116655 us
memcpy size 19 count 1000000 offset 0 took 127676 us
memcpy size 1f count 1000000 offset 0 took 127628 us
memcpy size 20 count 1000000 offset 0 took 128680 us
memcpy size 21 count 1000000 offset 0 took 142597 us
memcpy size 3f count 1000000 offset 0 took 178168 us
memcpy size 40 count 1000000 offset 0 took 172904 us
memcpy size 41 count 1000000 offset 0 took 185312 us
memcpy size 7c count 100000 offset 0 took 26748 us
memcpy size 7f count 100000 offset 0 took 26798 us
memcpy size ff count 100000 offset 0 took 34023 us
memcpy size ff count 100000 offset 0 took 34038 us
memcpy size 100 count 100000 offset 0 took 22458 us
memcpy size 200 count 100000 offset 0 took 36446 us
memcpy size 27f count 100000 offset 0 took 55046 us
memcpy size 400 count 100000 offset 0 took 65218 us
memcpy size 407 count 100000 offset 0 took 66900 us
memcpy size 800 count 100000 offset 0 took 122498 us
memcpy size 87f count 100000 offset 0 took 141846 us
memcpy size 87f count 100000 offset 3 took 143608 us
memcpy size 1000 count 100000 offset 0 took 233596 us
memcpy size 1000 count 100000 offset 1 took 256441 us
memcpy size 1000 count 100000 offset 3 took 257418 us
memcpy size 1000 count 100000 offset 4 took 256342 us
memcpy size 1000 count 100000 offset 5 took 256154 us
memcpy size 1000 count 100000 offset 7 took 256306 us
memcpy size 1000 count 100000 offset 8 took 233670 us
memcpy size 1000 count 100000 offset 9 took 257956 us
memcpy size 3000 count 50000 offset 0 took 506898 us
memcpy size 3000 count 50000 offset 1 took 516202 us
memcpy size 3000 count 50000 offset 3 took 516558 us
memcpy size 3000 count 50000 offset 4 took 519054 us
memcpy size 3000 count 50000 offset 5 took 520583 us
memcpy size 3000 count 50000 offset 7 took 515544 us
memcpy size 3000 count 50000 offset 8 took 504335 us
memcpy size 3000 count 50000 offset 9 took 518367 us

-Evan

>
> memcpy size 1 count 1000000 offset 0 took 109564 us
> memcpy size 3 count 1000000 offset 0 took 138425 us
> memcpy size 4 count 1000000 offset 0 took 148374 us
> memcpy size 7 count 1000000 offset 0 took 178433 us
> memcpy size 8 count 1000000 offset 0 took 188430 us
> memcpy size f count 1000000 offset 0 took 266118 us
> memcpy size f count 1000000 offset 1 took 265940 us
> memcpy size f count 1000000 offset 3 took 265934 us
> memcpy size f count 1000000 offset 7 took 266215 us
> memcpy size f count 1000000 offset 8 took 265954 us
> memcpy size f count 1000000 offset 9 took 265886 us
> memcpy size 10 count 1000000 offset 0 took 195308 us
> memcpy size 11 count 1000000 offset 0 took 205161 us
> memcpy size 17 count 1000000 offset 0 took 274376 us
> memcpy size 18 count 1000000 offset 0 took 199188 us
> memcpy size 19 count 1000000 offset 0 took 209258 us
> memcpy size 1f count 1000000 offset 0 took 278263 us
> memcpy size 20 count 1000000 offset 0 took 207364 us
> memcpy size 21 count 1000000 offset 0 took 217143 us
> memcpy size 3f count 1000000 offset 0 took 300023 us
> memcpy size 40 count 1000000 offset 0 took 231063 us
> memcpy size 41 count 1000000 offset 0 took 241259 us
> memcpy size 7c count 100000 offset 0 took 32807 us
> memcpy size 7f count 100000 offset 0 took 36274 us
> memcpy size ff count 100000 offset 0 took 47818 us
> memcpy size ff count 100000 offset 0 took 47932 us
> memcpy size 100 count 100000 offset 0 took 40468 us
> memcpy size 200 count 100000 offset 0 took 64245 us
> memcpy size 27f count 100000 offset 0 took 82549 us
> memcpy size 400 count 100000 offset 0 took 111254 us
> memcpy size 407 count 100000 offset 0 took 119364 us
> memcpy size 800 count 100000 offset 0 took 203899 us
> memcpy size 87f count 100000 offset 0 took 222465 us
> memcpy size 87f count 100000 offset 3 took 222289 us
> memcpy size 1000 count 100000 offset 0 took 388846 us
> memcpy size 1000 count 100000 offset 1 took 468827 us
> memcpy size 1000 count 100000 offset 3 took 397098 us
> memcpy size 1000 count 100000 offset 4 took 397379 us
> memcpy size 1000 count 100000 offset 5 took 397368 us
> memcpy size 1000 count 100000 offset 7 took 396867 us
> memcpy size 1000 count 100000 offset 8 took 389227 us
> memcpy size 1000 count 100000 offset 9 took 395949 us
> memcpy size 3000 count 50000 offset 0 took 674837 us
> memcpy size 3000 count 50000 offset 1 took 676944 us
> memcpy size 3000 count 50000 offset 3 took 679709 us
> memcpy size 3000 count 50000 offset 4 took 680829 us
> memcpy size 3000 count 50000 offset 5 took 678024 us
> memcpy size 3000 count 50000 offset 7 took 681097 us
> memcpy size 3000 count 50000 offset 8 took 670004 us
> memcpy size 3000 count 50000 offset 9 took 674553 us
>
> Here is that same test run with the assembly memcpy() in this series:
> memcpy size 1 count 1000000 offset 0 took 92703 us
> memcpy size 3 count 1000000 offset 0 took 112527 us
> memcpy size 4 count 1000000 offset 0 took 120481 us
> memcpy size 7 count 1000000 offset 0 took 149558 us
> memcpy size 8 count 1000000 offset 0 took 90617 us
> memcpy size f count 1000000 offset 0 took 174373 us
> memcpy size f count 1000000 offset 1 took 178615 us
> memcpy size f count 1000000 offset 3 took 178845 us
> memcpy size f count 1000000 offset 7 took 178636 us
> memcpy size f count 1000000 offset 8 took 174442 us
> memcpy size f count 1000000 offset 9 took 178660 us
> memcpy size 10 count 1000000 offset 0 took 99845 us
> memcpy size 11 count 1000000 offset 0 took 112522 us
> memcpy size 17 count 1000000 offset 0 took 179735 us
> memcpy size 18 count 1000000 offset 0 took 110870 us
> memcpy size 19 count 1000000 offset 0 took 121472 us
> memcpy size 1f count 1000000 offset 0 took 188231 us
> memcpy size 20 count 1000000 offset 0 took 119571 us
> memcpy size 21 count 1000000 offset 0 took 132429 us
> memcpy size 3f count 1000000 offset 0 took 227021 us
> memcpy size 40 count 1000000 offset 0 took 166416 us
> memcpy size 41 count 1000000 offset 0 took 180206 us
> memcpy size 7c count 100000 offset 0 took 28602 us
> memcpy size 7f count 100000 offset 0 took 31676 us
> memcpy size ff count 100000 offset 0 took 39257 us
> memcpy size ff count 100000 offset 0 took 39176 us
> memcpy size 100 count 100000 offset 0 took 21928 us
> memcpy size 200 count 100000 offset 0 took 35814 us
> memcpy size 27f count 100000 offset 0 took 60315 us
> memcpy size 400 count 100000 offset 0 took 63652 us
> memcpy size 407 count 100000 offset 0 took 73160 us
> memcpy size 800 count 100000 offset 0 took 121532 us
> memcpy size 87f count 100000 offset 0 took 147269 us
> memcpy size 87f count 100000 offset 3 took 144744 us
> memcpy size 1000 count 100000 offset 0 took 232057 us
> memcpy size 1000 count 100000 offset 1 took 254319 us
> memcpy size 1000 count 100000 offset 3 took 256973 us
> memcpy size 1000 count 100000 offset 4 took 257655 us
> memcpy size 1000 count 100000 offset 5 took 259456 us
> memcpy size 1000 count 100000 offset 7 took 260849 us
> memcpy size 1000 count 100000 offset 8 took 232347 us
> memcpy size 1000 count 100000 offset 9 took 254330 us
> memcpy size 3000 count 50000 offset 0 took 382376 us
> memcpy size 3000 count 50000 offset 1 took 389872 us
> memcpy size 3000 count 50000 offset 3 took 385310 us
> memcpy size 3000 count 50000 offset 4 took 389748 us
> memcpy size 3000 count 50000 offset 5 took 391707 us
> memcpy size 3000 count 50000 offset 7 took 386778 us
> memcpy size 3000 count 50000 offset 8 took 385691 us
> memcpy size 3000 count 50000 offset 9 took 392030 us
>
> The assembly routine is measurably better.
>
> [1] https://pastebin.com/DRyECNQW
>
>
> Changes in v5:
>  - Introduced __riscv_hwprobe_early()
>  - Do unaligned word access for final trailing bytes (Richard)
>
> Changes in v4:
>  - Remove __USE_GNU (Florian)
>  - __nonnull, __wur, __THROW, and  __fortified_attr_access decorations
>   (Florian)
>  - change long to long int (Florian)
>  - Fix comment formatting (Florian)
>  - Update backup kernel header content copy.
>  - Fix function declaration formatting (Florian)
>  - Changed export versions to 2.38
>  - Fixed comment style (Florian)
>
> Changes in v3:
>  - Update argument types to match v4 kernel interface
>  - Add the "return" to the vsyscall
>  - Fix up vdso arg types to match kernel v4 version
>  - Remove ifdef around INLINE_VSYSCALL (Adhemerval)
>  - Word align dest for large memcpy()s.
>  - Add tags
>  - Remove spurious blank line from sysdeps/riscv/memcpy.c
>
> Changes in v2:
>  - hwprobe.h: Use __has_include and duplicate Linux content to make
>    compilation work when Linux headers are absent (Adhemerval)
>  - hwprobe.h: Put declaration under __USE_GNU (Adhemerval)
>  - Use INLINE_SYSCALL_CALL (Adhemerval)
>  - Update versions
>  - Update UNALIGNED_MASK to match kernel v3 series.
>  - Add vDSO interface
>  - Used _MASK instead of _FAST value itself.
>
> Evan Green (4):
>   riscv: Add Linux hwprobe syscall support
>   riscv: Add hwprobe vdso call support
>   riscv: Add ifunc-compatible hwprobe function
>   riscv: Add and use alignment-ignorant memcpy
>
>  sysdeps/riscv/memcopy.h                       |  26 ++++
>  sysdeps/riscv/memcpy.c                        |  64 +++++++++
>  sysdeps/riscv/memcpy_noalignment.S            | 134 ++++++++++++++++++
>  sysdeps/unix/sysv/linux/dl-vdso-setup.c       |  10 ++
>  sysdeps/unix/sysv/linux/dl-vdso-setup.h       |   3 +
>  sysdeps/unix/sysv/linux/riscv/Makefile        |   9 +-
>  sysdeps/unix/sysv/linux/riscv/Versions        |   4 +
>  sysdeps/unix/sysv/linux/riscv/hwprobe.c       |  33 +++++
>  .../unix/sysv/linux/riscv/hwprobe_static.c    |  36 +++++
>  .../unix/sysv/linux/riscv/memcpy-generic.c    |  24 ++++
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |   2 +
>  .../unix/sysv/linux/riscv/rv64/libc.abilist   |   2 +
>  sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h   |  93 ++++++++++++
>  sysdeps/unix/sysv/linux/riscv/sysdep.h        |   1 +
>  14 files changed, 439 insertions(+), 2 deletions(-)
>  create mode 100644 sysdeps/riscv/memcopy.h
>  create mode 100644 sysdeps/riscv/memcpy.c
>  create mode 100644 sysdeps/riscv/memcpy_noalignment.S
>  create mode 100644 sysdeps/unix/sysv/linux/riscv/hwprobe.c
>  create mode 100644 sysdeps/unix/sysv/linux/riscv/hwprobe_static.c
>  create mode 100644 sysdeps/unix/sysv/linux/riscv/memcpy-generic.c
>  create mode 100644 sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
>
> --
> 2.34.1
>

Re: [PATCH v5 3/4] riscv: Add ifunc-compatible hwprobe function

[Florian Weimer]

* Evan Green:

> diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> index b27af5cb07..d739371806 100644
> --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> @@ -67,6 +67,27 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
>       __fortified_attr_access (__read_write__, 1, 2)
>       __fortified_attr_access (__read_only__, 4, 3);
>  
> +extern int __riscv_hwprobe_weak (struct riscv_hwprobe *pairs, size_t pair_count,
> +				 size_t cpu_count, unsigned long int *cpus,
> +				 unsigned int flags)
> +     __THROW __nonnull ((1)) __wur
> +     __fortified_attr_access (__read_write__, 1, 2)
> +     __fortified_attr_access (__read_only__, 4, 3);
> +
> +# ifdef weak_extern
> +weak_extern (__riscv_hwprobe_weak)
> +# else
> +#  pragma weak __riscv_hwprobe_weak
> +# endif
> +
> +extern int __riscv_hwprobe_early (struct riscv_hwprobe *pairs, size_t pair_count,
> +				  size_t cpu_count, unsigned long int *cpus,
> +				  unsigned int flags)
> +     __THROW __nonnull ((1)) __wur
> +     __fortified_attr_access (__read_write__, 1, 2)
> +     __fortified_attr_access (__read_only__, 4, 3)
> +     __attribute__ ((__visibility__ ("hidden")));
> +
>  __END_DECLS
>  
>  #endif /* sys/hwprobe.h */

I would call the dynamic symbol ___riscv_hwprobe, not
__riscv_hwprobe_weak.  The weak property is an implementation detail in
the C file that goes into libc_nonshared.  And ___riscv_hwprobe does not
need to be declared in an installed header, I think it's fine to offer
__riscv_hwprobe only.

Use of INTERNAL_SYSCALL is correct because without relocations, errno
will not work, but it needs to match across all implementations.  This
means using INTERNAL_VSYSCALL instead of INLINE_VSYSCALL.

There needs to be manual entry that documents the exact interface
conventions.  If the function returns 0 on success (no positive return
values), it may make sense to negate the result value that comes back
from the kernel because that matches what some pthread interfaces use.
We probabl use the -EINVAL convention on some external API (it's a big
library), but I think it's on the unusual side.

Thanks,
Florian

Re: [PATCH v5 3/4] riscv: Add ifunc-compatible hwprobe function

[Evan Green]

On Thu, Jul 13, 2023 at 12:07 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Evan Green:
>
> > diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> > index b27af5cb07..d739371806 100644
> > --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> > +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> > @@ -67,6 +67,27 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
> >       __fortified_attr_access (__read_write__, 1, 2)
> >       __fortified_attr_access (__read_only__, 4, 3);
> >
> > +extern int __riscv_hwprobe_weak (struct riscv_hwprobe *pairs, size_t pair_count,
> > +                              size_t cpu_count, unsigned long int *cpus,
> > +                              unsigned int flags)
> > +     __THROW __nonnull ((1)) __wur
> > +     __fortified_attr_access (__read_write__, 1, 2)
> > +     __fortified_attr_access (__read_only__, 4, 3);
> > +
> > +# ifdef weak_extern
> > +weak_extern (__riscv_hwprobe_weak)
> > +# else
> > +#  pragma weak __riscv_hwprobe_weak
> > +# endif
> > +
> > +extern int __riscv_hwprobe_early (struct riscv_hwprobe *pairs, size_t pair_count,
> > +                               size_t cpu_count, unsigned long int *cpus,
> > +                               unsigned int flags)
> > +     __THROW __nonnull ((1)) __wur
> > +     __fortified_attr_access (__read_write__, 1, 2)
> > +     __fortified_attr_access (__read_only__, 4, 3)
> > +     __attribute__ ((__visibility__ ("hidden")));
> > +
> >  __END_DECLS
> >
> >  #endif /* sys/hwprobe.h */
>
> I would call the dynamic symbol ___riscv_hwprobe, not
> __riscv_hwprobe_weak.  The weak property is an implementation detail in
> the C file that goes into libc_nonshared.  And ___riscv_hwprobe does not
> need to be declared in an installed header, I think it's fine to offer
> __riscv_hwprobe only.

Ok. Just to reiterate what I'm planning to export to users, I've got
__riscv_hwprobe() as a dynamic function from libc that non-ifunc
callers can use without the overhead of checking the weak symbol, as
well as __riscv_hwprobe_early() that ifunc selectors can use
pre-relocation. Based on your comment, I'll rename the
__riscv_hwprobe_weak alias to ___riscv_hwprobe.

>
> Use of INTERNAL_SYSCALL is correct because without relocations, errno
> will not work, but it needs to match across all implementations.  This
> means using INTERNAL_VSYSCALL instead of INLINE_VSYSCALL.

Oh yeah, otherwise the return values of __riscv_hwprobe_early() would
suddenly change from -EINVAL to -1 once the symbol is available. So
then we'll document errno is always left unchanged by this function.

>
> There needs to be manual entry that documents the exact interface
> conventions.  If the function returns 0 on success (no positive return
> values), it may make sense to negate the result value that comes back
> from the kernel because that matches what some pthread interfaces use.
> We probabl use the -EINVAL convention on some external API (it's a big
> library), but I think it's on the unusual side.

Correct, it's 0 on success or negative error code on failure. I guess
you're binning it in with pthreads because those functions also return
error codes directly? I can do that.

After discussing with Palmer, based on the timing of things we're
thinking it's probably too late to try and rush this into the upcoming
release. There was also a suggestion internally to try and get to the
vDSO pointer early, though I don't understand how/if this is possible.
I need to dig more on that one. I may still spin this today if I can,
but we'll see.

-Evan

Re: [PATCH v5 3/4] riscv: Add ifunc-compatible hwprobe function

[Palmer Dabbelt]

On Thu, 13 Jul 2023 09:33:20 PDT (-0700), Evan Green wrote:
> On Thu, Jul 13, 2023 at 12:07 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Evan Green:
>>
>> > diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
>> > index b27af5cb07..d739371806 100644
>> > --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
>> > +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
>> > @@ -67,6 +67,27 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
>> >       __fortified_attr_access (__read_write__, 1, 2)
>> >       __fortified_attr_access (__read_only__, 4, 3);
>> >
>> > +extern int __riscv_hwprobe_weak (struct riscv_hwprobe *pairs, size_t pair_count,
>> > +                              size_t cpu_count, unsigned long int *cpus,
>> > +                              unsigned int flags)
>> > +     __THROW __nonnull ((1)) __wur
>> > +     __fortified_attr_access (__read_write__, 1, 2)
>> > +     __fortified_attr_access (__read_only__, 4, 3);
>> > +
>> > +# ifdef weak_extern
>> > +weak_extern (__riscv_hwprobe_weak)
>> > +# else
>> > +#  pragma weak __riscv_hwprobe_weak
>> > +# endif
>> > +
>> > +extern int __riscv_hwprobe_early (struct riscv_hwprobe *pairs, size_t pair_count,
>> > +                               size_t cpu_count, unsigned long int *cpus,
>> > +                               unsigned int flags)
>> > +     __THROW __nonnull ((1)) __wur
>> > +     __fortified_attr_access (__read_write__, 1, 2)
>> > +     __fortified_attr_access (__read_only__, 4, 3)
>> > +     __attribute__ ((__visibility__ ("hidden")));
>> > +
>> >  __END_DECLS
>> >
>> >  #endif /* sys/hwprobe.h */
>>
>> I would call the dynamic symbol ___riscv_hwprobe, not
>> __riscv_hwprobe_weak.  The weak property is an implementation detail in
>> the C file that goes into libc_nonshared.  And ___riscv_hwprobe does not
>> need to be declared in an installed header, I think it's fine to offer
>> __riscv_hwprobe only.
>
> Ok. Just to reiterate what I'm planning to export to users, I've got
> __riscv_hwprobe() as a dynamic function from libc that non-ifunc
> callers can use without the overhead of checking the weak symbol, as
> well as __riscv_hwprobe_early() that ifunc selectors can use
> pre-relocation. Based on your comment, I'll rename the
> __riscv_hwprobe_weak alias to ___riscv_hwprobe.
>
>>
>> Use of INTERNAL_SYSCALL is correct because without relocations, errno
>> will not work, but it needs to match across all implementations.  This
>> means using INTERNAL_VSYSCALL instead of INLINE_VSYSCALL.
>
> Oh yeah, otherwise the return values of __riscv_hwprobe_early() would
> suddenly change from -EINVAL to -1 once the symbol is available. So
> then we'll document errno is always left unchanged by this function.
>
>>
>> There needs to be manual entry that documents the exact interface
>> conventions.  If the function returns 0 on success (no positive return
>> values), it may make sense to negate the result value that comes back
>> from the kernel because that matches what some pthread interfaces use.
>> We probabl use the -EINVAL convention on some external API (it's a big
>> library), but I think it's on the unusual side.
>
> Correct, it's 0 on success or negative error code on failure. I guess
> you're binning it in with pthreads because those functions also return
> error codes directly? I can do that.
>
> After discussing with Palmer, based on the timing of things we're
> thinking it's probably too late to try and rush this into the upcoming
> release. There was also a suggestion internally to try and get to the
> vDSO pointer early, though I don't understand how/if this is possible.
> I need to dig more on that one. I may still spin this today if I can,
> but we'll see.

Ya, thanks.  Given how late we are in the cycle I think it's best to 
avoid rushing in any new ABI here, it's just too risky.

The vDSO idea would be to pre-populate HWCAP2 with a pointer to the full 
output of a riscv_hwprobe() run on all harts.  We'd store that data in 
the vDSO, but IIUC it'd fix the symbol lookup problem as it'd come in 
via the auxvec so we wouldn't need to poke around for symbols.  We 
probably need to think through some extensibility issues as well, but I 
think that doesn't change the core of the IFUNC discussion.

We'd talked about this before, but it was just in the context of 
performance (it's essentially a pre-cached version of the syscall).  Had 
I realized all these IFUNC-related symbol lookup headaches we probably 
would have done it the first time.  We probably would have ended up with 
it anyway, though, so it's not super scary.

The syscall would still be useful, both because it's sometimes easier to 
get at than HWCAP2 and because we can handle the more complex cases like 
heterogenous systems.  It'll probably still be worth allowing IFUNC 
resolvers to plumb into the syscall, but if we can make life easier for 
users who just want the simple case that seems valuable too.

> -Evan

Re: [PATCH v5 3/4] riscv: Add ifunc-compatible hwprobe function

[Evan Green]

On Thu, Jul 13, 2023 at 9:47 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Thu, 13 Jul 2023 09:33:20 PDT (-0700), Evan Green wrote:
> > On Thu, Jul 13, 2023 at 12:07 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Evan Green:
> >>
> >> > diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> >> > index b27af5cb07..d739371806 100644
> >> > --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> >> > +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> >> > @@ -67,6 +67,27 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
> >> >       __fortified_attr_access (__read_write__, 1, 2)
> >> >       __fortified_attr_access (__read_only__, 4, 3);
> >> >
> >> > +extern int __riscv_hwprobe_weak (struct riscv_hwprobe *pairs, size_t pair_count,
> >> > +                              size_t cpu_count, unsigned long int *cpus,
> >> > +                              unsigned int flags)
> >> > +     __THROW __nonnull ((1)) __wur
> >> > +     __fortified_attr_access (__read_write__, 1, 2)
> >> > +     __fortified_attr_access (__read_only__, 4, 3);
> >> > +
> >> > +# ifdef weak_extern
> >> > +weak_extern (__riscv_hwprobe_weak)
> >> > +# else
> >> > +#  pragma weak __riscv_hwprobe_weak
> >> > +# endif
> >> > +
> >> > +extern int __riscv_hwprobe_early (struct riscv_hwprobe *pairs, size_t pair_count,
> >> > +                               size_t cpu_count, unsigned long int *cpus,
> >> > +                               unsigned int flags)
> >> > +     __THROW __nonnull ((1)) __wur
> >> > +     __fortified_attr_access (__read_write__, 1, 2)
> >> > +     __fortified_attr_access (__read_only__, 4, 3)
> >> > +     __attribute__ ((__visibility__ ("hidden")));
> >> > +
> >> >  __END_DECLS
> >> >
> >> >  #endif /* sys/hwprobe.h */
> >>
> >> I would call the dynamic symbol ___riscv_hwprobe, not
> >> __riscv_hwprobe_weak.  The weak property is an implementation detail in
> >> the C file that goes into libc_nonshared.  And ___riscv_hwprobe does not
> >> need to be declared in an installed header, I think it's fine to offer
> >> __riscv_hwprobe only.
> >
> > Ok. Just to reiterate what I'm planning to export to users, I've got
> > __riscv_hwprobe() as a dynamic function from libc that non-ifunc
> > callers can use without the overhead of checking the weak symbol, as
> > well as __riscv_hwprobe_early() that ifunc selectors can use
> > pre-relocation. Based on your comment, I'll rename the
> > __riscv_hwprobe_weak alias to ___riscv_hwprobe.
> >
> >>
> >> Use of INTERNAL_SYSCALL is correct because without relocations, errno
> >> will not work, but it needs to match across all implementations.  This
> >> means using INTERNAL_VSYSCALL instead of INLINE_VSYSCALL.
> >
> > Oh yeah, otherwise the return values of __riscv_hwprobe_early() would
> > suddenly change from -EINVAL to -1 once the symbol is available. So
> > then we'll document errno is always left unchanged by this function.
> >
> >>
> >> There needs to be manual entry that documents the exact interface
> >> conventions.  If the function returns 0 on success (no positive return
> >> values), it may make sense to negate the result value that comes back
> >> from the kernel because that matches what some pthread interfaces use.
> >> We probabl use the -EINVAL convention on some external API (it's a big
> >> library), but I think it's on the unusual side.
> >
> > Correct, it's 0 on success or negative error code on failure. I guess
> > you're binning it in with pthreads because those functions also return
> > error codes directly? I can do that.
> >
> > After discussing with Palmer, based on the timing of things we're
> > thinking it's probably too late to try and rush this into the upcoming
> > release. There was also a suggestion internally to try and get to the
> > vDSO pointer early, though I don't understand how/if this is possible.
> > I need to dig more on that one. I may still spin this today if I can,
> > but we'll see.
>
> Ya, thanks.  Given how late we are in the cycle I think it's best to
> avoid rushing in any new ABI here, it's just too risky.
>
> The vDSO idea would be to pre-populate HWCAP2 with a pointer to the full
> output of a riscv_hwprobe() run on all harts.  We'd store that data in
> the vDSO, but IIUC it'd fix the symbol lookup problem as it'd come in
> via the auxvec so we wouldn't need to poke around for symbols.  We
> probably need to think through some extensibility issues as well, but I
> think that doesn't change the core of the IFUNC discussion.

Palmer and I chatted a bit more about this idea. The evolution of it
was to define something like AT_RISCV_HWPROBE_FUNC whose value is a
pointer to the vDSO function. The __riscv_hwprobe_early() in
libc_nonshared can then just get the auxval and call it, with similar
semantics as we have here of not setting errno, etc.

Florian, one question about your other possible solution suggestion,
reproduced below:

> You could pass the function pointer to the IFUNC resolver
> (which may require a marker symbol and GCC changes).

I'm unsure what a marker symbol is, so I'm having trouble following
this suggestion. I'd like to understand what that suggestion was,
especially if you think it's better than what we're planning above.

-Evan

Re: [PATCH v5 3/4] riscv: Add ifunc-compatible hwprobe function

[Florian Weimer]

* Evan Green:

> Palmer and I chatted a bit more about this idea. The evolution of it
> was to define something like AT_RISCV_HWPROBE_FUNC whose value is a
> pointer to the vDSO function. The __riscv_hwprobe_early() in
> libc_nonshared can then just get the auxval and call it, with similar
> semantics as we have here of not setting errno, etc.

I don't think it can access the auxiliary vector without relocations.

> Florian, one question about your other possible solution suggestion,
> reproduced below:
>
>> You could pass the function pointer to the IFUNC resolver
>> (which may require a marker symbol and GCC changes).
>
> I'm unsure what a marker symbol is, so I'm having trouble following
> this suggestion. I'd like to understand what that suggestion was,
> especially if you think it's better than what we're planning above.

POWER does this to signal that a binary needs data in the TCB for use by
IFUNC resolvers:

/* Newer LIBCs explicitly export this symbol to declare that they provide
   the AT_PLATFORM and AT_HWCAP/AT_HWCAP2 values in the TCB.  We emit a
   reference to this symbol whenever we expand a CPU builtin, so that
   we never link against an old LIBC.  */
const char *tcb_verification_symbol = "__parse_hwcap_and_convert_at_platform";

You are already passing an additional unused argument to IFUNC
resolvers, though:

static inline ElfW(Addr)
__attribute ((always_inline))
elf_ifunc_invoke (ElfW(Addr) addr)
{
  /* The second argument is a void pointer to preserve the extension
     fexibility.  */
  return ((ElfW(Addr) (*) (uint64_t, void *)) (addr))
         (GLRO(dl_hwcap), NULL);
}

So you could pass something there (like the address of __riscv_hwprobe),
and add a third argument (NULL for now) for future extensions.

This change does not even have ABI impact, applications just need to
remember to check the function pointer for NULL.  I haven't check if GCC
changes are needed before the compiler accepts the additional argument
in IFUNC resolvers.

Thanks,
Florian

Re: [PATCH v5 1/4] riscv: Add Linux hwprobe syscall support

[Joseph Myers]

On Wed, 12 Jul 2023, Evan Green wrote:

> +extern int __riscv_hwprobe (struct riscv_hwprobe *pairs, size_t pair_count,
> +			    size_t cpu_count, unsigned long int *cpus,
> +			    unsigned int flags)

In an installed header, I'd expect leading __ on parameter names in 
function declarations, so that you don't take those names from the user's 
namespace for macros unnecessarily.

-- 
Joseph S. Myers
joseph@codesourcery.com