Re: [PATCH] selftests: vDSO: Do not rely on $ARCH for vdso_test_getrandom && vdso_test_chacha

Christophe Leroy
2024-09-01T18:00:33Z

Hi Jason,

Le 01/09/2024 à 15:22, Jason A. Donenfeld a écrit :
> Hi Christophe,
>
> Hmm, I'm not so sure I like this very much. I think it's important for
> these tests to fail when an arch tries to hook up the function to the
> vDSO, but it's still not exported for some reason. This also regresses
> the ARCH=x86_64 vs ARCH=x86 thing, which SRCARCH fixes.
>
> What about, instead, something like below, replacing the other commit?

I need to look at it in more details and perfom a test, but after first
look I can't figure out how it would work.

When I build selftests,

to build 32 bits selftests I do:

make ARCH=powerpc CROSS_COMPILE=ppc-linux-

to build a 64 bits BE selftests I do:

make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-

to build a 64 bits LE selftests I do:

make ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-

I addition, in case someone does the build on a native platform directly,

On 32 bits, uname -m returns 'ppc'
On 64 bits, uname -m returns 'ppc64'
On 64 bits little endian, uname -m returns 'ppc64le'

How would this fit in the logic where IIUC you just remove '_64' from
'x86_64' to get 'x86'

Christophe

>
> Jason
>
> From ccc53425c98f4f5c2a1edaf775089efb56bd106e Mon Sep 17 00:00:00 2001
> From: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Date: Sun, 1 Sep 2024 15:05:01 +0200
> Subject: [PATCH] selftests: vDSO: fix cross build for getrandom and chacha
> tests
>
> Unlike the check for the standalone x86 test, the check for building the
> vDSO getrandom and chacaha tests looks at the architecture for the host
> rather than the architecture for the target when deciding if they should
> be built. Since the chacha test includes some assembler code this means
> that cross building with x86 as either the target or host is broken.
>
> There's also some additional complications, where ARCH can legitimately
> be either x86_64 or x86, but the source code we need to compile lives in
> a directory path containing arch/x86. The standard SRCARCH variable
> handles that. And actually, all these variables and proper substitutions
> are already described in tools/scripts/Makefile.arch, so just include
> that to handle it.
>
> Similarly, ARCH=x86 can actually describe ARCH=x86_64,
> just with CONFIG_64BIT, so we can't rely on ARCH for selecting
> non-32-bit tests. For that, check against $(ARCH)$(CONFIG_X86_32). This
> won't help for people manually running this inside the vDSO selftest
> directory (which isn't really supported anyway and has problems on
> various archs), but it should work for builds of the kselftests, where
> the CONFIG_* variables are defined. On x86_64 machines,
> $(ARCH)$(CONFIG_X86_32) will evaluate to x86. On arm64 machines, it will
> evaluate to arm64. On 32-bit x86 machines, it will evaluate to x86y,
> which won't match the filter list.
>
> Reported-by: Mark Brown <broonie@kernel.org>
> Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> tools/testing/selftests/vDSO/Makefile | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> index e21e78aae24d..01a5805062b3 100644
> --- a/tools/testing/selftests/vDSO/Makefile
> +++ b/tools/testing/selftests/vDSO/Makefile
> @@ -1,6 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
> -uname_M := $(shell uname -m 2>/dev/null || echo not)
> -ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
> +include ../../../scripts/Makefile.arch
>
> TEST_GEN_PROGS := vdso_test_gettimeofday
> TEST_GEN_PROGS += vdso_test_getcpu
> @@ -10,7 +9,7 @@ ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
> TEST_GEN_PROGS += vdso_standalone_test_x86
> endif
> TEST_GEN_PROGS += vdso_test_correctness
> -ifeq ($(uname_M),x86_64)
> +ifeq ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86))
> TEST_GEN_PROGS += vdso_test_getrandom
> TEST_GEN_PROGS += vdso_test_chacha
> endif
> @@ -38,8 +37,8 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
> $(KHDR_INCLUDES) \
> -isystem $(top_srcdir)/include/uapi
>
> -$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
> +$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S
> $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
> - -idirafter $(top_srcdir)/arch/$(ARCH)/include \
> + -idirafter $(top_srcdir)/arch/$(SRCARCH)/include \
> -idirafter $(top_srcdir)/include \
> -D__ASSEMBLY__ -Wa,--noexecstack
> --
> 2.46.0
>
>