Re: [PATCH 2/2] powerpc/vdso: Implement __arch_get_vdso_rng_data()

[Thomas Weißschuh]

On Wed, Oct 02, 2024 at 10:39:29AM +0200, Christophe Leroy wrote:
> VDSO time functions do not call any other function, so they don't
> need to save/restore LR. However, retrieving the address of VDSO data
> page requires using LR hence saving then restoring it, which can be
> heavy on some CPUs. On the other hand, VDSO functions on powerpc are
> not standard functions and require a wrapper function to call C VDSO
> functions. And that wrapper has to save and restore LR in order to
> call the C VDSO function, so retrieving VDSO data page address in that
> wrapper doesn't require additional save/restore of LR.
> 
> For random VDSO functions it is a bit different. Because the function
> calls __arch_chacha20_blocks_nostack(), it saves and restores LR.
> Retrieving VDSO data page address can then be done there without
> additional save/restore of LR.
> 
> So lets implement __arch_get_vdso_rng_data() and simplify the wrapper.
> 
> It starts paving the way for the day powerpc will implement a more
> standard ABI for VDSO functions.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> ---
>  arch/powerpc/include/asm/vdso/getrandom.h | 15 +++++++++++++--
>  arch/powerpc/kernel/asm-offsets.c         |  1 -
>  arch/powerpc/kernel/vdso/getrandom.S      |  1 -
>  arch/powerpc/kernel/vdso/vgetrandom.c     |  4 ++--
>  4 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/vdso/getrandom.h b/arch/powerpc/include/asm/vdso/getrandom.h
> index 501d6bb14e8a..4302e7c67aa5 100644
> --- a/arch/powerpc/include/asm/vdso/getrandom.h
> +++ b/arch/powerpc/include/asm/vdso/getrandom.h
> @@ -7,6 +7,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <asm/vdso_datapage.h>
> +
>  static __always_inline int do_syscall_3(const unsigned long _r0, const unsigned long _r3,
>  					const unsigned long _r4, const unsigned long _r5)
>  {
> @@ -43,11 +45,20 @@ static __always_inline ssize_t getrandom_syscall(void *buffer, size_t len, unsig
>  
>  static __always_inline struct vdso_rng_data *__arch_get_vdso_rng_data(void)
>  {
> -	return NULL;
> +	struct vdso_arch_data *data;
> +
> +	asm(
> +		"	bcl	20, 31, .+4\n"
> +		"0:	mflr	%0\n"
> +		"	addis	%0, %0, (_vdso_datapage - 0b)@ha\n"
> +		"	addi	%0, %0, (_vdso_datapage - 0b)@l\n"
> +	: "=r" (data) :: "lr");
> +
> +	return &data->rng_data;
>  }

Did you also try something like this:

extern struct vdso_arch_data _vdso_datapage __attribute__((visibility("hidden")));

static __always_inline struct vdso_rng_data *__arch_get_vdso_rng_data(void)
{
       return &_vdso_datapage.rng_data;
}

Not knowing much about ppc asm the resulting assembly looks simpler.
And it would be more in line with what other archs are doing.

> [..]