This patch introduces a new probe to check whether the kernel supports
instruction set extensions v4. The v4 extension comprises several new
instructions: BPF_{SDIV,SMOD} (signed div and mod), BPF_{LD,LDX,ST,STX,MOV}
(sign-extended load/store/move), 32-bit BPF_JA (unconditional jump),
target-independent BPF_ALU64 BSWAP (byte-swapping 16/32/64). These have
been introduced in the following commits respectively:

* ec0e2da ("bpf: Support new signed div/mod instructions.")
* 1f9a1ea ("bpf: Support new sign-extension load insns")
* 8100928 ("bpf: Support new sign-extension mov insns")
* 4cd58e9 ("bpf: Support new 32bit offset jmp instruction")
* 0845c3d ("bpf: Support new unconditional bswap instruction")

Support in bpftool for previous ISA extensions were added in commit
0fd800b2 ("bpftool: Probe for instruction set extensions"). These probes
are useful for userspace BPF projects that want to use newer
instruction set extensions on newer kernels, to reduce the programs'
sizes or their complexity. LLVM provides the mcpu=v4 option since commit
"[BPF] support for BPF_ST instruction in codegen"
(https://github.com/llvm/llvm-project/commit/8f28e8069c4ba1110daee8bddc4d5049b6d4646e).

Changelog:

- v2:
  - moved BPF_JMP32_A macro after BPF_JMP_A in filter.h for consistency
    with include/linux/filter.h, noted by Quentin Monnet <qmo@kernel.org>

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
---
 tools/bpf/bpftool/feature.c  | 23 +++++++++++++++++++++++
 tools/include/linux/filter.h | 10 ++++++++++
 2 files changed, 33 insertions(+)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 4dbc4fcdf473..24fecdf8e430 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -885,6 +885,28 @@ probe_v3_isa_extension(const char *define_prefix, __u32 ifindex)
 			   "V3_ISA_EXTENSION");
 }

+/*
+ * Probe for the v4 instruction set extension introduced in commit 1f9a1ea821ff
+ * ("bpf: Support new sign-extension load insns").
+ */
+static void
+probe_v4_isa_extension(const char *define_prefix, __u32 ifindex)
+{
+	struct bpf_insn insns[5] = {
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_JMP32_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+		BPF_JMP32_A(1),
+		BPF_MOV64_IMM(BPF_REG_0, 1),
+		BPF_EXIT_INSN()
+	};
+
+	probe_misc_feature(insns, ARRAY_SIZE(insns),
+			   define_prefix, ifindex,
+			   "have_v4_isa_extension",
+			   "ISA extension v4",
+			   "V4_ISA_EXTENSION");
+}
+
 static void
 section_system_config(enum probe_component target, const char *define_prefix)
 {
@@ -1029,6 +1051,7 @@ static void section_misc(const char *define_prefix, __u32 ifindex)
 	probe_bounded_loops(define_prefix, ifindex);
 	probe_v2_isa_extension(define_prefix, ifindex);
 	probe_v3_isa_extension(define_prefix, ifindex);
+	probe_v4_isa_extension(define_prefix, ifindex);
 	print_end_section();
 }

diff --git a/tools/include/linux/filter.h b/tools/include/linux/filter.h
index 65aa8ce142e5..bcc6df79301a 100644
--- a/tools/include/linux/filter.h
+++ b/tools/include/linux/filter.h
@@ -273,6 +273,16 @@
 		.off   = OFF,					\
 		.imm   = 0 })

+/* Unconditional jumps, gotol pc + imm32 */
+
+#define BPF_JMP32_A(IMM)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_JMP32 | BPF_JA,			\
+		.dst_reg = 0,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = IMM })
+
 /* Function call */

 #define BPF_EMIT_CALL(FUNC)					\
--
2.43.0

Many thanks Quentin for the feedback!
Updated changelog in the commit message with v2, moving the macro after BPF_JMP_A.
Also, adding in cc other maintainers as you suggested.

On 09/12/2024 14:54, Simone Magnani wrote:
> This patch introduces a new probe to check whether the kernel supports
> instruction set extensions v4. The v4 extension comprises several new
> instructions: BPF_{SDIV,SMOD} (signed div and mod), BPF_{LD,LDX,ST,STX,MOV}
> (sign-extended load/store/move), 32-bit BPF_JA (unconditional jump),
> target-independent BPF_ALU64 BSWAP (byte-swapping 16/32/64). These have
> been introduced in the following commits respectively:
> 
> * ec0e2da ("bpf: Support new signed div/mod instructions.")
> * 1f9a1ea ("bpf: Support new sign-extension load insns")
> * 8100928 ("bpf: Support new sign-extension mov insns")
> * 4cd58e9 ("bpf: Support new 32bit offset jmp instruction")
> * 0845c3d ("bpf: Support new unconditional bswap instruction")
> 
> Support in bpftool for previous ISA extensions were added in commit
> 0fd800b2 ("bpftool: Probe for instruction set extensions"). These probes
> are useful for userspace BPF projects that want to use newer
> instruction set extensions on newer kernels, to reduce the programs'
> sizes or their complexity. LLVM provides the mcpu=v4 option since commit
> "[BPF] support for BPF_ST instruction in codegen"
> (https://github.com/llvm/llvm-project/commit/8f28e8069c4ba1110daee8bddc4d5049b6d4646e).
> 
> Changelog:
> 
> - v2:
>   - moved BPF_JMP32_A macro after BPF_JMP_A in filter.h for consistency
>     with include/linux/filter.h, noted by Quentin Monnet <qmo@kernel.org>
> 
> Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
> ---
>  tools/bpf/bpftool/feature.c  | 23 +++++++++++++++++++++++
>  tools/include/linux/filter.h | 10 ++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 4dbc4fcdf473..24fecdf8e430 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -885,6 +885,28 @@ probe_v3_isa_extension(const char *define_prefix, __u32 ifindex)
>  			   "V3_ISA_EXTENSION");
>  }
> 
> +/*
> + * Probe for the v4 instruction set extension introduced in commit 1f9a1ea821ff
> + * ("bpf: Support new sign-extension load insns").
> + */
> +static void
> +probe_v4_isa_extension(const char *define_prefix, __u32 ifindex)
> +{
> +	struct bpf_insn insns[5] = {
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_JMP32_IMM(BPF_JEQ, BPF_REG_0, 1, 1),


Looking again at the probe itself, does the second instruction serve any
practical purpose here? Don't you just need to test the BPF_JMP32_A?

Looks good otherwise, thank you!

Reviewed-by: Quentin Monnet <qmo@kernel.org>


> +		BPF_JMP32_A(1),
> +		BPF_MOV64_IMM(BPF_REG_0, 1),
> +		BPF_EXIT_INSN()
> +	};

On 09/12/24 16:20, Quentin Monnet wrote:
> Looking again at the probe itself, does the second instruction serve any
> practical purpose here? Don't you just need to test the BPF_JMP32_A?
> 
> Looks good otherwise, thank you!
> 
> Reviewed-by: Quentin Monnet <qmo@kernel.org>

I wanted to keep probes similar to the previous ones (especially v3
and v2), despite we never check their return codes. This means
having as 4th instruction `BPF_MOV64_IMM(BPF_REG_0, 1)`. However,
to do so, I also need the 2nd instruction, otherwise I'd hit an
`Invalid Argument` error while calling `bpf_prog_load()`: I think
that would be due to the fact that no execution paths would
execute that instruction otherwise.

An alternative approach less consistent with the others would be:

struct bpf_insn insns[3] = {
		BPF_MOV64_IMM(BPF_REG_0, 0),
		BPF_JMP32_A(0),
		BPF_EXIT_INSN()
	};

Please let me know if you have any further questions, need
additional information, or if I could improve the patch.

2024-12-09 18:01 UTC+0100 ~ Simone Magnani <simone.magnani@isovalent.com>
> On 09/12/24 16:20, Quentin Monnet wrote:
>> Looking again at the probe itself, does the second instruction serve any
>> practical purpose here? Don't you just need to test the BPF_JMP32_A?
>>
>> Looks good otherwise, thank you!
>>
>> Reviewed-by: Quentin Monnet <qmo@kernel.org>
> 
> I wanted to keep probes similar to the previous ones (especially v3
> and v2), despite we never check their return codes. This means
> having as 4th instruction `BPF_MOV64_IMM(BPF_REG_0, 1)`. However,
> to do so, I also need the 2nd instruction, otherwise I'd hit an
> `Invalid Argument` error while calling `bpf_prog_load()`: I think
> that would be due to the fact that no execution paths would
> execute that instruction otherwise.


Right, that's what I missed.


> 
> An alternative approach less consistent with the others would be:
> 
> struct bpf_insn insns[3] = {
> 		BPF_MOV64_IMM(BPF_REG_0, 0),
> 		BPF_JMP32_A(0),
> 		BPF_EXIT_INSN()
> 	};
> 
> Please let me know if you have any further questions, need
> additional information, or if I could improve the patch.


No it's all good to me in that case, thank you!

Quentin