[PATCH bpf-next v6 0/4] bpf: add cpu cycles kfuncss

[PATCH bpf-next v6 0/4] bpf: add cpu cycles kfuncss

[Vadim Fedorenko]

This patchset adds 2 kfuncs to provide a way to precisely measure the
time spent running some code. The first patch provides a way to get cpu
cycles counter which is used to feed CLOCK_MONOTONIC_RAW. On x86
architecture it is effectively rdtsc_ordered() function while on other
architectures it falls back to __arch_get_hw_counter(). The second patch
adds a kfunc to convert cpu cycles to nanoseconds using shift/mult
constants discovered by kernel. JIT version is done for x86 for now, on
other architectures it falls back to slightly simplified version of
vdso_calc_ns.

Selftests are also added to check whether the JIT implementation is
correct and to show the simplest usage example.

Change log:
v5 -> v6:
* added cover letter
* add comment about dropping S64_MAX manipulation in jitted
  implementation of rdtsc_oredered (Alexey)
* add comment about using 'lfence;rdtsc' variant (Alexey)
* change the check in fixup_kfunc_call() (Eduard)
* make __arch_get_hw_counter() call more aligned with vDSO
  implementation (Yonghong)
v4 -> v5:
* use #if instead of #ifdef with IS_ENABLED
v3 -> v4:
* change name of the helper to bpf_get_cpu_cycles (Andrii)
* Hide the helper behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing
  it on architectures which do not have vDSO functions and data
* reduce the scope of check of inlined functions in verifier to only 2,
  which are actually inlined.
* change helper name to bpf_cpu_cycles_to_ns.
* hide it behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing on
  unsupported architectures.
v2 -> v3:
* change name of the helper to bpf_get_cpu_cycles_counter to
* explicitly mention what counter it provides (Andrii)
* move kfunc definition to bpf.h to use it in JIT.
* introduce another kfunc to convert cycles into nanoseconds as
* more meaningful time units for generic tracing use case (Andrii)
v1 -> v2:
* Fix incorrect function return value type to u64
* Introduce bpf_jit_inlines_kfunc_call() and use it in
	mark_fastcall_pattern_for_call() to avoid clobbering in case
	of running programs with no JIT (Eduard)
* Avoid rewriting instruction and check function pointer directly
	in JIT (Alexei)
* Change includes to fix compile issues on non x86 architectures

Vadim Fedorenko (4):
  bpf: add bpf_get_cpu_cycles kfunc
  bpf: add bpf_cpu_cycles_to_ns helper
  selftests/bpf: add selftest to check rdtsc jit
  selftests/bpf: add usage example for cpu cycles kfuncs

 arch/x86/net/bpf_jit_comp.c                   |  60 ++++++++++
 arch/x86/net/bpf_jit_comp32.c                 |  33 ++++++
 include/linux/bpf.h                           |   6 +
 include/linux/filter.h                        |   1 +
 kernel/bpf/core.c                             |  11 ++
 kernel/bpf/helpers.c                          |  32 ++++++
 kernel/bpf/verifier.c                         |  41 ++++++-
 .../bpf/prog_tests/test_cpu_cycles.c          |  35 ++++++
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/test_cpu_cycles.c     |  25 +++++
 .../selftests/bpf/progs/verifier_cpu_cycles.c | 104 ++++++++++++++++++
 11 files changed, 344 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_cpu_cycles.c
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c

-- 
2.43.5

[PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc

[Vadim Fedorenko]

New kfunc to return ARCH-specific timecounter. For x86 BPF JIT converts
it into rdtsc ordered call. Other architectures will get JIT
implementation too if supported. The fallback is to
__arch_get_hw_counter().

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
v5 -> v6:
* add comment about dropping S64_MAX manipulation in jitted
  implementation of rdtsc_oredered (Alexey)
* add comment about using 'lfence;rdtsc' variant (Alexey)
* change the check in fixup_kfunc_call() (Eduard)
* make __arch_get_hw_counter() call more aligned with vDSO
  implementation (Yonghong)
v4 -> v5:
* use if instead of ifdef with IS_ENABLED
v3 -> v4:
* change name of the helper to bpf_get_cpu_cycles (Andrii)
* Hide the helper behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing
  it on architectures which do not have vDSO functions and data
* reduce the scope of check of inlined functions in verifier to only 2,
  which are actually inlined.
v2 -> v3:
* change name of the helper to bpf_get_cpu_cycles_counter to explicitly
  mention what counter it provides (Andrii)
* move kfunc definition to bpf.h to use it in JIT.
* introduce another kfunc to convert cycles into nanoseconds as more
  meaningful time units for generic tracing use case (Andrii)
v1 -> v2:
* Fix incorrect function return value type to u64
* Introduce bpf_jit_inlines_kfunc_call() and use it in
  mark_fastcall_pattern_for_call() to avoid clobbering in case of
  running programs with no JIT (Eduard)
* Avoid rewriting instruction and check function pointer directly
  in JIT (Alexei)
* Change includes to fix compile issues on non x86 architectures
---
 arch/x86/net/bpf_jit_comp.c   | 39 +++++++++++++++++++++++++++++++++
 arch/x86/net/bpf_jit_comp32.c | 14 ++++++++++++
 include/linux/bpf.h           |  5 +++++
 include/linux/filter.h        |  1 +
 kernel/bpf/core.c             | 11 ++++++++++
 kernel/bpf/helpers.c          | 21 ++++++++++++++++++
 kernel/bpf/verifier.c         | 41 ++++++++++++++++++++++++++++++-----
 7 files changed, 126 insertions(+), 6 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index a43fc5af973d..107bd921f104 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2185,6 +2185,37 @@ st:			if (is_imm8(insn->off))
 		case BPF_JMP | BPF_CALL: {
 			u8 *ip = image + addrs[i - 1];
 
+			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
+			    imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
+				/* The default implementation of this kfunc uses
+				 * __arch_get_hw_counter() which is implemented as
+				 * `(u64)rdtsc_ordered() & S64_MAX`. We skip masking
+				 * part because we assume it's not needed in BPF
+				 * use case (two measurements close in time).
+				 * Original code for rdtsc_ordered() uses sequence:
+				 * 'rdtsc; nop; nop; nop' to patch it into
+				 * 'lfence; rdtsc' or 'rdtscp' depending on CPU features.
+				 * JIT uses 'lfence; rdtsc' variant because BPF program
+				 * doesn't care about cookie provided by rdtsp in RCX.
+				 * Save RDX because RDTSC will use EDX:EAX to return u64
+				 */
+				emit_mov_reg(&prog, true, AUX_REG, BPF_REG_3);
+				if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
+					EMIT_LFENCE();
+				EMIT2(0x0F, 0x31);
+
+				/* shl RDX, 32 */
+				maybe_emit_1mod(&prog, BPF_REG_3, true);
+				EMIT3(0xC1, add_1reg(0xE0, BPF_REG_3), 32);
+				/* or RAX, RDX */
+				maybe_emit_mod(&prog, BPF_REG_0, BPF_REG_3, true);
+				EMIT2(0x09, add_2reg(0xC0, BPF_REG_0, BPF_REG_3));
+				/* restore RDX from R11 */
+				emit_mov_reg(&prog, true, BPF_REG_3, AUX_REG);
+
+				break;
+			}
+
 			func = (u8 *) __bpf_call_base + imm32;
 			if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
 				LOAD_TAIL_CALL_CNT_PTR(stack_depth);
@@ -3791,3 +3822,11 @@ u64 bpf_arch_uaddress_limit(void)
 {
 	return 0;
 }
+
+/* x86-64 JIT can inline kfunc */
+bool bpf_jit_inlines_kfunc_call(s32 imm)
+{
+	if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
+		return true;
+	return false;
+}
diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index de0f9e5f9f73..e6097a371b69 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -2094,6 +2094,13 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
 				int err;
 
+				if (imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
+					if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
+						EMIT3(0x0F, 0xAE, 0xE8);
+					EMIT2(0x0F, 0x31);
+					break;
+				}
+
 				err = emit_kfunc_call(bpf_prog,
 						      image + addrs[i],
 						      insn, &prog);
@@ -2621,3 +2628,10 @@ bool bpf_jit_supports_kfunc_call(void)
 {
 	return true;
 }
+
+bool bpf_jit_inlines_kfunc_call(s32 imm)
+{
+	if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
+		return true;
+	return false;
+}
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3ace0d6227e3..43a5207a1591 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3333,6 +3333,11 @@ void bpf_user_rnd_init_once(void);
 u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 u64 bpf_get_raw_cpu_id(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 
+/* Inlined kfuncs */
+#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
+u64 bpf_get_cpu_cycles(void);
+#endif
+
 #if defined(CONFIG_NET)
 bool bpf_sock_common_is_valid_access(int off, int size,
 				     enum bpf_access_type type,
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 3a21947f2fd4..9cf57233874f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1111,6 +1111,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog);
 void bpf_jit_compile(struct bpf_prog *prog);
 bool bpf_jit_needs_zext(void);
 bool bpf_jit_inlines_helper_call(s32 imm);
+bool bpf_jit_inlines_kfunc_call(s32 imm);
 bool bpf_jit_supports_subprog_tailcalls(void);
 bool bpf_jit_supports_percpu_insn(void);
 bool bpf_jit_supports_kfunc_call(void);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 14d9288441f2..daa3ab458c8a 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2965,6 +2965,17 @@ bool __weak bpf_jit_inlines_helper_call(s32 imm)
 	return false;
 }
 
+/* Return true if the JIT inlines the call to the kfunc corresponding to
+ * the imm.
+ *
+ * The verifier will not patch the insn->imm for the call to the helper if
+ * this returns true.
+ */
+bool __weak bpf_jit_inlines_kfunc_call(s32 imm)
+{
+	return false;
+}
+
 /* Return TRUE if the JIT backend supports mixing bpf2bpf and tailcalls. */
 bool __weak bpf_jit_supports_subprog_tailcalls(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 751c150f9e1c..12d40537e57b 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -23,6 +23,10 @@
 #include <linux/btf_ids.h>
 #include <linux/bpf_mem_alloc.h>
 #include <linux/kasan.h>
+#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
+#include <vdso/datapage.h>
+#include <asm/vdso/vsyscall.h>
+#endif
 
 #include "../../lib/kstrtox.h"
 
@@ -3057,6 +3061,20 @@ __bpf_kfunc int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void __user
 	return ret + 1;
 }
 
+#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
+__bpf_kfunc u64 bpf_get_cpu_cycles(void)
+{
+	const struct vdso_data *vd = __arch_get_k_vdso_data();
+
+	vd = &vd[CS_RAW];
+
+	/* CS_RAW clock_mode translates to VDSO_CLOCKMODE_TSC on x86 and
+	 * to VDSO_CLOCKMODE_ARCHTIMER on aarch64/risc-v.
+	 */
+	return __arch_get_hw_counter(vd->clock_mode, vd);
+}
+#endif
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(generic_btf_ids)
@@ -3149,6 +3167,9 @@ BTF_ID_FLAGS(func, bpf_get_kmem_cache)
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
+#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
+BTF_ID_FLAGS(func, bpf_get_cpu_cycles, KF_FASTCALL)
+#endif
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 09f7fa635f67..a08e05a420e1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -16411,6 +16411,24 @@ static bool verifier_inlines_helper_call(struct bpf_verifier_env *env, s32 imm)
 	}
 }
 
+/* True if fixup_kfunc_call() replaces calls to kfunc number 'imm',
+ * replacement patch is presumed to follow bpf_fastcall contract
+ * (see mark_fastcall_pattern_for_call() below).
+ */
+static bool verifier_inlines_kfunc_call(struct bpf_verifier_env *env, s32 imm)
+{
+	const struct bpf_kfunc_desc *desc = find_kfunc_desc(env->prog, imm, 0);
+
+	if (!env->prog->jit_requested)
+		return false;
+
+	if (desc->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx] ||
+	    desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast])
+		return true;
+
+	return false;
+}
+
 /* Same as helper_fastcall_clobber_mask() but for kfuncs, see comment above */
 static u32 kfunc_fastcall_clobber_mask(struct bpf_kfunc_call_arg_meta *meta)
 {
@@ -16538,7 +16556,10 @@ static void mark_fastcall_pattern_for_call(struct bpf_verifier_env *env,
 			return;
 
 		clobbered_regs_mask = kfunc_fastcall_clobber_mask(&meta);
-		can_be_inlined = is_fastcall_kfunc_call(&meta);
+		can_be_inlined = is_fastcall_kfunc_call(&meta) &&
+				 (verifier_inlines_kfunc_call(env, call->imm) ||
+				 (meta.btf == btf_vmlinux &&
+				  bpf_jit_inlines_kfunc_call(call->imm)));
 	}
 
 	if (clobbered_regs_mask == ALL_CALLER_SAVED_REGS)
@@ -20545,6 +20566,7 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			    struct bpf_insn *insn_buf, int insn_idx, int *cnt)
 {
 	const struct bpf_kfunc_desc *desc;
+	s32 imm = insn->imm;
 
 	if (!insn->imm) {
 		verbose(env, "invalid kernel function call not eliminated in verifier pass\n");
@@ -20568,7 +20590,18 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		insn->imm = BPF_CALL_IMM(desc->addr);
 	if (insn->off)
 		return 0;
-	if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] ||
+	if (verifier_inlines_kfunc_call(env, imm)) {
+		if (desc->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx] ||
+		    desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
+			insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1);
+			*cnt = 1;
+		} else {
+			verbose(env, "verifier internal error: kfunc id %d has no inline code\n",
+				desc->func_id);
+			return -EFAULT;
+		}
+
+	} else if (desc->func_id == special_kfunc_list[KF_bpf_obj_new_impl] ||
 	    desc->func_id == special_kfunc_list[KF_bpf_percpu_obj_new_impl]) {
 		struct btf_struct_meta *kptr_struct_meta = env->insn_aux_data[insn_idx].kptr_struct_meta;
 		struct bpf_insn addr[2] = { BPF_LD_IMM64(BPF_REG_2, (long)kptr_struct_meta) };
@@ -20629,10 +20662,6 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 
 		__fixup_collection_insert_kfunc(&env->insn_aux_data[insn_idx], struct_meta_reg,
 						node_offset_reg, insn, insn_buf, cnt);
-	} else if (desc->func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx] ||
-		   desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) {
-		insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1);
-		*cnt = 1;
 	} else if (is_bpf_wq_set_callback_impl_kfunc(desc->func_id)) {
 		struct bpf_insn ld_addrs[2] = { BPF_LD_IMM64(BPF_REG_4, (long)env->prog->aux) };
 
-- 
2.43.5

[PATCH bpf-next v6 2/4] bpf: add bpf_cpu_cycles_to_ns helper

[Vadim Fedorenko]

The new helper should be used to convert cycles received by
bpf_get_cpu_cycle() into nanoseconds.

Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
v4 -> v6:
* add comment about simplified implementation (Eduard)
v4:
* change helper name to bpf_cpu_cycles_to_ns.
* hide it behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing on
  unsupported architectures.
---
 arch/x86/net/bpf_jit_comp.c   | 22 ++++++++++++++++++++++
 arch/x86/net/bpf_jit_comp32.c | 19 +++++++++++++++++++
 include/linux/bpf.h           |  1 +
 kernel/bpf/helpers.c          | 13 ++++++++++++-
 4 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 107bd921f104..b87233d41a75 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -11,6 +11,7 @@
 #include <linux/bpf.h>
 #include <linux/memory.h>
 #include <linux/sort.h>
+#include <linux/clocksource.h>
 #include <asm/extable.h>
 #include <asm/ftrace.h>
 #include <asm/set_memory.h>
@@ -2216,6 +2217,24 @@ st:			if (is_imm8(insn->off))
 				break;
 			}
 
+			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
+			    imm32 == BPF_CALL_IMM(bpf_cpu_cycles_to_ns) &&
+			    boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
+				u32 mult, shift;
+
+				clocks_calc_mult_shift(&mult, &shift, tsc_khz, USEC_PER_SEC, 0);
+				/* imul RAX, RDI, mult */
+				maybe_emit_mod(&prog, BPF_REG_1, BPF_REG_0, true);
+				EMIT2_off32(0x69, add_2reg(0xC0, BPF_REG_1, BPF_REG_0),
+					    mult);
+
+				/* shr RAX, shift (which is less than 64) */
+				maybe_emit_1mod(&prog, BPF_REG_0, true);
+				EMIT3(0xC1, add_1reg(0xE8, BPF_REG_0), shift);
+
+				break;
+			}
+
 			func = (u8 *) __bpf_call_base + imm32;
 			if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
 				LOAD_TAIL_CALL_CNT_PTR(stack_depth);
@@ -3828,5 +3847,8 @@ bool bpf_jit_inlines_kfunc_call(s32 imm)
 {
 	if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
 		return true;
+	if (imm == BPF_CALL_IMM(bpf_cpu_cycles_to_ns) &&
+	    boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
+		return true;
 	return false;
 }
diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index e6097a371b69..34f762f28c82 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -12,6 +12,7 @@
 #include <linux/netdevice.h>
 #include <linux/filter.h>
 #include <linux/if_vlan.h>
+#include <linux/clocksource.h>
 #include <asm/cacheflush.h>
 #include <asm/set_memory.h>
 #include <asm/nospec-branch.h>
@@ -2100,6 +2101,24 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 					EMIT2(0x0F, 0x31);
 					break;
 				}
+				if (imm32 == BPF_CALL_IMM(bpf_cpu_cycles_to_ns) &&
+				    boot_cpu_has(X86_FEATURE_CONSTANT_TSC)) {
+					u32 mult, shift;
+
+					clocks_calc_mult_shift(&mult, &shift, tsc_khz,
+							       USEC_PER_SEC, 0);
+
+					/* move parameter to BPF_REG_0 */
+					emit_ia32_mov_r64(true, bpf2ia32[BPF_REG_0],
+							  bpf2ia32[BPF_REG_1], true, true,
+							  &prog, bpf_prog->aux);
+					/* multiply parameter by mut */
+					emit_ia32_mul_i64(bpf2ia32[BPF_REG_0],
+							  mult, true, &prog);
+					/* shift parameter by shift which is less than 64 */
+					emit_ia32_rsh_i64(bpf2ia32[BPF_REG_0],
+							  shift, true, &prog);
+				}
 
 				err = emit_kfunc_call(bpf_prog,
 						      image + addrs[i],
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 43a5207a1591..af47704afeaa 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3336,6 +3336,7 @@ u64 bpf_get_raw_cpu_id(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
 /* Inlined kfuncs */
 #if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
 u64 bpf_get_cpu_cycles(void);
+u64 bpf_cpu_cycles_to_ns(u64 cycles);
 #endif
 
 #if defined(CONFIG_NET)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 12d40537e57b..e89eff53c340 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3073,8 +3073,18 @@ __bpf_kfunc u64 bpf_get_cpu_cycles(void)
 	 */
 	return __arch_get_hw_counter(vd->clock_mode, vd);
 }
-#endif
 
+__bpf_kfunc u64 bpf_cpu_cycles_to_ns(u64 cycles)
+{
+	const struct vdso_data *vd = __arch_get_k_vdso_data();
+
+	/* kfunc implementation does less manipulations than vDSO
+	 * implementation. BPF use-case assumes two measurements are close
+	 * in time and can simplify the logic.
+	 */
+	return mul_u64_u32_shr(cycles, vd->mult, vd->shift);
+}
+#endif
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(generic_btf_ids)
@@ -3169,6 +3179,7 @@ BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLE
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
 #if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
 BTF_ID_FLAGS(func, bpf_get_cpu_cycles, KF_FASTCALL)
+BTF_ID_FLAGS(func, bpf_cpu_cycles_to_ns, KF_FASTCALL)
 #endif
 BTF_KFUNCS_END(common_btf_ids)
 
-- 
2.43.5

[PATCH bpf-next v6 3/4] selftests/bpf: add selftest to check rdtsc jit

[Vadim Fedorenko]

get_cpu_cycles() is replaced with rdtsc instruction on x86_64. Add
tests to check it.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../selftests/bpf/progs/verifier_cpu_cycles.c | 104 ++++++++++++++++++
 2 files changed, 106 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index d9f65adb456b..6cbb8949164a 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -98,6 +98,7 @@
 #include "verifier_xdp_direct_packet_access.skel.h"
 #include "verifier_bits_iter.skel.h"
 #include "verifier_lsm.skel.h"
+#include "verifier_cpu_cycles.skel.h"
 
 #define MAX_ENTRIES 11
 
@@ -225,6 +226,7 @@ void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
 void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
 void test_verifier_bits_iter(void) { RUN(verifier_bits_iter); }
 void test_verifier_lsm(void)                  { RUN(verifier_lsm); }
+void test_verifier_cpu_cycles(void)           { RUN(verifier_cpu_cycles); }
 
 void test_verifier_mtu(void)
 {
diff --git a/tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c b/tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c
new file mode 100644
index 000000000000..88bfa7211858
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Meta Inc. */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+extern u64 bpf_cpu_cycles_to_ns(u64 cycles) __weak __ksym;
+extern u64 bpf_get_cpu_cycles(void) __weak __ksym;
+
+SEC("syscall")
+__arch_x86_64
+__xlated("0: call kernel-function")
+__naked int bpf_rdtsc(void)
+{
+	asm volatile(
+	"call %[bpf_get_cpu_cycles];"
+	"exit"
+	:
+	: __imm(bpf_get_cpu_cycles)
+	: __clobber_all
+	);
+}
+
+SEC("syscall")
+__arch_x86_64
+/* program entry for bpf_rdtsc_jit_x86_64(), regular function prologue */
+__jited("	endbr64")
+__jited("	nopl	(%rax,%rax)")
+__jited("	nopl	(%rax)")
+__jited("	pushq	%rbp")
+__jited("	movq	%rsp, %rbp")
+__jited("	endbr64")
+/* save RDX in R11 as it will be overwritten */
+__jited("	movq	%rdx, %r11")
+/* lfence may not be executed depending on cpu features */
+__jited("	{{(lfence|)}}")
+__jited("	rdtsc")
+/* combine EDX:EAX into RAX */
+__jited("	shlq	${{(32|0x20)}}, %rdx")
+__jited("	orq	%rdx, %rax")
+/* restore RDX from R11 */
+__jited("	movq	%r11, %rdx")
+__jited("	leave")
+__naked int bpf_rdtsc_jit_x86_64(void)
+{
+	asm volatile(
+	"call %[bpf_get_cpu_cycles];"
+	"exit"
+	:
+	: __imm(bpf_get_cpu_cycles)
+	: __clobber_all
+	);
+}
+
+SEC("syscall")
+__arch_x86_64
+__xlated("0: r1 = 42")
+__xlated("1: call kernel-function")
+__naked int bpf_cyc2ns(void)
+{
+	asm volatile(
+	"r1=0x2a;"
+	"call %[bpf_cpu_cycles_to_ns];"
+	"exit"
+	:
+	: __imm(bpf_cpu_cycles_to_ns)
+	: __clobber_all
+	);
+}
+
+SEC("syscall")
+__arch_x86_64
+/* program entry for bpf_rdtsc_jit_x86_64(), regular function prologue */
+__jited("	endbr64")
+__jited("	nopl	(%rax,%rax)")
+__jited("	nopl	(%rax)")
+__jited("	pushq	%rbp")
+__jited("	movq	%rsp, %rbp")
+__jited("	endbr64")
+/* save RDX in R11 as it will be overwritten */
+__jited("	movabsq	$0x2a2a2a2a2a, %rdi")
+__jited("	imulq	${{.*}}, %rdi, %rax")
+__jited("	shrq	${{.*}}, %rax")
+__jited("	leave")
+__naked int bpf_cyc2ns_jit_x86(void)
+{
+	asm volatile(
+	"r1=0x2a2a2a2a2a ll;"
+	"call %[bpf_cpu_cycles_to_ns];"
+	"exit"
+	:
+	: __imm(bpf_cpu_cycles_to_ns)
+	: __clobber_all
+	);
+}
+
+void rdtsc(void)
+{
+	bpf_get_cpu_cycles();
+	bpf_cpu_cycles_to_ns(42);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.5

[PATCH bpf-next v6 4/4] selftests/bpf: add usage example for cpu cycles kfuncs

[Vadim Fedorenko]

The selftest provides an example of how to measure the latency of bpf
kfunc/helper call in cycles and nanoseconds.

Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
 .../bpf/prog_tests/test_cpu_cycles.c          | 35 +++++++++++++++++++
 .../selftests/bpf/progs/test_cpu_cycles.c     | 25 +++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_cpu_cycles.c

diff --git a/tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c b/tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
new file mode 100644
index 000000000000..d7f3b66594b3
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Inc. */
+
+#include <test_progs.h>
+#include "test_cpu_cycles.skel.h"
+
+static void cpu_cycles(void)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+	struct test_cpu_cycles *skel;
+	int err, pfd;
+
+	skel = test_cpu_cycles__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "test_cpu_cycles open and load"))
+		return;
+
+	pfd = bpf_program__fd(skel->progs.bpf_cpu_cycles);
+	if (!ASSERT_GT(pfd, 0, "test_cpu_cycles fd"))
+		goto fail;
+
+	err = bpf_prog_test_run_opts(pfd, &opts);
+	if (!ASSERT_OK(err, "test_cpu_cycles test run"))
+		goto fail;
+
+	ASSERT_NEQ(skel->bss->cycles, 0, "test_cpu_cycles 0 cycles");
+	ASSERT_NEQ(skel->bss->ns, 0, "test_cpu_cycles 0 ns");
+fail:
+	test_cpu_cycles__destroy(skel);
+}
+
+void test_cpu_cycles(void)
+{
+	if (test__start_subtest("cpu_cycles"))
+		cpu_cycles();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_cpu_cycles.c b/tools/testing/selftests/bpf/progs/test_cpu_cycles.c
new file mode 100644
index 000000000000..48762f07ad7f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_cpu_cycles.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Inc. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+extern u64 bpf_cpu_cycles_to_ns(u64 cycles) __weak __ksym;
+extern u64 bpf_get_cpu_cycles(void) __weak __ksym;
+
+__u64 cycles, ns;
+
+SEC("syscall")
+int bpf_cpu_cycles(void)
+{
+	struct bpf_pidns_info pidns;
+	__u64 start;
+
+	start = bpf_get_cpu_cycles();
+	bpf_get_ns_current_pid_tgid(0, 0, &pidns, sizeof(struct bpf_pidns_info));
+	cycles = bpf_get_cpu_cycles() - start;
+	ns = bpf_cpu_cycles_to_ns(cycles);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.5

Re: [PATCH bpf-next v6 0/4] bpf: add cpu cycles kfuncss

[Borislav Petkov]

On Fri, Nov 15, 2024 at 11:48:37AM -0800, Vadim Fedorenko wrote:
>  arch/x86/net/bpf_jit_comp.c                   |  60 ++++++++++
>  arch/x86/net/bpf_jit_comp32.c                 |  33 ++++++
>  include/linux/bpf.h                           |   6 +
>  include/linux/filter.h                        |   1 +
>  kernel/bpf/core.c                             |  11 ++
>  kernel/bpf/helpers.c                          |  32 ++++++
>  kernel/bpf/verifier.c                         |  41 ++++++-
>  .../bpf/prog_tests/test_cpu_cycles.c          |  35 ++++++
>  .../selftests/bpf/prog_tests/verifier.c       |   2 +
>  .../selftests/bpf/progs/test_cpu_cycles.c     |  25 +++++
>  .../selftests/bpf/progs/verifier_cpu_cycles.c | 104 ++++++++++++++++++
>  11 files changed, 344 insertions(+), 6 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_cpu_cycles.c
>  create mode 100644 tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c

For your whole set:

s/boot_cpu_has/cpu_feature_enabled/g

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH bpf-next v6 0/4] bpf: add cpu cycles kfuncss

[Vadim Fedorenko]

On 15/11/2024 20:15, Borislav Petkov wrote:
> On Fri, Nov 15, 2024 at 11:48:37AM -0800, Vadim Fedorenko wrote:
>>   arch/x86/net/bpf_jit_comp.c                   |  60 ++++++++++
>>   arch/x86/net/bpf_jit_comp32.c                 |  33 ++++++
>>   include/linux/bpf.h                           |   6 +
>>   include/linux/filter.h                        |   1 +
>>   kernel/bpf/core.c                             |  11 ++
>>   kernel/bpf/helpers.c                          |  32 ++++++
>>   kernel/bpf/verifier.c                         |  41 ++++++-
>>   .../bpf/prog_tests/test_cpu_cycles.c          |  35 ++++++
>>   .../selftests/bpf/prog_tests/verifier.c       |   2 +
>>   .../selftests/bpf/progs/test_cpu_cycles.c     |  25 +++++
>>   .../selftests/bpf/progs/verifier_cpu_cycles.c | 104 ++++++++++++++++++
>>   11 files changed, 344 insertions(+), 6 deletions(-)
>>   create mode 100644 tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
>>   create mode 100644 tools/testing/selftests/bpf/progs/test_cpu_cycles.c
>>   create mode 100644 tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c
> 
> For your whole set:
> 
> s/boot_cpu_has/cpu_feature_enabled/g

thanks, will change it for the next version
> 
> Thx.
> 

Re: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc

[Yonghong Song]

On 11/15/24 11:48 AM, Vadim Fedorenko wrote:
> New kfunc to return ARCH-specific timecounter. For x86 BPF JIT converts
> it into rdtsc ordered call. Other architectures will get JIT
> implementation too if supported. The fallback is to
> __arch_get_hw_counter().
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>

LGTM with a small nit below.

Acked-by: Yonghong Song <yonghong.song@linux.dev>

> ---
> v5 -> v6:
> * add comment about dropping S64_MAX manipulation in jitted
>    implementation of rdtsc_oredered (Alexey)
> * add comment about using 'lfence;rdtsc' variant (Alexey)
> * change the check in fixup_kfunc_call() (Eduard)
> * make __arch_get_hw_counter() call more aligned with vDSO
>    implementation (Yonghong)
> v4 -> v5:
> * use if instead of ifdef with IS_ENABLED
> v3 -> v4:
> * change name of the helper to bpf_get_cpu_cycles (Andrii)
> * Hide the helper behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing
>    it on architectures which do not have vDSO functions and data
> * reduce the scope of check of inlined functions in verifier to only 2,
>    which are actually inlined.
> v2 -> v3:
> * change name of the helper to bpf_get_cpu_cycles_counter to explicitly
>    mention what counter it provides (Andrii)
> * move kfunc definition to bpf.h to use it in JIT.
> * introduce another kfunc to convert cycles into nanoseconds as more
>    meaningful time units for generic tracing use case (Andrii)
> v1 -> v2:
> * Fix incorrect function return value type to u64
> * Introduce bpf_jit_inlines_kfunc_call() and use it in
>    mark_fastcall_pattern_for_call() to avoid clobbering in case of
>    running programs with no JIT (Eduard)
> * Avoid rewriting instruction and check function pointer directly
>    in JIT (Alexei)
> * Change includes to fix compile issues on non x86 architectures
> ---
>   arch/x86/net/bpf_jit_comp.c   | 39 +++++++++++++++++++++++++++++++++
>   arch/x86/net/bpf_jit_comp32.c | 14 ++++++++++++
>   include/linux/bpf.h           |  5 +++++
>   include/linux/filter.h        |  1 +
>   kernel/bpf/core.c             | 11 ++++++++++
>   kernel/bpf/helpers.c          | 21 ++++++++++++++++++
>   kernel/bpf/verifier.c         | 41 ++++++++++++++++++++++++++++++-----
>   7 files changed, 126 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index a43fc5af973d..107bd921f104 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2185,6 +2185,37 @@ st:			if (is_imm8(insn->off))
>   		case BPF_JMP | BPF_CALL: {
>   			u8 *ip = image + addrs[i - 1];
>   
> +			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
> +			    imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
> +				/* The default implementation of this kfunc uses
> +				 * __arch_get_hw_counter() which is implemented as
> +				 * `(u64)rdtsc_ordered() & S64_MAX`. We skip masking
> +				 * part because we assume it's not needed in BPF
> +				 * use case (two measurements close in time).
> +				 * Original code for rdtsc_ordered() uses sequence:
> +				 * 'rdtsc; nop; nop; nop' to patch it into
> +				 * 'lfence; rdtsc' or 'rdtscp' depending on CPU features.
> +				 * JIT uses 'lfence; rdtsc' variant because BPF program
> +				 * doesn't care about cookie provided by rdtsp in RCX.

rdtsp -> tdtscp?

> +				 * Save RDX because RDTSC will use EDX:EAX to return u64
> +				 */
> +				emit_mov_reg(&prog, true, AUX_REG, BPF_REG_3);
> +				if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
> +					EMIT_LFENCE();
> +				EMIT2(0x0F, 0x31);
> +
> +				/* shl RDX, 32 */
> +				maybe_emit_1mod(&prog, BPF_REG_3, true);
> +				EMIT3(0xC1, add_1reg(0xE0, BPF_REG_3), 32);
> +				/* or RAX, RDX */
> +				maybe_emit_mod(&prog, BPF_REG_0, BPF_REG_3, true);
> +				EMIT2(0x09, add_2reg(0xC0, BPF_REG_0, BPF_REG_3));
> +				/* restore RDX from R11 */
> +				emit_mov_reg(&prog, true, BPF_REG_3, AUX_REG);
> +
> +				break;
> +			}
> +
>   			func = (u8 *) __bpf_call_base + imm32;
>   			if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
>   				LOAD_TAIL_CALL_CNT_PTR(stack_depth);
> @@ -3791,3 +3822,11 @@ u64 bpf_arch_uaddress_limit(void)
>   {
>   	return 0;
>   }
> +
> +/* x86-64 JIT can inline kfunc */
> +bool bpf_jit_inlines_kfunc_call(s32 imm)
> +{
> +	if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
> +		return true;
> +	return false;
> +}
> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
> index de0f9e5f9f73..e6097a371b69 100644
> --- a/arch/x86/net/bpf_jit_comp32.c
> +++ b/arch/x86/net/bpf_jit_comp32.c
> @@ -2094,6 +2094,13 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
>   			if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
>   				int err;
>   
> +				if (imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
> +					if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
> +						EMIT3(0x0F, 0xAE, 0xE8);
> +					EMIT2(0x0F, 0x31);
> +					break;
> +				}
> +
>   				err = emit_kfunc_call(bpf_prog,
>   						      image + addrs[i],
>   						      insn, &prog);
> @@ -2621,3 +2628,10 @@ bool bpf_jit_supports_kfunc_call(void)
>   {
>   	return true;
>   }
> +
> +bool bpf_jit_inlines_kfunc_call(s32 imm)
> +{
> +	if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
> +		return true;
> +	return false;
> +}
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 3ace0d6227e3..43a5207a1591 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3333,6 +3333,11 @@ void bpf_user_rnd_init_once(void);
>   u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>   u64 bpf_get_raw_cpu_id(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>   
> +/* Inlined kfuncs */
> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
> +u64 bpf_get_cpu_cycles(void);
> +#endif
> +
>   #if defined(CONFIG_NET)
>   bool bpf_sock_common_is_valid_access(int off, int size,
>   				     enum bpf_access_type type,
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 3a21947f2fd4..9cf57233874f 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -1111,6 +1111,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog);
>   void bpf_jit_compile(struct bpf_prog *prog);
>   bool bpf_jit_needs_zext(void);
>   bool bpf_jit_inlines_helper_call(s32 imm);
> +bool bpf_jit_inlines_kfunc_call(s32 imm);
>   bool bpf_jit_supports_subprog_tailcalls(void);
>   bool bpf_jit_supports_percpu_insn(void);
>   bool bpf_jit_supports_kfunc_call(void);
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 14d9288441f2..daa3ab458c8a 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2965,6 +2965,17 @@ bool __weak bpf_jit_inlines_helper_call(s32 imm)
>   	return false;
>   }
>   
> +/* Return true if the JIT inlines the call to the kfunc corresponding to
> + * the imm.
> + *
> + * The verifier will not patch the insn->imm for the call to the helper if
> + * this returns true.
> + */
> +bool __weak bpf_jit_inlines_kfunc_call(s32 imm)
> +{
> +	return false;
> +}
> +
>   /* Return TRUE if the JIT backend supports mixing bpf2bpf and tailcalls. */
>   bool __weak bpf_jit_supports_subprog_tailcalls(void)
>   {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 751c150f9e1c..12d40537e57b 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -23,6 +23,10 @@
>   #include <linux/btf_ids.h>
>   #include <linux/bpf_mem_alloc.h>
>   #include <linux/kasan.h>
> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
> +#include <vdso/datapage.h>
> +#include <asm/vdso/vsyscall.h>
> +#endif
>   
>   #include "../../lib/kstrtox.h"
>   
> @@ -3057,6 +3061,20 @@ __bpf_kfunc int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void __user
>   	return ret + 1;
>   }
>   
> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
> +__bpf_kfunc u64 bpf_get_cpu_cycles(void)
> +{
> +	const struct vdso_data *vd = __arch_get_k_vdso_data();
> +
> +	vd = &vd[CS_RAW];
> +
> +	/* CS_RAW clock_mode translates to VDSO_CLOCKMODE_TSC on x86 and
> +	 * to VDSO_CLOCKMODE_ARCHTIMER on aarch64/risc-v.
> +	 */
> +	return __arch_get_hw_counter(vd->clock_mode, vd);
> +}
> +#endif
> +
>   __bpf_kfunc_end_defs();
>   
>   BTF_KFUNCS_START(generic_btf_ids)
> @@ -3149,6 +3167,9 @@ BTF_ID_FLAGS(func, bpf_get_kmem_cache)
>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE)
>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLEEPABLE)
>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
> +BTF_ID_FLAGS(func, bpf_get_cpu_cycles, KF_FASTCALL)
> +#endif
>   BTF_KFUNCS_END(common_btf_ids)
>   
>   static const struct btf_kfunc_id_set common_kfunc_set = {
[...]

Re: [PATCH bpf-next v6 0/4] bpf: add cpu cycles kfuncss

[Andrii Nakryiko]

On Fri, Nov 15, 2024 at 11:49 AM Vadim Fedorenko <vadfed@meta.com> wrote:
>
> This patchset adds 2 kfuncs to provide a way to precisely measure the
> time spent running some code. The first patch provides a way to get cpu
> cycles counter which is used to feed CLOCK_MONOTONIC_RAW. On x86
> architecture it is effectively rdtsc_ordered() function while on other
> architectures it falls back to __arch_get_hw_counter(). The second patch
> adds a kfunc to convert cpu cycles to nanoseconds using shift/mult
> constants discovered by kernel. JIT version is done for x86 for now, on
> other architectures it falls back to slightly simplified version of
> vdso_calc_ns.
>
> Selftests are also added to check whether the JIT implementation is
> correct and to show the simplest usage example.
>
> Change log:
> v5 -> v6:
> * added cover letter
> * add comment about dropping S64_MAX manipulation in jitted
>   implementation of rdtsc_oredered (Alexey)
> * add comment about using 'lfence;rdtsc' variant (Alexey)
> * change the check in fixup_kfunc_call() (Eduard)
> * make __arch_get_hw_counter() call more aligned with vDSO
>   implementation (Yonghong)
> v4 -> v5:
> * use #if instead of #ifdef with IS_ENABLED
> v3 -> v4:
> * change name of the helper to bpf_get_cpu_cycles (Andrii)
> * Hide the helper behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing
>   it on architectures which do not have vDSO functions and data
> * reduce the scope of check of inlined functions in verifier to only 2,
>   which are actually inlined.
> * change helper name to bpf_cpu_cycles_to_ns.
> * hide it behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing on
>   unsupported architectures.
> v2 -> v3:
> * change name of the helper to bpf_get_cpu_cycles_counter to
> * explicitly mention what counter it provides (Andrii)
> * move kfunc definition to bpf.h to use it in JIT.
> * introduce another kfunc to convert cycles into nanoseconds as
> * more meaningful time units for generic tracing use case (Andrii)
> v1 -> v2:
> * Fix incorrect function return value type to u64
> * Introduce bpf_jit_inlines_kfunc_call() and use it in
>         mark_fastcall_pattern_for_call() to avoid clobbering in case
>         of running programs with no JIT (Eduard)
> * Avoid rewriting instruction and check function pointer directly
>         in JIT (Alexei)
> * Change includes to fix compile issues on non x86 architectures
>
> Vadim Fedorenko (4):
>   bpf: add bpf_get_cpu_cycles kfunc
>   bpf: add bpf_cpu_cycles_to_ns helper
>   selftests/bpf: add selftest to check rdtsc jit
>   selftests/bpf: add usage example for cpu cycles kfuncs
>
>  arch/x86/net/bpf_jit_comp.c                   |  60 ++++++++++
>  arch/x86/net/bpf_jit_comp32.c                 |  33 ++++++
>  include/linux/bpf.h                           |   6 +
>  include/linux/filter.h                        |   1 +
>  kernel/bpf/core.c                             |  11 ++
>  kernel/bpf/helpers.c                          |  32 ++++++
>  kernel/bpf/verifier.c                         |  41 ++++++-
>  .../bpf/prog_tests/test_cpu_cycles.c          |  35 ++++++
>  .../selftests/bpf/prog_tests/verifier.c       |   2 +
>  .../selftests/bpf/progs/test_cpu_cycles.c     |  25 +++++
>  .../selftests/bpf/progs/verifier_cpu_cycles.c | 104 ++++++++++++++++++
>  11 files changed, 344 insertions(+), 6 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/test_cpu_cycles.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_cpu_cycles.c
>  create mode 100644 tools/testing/selftests/bpf/progs/verifier_cpu_cycles.c
>
> --
> 2.43.5
>

typo in subject: kfuncss -> kfuncs

LGTM overall, thanks a lot for adding this!

For the series:

Acked-by: Andrii Nakryiko <andrii@kernel.org>

Re: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc

[kernel test robot]

Hi Vadim,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bpf-add-bpf_get_cpu_cycles-kfunc/20241117-002106
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20241115194841.2108634-2-vadfed%40meta.com
patch subject: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20241117/202411171347.9Yb9hhnX-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241117/202411171347.9Yb9hhnX-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411171347.9Yb9hhnX-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined hidden symbol: pvclock_page
   >>> referenced by pvclock.h:25 (arch/x86/include/asm/pvclock.h:25)
   >>>               vmlinux.o:(vread_pvclock)
   >>> referenced by gettimeofday.h:228 (arch/x86/include/asm/vdso/gettimeofday.h:228)
   >>>               vmlinux.o:(vread_pvclock)
   >>> referenced by pvclock.h:86 (arch/x86/include/asm/pvclock.h:86)
   >>>               vmlinux.o:(vread_pvclock)
   >>> referenced 4 more times
--
>> ld.lld: error: undefined hidden symbol: hvclock_page
   >>> referenced by hyperv_timer.h:65 (include/clocksource/hyperv_timer.h:65)
   >>>               vmlinux.o:(vread_hvclock)
   >>> referenced by hyperv_timer.h:74 (include/clocksource/hyperv_timer.h:74)
   >>>               vmlinux.o:(vread_hvclock)
   >>> referenced by hyperv_timer.h:75 (include/clocksource/hyperv_timer.h:75)
   >>>               vmlinux.o:(vread_hvclock)
   >>> referenced 1 more times

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc

[Vadim Fedorenko]

On 15/11/2024 21:54, Yonghong Song wrote:
> 
> 
> On 11/15/24 11:48 AM, Vadim Fedorenko wrote:
>> New kfunc to return ARCH-specific timecounter. For x86 BPF JIT converts
>> it into rdtsc ordered call. Other architectures will get JIT
>> implementation too if supported. The fallback is to
>> __arch_get_hw_counter().
>>
>> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
> 
> LGTM with a small nit below.
> 
> Acked-by: Yonghong Song <yonghong.song@linux.dev>

@Yonghong The changes to align to vdso call bring this patch to the
state when the kernel fails to compile with CONFIG_PARAVIRT_CLOCK or
CONFIG_HYPERV_TIMER enabled. This happens because on x86 there is
special way to grab cpu cycle counter in PARAVIRT mode. The paravirt
memory structure is hidden for kernel and linked for vDSO only using
arch/x86/entry/vdso/vdso-layout.lds.S. But in anycase both
vread_pvclock() and vread_hvclock() end up doing rdtsc_ordered().
I believe we can have constant clock_mode for x86 equal to
VDSO_CLOCKMODE_TSC given we have JIT for x86 ready.

Another way is to switch to use get_cycles() which is also defined for
all architectures. But that will bring up another discussion whether we
should use rdtsc_ordered in JIT, because on x86 get_cycles() ends up
calling rdtsc() which has no LFENCE in assembly. If I remember correctly
there was a question of maybe using simple rdtsc() in this patchset as
ordered version might be slow on modern CPUs. We still can use shift
and mult values for cycles2ns helper because we know that CS_RAW uses
the same cpu cycles counter.

I'm up for any option, but let's just agree on how to proceed.

Thanks.
> 
>> ---
>> v5 -> v6:
>> * add comment about dropping S64_MAX manipulation in jitted
>>    implementation of rdtsc_oredered (Alexey)
>> * add comment about using 'lfence;rdtsc' variant (Alexey)
>> * change the check in fixup_kfunc_call() (Eduard)
>> * make __arch_get_hw_counter() call more aligned with vDSO
>>    implementation (Yonghong)
>> v4 -> v5:
>> * use if instead of ifdef with IS_ENABLED
>> v3 -> v4:
>> * change name of the helper to bpf_get_cpu_cycles (Andrii)
>> * Hide the helper behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing
>>    it on architectures which do not have vDSO functions and data
>> * reduce the scope of check of inlined functions in verifier to only 2,
>>    which are actually inlined.
>> v2 -> v3:
>> * change name of the helper to bpf_get_cpu_cycles_counter to explicitly
>>    mention what counter it provides (Andrii)
>> * move kfunc definition to bpf.h to use it in JIT.
>> * introduce another kfunc to convert cycles into nanoseconds as more
>>    meaningful time units for generic tracing use case (Andrii)
>> v1 -> v2:
>> * Fix incorrect function return value type to u64
>> * Introduce bpf_jit_inlines_kfunc_call() and use it in
>>    mark_fastcall_pattern_for_call() to avoid clobbering in case of
>>    running programs with no JIT (Eduard)
>> * Avoid rewriting instruction and check function pointer directly
>>    in JIT (Alexei)
>> * Change includes to fix compile issues on non x86 architectures
>> ---
>>   arch/x86/net/bpf_jit_comp.c   | 39 +++++++++++++++++++++++++++++++++
>>   arch/x86/net/bpf_jit_comp32.c | 14 ++++++++++++
>>   include/linux/bpf.h           |  5 +++++
>>   include/linux/filter.h        |  1 +
>>   kernel/bpf/core.c             | 11 ++++++++++
>>   kernel/bpf/helpers.c          | 21 ++++++++++++++++++
>>   kernel/bpf/verifier.c         | 41 ++++++++++++++++++++++++++++++-----
>>   7 files changed, 126 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index a43fc5af973d..107bd921f104 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -2185,6 +2185,37 @@ st:            if (is_imm8(insn->off))
>>           case BPF_JMP | BPF_CALL: {
>>               u8 *ip = image + addrs[i - 1];
>> +            if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
>> +                imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
>> +                /* The default implementation of this kfunc uses
>> +                 * __arch_get_hw_counter() which is implemented as
>> +                 * `(u64)rdtsc_ordered() & S64_MAX`. We skip masking
>> +                 * part because we assume it's not needed in BPF
>> +                 * use case (two measurements close in time).
>> +                 * Original code for rdtsc_ordered() uses sequence:
>> +                 * 'rdtsc; nop; nop; nop' to patch it into
>> +                 * 'lfence; rdtsc' or 'rdtscp' depending on CPU 
>> features.
>> +                 * JIT uses 'lfence; rdtsc' variant because BPF program
>> +                 * doesn't care about cookie provided by rdtsp in RCX.
> 
> rdtsp -> tdtscp?
> 
>> +                 * Save RDX because RDTSC will use EDX:EAX to return u64
>> +                 */
>> +                emit_mov_reg(&prog, true, AUX_REG, BPF_REG_3);
>> +                if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
>> +                    EMIT_LFENCE();
>> +                EMIT2(0x0F, 0x31);
>> +
>> +                /* shl RDX, 32 */
>> +                maybe_emit_1mod(&prog, BPF_REG_3, true);
>> +                EMIT3(0xC1, add_1reg(0xE0, BPF_REG_3), 32);
>> +                /* or RAX, RDX */
>> +                maybe_emit_mod(&prog, BPF_REG_0, BPF_REG_3, true);
>> +                EMIT2(0x09, add_2reg(0xC0, BPF_REG_0, BPF_REG_3));
>> +                /* restore RDX from R11 */
>> +                emit_mov_reg(&prog, true, BPF_REG_3, AUX_REG);
>> +
>> +                break;
>> +            }
>> +
>>               func = (u8 *) __bpf_call_base + imm32;
>>               if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
>>                   LOAD_TAIL_CALL_CNT_PTR(stack_depth);
>> @@ -3791,3 +3822,11 @@ u64 bpf_arch_uaddress_limit(void)
>>   {
>>       return 0;
>>   }
>> +
>> +/* x86-64 JIT can inline kfunc */
>> +bool bpf_jit_inlines_kfunc_call(s32 imm)
>> +{
>> +    if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
>> +        return true;
>> +    return false;
>> +}
>> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/ 
>> bpf_jit_comp32.c
>> index de0f9e5f9f73..e6097a371b69 100644
>> --- a/arch/x86/net/bpf_jit_comp32.c
>> +++ b/arch/x86/net/bpf_jit_comp32.c
>> @@ -2094,6 +2094,13 @@ static int do_jit(struct bpf_prog *bpf_prog, 
>> int *addrs, u8 *image,
>>               if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
>>                   int err;
>> +                if (imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
>> +                    if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
>> +                        EMIT3(0x0F, 0xAE, 0xE8);
>> +                    EMIT2(0x0F, 0x31);
>> +                    break;
>> +                }
>> +
>>                   err = emit_kfunc_call(bpf_prog,
>>                                 image + addrs[i],
>>                                 insn, &prog);
>> @@ -2621,3 +2628,10 @@ bool bpf_jit_supports_kfunc_call(void)
>>   {
>>       return true;
>>   }
>> +
>> +bool bpf_jit_inlines_kfunc_call(s32 imm)
>> +{
>> +    if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
>> +        return true;
>> +    return false;
>> +}
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 3ace0d6227e3..43a5207a1591 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -3333,6 +3333,11 @@ void bpf_user_rnd_init_once(void);
>>   u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>>   u64 bpf_get_raw_cpu_id(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>> +/* Inlined kfuncs */
>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>> +u64 bpf_get_cpu_cycles(void);
>> +#endif
>> +
>>   #if defined(CONFIG_NET)
>>   bool bpf_sock_common_is_valid_access(int off, int size,
>>                        enum bpf_access_type type,
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index 3a21947f2fd4..9cf57233874f 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -1111,6 +1111,7 @@ struct bpf_prog *bpf_int_jit_compile(struct 
>> bpf_prog *prog);
>>   void bpf_jit_compile(struct bpf_prog *prog);
>>   bool bpf_jit_needs_zext(void);
>>   bool bpf_jit_inlines_helper_call(s32 imm);
>> +bool bpf_jit_inlines_kfunc_call(s32 imm);
>>   bool bpf_jit_supports_subprog_tailcalls(void);
>>   bool bpf_jit_supports_percpu_insn(void);
>>   bool bpf_jit_supports_kfunc_call(void);
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 14d9288441f2..daa3ab458c8a 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -2965,6 +2965,17 @@ bool __weak bpf_jit_inlines_helper_call(s32 imm)
>>       return false;
>>   }
>> +/* Return true if the JIT inlines the call to the kfunc corresponding to
>> + * the imm.
>> + *
>> + * The verifier will not patch the insn->imm for the call to the 
>> helper if
>> + * this returns true.
>> + */
>> +bool __weak bpf_jit_inlines_kfunc_call(s32 imm)
>> +{
>> +    return false;
>> +}
>> +
>>   /* Return TRUE if the JIT backend supports mixing bpf2bpf and 
>> tailcalls. */
>>   bool __weak bpf_jit_supports_subprog_tailcalls(void)
>>   {
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index 751c150f9e1c..12d40537e57b 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -23,6 +23,10 @@
>>   #include <linux/btf_ids.h>
>>   #include <linux/bpf_mem_alloc.h>
>>   #include <linux/kasan.h>
>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>> +#include <vdso/datapage.h>
>> +#include <asm/vdso/vsyscall.h>
>> +#endif
>>   #include "../../lib/kstrtox.h"
>> @@ -3057,6 +3061,20 @@ __bpf_kfunc int bpf_copy_from_user_str(void 
>> *dst, u32 dst__sz, const void __user
>>       return ret + 1;
>>   }
>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>> +__bpf_kfunc u64 bpf_get_cpu_cycles(void)
>> +{
>> +    const struct vdso_data *vd = __arch_get_k_vdso_data();
>> +
>> +    vd = &vd[CS_RAW];
>> +
>> +    /* CS_RAW clock_mode translates to VDSO_CLOCKMODE_TSC on x86 and
>> +     * to VDSO_CLOCKMODE_ARCHTIMER on aarch64/risc-v.
>> +     */
>> +    return __arch_get_hw_counter(vd->clock_mode, vd);
>> +}
>> +#endif
>> +
>>   __bpf_kfunc_end_defs();
>>   BTF_KFUNCS_START(generic_btf_ids)
>> @@ -3149,6 +3167,9 @@ BTF_ID_FLAGS(func, bpf_get_kmem_cache)
>>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | KF_SLEEPABLE)
>>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | 
>> KF_RET_NULL | KF_SLEEPABLE)
>>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | 
>> KF_SLEEPABLE)
>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>> +BTF_ID_FLAGS(func, bpf_get_cpu_cycles, KF_FASTCALL)
>> +#endif
>>   BTF_KFUNCS_END(common_btf_ids)
>>   static const struct btf_kfunc_id_set common_kfunc_set = {
> [...]

Re: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc

[Yonghong Song]

On 11/17/24 10:11 AM, Vadim Fedorenko wrote:
> On 15/11/2024 21:54, Yonghong Song wrote:
>>
>>
>> On 11/15/24 11:48 AM, Vadim Fedorenko wrote:
>>> New kfunc to return ARCH-specific timecounter. For x86 BPF JIT converts
>>> it into rdtsc ordered call. Other architectures will get JIT
>>> implementation too if supported. The fallback is to
>>> __arch_get_hw_counter().
>>>
>>> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>>> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
>>
>> LGTM with a small nit below.
>>
>> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
> @Yonghong The changes to align to vdso call bring this patch to the
> state when the kernel fails to compile with CONFIG_PARAVIRT_CLOCK or
> CONFIG_HYPERV_TIMER enabled. This happens because on x86 there is
> special way to grab cpu cycle counter in PARAVIRT mode. The paravirt
> memory structure is hidden for kernel and linked for vDSO only using
> arch/x86/entry/vdso/vdso-layout.lds.S. But in anycase both
> vread_pvclock() and vread_hvclock() end up doing rdtsc_ordered().
> I believe we can have constant clock_mode for x86 equal to
> VDSO_CLOCKMODE_TSC given we have JIT for x86 ready.

What you described is correct. For x86, we have

static inline u64 __arch_get_hw_counter(s32 clock_mode,
                                         const struct vdso_data *vd)
{
         if (likely(clock_mode == VDSO_CLOCKMODE_TSC))
                 return (u64)rdtsc_ordered() & S64_MAX;
         /*
          * For any memory-mapped vclock type, we need to make sure that gcc
          * doesn't cleverly hoist a load before the mode check.  Otherwise we
          * might end up touching the memory-mapped page even if the vclock in
          * question isn't enabled, which will segfault.  Hence the barriers.
          */
#ifdef CONFIG_PARAVIRT_CLOCK
         if (clock_mode == VDSO_CLOCKMODE_PVCLOCK) {
                 barrier();
                 return vread_pvclock();
         }
#endif
#ifdef CONFIG_HYPERV_TIMER
         if (clock_mode == VDSO_CLOCKMODE_HVCLOCK) {
                 barrier();
                 return vread_hvclock();
         }
#endif
         return U64_MAX;
}

Even if CONFIG_PARAVIRT_CLOCK and CONFIG_HYPERV_TIMER are enabled,
if clock_mode is constant 1 (== VDSO_CLOCKMODE_TSC), dead code
elimination will happen to remove vread_pvclock() and vread_hvclock()
so we are fine.

And actually the above PARAVIRT_CLOCK and HYPERV_TIMER are x86 specific.
Other arch's do not have such things.

s390 has the following implementation:

static inline u64 __arch_get_hw_counter(s32 clock_mode, const struct vdso_data *vd)
{
         u64 adj, now;

         now = get_tod_clock();
         adj = vd->arch_data.tod_steering_end - now;
         if (unlikely((s64) adj > 0))
                 now += (vd->arch_data.tod_steering_delta < 0) ? (adj >> 15) : -(adj >> 15);
         return now;
}

So I think __arch_get_hw_counter(1, vd) probably works.
But you need to double check that.

This makes us to do __arch_get_hw_counter(const_clock_mode, vd).
As you mentioned earlier, const_clock_mode = 1 is working for all
architectures. But please add a comment for that.

>
> Another way is to switch to use get_cycles() which is also defined for
> all architectures. But that will bring up another discussion whether we
> should use rdtsc_ordered in JIT, because on x86 get_cycles() ends up
> calling rdtsc() which has no LFENCE in assembly. If I remember correctly
> there was a question of maybe using simple rdtsc() in this patchset as
> ordered version might be slow on modern CPUs. We still can use shift
> and mult values for cycles2ns helper because we know that CS_RAW uses
> the same cpu cycles counter.

I think some previous discussion favors rdtsc_ordered to ensure counting
results are precise. So let us go with rdtsc_ordered.

One more thing, maybe we can replace asm/vdso/vsyscall.h to vdso/vsyscall.h
(from kernel/time/vsyscall.c)?


diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e89eff53c340..fb93f003ecf8 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -25,7 +25,7 @@
  #include <linux/kasan.h>
  #if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
  #include <vdso/datapage.h>
-#include <asm/vdso/vsyscall.h>
+#include <vdso/vsyscall.h>
  #endif
  
  #include "../../lib/kstrtox.h"

>
> I'm up for any option, but let's just agree on how to proceed.
>
> Thanks.
>>
>>> ---
>>> v5 -> v6:
>>> * add comment about dropping S64_MAX manipulation in jitted
>>>    implementation of rdtsc_oredered (Alexey)
>>> * add comment about using 'lfence;rdtsc' variant (Alexey)
>>> * change the check in fixup_kfunc_call() (Eduard)
>>> * make __arch_get_hw_counter() call more aligned with vDSO
>>>    implementation (Yonghong)
>>> v4 -> v5:
>>> * use if instead of ifdef with IS_ENABLED
>>> v3 -> v4:
>>> * change name of the helper to bpf_get_cpu_cycles (Andrii)
>>> * Hide the helper behind CONFIG_GENERIC_GETTIMEOFDAY to avoid exposing
>>>    it on architectures which do not have vDSO functions and data
>>> * reduce the scope of check of inlined functions in verifier to only 2,
>>>    which are actually inlined.
>>> v2 -> v3:
>>> * change name of the helper to bpf_get_cpu_cycles_counter to explicitly
>>>    mention what counter it provides (Andrii)
>>> * move kfunc definition to bpf.h to use it in JIT.
>>> * introduce another kfunc to convert cycles into nanoseconds as more
>>>    meaningful time units for generic tracing use case (Andrii)
>>> v1 -> v2:
>>> * Fix incorrect function return value type to u64
>>> * Introduce bpf_jit_inlines_kfunc_call() and use it in
>>>    mark_fastcall_pattern_for_call() to avoid clobbering in case of
>>>    running programs with no JIT (Eduard)
>>> * Avoid rewriting instruction and check function pointer directly
>>>    in JIT (Alexei)
>>> * Change includes to fix compile issues on non x86 architectures
>>> ---
>>>   arch/x86/net/bpf_jit_comp.c   | 39 +++++++++++++++++++++++++++++++++
>>>   arch/x86/net/bpf_jit_comp32.c | 14 ++++++++++++
>>>   include/linux/bpf.h           |  5 +++++
>>>   include/linux/filter.h        |  1 +
>>>   kernel/bpf/core.c             | 11 ++++++++++
>>>   kernel/bpf/helpers.c          | 21 ++++++++++++++++++
>>>   kernel/bpf/verifier.c         | 41 
>>> ++++++++++++++++++++++++++++++-----
>>>   7 files changed, 126 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>>> index a43fc5af973d..107bd921f104 100644
>>> --- a/arch/x86/net/bpf_jit_comp.c
>>> +++ b/arch/x86/net/bpf_jit_comp.c
>>> @@ -2185,6 +2185,37 @@ st:            if (is_imm8(insn->off))
>>>           case BPF_JMP | BPF_CALL: {
>>>               u8 *ip = image + addrs[i - 1];
>>> +            if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
>>> +                imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
>>> +                /* The default implementation of this kfunc uses
>>> +                 * __arch_get_hw_counter() which is implemented as
>>> +                 * `(u64)rdtsc_ordered() & S64_MAX`. We skip masking
>>> +                 * part because we assume it's not needed in BPF
>>> +                 * use case (two measurements close in time).
>>> +                 * Original code for rdtsc_ordered() uses sequence:
>>> +                 * 'rdtsc; nop; nop; nop' to patch it into
>>> +                 * 'lfence; rdtsc' or 'rdtscp' depending on CPU 
>>> features.
>>> +                 * JIT uses 'lfence; rdtsc' variant because BPF 
>>> program
>>> +                 * doesn't care about cookie provided by rdtsp in RCX.
>>
>> rdtsp -> tdtscp?
>>
>>> +                 * Save RDX because RDTSC will use EDX:EAX to 
>>> return u64
>>> +                 */
>>> +                emit_mov_reg(&prog, true, AUX_REG, BPF_REG_3);
>>> +                if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
>>> +                    EMIT_LFENCE();
>>> +                EMIT2(0x0F, 0x31);
>>> +
>>> +                /* shl RDX, 32 */
>>> +                maybe_emit_1mod(&prog, BPF_REG_3, true);
>>> +                EMIT3(0xC1, add_1reg(0xE0, BPF_REG_3), 32);
>>> +                /* or RAX, RDX */
>>> +                maybe_emit_mod(&prog, BPF_REG_0, BPF_REG_3, true);
>>> +                EMIT2(0x09, add_2reg(0xC0, BPF_REG_0, BPF_REG_3));
>>> +                /* restore RDX from R11 */
>>> +                emit_mov_reg(&prog, true, BPF_REG_3, AUX_REG);
>>> +
>>> +                break;
>>> +            }
>>> +
>>>               func = (u8 *) __bpf_call_base + imm32;
>>>               if (src_reg == BPF_PSEUDO_CALL && tail_call_reachable) {
>>>                   LOAD_TAIL_CALL_CNT_PTR(stack_depth);
>>> @@ -3791,3 +3822,11 @@ u64 bpf_arch_uaddress_limit(void)
>>>   {
>>>       return 0;
>>>   }
>>> +
>>> +/* x86-64 JIT can inline kfunc */
>>> +bool bpf_jit_inlines_kfunc_call(s32 imm)
>>> +{
>>> +    if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
>>> +        return true;
>>> +    return false;
>>> +}
>>> diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/ 
>>> bpf_jit_comp32.c
>>> index de0f9e5f9f73..e6097a371b69 100644
>>> --- a/arch/x86/net/bpf_jit_comp32.c
>>> +++ b/arch/x86/net/bpf_jit_comp32.c
>>> @@ -2094,6 +2094,13 @@ static int do_jit(struct bpf_prog *bpf_prog, 
>>> int *addrs, u8 *image,
>>>               if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
>>>                   int err;
>>> +                if (imm32 == BPF_CALL_IMM(bpf_get_cpu_cycles)) {
>>> +                    if (boot_cpu_has(X86_FEATURE_LFENCE_RDTSC))
>>> +                        EMIT3(0x0F, 0xAE, 0xE8);
>>> +                    EMIT2(0x0F, 0x31);
>>> +                    break;
>>> +                }
>>> +
>>>                   err = emit_kfunc_call(bpf_prog,
>>>                                 image + addrs[i],
>>>                                 insn, &prog);
>>> @@ -2621,3 +2628,10 @@ bool bpf_jit_supports_kfunc_call(void)
>>>   {
>>>       return true;
>>>   }
>>> +
>>> +bool bpf_jit_inlines_kfunc_call(s32 imm)
>>> +{
>>> +    if (imm == BPF_CALL_IMM(bpf_get_cpu_cycles))
>>> +        return true;
>>> +    return false;
>>> +}
>>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>>> index 3ace0d6227e3..43a5207a1591 100644
>>> --- a/include/linux/bpf.h
>>> +++ b/include/linux/bpf.h
>>> @@ -3333,6 +3333,11 @@ void bpf_user_rnd_init_once(void);
>>>   u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>>>   u64 bpf_get_raw_cpu_id(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>>> +/* Inlined kfuncs */
>>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>>> +u64 bpf_get_cpu_cycles(void);
>>> +#endif
>>> +
>>>   #if defined(CONFIG_NET)
>>>   bool bpf_sock_common_is_valid_access(int off, int size,
>>>                        enum bpf_access_type type,
>>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>>> index 3a21947f2fd4..9cf57233874f 100644
>>> --- a/include/linux/filter.h
>>> +++ b/include/linux/filter.h
>>> @@ -1111,6 +1111,7 @@ struct bpf_prog *bpf_int_jit_compile(struct 
>>> bpf_prog *prog);
>>>   void bpf_jit_compile(struct bpf_prog *prog);
>>>   bool bpf_jit_needs_zext(void);
>>>   bool bpf_jit_inlines_helper_call(s32 imm);
>>> +bool bpf_jit_inlines_kfunc_call(s32 imm);
>>>   bool bpf_jit_supports_subprog_tailcalls(void);
>>>   bool bpf_jit_supports_percpu_insn(void);
>>>   bool bpf_jit_supports_kfunc_call(void);
>>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>>> index 14d9288441f2..daa3ab458c8a 100644
>>> --- a/kernel/bpf/core.c
>>> +++ b/kernel/bpf/core.c
>>> @@ -2965,6 +2965,17 @@ bool __weak bpf_jit_inlines_helper_call(s32 imm)
>>>       return false;
>>>   }
>>> +/* Return true if the JIT inlines the call to the kfunc 
>>> corresponding to
>>> + * the imm.
>>> + *
>>> + * The verifier will not patch the insn->imm for the call to the 
>>> helper if
>>> + * this returns true.
>>> + */
>>> +bool __weak bpf_jit_inlines_kfunc_call(s32 imm)
>>> +{
>>> +    return false;
>>> +}
>>> +
>>>   /* Return TRUE if the JIT backend supports mixing bpf2bpf and 
>>> tailcalls. */
>>>   bool __weak bpf_jit_supports_subprog_tailcalls(void)
>>>   {
>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>> index 751c150f9e1c..12d40537e57b 100644
>>> --- a/kernel/bpf/helpers.c
>>> +++ b/kernel/bpf/helpers.c
>>> @@ -23,6 +23,10 @@
>>>   #include <linux/btf_ids.h>
>>>   #include <linux/bpf_mem_alloc.h>
>>>   #include <linux/kasan.h>
>>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>>> +#include <vdso/datapage.h>
>>> +#include <asm/vdso/vsyscall.h>
>>> +#endif
>>>   #include "../../lib/kstrtox.h"
>>> @@ -3057,6 +3061,20 @@ __bpf_kfunc int bpf_copy_from_user_str(void 
>>> *dst, u32 dst__sz, const void __user
>>>       return ret + 1;
>>>   }
>>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>>> +__bpf_kfunc u64 bpf_get_cpu_cycles(void)
>>> +{
>>> +    const struct vdso_data *vd = __arch_get_k_vdso_data();
>>> +
>>> +    vd = &vd[CS_RAW];
>>> +
>>> +    /* CS_RAW clock_mode translates to VDSO_CLOCKMODE_TSC on x86 and
>>> +     * to VDSO_CLOCKMODE_ARCHTIMER on aarch64/risc-v.
>>> +     */
>>> +    return __arch_get_hw_counter(vd->clock_mode, vd);
>>> +}
>>> +#endif
>>> +
>>>   __bpf_kfunc_end_defs();
>>>   BTF_KFUNCS_START(generic_btf_ids)
>>> @@ -3149,6 +3167,9 @@ BTF_ID_FLAGS(func, bpf_get_kmem_cache)
>>>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_new, KF_ITER_NEW | 
>>> KF_SLEEPABLE)
>>>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | 
>>> KF_RET_NULL | KF_SLEEPABLE)
>>>   BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | 
>>> KF_SLEEPABLE)
>>> +#if IS_ENABLED(CONFIG_GENERIC_GETTIMEOFDAY)
>>> +BTF_ID_FLAGS(func, bpf_get_cpu_cycles, KF_FASTCALL)
>>> +#endif
>>>   BTF_KFUNCS_END(common_btf_ids)
>>>   static const struct btf_kfunc_id_set common_kfunc_set = {
>> [...]
>

Re: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc

[kernel test robot]

Hi Vadim,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/bpf-add-bpf_get_cpu_cycles-kfunc/20241117-002106
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20241115194841.2108634-2-vadfed%40meta.com
patch subject: [PATCH bpf-next v6 1/4] bpf: add bpf_get_cpu_cycles kfunc
config: x86_64-randconfig-075-20241117 (https://download.01.org/0day-ci/archive/20241118/202411180657.c0eoNysc-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241118/202411180657.c0eoNysc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411180657.c0eoNysc-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `vread_pvclock':
>> arch/x86/include/asm/vdso/gettimeofday.h:198: undefined reference to `pvclock_page'
>> ld: arch/x86/include/asm/vdso/gettimeofday.h:198: undefined reference to `pvclock_page'
   ld: vmlinux.o: in function `pvclock_read_begin':
>> arch/x86/include/asm/pvclock.h:25: undefined reference to `pvclock_page'
   ld: vmlinux.o: in function `vread_pvclock':
   arch/x86/include/asm/vdso/gettimeofday.h:228: undefined reference to `pvclock_page'
   ld: vmlinux.o: in function `__pvclock_read_cycles':
   arch/x86/include/asm/pvclock.h:86: undefined reference to `pvclock_page'
   ld: vmlinux.o:arch/x86/include/asm/pvclock.h:88: more undefined references to `pvclock_page' follow
   ld: .tmp_vmlinux1: hidden symbol `pvclock_page' isn't defined
   ld: final link failed: bad value


vim +198 arch/x86/include/asm/vdso/gettimeofday.h

7ac8707479886c Vincenzo Frascino 2019-06-21  195  
7ac8707479886c Vincenzo Frascino 2019-06-21  196  #ifdef CONFIG_PARAVIRT_CLOCK
7ac8707479886c Vincenzo Frascino 2019-06-21  197  static u64 vread_pvclock(void)
7ac8707479886c Vincenzo Frascino 2019-06-21 @198  {
ecf9db3d1f1a8f Andy Lutomirski   2019-06-22  199  	const struct pvclock_vcpu_time_info *pvti = &pvclock_page.pvti;
7ac8707479886c Vincenzo Frascino 2019-06-21  200  	u32 version;
7ac8707479886c Vincenzo Frascino 2019-06-21  201  	u64 ret;
7ac8707479886c Vincenzo Frascino 2019-06-21  202  
7ac8707479886c Vincenzo Frascino 2019-06-21  203  	/*
7ac8707479886c Vincenzo Frascino 2019-06-21  204  	 * Note: The kernel and hypervisor must guarantee that cpu ID
7ac8707479886c Vincenzo Frascino 2019-06-21  205  	 * number maps 1:1 to per-CPU pvclock time info.
7ac8707479886c Vincenzo Frascino 2019-06-21  206  	 *
7ac8707479886c Vincenzo Frascino 2019-06-21  207  	 * Because the hypervisor is entirely unaware of guest userspace
7ac8707479886c Vincenzo Frascino 2019-06-21  208  	 * preemption, it cannot guarantee that per-CPU pvclock time
7ac8707479886c Vincenzo Frascino 2019-06-21  209  	 * info is updated if the underlying CPU changes or that that
7ac8707479886c Vincenzo Frascino 2019-06-21  210  	 * version is increased whenever underlying CPU changes.
7ac8707479886c Vincenzo Frascino 2019-06-21  211  	 *
7ac8707479886c Vincenzo Frascino 2019-06-21  212  	 * On KVM, we are guaranteed that pvti updates for any vCPU are
7ac8707479886c Vincenzo Frascino 2019-06-21  213  	 * atomic as seen by *all* vCPUs.  This is an even stronger
7ac8707479886c Vincenzo Frascino 2019-06-21  214  	 * guarantee than we get with a normal seqlock.
7ac8707479886c Vincenzo Frascino 2019-06-21  215  	 *
7ac8707479886c Vincenzo Frascino 2019-06-21  216  	 * On Xen, we don't appear to have that guarantee, but Xen still
7ac8707479886c Vincenzo Frascino 2019-06-21  217  	 * supplies a valid seqlock using the version field.
7ac8707479886c Vincenzo Frascino 2019-06-21  218  	 *
7ac8707479886c Vincenzo Frascino 2019-06-21  219  	 * We only do pvclock vdso timing at all if
7ac8707479886c Vincenzo Frascino 2019-06-21  220  	 * PVCLOCK_TSC_STABLE_BIT is set, and we interpret that bit to
7ac8707479886c Vincenzo Frascino 2019-06-21  221  	 * mean that all vCPUs have matching pvti and that the TSC is
7ac8707479886c Vincenzo Frascino 2019-06-21  222  	 * synced, so we can just look at vCPU 0's pvti.
7ac8707479886c Vincenzo Frascino 2019-06-21  223  	 */
7ac8707479886c Vincenzo Frascino 2019-06-21  224  
7ac8707479886c Vincenzo Frascino 2019-06-21  225  	do {
7ac8707479886c Vincenzo Frascino 2019-06-21  226  		version = pvclock_read_begin(pvti);
7ac8707479886c Vincenzo Frascino 2019-06-21  227  
7ac8707479886c Vincenzo Frascino 2019-06-21  228  		if (unlikely(!(pvti->flags & PVCLOCK_TSC_STABLE_BIT)))
7ac8707479886c Vincenzo Frascino 2019-06-21  229  			return U64_MAX;
7ac8707479886c Vincenzo Frascino 2019-06-21  230  
7ac8707479886c Vincenzo Frascino 2019-06-21  231  		ret = __pvclock_read_cycles(pvti, rdtsc_ordered());
7ac8707479886c Vincenzo Frascino 2019-06-21  232  	} while (pvclock_read_retry(pvti, version));
7ac8707479886c Vincenzo Frascino 2019-06-21  233  
77750f78b0b324 Peter Zijlstra    2023-05-19  234  	return ret & S64_MAX;
7ac8707479886c Vincenzo Frascino 2019-06-21  235  }
7ac8707479886c Vincenzo Frascino 2019-06-21  236  #endif
7ac8707479886c Vincenzo Frascino 2019-06-21  237  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki