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

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 06/02/23 16:48, Evan Green wrote:
> 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.

Do you want to use this symbol on external projects, for instance to
implement builtin_cpu_supports (as powerpc does)? If so, it has the 
drawback that such feature will only work on glibc.

> 
> Signed-off-by: Evan Green <evan@rivosinc.com>
> ---
> 
>  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/arch-syscall.h |  1 +
>  .../unix/sysv/linux/riscv/rv32/libc.abilist   |  1 +
>  .../unix/sysv/linux/riscv/rv64/arch-syscall.h |  1 +
>  .../unix/sysv/linux/riscv/rv64/libc.abilist   |  1 +
>  sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h   | 34 +++++++++++++++++++
>  sysdeps/unix/sysv/linux/syscall-names.list    |  1 +
>  9 files changed, 74 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..891ae05730 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.37 {
> +    __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..ef6dccb9db
> --- /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 <asm/hwprobe.h>
> +#include <sysdep.h>
> +
> +int
> +__riscv_hwprobe (struct riscv_hwprobe *pairs, long pair_count,
> +  long cpu_count, unsigned long *cpus, unsigned long flags)
> +{
> +  return INLINE_SYSCALL (riscv_hwprobe, 5, pairs, pair_count,
> +                         cpu_count, cpus, flags);
> +}

Use INLINE_SYSCALL_CALL to avoid the need to specify the argument size.

> diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/arch-syscall.h b/sysdeps/unix/sysv/linux/riscv/rv32/arch-syscall.h
> index 202520ee25..2416e041c8 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv32/arch-syscall.h
> +++ b/sysdeps/unix/sysv/linux/riscv/rv32/arch-syscall.h
> @@ -198,6 +198,7 @@
>  #define __NR_request_key 218
>  #define __NR_restart_syscall 128
>  #define __NR_riscv_flush_icache 259
> +#define __NR_riscv_hwprobe 258
>  #define __NR_rseq 293
>  #define __NR_rt_sigaction 134
>  #define __NR_rt_sigpending 136
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> index ff90d1bff2..f4c391d3be 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv32/libc.abilist
> @@ -2396,3 +2396,4 @@ GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
>  GLIBC_2.36 process_madvise F
>  GLIBC_2.36 process_mrelease F
> +GLIBC_2.37 __riscv_hwprobe F
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/arch-syscall.h b/sysdeps/unix/sysv/linux/riscv/rv64/arch-syscall.h
> index 4e65f337d4..a32bc82f60 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv64/arch-syscall.h
> +++ b/sysdeps/unix/sysv/linux/riscv/rv64/arch-syscall.h
> @@ -205,6 +205,7 @@
>  #define __NR_request_key 218
>  #define __NR_restart_syscall 128
>  #define __NR_riscv_flush_icache 259
> +#define __NR_riscv_hwprobe 258
>  #define __NR_rseq 293
>  #define __NR_rt_sigaction 134
>  #define __NR_rt_sigpending 136
> diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> index f1017f6ec5..0f57bbe9e1 100644
> --- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> +++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
> @@ -2596,3 +2596,4 @@ GLIBC_2.36 pidfd_open F
>  GLIBC_2.36 pidfd_send_signal F
>  GLIBC_2.36 process_madvise F
>  GLIBC_2.36 process_mrelease F
> +GLIBC_2.37 __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..da8cdc90bf
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h
> @@ -0,0 +1,34 @@
> +/* 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 <asm/hwprobe.h>

Do we really want to tie its usage to the supplied kernel headers? It means that
if kernel headers is too old a program that includes sys/hwprobe.h will just
fail to compile, which is not ideal.

For user exported header the usual way is to use __has_include and redefine the
required fields (check sysdeps/unix/sysv/linux/bits/unistd_ext.h or grep for
__has_include).  It has the drawnback of require duplicate and constant sync 
kernel definition to exported header.

> +
> +__BEGIN_DECLS
> +
> +int
> +__riscv_hwprobe (struct riscv_hwprobe *pairs, long pair_count,
> +  long cpu_count, unsigned long *cpus, unsigned long flags);
> +
> +__END_DECLS
> +
> +#endif /* sys/hwprobe.h */


I think we should also only export it as a GNU extension (__USE_GNU).

> diff --git a/sysdeps/unix/sysv/linux/syscall-names.list b/sysdeps/unix/sysv/linux/syscall-names.list
> index 822498d3e3..4f4a62e91c 100644
> --- a/sysdeps/unix/sysv/linux/syscall-names.list
> +++ b/sysdeps/unix/sysv/linux/syscall-names.list
> @@ -477,6 +477,7 @@ renameat2
>  request_key
>  restart_syscall
>  riscv_flush_icache
> +riscv_hwprobe
>  rmdir
>  rseq
>  rt_sigaction