When trying to run the arm64 MTE (Memory Tagging Extension) selftests on a model with the new FEAT_MTE3 capability, the MTE feature detection failed, because it was overzealously checking for one exact feature version only (0b0010). Trying to fix that (patch 06/11) led me into the rabbit hole of userland tool compilation, which triggered patches 01-05/11, to let me actually compile the selftests on an arm64 machine running Ubuntu 20.04. Before I actually fixed that, I tried some other compiler and distro; patches 07 and 08 are my witnesses. Then I got brave and tried clang: entering patches 09/11 and 10/11. Eventually I tried to run the whole thing on that model again, and, you guessed it, patch 11/11 concludes this apparent "2 minute job".
Eventually I can now compile the mte selftests on Ubuntu 20.04 with both the native gcc and clang without warnings, also with some custom made cross compiler. And they even run now!
Please have a look, also you may try to compile it on your setup, if you feel adventurous: $ make -C tools/testing/selftests TARGETS=arm64 ARM64_SUBTARGETS=mte
Cheers, Andre
Andre Przywara (11): kselftest/arm64: mte: Fix compilation with native compiler kselftest/arm64: mte: Fix pthread linking kselftest/arm64: mte: ksm_options: Fix fscanf warning kselftest/arm64: mte: user_mem: Fix write() warning kselftest/arm64: mte: common: Fix write() warnings kselftest/arm64: mte: Fix MTE feature detection kselftest/arm64: mte: Use cross-compiler if specified kselftest/arm64: mte: Output warning about failing compiler kselftest/arm64: mte: Makefile: Fix clang compilation kselftest/arm64: mte: Fix clang warning kselftest/arm64: mte: Report filename on failing temp file creation
tools/testing/selftests/arm64/mte/Makefile | 15 +++++-- .../selftests/arm64/mte/check_ksm_options.c | 5 ++- .../selftests/arm64/mte/check_user_mem.c | 3 +- .../selftests/arm64/mte/mte_common_util.c | 39 +++++++++++-------- 4 files changed, 39 insertions(+), 23 deletions(-)
The mte selftest Makefile contains a check for GCC, to add the memtag -march flag to the compiler options. This check fails if the compiler is not explicitly specified, so reverts to the standard "cc", in which case --version doesn't mention the "gcc" string we match against: $ cc --version | head -n 1 cc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
This will not add the -march switch to the command line, so compilation fails: mte_helper.S: Assembler messages: mte_helper.S:25: Error: selected processor does not support `irg x0,x0,xzr' mte_helper.S:38: Error: selected processor does not support `gmi x1,x0,xzr' ...
Actually clang accepts the same -march option as well, so we can just drop this check and add this unconditionally to the command line, to avoid any future issues with this check altogether (gcc actually prints basename(argv[0]) when called with --version).
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/Makefile | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index 0b3af552632a..df15d44aeb8d 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -6,9 +6,7 @@ SRCS := $(filter-out mte_common_util.c,$(wildcard *.c)) PROGS := $(patsubst %.c,%,$(SRCS))
#Add mte compiler option -ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep gcc),) CFLAGS += -march=armv8.5-a+memtag -endif
#check if the compiler works well mte_cc_support := $(shell if ($(CC) $(CFLAGS) -E -x c /dev/null -o /dev/null 2>&1) then echo "1"; fi)
On Fri, Mar 19, 2021 at 9:53 AM Andre Przywara andre.przywara@arm.com wrote:
The mte selftest Makefile contains a check for GCC, to add the memtag -march flag to the compiler options. This check fails if the compiler is not explicitly specified, so reverts to the standard "cc", in which case --version doesn't mention the "gcc" string we match against: $ cc --version | head -n 1 cc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
This will not add the -march switch to the command line, so compilation fails: mte_helper.S: Assembler messages: mte_helper.S:25: Error: selected processor does not support `irg x0,x0,xzr' mte_helper.S:38: Error: selected processor does not support `gmi x1,x0,xzr' ...
Actually clang accepts the same -march option as well, so we can just drop this check and add this unconditionally to the command line, to avoid any future issues with this check altogether (gcc actually prints basename(argv[0]) when called with --version).
Signed-off-by: Andre Przywara andre.przywara@arm.com
Reviewed-by: Nick Desaulniers ndesaulniers@google.com
tools/testing/selftests/arm64/mte/Makefile | 2 -- 1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index 0b3af552632a..df15d44aeb8d 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -6,9 +6,7 @@ SRCS := $(filter-out mte_common_util.c,$(wildcard *.c)) PROGS := $(patsubst %.c,%,$(SRCS))
#Add mte compiler option -ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep gcc),) CFLAGS += -march=armv8.5-a+memtag -endif
#check if the compiler works well mte_cc_support := $(shell if ($(CC) $(CFLAGS) -E -x c /dev/null -o /dev/null 2>&1) then echo "1"; fi) -- 2.17.5
The GCC manual suggests to use -pthread, when linking with the PThread library, also to add this switch to both the compilation and linking stages.
Do as the manual says, to fix compilation with Ubuntu's 20.04 toolchain, which was getting -lpthread too early on the command line: ------------ /usr/bin/ld: /tmp/cc5zbo2A.o: in function `execute_test': tools/testing/selftests/arm64/mte/check_gcr_el1_cswitch.c:86: undefined reference to `pthread_create' /usr/bin/ld: tools/testing/selftests/arm64/mte/check_gcr_el1_cswitch.c:90: undefined reference to `pthread_join' ------------
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index df15d44aeb8d..90aadd86fa0d 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -1,7 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 # Copyright (C) 2020 ARM Limited
-CFLAGS += -std=gnu99 -I. -lpthread +CFLAGS += -std=gnu99 -I. -pthread +LDFLAGS += -pthread SRCS := $(filter-out mte_common_util.c,$(wildcard *.c)) PROGS := $(patsubst %.c,%,$(SRCS))
Out of the box Ubuntu's 20.04 compiler warns about missing return value checks for fscanf() calls.
Make GCC happy by checking whether we actually parsed one integer.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/check_ksm_options.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/mte/check_ksm_options.c b/tools/testing/selftests/arm64/mte/check_ksm_options.c index 3b23c4d61d38..88c74bc46d4f 100644 --- a/tools/testing/selftests/arm64/mte/check_ksm_options.c +++ b/tools/testing/selftests/arm64/mte/check_ksm_options.c @@ -33,7 +33,10 @@ static unsigned long read_sysfs(char *str) ksft_print_msg("ERR: missing %s\n", str); return 0; } - fscanf(f, "%lu", &val); + if (fscanf(f, "%lu", &val) != 1) { + ksft_print_msg("ERR: parsing %s\n", str); + val = 0; + } fclose(f); return val; }
Out of the box Ubuntu's 20.04 compiler warns about missing return value checks for write() (sys)calls.
Make GCC happy by checking whether we actually managed to write "val".
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/check_user_mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c index 4bfa80f2a8c3..1de7a0abd0ae 100644 --- a/tools/testing/selftests/arm64/mte/check_user_mem.c +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c @@ -33,7 +33,8 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping) if (fd == -1) return KSFT_FAIL; for (i = 0; i < len; i++) - write(fd, &val, sizeof(val)); + if (write(fd, &val, sizeof(val)) != sizeof(val)) + return KSFT_FAIL; lseek(fd, 0, 0); ptr = mte_allocate_memory(len, mem_type, mapping, true); if (check_allocated_memory(ptr, len, mem_type, true) != KSFT_PASS) {
Out of the box Ubuntu's 20.04 compiler warns about missing return value checks for write() (sys)calls.
Make GCC happy by checking whether we actually managed to write out our buffer.
Reviewed-by: Andre Przywara andre.przywara@arm.com --- .../selftests/arm64/mte/mte_common_util.c | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 39f8908988ea..4e887dad762d 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -181,10 +181,17 @@ void *mte_allocate_file_memory(size_t size, int mem_type, int mapping, bool tags } /* Initialize the file for mappable size */ lseek(fd, 0, SEEK_SET); - for (index = INIT_BUFFER_SIZE; index < size; index += INIT_BUFFER_SIZE) - write(fd, buffer, INIT_BUFFER_SIZE); + for (index = INIT_BUFFER_SIZE; index < size; index += INIT_BUFFER_SIZE) { + if (write(fd, buffer, INIT_BUFFER_SIZE) != INIT_BUFFER_SIZE) { + perror("initialising buffer"); + return NULL; + } + } index -= INIT_BUFFER_SIZE; - write(fd, buffer, size - index); + if (write(fd, buffer, size - index) != size - index) { + perror("initialising buffer"); + return NULL; + } return __mte_allocate_memory_range(size, mem_type, mapping, 0, 0, tags, fd); }
@@ -202,9 +209,15 @@ void *mte_allocate_file_memory_tag_range(size_t size, int mem_type, int mapping, /* Initialize the file for mappable size */ lseek(fd, 0, SEEK_SET); for (index = INIT_BUFFER_SIZE; index < map_size; index += INIT_BUFFER_SIZE) - write(fd, buffer, INIT_BUFFER_SIZE); + if (write(fd, buffer, INIT_BUFFER_SIZE) != INIT_BUFFER_SIZE) { + perror("initialising buffer"); + return NULL; + } index -= INIT_BUFFER_SIZE; - write(fd, buffer, map_size - index); + if (write(fd, buffer, map_size - index) != map_size - index) { + perror("initialising buffer"); + return NULL; + } return __mte_allocate_memory_range(size, mem_type, mapping, range_before, range_after, true, fd); }
To check whether the CPU and kernel support the MTE features we want to test, we use an (emulated) CPU ID register read. However we only check against a very particular feature version (0b0010), even though the ARM ARM promises ID register features to be backwards compatible.
While this could be fixed by using ">=" instead of "==", we should actually use the explicit HWCAP2_MTE hardware capability, exposed by the kernel via the ELF auxiliary vectors.
That moves this responsibility to the kernel, and fixes running the tests on machines with FEAT_MTE3 capability.
Reviewed-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/mte_common_util.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 4e887dad762d..aa8a8a6b8b6d 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -291,22 +291,13 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask) return 0; }
-#define ID_AA64PFR1_MTE_SHIFT 8 -#define ID_AA64PFR1_MTE 2 - int mte_default_setup(void) { - unsigned long hwcaps = getauxval(AT_HWCAP); + unsigned long hwcaps2 = getauxval(AT_HWCAP2); unsigned long en = 0; int ret;
- if (!(hwcaps & HWCAP_CPUID)) { - ksft_print_msg("FAIL: CPUID registers unavailable\n"); - return KSFT_FAIL; - } - /* Read ID_AA64PFR1_EL1 register */ - asm volatile("mrs %0, id_aa64pfr1_el1" : "=r"(hwcaps) : : "memory"); - if (((hwcaps >> ID_AA64PFR1_MTE_SHIFT) & MT_TAG_MASK) != ID_AA64PFR1_MTE) { + if (!(hwcaps2 & HWCAP2_MTE)) { ksft_print_msg("FAIL: MTE features unavailable\n"); return KSFT_SKIP; }
At the moment we either need to provide CC explicitly, or use a native machine to get the ARM64 MTE selftest compiled.
It seems useful to use the same (cross-)compiler as we use for the kernel, so copy the recipe we use in the pauth selftest.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/Makefile | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index 90aadd86fa0d..fa282e5f2069 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -1,6 +1,11 @@ # SPDX-License-Identifier: GPL-2.0 # Copyright (C) 2020 ARM Limited
+# preserve CC value from top level Makefile +ifeq ($(CC),cc) +CC := $(CROSS_COMPILE)gcc +endif + CFLAGS += -std=gnu99 -I. -pthread LDFLAGS += -pthread SRCS := $(filter-out mte_common_util.c,$(wildcard *.c))
At the moment we check the compiler's ability to compile MTE enabled code, but guard all the Makefile rules by it. As a consequence a broken or not capable compiler just doesn't do anything, and make happily returns without any error message, but with no programs created.
Since the MTE feature is only supported by recent aarch64 compilers (not all stable distro compilers support it), having an explicit message seems like a good idea. To not break building multiple targets, we let make proceed without errors.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/Makefile | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index fa282e5f2069..e0d43cea3cd1 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -23,6 +23,9 @@ TEST_GEN_PROGS := $(PROGS)
# Get Kernel headers installed and use them. KSFT_KHDR_INSTALL := 1 +else + $(warning compiler "$(CC)" does not support the ARMv8.5 MTE extension.) + $(warning test program "mte" will not be created.) endif
# Include KSFT lib.mk.
When clang finds a header file on the command line, it wants to precompile that, which would end up in a separate output file. Specifying -o on that same command line collides with that effort, so the compiler complains:
clang: error: cannot specify -o when generating multiple output files
Since we are not really after a precompiled header, just drop the header file from the command line, by removing it from the list of source files in the Makefile.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index e0d43cea3cd1..409e3e53d00a 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -32,5 +32,5 @@ endif include ../../lib.mk
ifeq ($(mte_cc_support),1) -$(TEST_GEN_PROGS): mte_common_util.c mte_common_util.h mte_helper.S +$(TEST_GEN_PROGS): mte_common_util.c mte_helper.S endif
if (!prctl(...) == 0) is not only cumbersome to read, it also upsets clang and triggers a warning: ------------ mte_common_util.c:287:6: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses] ....
Fix that by just comparing against "not 0" instead.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/mte_common_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index aa8a8a6b8b6d..040abdca079d 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -284,7 +284,7 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
en |= (incl_mask << PR_MTE_TAG_SHIFT); /* Enable address tagging ABI, mte error reporting mode and tag inclusion mask. */ - if (!prctl(PR_SET_TAGGED_ADDR_CTRL, en, 0, 0, 0) == 0) { + if (prctl(PR_SET_TAGGED_ADDR_CTRL, en, 0, 0, 0) != 0) { ksft_print_msg("FAIL:prctl PR_SET_TAGGED_ADDR_CTRL for mte mode\n"); return -EINVAL; }
On Fri, Mar 19, 2021 at 9:53 AM Andre Przywara andre.przywara@arm.com wrote:
if (!prctl(...) == 0) is not only cumbersome to read, it also upsets clang and triggers a warning:
mte_common_util.c:287:6: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses] ....
Fix that by just comparing against "not 0" instead.
Signed-off-by: Andre Przywara andre.przywara@arm.com
tools/testing/selftests/arm64/mte/mte_common_util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index aa8a8a6b8b6d..040abdca079d 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -284,7 +284,7 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask)
en |= (incl_mask << PR_MTE_TAG_SHIFT); /* Enable address tagging ABI, mte error reporting mode and tag inclusion mask. */
if (!prctl(PR_SET_TAGGED_ADDR_CTRL, en, 0, 0, 0) == 0) {
if (prctl(PR_SET_TAGGED_ADDR_CTRL, en, 0, 0, 0) != 0) {
How about `if (prctl(...)) {`?
ksft_print_msg("FAIL:prctl PR_SET_TAGGED_ADDR_CTRL for mte mode\n"); return -EINVAL; }-- 2.17.5
The MTE selftests create temporary files in /dev/shm, for later mmap-ing them. When there is no tmpfs mounted on /dev/shm, or /dev/shm does not exist in the first place (on minimal filesystems), the error message is not giving good hints: # FAIL: Unable to open temporary file # FAIL: memory allocation not ok 17 Check initial tags with private mapping, ...
Add a perror() call, that gives both the filename and the actual error reason, so that users get a chance of correcting that.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- tools/testing/selftests/arm64/mte/mte_common_util.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 040abdca079d..f50ac31920d1 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -337,6 +337,7 @@ int create_temp_file(void) /* Create a file in the tmpfs filesystem */ fd = mkstemp(&filename[0]); if (fd == -1) { + perror(filename); ksft_print_msg("FAIL: Unable to open temporary file\n"); return 0; }
On Fri, Mar 19, 2021 at 04:53:23PM +0000, Andre Przywara wrote:
When trying to run the arm64 MTE (Memory Tagging Extension) selftests on a model with the new FEAT_MTE3 capability, the MTE feature detection failed, because it was overzealously checking for one exact feature version only (0b0010). Trying to fix that (patch 06/11) led me into the rabbit hole of userland tool compilation, which triggered patches
Reviewed-by: Mark Brown broone@kernel.org
On Fri, 19 Mar 2021 16:53:23 +0000, Andre Przywara wrote:
When trying to run the arm64 MTE (Memory Tagging Extension) selftests on a model with the new FEAT_MTE3 capability, the MTE feature detection failed, because it was overzealously checking for one exact feature version only (0b0010). Trying to fix that (patch 06/11) led me into the rabbit hole of userland tool compilation, which triggered patches 01-05/11, to let me actually compile the selftests on an arm64 machine running Ubuntu 20.04. Before I actually fixed that, I tried some other compiler and distro; patches 07 and 08 are my witnesses. Then I got brave and tried clang: entering patches 09/11 and 10/11. Eventually I tried to run the whole thing on that model again, and, you guessed it, patch 11/11 concludes this apparent "2 minute job".
[...]
Applied to arm64 (for-next/kselftest), thanks!
[01/11] kselftest/arm64: mte: Fix compilation with native compiler https://git.kernel.org/arm64/c/4a423645bc26 [02/11] kselftest/arm64: mte: Fix pthread linking https://git.kernel.org/arm64/c/e5decefd884d [03/11] kselftest/arm64: mte: ksm_options: Fix fscanf warning https://git.kernel.org/arm64/c/31c88729a7ad [04/11] kselftest/arm64: mte: user_mem: Fix write() warning https://git.kernel.org/arm64/c/40de85226fec [05/11] kselftest/arm64: mte: common: Fix write() warnings https://git.kernel.org/arm64/c/4dfc9d30a8ab [06/11] kselftest/arm64: mte: Fix MTE feature detection https://git.kernel.org/arm64/c/b17f265bb4cc [07/11] kselftest/arm64: mte: Use cross-compiler if specified https://git.kernel.org/arm64/c/28cc9b3d8996 [08/11] kselftest/arm64: mte: Output warning about failing compiler https://git.kernel.org/arm64/c/6b9bbb7f934d [09/11] kselftest/arm64: mte: Makefile: Fix clang compilation https://git.kernel.org/arm64/c/adb73140eec7 [10/11] kselftest/arm64: mte: Fix clang warning https://git.kernel.org/arm64/c/005a62f6d269 [11/11] kselftest/arm64: mte: Report filename on failing temp file creation https://git.kernel.org/arm64/c/b4985bb88afb
linux-kselftest-mirror@lists.linaro.org
-
Andre Przywara -
Catalin Marinas -
Mark Brown -
Nick Desaulniers