| Message ID | 20240901061315.15693-2-xry111@xry111.site (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | LoongArch: Implement getrandom() in vDSO | expand |
Hi, Ruoyao, On Sun, Sep 1, 2024 at 2:13 PM Xi Ruoyao <xry111@xry111.site> wrote: > > Without a prototype, we'll have to add a prototype for each architecture > implementing vDSO getrandom. As most architectures will likely have the > vDSO getrandom implemented in a near future, and we'd like to keep the > declarations compatible everywhere (to ease the Glibc work), we should > really just have one copy of the prototype. > > Suggested-by: Huacai Chen <chenhuacai@kernel.org> > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > --- > arch/x86/entry/vdso/vgetrandom.c | 2 -- > include/vdso/getrandom.h | 5 +++++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c > index 52d3c7faae2e..430862b8977c 100644 > --- a/arch/x86/entry/vdso/vgetrandom.c > +++ b/arch/x86/entry/vdso/vgetrandom.c > @@ -6,8 +6,6 @@ > > #include "../../../../lib/vdso/getrandom.c" > > -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > - > ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) > { > return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); > diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h > index 4cf02e678f5e..08b47b002bf7 100644 > --- a/include/vdso/getrandom.h > +++ b/include/vdso/getrandom.h > @@ -56,4 +56,9 @@ struct vgetrandom_state { > */ > extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); > > +/** Though in this file there are already comments beginning with /**, but it seems the kernel's code style suggests beginning with /*. Huacai > + * __vdso_getrandom: Prototype of vDSO getrandom. > + */ > +extern ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > + > #endif /* _VDSO_GETRANDOM_H */ > -- > 2.46.0 >
On Sun, Sep 01, 2024 at 04:44:40PM +0800, Huacai Chen wrote: > Hi, Ruoyao, > > On Sun, Sep 1, 2024 at 2:13 PM Xi Ruoyao <xry111@xry111.site> wrote: > > > > Without a prototype, we'll have to add a prototype for each architecture > > implementing vDSO getrandom. As most architectures will likely have the > > vDSO getrandom implemented in a near future, and we'd like to keep the > > declarations compatible everywhere (to ease the Glibc work), we should > > really just have one copy of the prototype. > > > > Suggested-by: Huacai Chen <chenhuacai@kernel.org> > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > > --- > > arch/x86/entry/vdso/vgetrandom.c | 2 -- > > include/vdso/getrandom.h | 5 +++++ > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c > > index 52d3c7faae2e..430862b8977c 100644 > > --- a/arch/x86/entry/vdso/vgetrandom.c > > +++ b/arch/x86/entry/vdso/vgetrandom.c > > @@ -6,8 +6,6 @@ > > > > #include "../../../../lib/vdso/getrandom.c" > > > > -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > > - > > ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) > > { > > return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); > > diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h > > index 4cf02e678f5e..08b47b002bf7 100644 > > --- a/include/vdso/getrandom.h > > +++ b/include/vdso/getrandom.h > > @@ -56,4 +56,9 @@ struct vgetrandom_state { > > */ > > extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); > > > > +/** > Though in this file there are already comments beginning with /**, but > it seems the kernel's code style suggests beginning with /*. /** is for docbook comments.
On Sun, Sep 01, 2024 at 02:13:10PM +0800, Xi Ruoyao wrote: > Without a prototype, we'll have to add a prototype for each architecture > implementing vDSO getrandom. As most architectures will likely have the > vDSO getrandom implemented in a near future, and we'd like to keep the > declarations compatible everywhere (to ease the Glibc work), we should > really just have one copy of the prototype. > > Suggested-by: Huacai Chen <chenhuacai@kernel.org> > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > --- > arch/x86/entry/vdso/vgetrandom.c | 2 -- > include/vdso/getrandom.h | 5 +++++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c > index 52d3c7faae2e..430862b8977c 100644 > --- a/arch/x86/entry/vdso/vgetrandom.c > +++ b/arch/x86/entry/vdso/vgetrandom.c > @@ -6,8 +6,6 @@ > > #include "../../../../lib/vdso/getrandom.c" > > -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > - > ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) > { > return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); > diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h > index 4cf02e678f5e..08b47b002bf7 100644 > --- a/include/vdso/getrandom.h > +++ b/include/vdso/getrandom.h > @@ -56,4 +56,9 @@ struct vgetrandom_state { > */ > extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); > > +/** > + * __vdso_getrandom: Prototype of vDSO getrandom. > + */ > +extern ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > + My inclination was that this isn't correct because arm64 uses __kernel_* symbols instead, but it seems like arm handles that on their own, and include/vdso/gettime.h follows this same format. So I'll queue this up, possibly fixing up the comment.
On Sun, Sep 01, 2024 at 03:23:39PM +0200, Jason A. Donenfeld wrote: > On Sun, Sep 01, 2024 at 04:44:40PM +0800, Huacai Chen wrote: > > Hi, Ruoyao, > > > > On Sun, Sep 1, 2024 at 2:13 PM Xi Ruoyao <xry111@xry111.site> wrote: > > > > > > Without a prototype, we'll have to add a prototype for each architecture > > > implementing vDSO getrandom. As most architectures will likely have the > > > vDSO getrandom implemented in a near future, and we'd like to keep the > > > declarations compatible everywhere (to ease the Glibc work), we should > > > really just have one copy of the prototype. > > > > > > Suggested-by: Huacai Chen <chenhuacai@kernel.org> > > > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > > > --- > > > arch/x86/entry/vdso/vgetrandom.c | 2 -- > > > include/vdso/getrandom.h | 5 +++++ > > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c > > > index 52d3c7faae2e..430862b8977c 100644 > > > --- a/arch/x86/entry/vdso/vgetrandom.c > > > +++ b/arch/x86/entry/vdso/vgetrandom.c > > > @@ -6,8 +6,6 @@ > > > > > > #include "../../../../lib/vdso/getrandom.c" > > > > > > -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > > > - > > > ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) > > > { > > > return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); > > > diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h > > > index 4cf02e678f5e..08b47b002bf7 100644 > > > --- a/include/vdso/getrandom.h > > > +++ b/include/vdso/getrandom.h > > > @@ -56,4 +56,9 @@ struct vgetrandom_state { > > > */ > > > extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); > > > > > > +/** > > Though in this file there are already comments beginning with /**, but > > it seems the kernel's code style suggests beginning with /*. > > /** is for docbook comments. I'll fix this commit up as follows: From de99263bbd61f5199ecbd7ce6cf57ede8bc42c84 Mon Sep 17 00:00:00 2001 From: Xi Ruoyao <xry111@xry111.site> Date: Sun, 1 Sep 2024 14:13:10 +0800 Subject: [PATCH] random: vDSO: add a __vdso_getrandom prototype for all architectures Without a prototype, we'll have to add a prototype for each architecture implementing vDSO getrandom. As most architectures will likely have the vDSO getrandom implemented in a near future, and we'd like to keep the declarations compatible everywhere (to ease the libc implementor work), we should really just have one copy of the prototype. This also is what's already done inside of include/vdso/gettime.h for those vDSO functions, so this continues that convention. Suggested-by: Huacai Chen <chenhuacai@kernel.org> Signed-off-by: Xi Ruoyao <xry111@xry111.site> Acked-by: Huacai Chen <chenhuacai@kernel.org> [Jason: rewrite docbook comment for prototype.] Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- arch/x86/entry/vdso/vgetrandom.c | 2 -- include/vdso/getrandom.h | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c index 52d3c7faae2e..430862b8977c 100644 --- a/arch/x86/entry/vdso/vgetrandom.c +++ b/arch/x86/entry/vdso/vgetrandom.c @@ -6,8 +6,6 @@ #include "../../../../lib/vdso/getrandom.c" -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); - ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) { return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h index 4cf02e678f5e..0d3849145a89 100644 --- a/include/vdso/getrandom.h +++ b/include/vdso/getrandom.h @@ -56,4 +56,20 @@ struct vgetrandom_state { */ extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); +/** + * __vdso_getrandom - Architecture-specific vDSO implementation of getrandom() syscall. + * @buffer: Passed to __cvdso_getrandom_data(). + * @len: Passed to __cvdso_getrandom_data(). + * @flags: Passed to __cvdso_getrandom_data(). + * @opaque_state: Passed to __cvdso_getrandom_data(). + * @opaque_len: Passed to __cvdso_getrandom_data(); + * + * This function acquires a pointer to the architecture-specific shared vDSO RNG datapage, and + * passes it, along with all of its arguments, to __cvdso_getrandom_data(), whose documentation may + * be consulted for more information. + * + * Returns: The value of __cvdso_getrandom_data(). + */ +extern ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); + #endif /* _VDSO_GETRANDOM_H */ -- 2.46.0
On Sun, Sep 01, 2024 at 03:39:52PM +0200, Jason A. Donenfeld wrote: > > > Though in this file there are already comments beginning with /**, but > > > it seems the kernel's code style suggests beginning with /*. > > > > /** is for docbook comments. > > I'll fix this commit up as follows: Sorry, I was confused. Better: From 6839d0a67e1c4d58980977d99d1b86b4649be4e7 Mon Sep 17 00:00:00 2001 From: Xi Ruoyao <xry111@xry111.site> Date: Sun, 1 Sep 2024 14:13:10 +0800 Subject: [PATCH] random: vDSO: add a __vdso_getrandom prototype for all architectures Without a prototype, we'll have to add a prototype for each architecture implementing vDSO getrandom. As most architectures will likely have the vDSO getrandom implemented in a near future, and we'd like to keep the declarations compatible everywhere (to ease the libc implementor work), we should really just have one copy of the prototype. This also is what's already done inside of include/vdso/gettime.h for those vDSO functions, so this continues that convention. Suggested-by: Huacai Chen <chenhuacai@kernel.org> Signed-off-by: Xi Ruoyao <xry111@xry111.site> Acked-by: Huacai Chen <chenhuacai@kernel.org> [Jason: rewrite docbook comment for prototype.] Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- arch/x86/entry/vdso/vgetrandom.c | 2 -- include/vdso/getrandom.h | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c index 52d3c7faae2e..430862b8977c 100644 --- a/arch/x86/entry/vdso/vgetrandom.c +++ b/arch/x86/entry/vdso/vgetrandom.c @@ -6,8 +6,6 @@ #include "../../../../lib/vdso/getrandom.c" -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); - ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) { return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h index 4cf02e678f5e..6ca4d6de9e46 100644 --- a/include/vdso/getrandom.h +++ b/include/vdso/getrandom.h @@ -56,4 +56,19 @@ struct vgetrandom_state { */ extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); +/** + * __vdso_getrandom - Architecture-specific vDSO implementation of getrandom() syscall. + * @buffer: Passed to __cvdso_getrandom(). + * @len: Passed to __cvdso_getrandom(). + * @flags: Passed to __cvdso_getrandom(). + * @opaque_state: Passed to __cvdso_getrandom(). + * @opaque_len: Passed to __cvdso_getrandom(); + * + * This function is implemented by making a single call to to __cvdso_getrandom(), whose + * documentation may be consulted for more information. + * + * Returns: The return value of __cvdso_getrandom(). + */ +extern ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); + #endif /* _VDSO_GETRANDOM_H */ -- 2.46.0
Le 01/09/2024 à 08:13, Xi Ruoyao a écrit : > Without a prototype, we'll have to add a prototype for each architecture > implementing vDSO getrandom. As most architectures will likely have the > vDSO getrandom implemented in a near future, and we'd like to keep the > declarations compatible everywhere (to ease the Glibc work), we should > really just have one copy of the prototype. It is a good idea but it have to handle all architectures, not only half of them. If you look into vdso_config.h in selftests, you can see that there are two names: __kernel_getrandom is used on arm64, powerpc, s390, __vdso_getrandom is used on arm, mips, sparc, x86, riscv, loongarch Christophe > > Suggested-by: Huacai Chen <chenhuacai@kernel.org> > Signed-off-by: Xi Ruoyao <xry111@xry111.site> > --- > arch/x86/entry/vdso/vgetrandom.c | 2 -- > include/vdso/getrandom.h | 5 +++++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c > index 52d3c7faae2e..430862b8977c 100644 > --- a/arch/x86/entry/vdso/vgetrandom.c > +++ b/arch/x86/entry/vdso/vgetrandom.c > @@ -6,8 +6,6 @@ > > #include "../../../../lib/vdso/getrandom.c" > > -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > - > ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) > { > return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); > diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h > index 4cf02e678f5e..08b47b002bf7 100644 > --- a/include/vdso/getrandom.h > +++ b/include/vdso/getrandom.h > @@ -56,4 +56,9 @@ struct vgetrandom_state { > */ > extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); > > +/** > + * __vdso_getrandom: Prototype of vDSO getrandom. > + */ > +extern ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); > + > #endif /* _VDSO_GETRANDOM_H */
On Sun, Sep 01, 2024 at 08:06:51PM +0200, Christophe Leroy wrote: > > > Le 01/09/2024 à 08:13, Xi Ruoyao a écrit : > > Without a prototype, we'll have to add a prototype for each architecture > > implementing vDSO getrandom. As most architectures will likely have the > > vDSO getrandom implemented in a near future, and we'd like to keep the > > declarations compatible everywhere (to ease the Glibc work), we should > > really just have one copy of the prototype. > > It is a good idea but it have to handle all architectures, not only half > of them. If you look into vdso_config.h in selftests, you can see that > there are two names: > > __kernel_getrandom is used on arm64, powerpc, s390, > > __vdso_getrandom is used on arm, mips, sparc, x86, riscv, loongarch I thought about this too, but actually it looks like the __vdso_* ones are already being handled this way, while the __kernel_* ones have their own special thing going on. See include/vdso/gettime.h. So this patch makes __vdso_getrandom() just like the other ones in gettime.h, which is fine with me.
diff --git a/arch/x86/entry/vdso/vgetrandom.c b/arch/x86/entry/vdso/vgetrandom.c index 52d3c7faae2e..430862b8977c 100644 --- a/arch/x86/entry/vdso/vgetrandom.c +++ b/arch/x86/entry/vdso/vgetrandom.c @@ -6,8 +6,6 @@ #include "../../../../lib/vdso/getrandom.c" -ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); - ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len) { return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len); diff --git a/include/vdso/getrandom.h b/include/vdso/getrandom.h index 4cf02e678f5e..08b47b002bf7 100644 --- a/include/vdso/getrandom.h +++ b/include/vdso/getrandom.h @@ -56,4 +56,9 @@ struct vgetrandom_state { */ extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks); +/** + * __vdso_getrandom: Prototype of vDSO getrandom. + */ +extern ssize_t __vdso_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len); + #endif /* _VDSO_GETRANDOM_H */
Without a prototype, we'll have to add a prototype for each architecture implementing vDSO getrandom. As most architectures will likely have the vDSO getrandom implemented in a near future, and we'd like to keep the declarations compatible everywhere (to ease the Glibc work), we should really just have one copy of the prototype. Suggested-by: Huacai Chen <chenhuacai@kernel.org> Signed-off-by: Xi Ruoyao <xry111@xry111.site> --- arch/x86/entry/vdso/vgetrandom.c | 2 -- include/vdso/getrandom.h | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-)