Re: [PATCH 2/2] Fixup for 3279be36b671 ("powerpc/vdso: Wire up getrandom() vDSO implementation on VDSO32")

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 06/09/2024 à 16:46, Jason A. Donenfeld a écrit :
> On Fri, Sep 06, 2024 at 04:26:32PM +0200, Christophe Leroy wrote:
> 
>> On the long run I wonder if we should try to find a more generic
>> solution for getrandom instead of requiring each architecture to handle
>> it. On gettimeofday the selection of the right page is embeded in the
>> generic part, see for instance :
>>
>> static __maybe_unused __kernel_old_time_t
>> __cvdso_time_data(const struct vdso_data *vd, __kernel_old_time_t *time)
>> {
>> 	__kernel_old_time_t t;
>>
>> 	if (IS_ENABLED(CONFIG_TIME_NS) &&
>> 	    vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
>> 		vd = __arch_get_timens_vdso_data(vd);
>>
>> 	t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec);
>>
>> 	if (time)
>> 		*time = t;
>>
>> 	return t;
>> }
>>
>> and powerpc just provides:
>>
>> static __always_inline
>> const struct vdso_data *__arch_get_timens_vdso_data(const struct
>> vdso_data *vd)
>> {
>> 	return (void *)vd + (1U << CONFIG_PAGE_SHIFT);
>> }
> 
> It's tempting, but maybe a bit tricky. LoongArch, for example, doesn't
> have this problem at all, because the layout of their vvars doesn't
> require it. So the vd->clock_mode access is unnecessary.
> 
>> Or another solution could be to put random data in a third page that is
>> always at the same place regardless of timens ?
> 
> Maybe that's the easier way, yea. Potentially wasteful, though.
> 

Indeed I just looked at Loongarch and that's exactly what they do: they 
have a third page after the two pages dedicated to TIME for arch 
specific data, and they have added getrandom data there.

The third page is common to every process so it won't waste more than a 
few bytes. It doesn't worry me even on the older boards that only have 
32 Mbytes of RAM.

So yes, I may have a look at that in the future, what we have at the 
moment is good enough to move forward.

Christophe