[v9,2/6] riscv: Add hwprobe vdso call support
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
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 v7)
Changes in v7:
- Use INTERNAL_VSYSCALL_CALL (Florian)
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 | 17 ++++++++++++++---
sysdeps/unix/sysv/linux/riscv/sysdep.h | 1 +
4 files changed, 28 insertions(+), 3 deletions(-)
Comments
On 30/11/23 15:32, Evan Green wrote:
> 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 v7)
>
> Changes in v7:
> - Use INTERNAL_VSYSCALL_CALL (Florian)
>
> 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 | 17 ++++++++++++++---
> sysdeps/unix/sysv/linux/riscv/sysdep.h | 1 +
> 4 files changed, 28 insertions(+), 3 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 e28194e344..6a9a44657f 100644
> --- a/sysdeps/unix/sysv/linux/riscv/hwprobe.c
> +++ b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
> @@ -27,9 +27,20 @@ int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count,
> unsigned int __flags)
> {
> int r;
> -
> - r = INTERNAL_SYSCALL_CALL (riscv_hwprobe, 5, __pairs, __pair_count,
> - __cpu_count, __cpus, __flags);
> + __riscv_hwprobe_t vdso_hwprobe =
> + (__riscv_hwprobe_t)GLRO(dl_vdso_riscv_hwprobe);
> +
> + if (vdso_hwprobe != NULL)
> + {
> + r = INTERNAL_VSYSCALL_CALL (vdso_hwprobe, 5, __pairs, __pair_count,
> + __cpu_count, __cpus, __flags);
> + }
> + else
> + {
> + r = INTERNAL_SYSCALL_CALL (riscv_hwprobe, 5, __pairs, __pair_count,
> + __cpu_count, __cpus, __flags);
> +
> + }
>
> /* Negate negative errno values to match pthreads API. */
> return -r;
Maybe add a INTERNAL_VSYSCALL macro instead, similar to the INLINE_VSYSCALL?
Something like:
#define INTERNAL_VSYSCALL(name, nr, args...) \
({ \
__label__ out; \
__label__ iserr; \
long int sc_ret; \
\
__typeof (GLRO(dl_vdso_##name)) vdsop = GLRO(dl_vdso_##name); \
if (vdsop != NULL) \
{ \
sc_ret = INTERNAL_VSYSCALL_CALL (vdsop, nr, ##args); \
if (!INTERNAL_SYSCALL_ERROR_P (sc_ret)) \
goto out; \
if (INTERNAL_SYSCALL_ERRNO (sc_ret) != ENOSYS) \
goto iserr; \
} \
\
sc_ret = INTERNAL_SYSCALL_CALL (name, ##args); \
if (INTERNAL_SYSCALL_ERROR_P (sc_ret)) \
{ \
iserr: \
sc_ret = -1L; \
} \
out: \
sc_ret; \
})
It might be redundant for the riscv case, but it would be helpful for some
archs that require either to fallback to syscall (it is just for some kernel
version on specific archs) or require some extra care to call the vDSO
(powerpc).
> 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
On Wed, Dec 6, 2023 at 9:34 AM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 30/11/23 15:32, Evan Green wrote:
> > 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 v7)
> >
> > Changes in v7:
> > - Use INTERNAL_VSYSCALL_CALL (Florian)
> >
> > 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 | 17 ++++++++++++++---
> > sysdeps/unix/sysv/linux/riscv/sysdep.h | 1 +
> > 4 files changed, 28 insertions(+), 3 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 e28194e344..6a9a44657f 100644
> > --- a/sysdeps/unix/sysv/linux/riscv/hwprobe.c
> > +++ b/sysdeps/unix/sysv/linux/riscv/hwprobe.c
> > @@ -27,9 +27,20 @@ int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count,
> > unsigned int __flags)
> > {
> > int r;
> > -
> > - r = INTERNAL_SYSCALL_CALL (riscv_hwprobe, 5, __pairs, __pair_count,
> > - __cpu_count, __cpus, __flags);
> > + __riscv_hwprobe_t vdso_hwprobe =
> > + (__riscv_hwprobe_t)GLRO(dl_vdso_riscv_hwprobe);
> > +
> > + if (vdso_hwprobe != NULL)
> > + {
> > + r = INTERNAL_VSYSCALL_CALL (vdso_hwprobe, 5, __pairs, __pair_count,
> > + __cpu_count, __cpus, __flags);
> > + }
> > + else
> > + {
> > + r = INTERNAL_SYSCALL_CALL (riscv_hwprobe, 5, __pairs, __pair_count,
> > + __cpu_count, __cpus, __flags);
> > +
> > + }
> >
> > /* Negate negative errno values to match pthreads API. */
> > return -r;
>
> Maybe add a INTERNAL_VSYSCALL macro instead, similar to the INLINE_VSYSCALL?
>
> Something like:
>
> #define INTERNAL_VSYSCALL(name, nr, args...) \
> ({ \
> __label__ out; \
> __label__ iserr; \
> long int sc_ret; \
> \
> __typeof (GLRO(dl_vdso_##name)) vdsop = GLRO(dl_vdso_##name); \
> if (vdsop != NULL) \
> { \
> sc_ret = INTERNAL_VSYSCALL_CALL (vdsop, nr, ##args); \
> if (!INTERNAL_SYSCALL_ERROR_P (sc_ret)) \
> goto out; \
> if (INTERNAL_SYSCALL_ERRNO (sc_ret) != ENOSYS) \
> goto iserr; \
> } \
> \
> sc_ret = INTERNAL_SYSCALL_CALL (name, ##args); \
> if (INTERNAL_SYSCALL_ERROR_P (sc_ret)) \
> { \
> iserr: \
> sc_ret = -1L; \
> } \
> out: \
> sc_ret; \
> })
>
> It might be redundant for the riscv case, but it would be helpful for some
> archs that require either to fallback to syscall (it is just for some kernel
> version on specific archs) or require some extra care to call the vDSO
> (powerpc).
>
I think I see what you're going for. As per Florian's suggestion, I'd
like to return the positive error value on failure though, so it's
better if the macro returns the errno rather than discarding it. How
about this:
diff --git a/sysdeps/unix/sysv/linux/sysdep-vdso.h
b/sysdeps/unix/sysv/linux/sysdep-vdso.h
index a51b5deb37..f63fcc510a 100644
--- a/sysdeps/unix/sysv/linux/sysdep-vdso.h
+++ b/sysdeps/unix/sysv/linux/sysdep-vdso.h
@@ -53,4 +53,24 @@
sc_ret; \
})
+#define INTERNAL_VSYSCALL(name, nr, args...) \
+ ({ \
+ __label__ out; \
+ __label__ iserr; \
+ long int sc_ret; \
+ \
+ __typeof (GLRO(dl_vdso_##name)) vdsop = GLRO(dl_vdso_##name); \
+ if (vdsop != NULL) \
+ {
\
+ sc_ret = INTERNAL_VSYSCALL_CALL (vdsop, nr, ##args); \
+ if ((!INTERNAL_SYSCALL_ERROR_P (sc_ret)) || \
+ (INTERNAL_SYSCALL_ERRNO (sc_ret) != ENOSYS)) \
+ goto out; \
+ }
\
+ \
+ sc_ret = INTERNAL_SYSCALL_CALL (name, ##args); \
+ out: \
+ sc_ret; \
+ })
+
#endif /* SYSDEP_VDSO_LINUX_H */
Holler if you see some changes you want (or are not a fan), otherwise
I think I've got the rest of your comments incorporated, and can send
out a v10 tomorrow.
-Evan
@@ -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
@@ -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
@@ -27,9 +27,20 @@ int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count,
unsigned int __flags)
{
int r;
-
- r = INTERNAL_SYSCALL_CALL (riscv_hwprobe, 5, __pairs, __pair_count,
- __cpu_count, __cpus, __flags);
+ __riscv_hwprobe_t vdso_hwprobe =
+ (__riscv_hwprobe_t)GLRO(dl_vdso_riscv_hwprobe);
+
+ if (vdso_hwprobe != NULL)
+ {
+ r = INTERNAL_VSYSCALL_CALL (vdso_hwprobe, 5, __pairs, __pair_count,
+ __cpu_count, __cpus, __flags);
+ }
+ else
+ {
+ r = INTERNAL_SYSCALL_CALL (riscv_hwprobe, 5, __pairs, __pair_count,
+ __cpu_count, __cpus, __flags);
+
+ }
/* Negate negative errno values to match pthreads API. */
return -r;
@@ -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