| Message ID | bd6825832c0cb376fc68ad61ffec6d829401ed0e.1605046192.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v9,01/44] kasan: drop unnecessary GPL text from comment headers | expand |
On Tue, Nov 10, 2020 at 11:10:41PM +0100, Andrey Konovalov wrote: > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > This test is specific to MTE and verifies that the GCR_EL1 register > is context switched correctly. > > It spawn 1024 processes and each process spawns 5 threads. Each thread > writes a random setting of GCR_EL1 through the prctl() system call and > reads it back verifying that it is the same. If the values are not the > same it reports a failure. > > Note: The test has been extended to verify that even SYNC and ASYNC mode > setting is preserved correctly over context switching. > > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
On Tue, Nov 10, 2020 at 11:12 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > This test is specific to MTE and verifies that the GCR_EL1 register > is context switched correctly. > > It spawn 1024 processes and each process spawns 5 threads. Each thread Nit: "spawns" > + srand(time(NULL) ^ (pid << 16) ^ (tid << 16)); > + > + prctl_tag_mask = rand() % 0xffff; Nit: if you want values between 0 and 0xffff you probably want to use bitwise AND. > + > +int execute_test(pid_t pid) > +{ > + pthread_t thread_id[MAX_THREADS]; > + int thread_data[MAX_THREADS]; > + > + for (int i = 0; i < MAX_THREADS; i++) > + pthread_create(&thread_id[i], NULL, > + execute_thread, (void *)&pid); It might be simpler to call getpid() in execute_thread() instead. > +int mte_gcr_fork_test() > +{ > + pid_t pid[NUM_ITERATIONS]; > + int results[NUM_ITERATIONS]; > + pid_t cpid; > + int res; > + > + for (int i = 0; i < NUM_ITERATIONS; i++) { > + pid[i] = fork(); > + > + if (pid[i] == 0) { pid[i] isn't used anywhere else. Did you want to keep the pids to ensure that all children finished the work? If not, we can probably go with a scalar here. > + for (int i = 0; i < NUM_ITERATIONS; i++) { > + wait(&res); > + > + if(WIFEXITED(res)) > + results[i] = WEXITSTATUS(res); > + else > + --i; Won't we get stuck in this loop if fork() returns -1 for one of the processes? > + } > + > + for (int i = 0; i < NUM_ITERATIONS; i++) > + if (results[i] == KSFT_FAIL) > + return KSFT_FAIL; > + > + return KSFT_PASS; > +} > +
On Thu, 12 Nov 2020 at 16:59, Alexander Potapenko <glider@google.com> wrote: > > On Tue, Nov 10, 2020 at 11:12 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > > > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > > > This test is specific to MTE and verifies that the GCR_EL1 register > > is context switched correctly. > > > > It spawn 1024 processes and each process spawns 5 threads. Each thread > > Nit: "spawns" > > > > + srand(time(NULL) ^ (pid << 16) ^ (tid << 16)); > > + > > + prctl_tag_mask = rand() % 0xffff; > > Nit: if you want values between 0 and 0xffff you probably want to use > bitwise AND. Another question would be, is the max here meant to be 0xffff or 0xffff-1. Because, as-is now, it's 0xffff-1. Only one of them has a trivial conversion to bitwise AND ( x % 2^n == x & (2^n - 1) ).
On Thu, Nov 12, 2020 at 5:09 PM Marco Elver <elver@google.com> wrote: > > On Thu, 12 Nov 2020 at 16:59, Alexander Potapenko <glider@google.com> wrote: > > > > On Tue, Nov 10, 2020 at 11:12 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > > > > > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > > > > > This test is specific to MTE and verifies that the GCR_EL1 register > > > is context switched correctly. > > > > > > It spawn 1024 processes and each process spawns 5 threads. Each thread > > > > Nit: "spawns" > > > > > > > + srand(time(NULL) ^ (pid << 16) ^ (tid << 16)); > > > + > > > + prctl_tag_mask = rand() % 0xffff; > > > > Nit: if you want values between 0 and 0xffff you probably want to use > > bitwise AND. > > Another question would be, is the max here meant to be 0xffff or > 0xffff-1. Because, as-is now, it's 0xffff-1. Only one of them has a > trivial conversion to bitwise AND ( x % 2^n == x & (2^n - 1) ). Yes, that is basically what I meant, assuming that Vincenzo wanted the max to be 0xffff
Hi Alexander, thank you for the review. On 11/12/20 3:59 PM, Alexander Potapenko wrote: > On Tue, Nov 10, 2020 at 11:12 PM Andrey Konovalov <andreyknvl@google.com> wrote: >> >> From: Vincenzo Frascino <vincenzo.frascino@arm.com> >> >> This test is specific to MTE and verifies that the GCR_EL1 register >> is context switched correctly. >> >> It spawn 1024 processes and each process spawns 5 threads. Each thread > > Nit: "spawns" > I will fix it in the next iteration. > >> + srand(time(NULL) ^ (pid << 16) ^ (tid << 16)); >> + >> + prctl_tag_mask = rand() % 0xffff; > > Nit: if you want values between 0 and 0xffff you probably want to use > bitwise AND. > The main goal here is to have a good probability of having a different setting to the GCR_EL1 register. Hence the difference in between 0xffff and 0xffff-1 is negligible. Anyway I agree that we should aim to cover all the possible combinations. > >> + >> +int execute_test(pid_t pid) >> +{ >> + pthread_t thread_id[MAX_THREADS]; >> + int thread_data[MAX_THREADS]; >> + >> + for (int i = 0; i < MAX_THREADS; i++) >> + pthread_create(&thread_id[i], NULL, >> + execute_thread, (void *)&pid); > > It might be simpler to call getpid() in execute_thread() instead. > Yes it might, but I would like to avoid another syscall if I can. >> +int mte_gcr_fork_test() >> +{ >> + pid_t pid[NUM_ITERATIONS]; >> + int results[NUM_ITERATIONS]; >> + pid_t cpid; >> + int res; >> + >> + for (int i = 0; i < NUM_ITERATIONS; i++) { >> + pid[i] = fork(); >> + >> + if (pid[i] == 0) { > > pid[i] isn't used anywhere else. Did you want to keep the pids to > ensure that all children finished the work? > If not, we can probably go with a scalar here. > Yes, I agree, I had some debug code making use of it, but I removed it in the end. > >> + for (int i = 0; i < NUM_ITERATIONS; i++) { >> + wait(&res); >> + >> + if(WIFEXITED(res)) >> + results[i] = WEXITSTATUS(res); >> + else >> + --i; > > Won't we get stuck in this loop if fork() returns -1 for one of the processes? > Yes I agree, I forgot to check a condition. We should abort the test in such a case returning KSFT_FAIL directly. >> + } >> + >> + for (int i = 0; i < NUM_ITERATIONS; i++) >> + if (results[i] == KSFT_FAIL) >> + return KSFT_FAIL; >> + >> + return KSFT_PASS; >> +} >> + > >
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile index 2480226dfe57..0b3af552632a 100644 --- a/tools/testing/selftests/arm64/mte/Makefile +++ b/tools/testing/selftests/arm64/mte/Makefile @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 # Copyright (C) 2020 ARM Limited -CFLAGS += -std=gnu99 -I. +CFLAGS += -std=gnu99 -I. -lpthread SRCS := $(filter-out mte_common_util.c,$(wildcard *.c)) PROGS := $(patsubst %.c,%,$(SRCS)) diff --git a/tools/testing/selftests/arm64/mte/check_gcr_el1_cswitch.c b/tools/testing/selftests/arm64/mte/check_gcr_el1_cswitch.c new file mode 100644 index 000000000000..55e33d96794c --- /dev/null +++ b/tools/testing/selftests/arm64/mte/check_gcr_el1_cswitch.c @@ -0,0 +1,152 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2020 ARM Limited + +#define _GNU_SOURCE + +#include <errno.h> +#include <pthread.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <unistd.h> +#include <sys/auxv.h> +#include <sys/mman.h> +#include <sys/prctl.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include "kselftest.h" +#include "mte_common_util.h" + +#define PR_SET_TAGGED_ADDR_CTRL 55 +#define PR_GET_TAGGED_ADDR_CTRL 56 +# define PR_TAGGED_ADDR_ENABLE (1UL << 0) +# define PR_MTE_TCF_SHIFT 1 +# define PR_MTE_TCF_NONE (0UL << PR_MTE_TCF_SHIFT) +# define PR_MTE_TCF_SYNC (1UL << PR_MTE_TCF_SHIFT) +# define PR_MTE_TCF_ASYNC (2UL << PR_MTE_TCF_SHIFT) +# define PR_MTE_TCF_MASK (3UL << PR_MTE_TCF_SHIFT) +# define PR_MTE_TAG_SHIFT 3 +# define PR_MTE_TAG_MASK (0xffffUL << PR_MTE_TAG_SHIFT) + +#include "mte_def.h" + +#define NUM_ITERATIONS 1024 +#define MAX_THREADS 5 +#define THREAD_ITERATIONS 1000 + +void *execute_thread(void *x) +{ + pid_t pid = *((pid_t *)x); + pid_t tid = gettid(); + uint64_t prctl_tag_mask; + uint64_t prctl_set; + uint64_t prctl_get; + uint64_t prctl_tcf; + + srand(time(NULL) ^ (pid << 16) ^ (tid << 16)); + + prctl_tag_mask = rand() % 0xffff; + + if (prctl_tag_mask % 2) + prctl_tcf = PR_MTE_TCF_SYNC; + else + prctl_tcf = PR_MTE_TCF_ASYNC; + + prctl_set = PR_TAGGED_ADDR_ENABLE | prctl_tcf | (prctl_tag_mask << PR_MTE_TAG_SHIFT); + + for (int j = 0; j < THREAD_ITERATIONS; j++) { + if (prctl(PR_SET_TAGGED_ADDR_CTRL, prctl_set, 0, 0, 0)) { + perror("prctl() failed"); + goto fail; + } + + prctl_get = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0); + + if (prctl_set != prctl_get) { + ksft_print_msg("Error: prctl_set: 0x%lx != prctl_get: 0x%lx\n", + prctl_set, prctl_get); + goto fail; + } + } + + return (void *)KSFT_PASS; + +fail: + return (void *)KSFT_FAIL; +} + +int execute_test(pid_t pid) +{ + pthread_t thread_id[MAX_THREADS]; + int thread_data[MAX_THREADS]; + + for (int i = 0; i < MAX_THREADS; i++) + pthread_create(&thread_id[i], NULL, + execute_thread, (void *)&pid); + + for (int i = 0; i < MAX_THREADS; i++) + pthread_join(thread_id[i], (void *)&thread_data[i]); + + for (int i = 0; i < MAX_THREADS; i++) + if (thread_data[i] == KSFT_FAIL) + return KSFT_FAIL; + + return KSFT_PASS; +} + +int mte_gcr_fork_test() +{ + pid_t pid[NUM_ITERATIONS]; + int results[NUM_ITERATIONS]; + pid_t cpid; + int res; + + for (int i = 0; i < NUM_ITERATIONS; i++) { + pid[i] = fork(); + + if (pid[i] == 0) { + cpid = getpid(); + + res = execute_test(cpid); + + exit(res); + } + } + + for (int i = 0; i < NUM_ITERATIONS; i++) { + wait(&res); + + if(WIFEXITED(res)) + results[i] = WEXITSTATUS(res); + else + --i; + } + + for (int i = 0; i < NUM_ITERATIONS; i++) + if (results[i] == KSFT_FAIL) + return KSFT_FAIL; + + return KSFT_PASS; +} + +int main(int argc, char *argv[]) +{ + int err; + + err = mte_default_setup(); + if (err) + return err; + + ksft_set_plan(1); + + evaluate_test(mte_gcr_fork_test(), + "Verify that GCR_EL1 is set correctly on context switch\n"); + + mte_restore_setup(); + ksft_print_cnts(); + + return ksft_get_fail_cnt() == 0 ? KSFT_PASS : KSFT_FAIL; +} +