Hi Christophe,

On Wed, Dec 18, 2024 at 08:20:51AM +0100, Christophe Leroy wrote:

Le 16/12/2024 à 15:10, Thomas Weißschuh a écrit :

[..]

quoted

  #ifdef CONFIG_TIME_NS
-static __always_inline
-const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
+static __always_inline const struct vdso_time_data *__ppc_get_vdso_u_timens_data(void)
  {
-	return (void *)vd + (1U << CONFIG_PAGE_SHIFT);
+	struct vdso_time_data *time_data;
+
+	asm(
+		"	bcl	20, 31, .+4\n"
+		"0:	mflr	%0\n"
+		"	addis	%0, %0, (vdso_u_timens_data - 0b)@ha\n"
+		"	addi	%0, %0, (vdso_u_timens_data - 0b)@l\n"
+	: "=r" (time_data) :: "lr");
+
+	return time_data;

Please don't do that, it kills optimisation efforts done when implementing
VDSO time. Commit ce7d8056e38b ("powerpc/vdso: Prepare for switching VDSO to
generic C implementation.") explains why.

For time data, the bcl/mflr dance is done by get_datapage macro called by
cvdso_call macro in gettimeofday.S, and given to
__cvdso_clock_gettime_data() by __c_kernel_clock_gettime() in
vgettimeofday.c . Use that information and don't redo the bcl/mflr sequence.

So instead keeping the logic of this:

static __always_inline
const struct vdso_data *__arch_get_timens_vdso_data(const struct vdso_data *vd)
{
	return (void *)vd + (1U << CONFIG_PAGE_SHIFT);
}

Makes sense.

Adding a constant value should be cheaper or just as cheap as a
PC-relative addressing for all architectures, so it can go into the
generic code, too.

[..]

quoted

  }
+#define __arch_get_vdso_u_timens_data __ppc_get_vdso_u_timens_data

There is not #ifdef __arch_get_vdso_u_timens_data anywhere, this #define is
not needed, the function should be called __arch_get_vdso_u_timens_data()
directly as before, unnecessary indirections reduce readability.

I'll see how this works out with the include order and conflicts with
symbols in include/vdso/datapage.h.

quoted

  #endif
-static inline bool vdso_clocksource_ok(const struct vdso_data *vd)
+static inline bool vdso_clocksource_ok(const struct vdso_time_data *vd)
  {
  	return true;
  }

Thanks!
Thomas