On Thu, Jul 4, 2024 at 3:24 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Add annotations __arch_x86_64, __arch_arm64, __arch_riscv64
> to specify on which architecture the test case should be tested.
> Several __arch_* annotations could be specified at once.
> When test case is not run on current arch it is marked as skipped.
>
> For example, the following would be tested only on arm64 and riscv64:
>
> SEC("raw_tp")
> __arch_arm64
> __arch_riscv64
> __xlated("1: *(u64 *)(r10 - 16) = r1")
> __xlated("2: call")
> __xlated("3: r1 = *(u64 *)(r10 - 16);")
> __success
> __naked void canary_arm64_riscv64(void)
> {
> asm volatile (
> "r1 = 1;"
> "*(u64 *)(r10 - 16) = r1;"
> "call %[bpf_get_smp_processor_id];"
> "r1 = *(u64 *)(r10 - 16);"
> "exit;"
> :
> : __imm(bpf_get_smp_processor_id)
> : __clobber_all);
> }
>
> On x86 it would be skipped:
>
> #467/2 verifier_nocsr/canary_arm64_riscv64:SKIP
>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> tools/testing/selftests/bpf/progs/bpf_misc.h | 8 ++++
> tools/testing/selftests/bpf/test_loader.c | 43 ++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
LGTM, just being pedantic below, because why not? ;)
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
[...]
> + spec->arch_mask = arch_mask;
> +
> if (spec->mode_mask == 0)
> spec->mode_mask = PRIV;
>
> @@ -677,6 +701,20 @@ static int get_xlated_program_text(int prog_fd, char *text, size_t text_sz)
> return err;
> }
>
> +static bool run_on_current_arch(int arch_mask)
> +{
> + if (arch_mask == 0)
> + return true;
> +#if defined(__x86_64__)
> + return !!(arch_mask & ARCH_X86_64);
nit: !! is needed if you assign the result to integer and you want
either 0 or 1 (and not whatever the bit mask value is). In this case
it's well defined that a non-zero value will be implicitly converted
to true for function result, so just `return arch_mask & ARCH_X86_64;`
is totally fine and cleaner.
> +#elif defined(__aarch64__)
> + return !!(arch_mask & ARCH_ARM64);
> +#elif defined(__riscv) && __riscv_xlen == 64
> + return !!(arch_mask & ARCH_RISCV64);
> +#endif
> + return false;
> +}
> +
> /* this function is forced noinline and has short generic name to look better
> * in test_progs output (in case of a failure)
> */
> @@ -701,6 +739,11 @@ void run_subtest(struct test_loader *tester,
> if (!test__start_subtest(subspec->name))
> return;
>
> + if (!run_on_current_arch(spec->arch_mask)) {
> + test__skip();
> + return;
> + }
> +
> if (unpriv) {
> if (!can_execute_unpriv(tester, spec)) {
> test__skip();
> --
> 2.45.2
>
