Re: [PATCH v10 4/8] selftest/bpf: Enable verifier selftest for powerpc64

[email protected]
Newsgroups org.ozlabs.lists.linuxppc-dev,org.kernel.vger.bpf,org.kernel.vger.linux-kselftest,org.kernel.vger.stable
Message-ID <5ef8aa40184d48efe9c1fb0def756263a654c22e4908bc803d0d92426e07ef52@mail.kernel.org>
> selftest/bpf: Enable verifier selftest for powerpc64
>
> This patch enables arch specifier "__powerpc64" in verifier
> selftest for ppc64. Power 32-bit would require separate
> handling. Changes tested for 64-bit only.

This isn't a bug, but would it read more precisely to name the new tag
as __arch_powerpc64 (matching the macro added in bpf_misc.h) rather
than "__powerpc64"?  The actual macro expands to __arch("POWERPC64"),
and "__powerpc64__" is the compiler predefine used in test_loader.c,
but there's no __powerpc64 tag itself.

Also, would phrasing it imperatively match the series style better?
For example, "Add __arch_powerpc64 so verifier tests can gate on ppc64"
instead of "This patch enables..."?

> diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
> index 5eacf1b432521..c35359c97e44b 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_misc.h
> +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
> @@ -159,6 +159,7 @@
>  #define __arch_riscv64		__arch("RISCV64")
>  #define __arch_s390x		__arch("s390x")
>  #define __arch_loongarch	__arch("LOONGARCH")
> +#define __arch_powerpc64	__arch("POWERPC64")
>  #define __caps_unpriv(caps)	__test_tag("test_caps_unpriv=" EXPAND_QUOTE(caps))
>  #define __load_if_JITed()	__test_tag("load_mode=jited")
>  #define __load_if_no_JITed()	__test_tag("load_mode=no_jited")
> diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
> index 07807757b518d..221393f2a8ef4 100644
> --- a/tools/testing/selftests/bpf/test_loader.c
> +++ b/tools/testing/selftests/bpf/test_loader.c
> @@ -378,6 +378,7 @@ enum arch {
>  	ARCH_RISCV64	= 0x8,
>  	ARCH_S390X	= 0x10,
>  	ARCH_LOONGARCH	= 0x20,
> +	ARCH_POWERPC64	= 0x40,
>  };
>
>  static int get_current_arch(void)
> @@ -392,6 +393,8 @@ static int get_current_arch(void)
>  	return ARCH_S390X;
>  #elif defined(__loongarch__)
>  	return ARCH_LOONGARCH;
> +#elif defined(__powerpc64__)
> +	return ARCH_POWERPC64;
>  #endif
>  	return ARCH_UNKNOWN;
>  }
> @@ -585,6 +588,8 @@ static int parse_test_spec(struct test_loader *tester,
>  				arch = ARCH_S390X;
>  			} else if (strcmp(val, "LOONGARCH") == 0) {
>  				arch = ARCH_LOONGARCH;
> +			} else if (strcmp(val, "POWERPC64") == 0) {
> +				arch = ARCH_POWERPC64;
>  			} else {
>  				PRINT_FAIL("bad arch spec: '%s'\n", val);
>  				err = -EINVAL;



---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30981212698
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.