Hi Conor,

On Wed, Dec 18, 2024 at 03:08:28PM +0000, Conor Dooley wrote:

On Mon, Dec 16, 2024 at 03:10:03PM +0100, Thomas Weißschuh wrote:

quoted

The generic storage implementation provides the same features as the
custom one. However it can be shared between architectures, making
maintenance easier.

Co-developed-by: Nam Cao <redacted>
Signed-off-by: Nam Cao <redacted>
Signed-off-by: Thomas Weißschuh <redacted>
---
 arch/riscv/Kconfig                                 |  3 +-
 arch/riscv/include/asm/vdso.h                      |  2 +-
 .../include/asm/vdso/{time_data.h => arch_data.h}  |  8 +-
 arch/riscv/include/asm/vdso/gettimeofday.h         | 14 +---
 arch/riscv/include/asm/vdso/vsyscall.h             |  9 ---
 arch/riscv/kernel/sys_hwprobe.c                    |  3 +-
 arch/riscv/kernel/vdso.c                           | 90 +---------------------
 arch/riscv/kernel/vdso/hwprobe.c                   |  6 +-
 arch/riscv/kernel/vdso/vdso.lds.S                  |  7 +-
 9 files changed, 18 insertions(+), 124 deletions(-)

Fails to build:
Patch 7/17: Test 1/12: .github/scripts/patches/tests/build_rv32_defconfig.sh
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:11:33: warning: declaration of 'struct riscv_hwprobe' will not be visible outside of this function [-Wvisibility]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:15:41: warning: declaration of 'struct riscv_hwprobe' will not be visible outside of this function [-Wvisibility]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:19:37: error: call to undeclared function '__arch_get_vdso_u_arch_data'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:19:31: error: incompatible integer to pointer conversion initializing 'const struct vdso_arch_data *' with an expression of type 'int' [-Wint-conversion]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:22:36: error: arithmetic on a pointer to an incomplete type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:30:40: error: incomplete definition of type 'const struct vdso_arch_data'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:31:24: error: incompatible pointer types passing 'struct riscv_hwprobe *' to parameter of type 'struct riscv_hwprobe *' [-Werror,-Wincompatible-pointer-types]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:35:7: error: call to undeclared function 'riscv_hwprobe_key_is_valid'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:35:35: error: incomplete definition of type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:36:5: error: incomplete definition of type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:36:18: error: incomplete definition of type 'const struct vdso_arch_data'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:36:44: error: incomplete definition of type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:39:5: error: incomplete definition of type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:40:5: error: incomplete definition of type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:43:4: error: arithmetic on a pointer to an incomplete type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:49:39: warning: declaration of 'struct riscv_hwprobe' will not be visible outside of this function [-Wvisibility]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:53:37: error: call to undeclared function '__arch_get_vdso_u_arch_data'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:53:31: error: incompatible integer to pointer conversion initializing 'const struct vdso_arch_data *' with an expression of type 'int' [-Wint-conversion]
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:55:36: error: arithmetic on a pointer to an incomplete type 'struct riscv_hwprobe'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:71:61: error: incomplete definition of type 'const struct vdso_arch_data'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:71:29: error: use of undeclared identifier 'RISCV_HWPROBE_WHICH_CPUS'
  /build/tmp.cAQOaMfDA3/arch/riscv/kernel/vdso/hwprobe.c:72:24: error: incompatible pointer types passing 'struct riscv_hwprobe *' to parameter of type 'struct riscv_hwprobe *' [-Werror,-Wincompatible-pointer-types]
  fatal error: too many errors emitted, stopping now [-ferror-limit=]

Might be a clang thing, allmodconfig with clang doesn't build either.

The proposed generic storage infrastructure currently expects that all
its users also use HAVE_GENERIC_VDSO.
I missed rv32 when checking this assumption.

I can add a bunch of ifdefs into the storage code to handle this.

Or we re-add the time vDSO functions which were removed in commit
d4c08b9776b3 ("riscv: Use latest system call ABI").
Today there are upstream ports of musl and glibc which can use them.
(currently musl even tries to use __vdso_clock_gettime() as 64-bit only
on rv32 due to a copy-and-paste error from its rv64 code)

There is precedence in providing 64bit only vDSO functions, for example
__vdso_clock_gettime64() in arm.
I do have a small, so far untested, proof-of-concept patch for it.
This would even be less code than the ifdefs.

What do you think about it?


Thomas