This series is just a set of minor tweaks and improvements for the MTE tests that I did while working on the asymmetric mode support for userspace which seemed like they might be worth keeping even though the prctl() for asymmetric mode got removed.
Mark Brown (4): kselftest/arm64: Handle more kselftest result codes in MTE helpers kselftest/arm64: Log unexpected asynchronous MTE faults kselftest/arm64: Refactor parameter checking in mte_switch_mode() kselftest/arm64: Add simple test for MTE prctl
tools/testing/selftests/arm64/mte/.gitignore | 1 + .../testing/selftests/arm64/mte/check_prctl.c | 123 ++++++++++++++++++ .../selftests/arm64/mte/mte_common_util.c | 17 ++- .../selftests/arm64/mte/mte_common_util.h | 15 ++- 4 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 tools/testing/selftests/arm64/mte/check_prctl.c
base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
The MTE selftests have a helper evaluate_test() which translates a return code into a call to ksft_test_result_*(). Currently this only handles pass and fail, silently ignoring any other code. Update the helper to support skipped tests and log any unknown return codes as an error so we get at least some diagnostic if anything goes wrong.
Signed-off-by: Mark Brown broonie@kernel.org --- .../testing/selftests/arm64/mte/mte_common_util.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.h b/tools/testing/selftests/arm64/mte/mte_common_util.h index 195a7d1879e6..2d3e71724e55 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.h +++ b/tools/testing/selftests/arm64/mte/mte_common_util.h @@ -75,10 +75,21 @@ unsigned int mte_get_pstate_tco(void); /* Test framework static inline functions/macros */ static inline void evaluate_test(int err, const char *msg) { - if (err == KSFT_PASS) + switch (err) { + case KSFT_PASS: ksft_test_result_pass(msg); - else if (err == KSFT_FAIL) + break; + case KSFT_FAIL: ksft_test_result_fail(msg); + break; + case KSFT_SKIP: + ksft_test_result_skip(msg); + break; + default: + ksft_test_result_error("Unknown return code %d from %s", + err, msg); + break; + } }
static inline int check_allocated_memory(void *ptr, size_t size,
On 3/10/22 7:43 AM, Mark Brown wrote:
The MTE selftests have a helper evaluate_test() which translates a return code into a call to ksft_test_result_*(). Currently this only handles pass and fail, silently ignoring any other code. Update the helper to support skipped tests and log any unknown return codes as an error so we get at least some diagnostic if anything goes wrong.
Signed-off-by: Mark Brown broonie@kernel.org
.../testing/selftests/arm64/mte/mte_common_util.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.h b/tools/testing/selftests/arm64/mte/mte_common_util.h index 195a7d1879e6..2d3e71724e55 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.h +++ b/tools/testing/selftests/arm64/mte/mte_common_util.h @@ -75,10 +75,21 @@ unsigned int mte_get_pstate_tco(void); /* Test framework static inline functions/macros */ static inline void evaluate_test(int err, const char *msg) {
- if (err == KSFT_PASS)
- switch (err) {
- case KSFT_PASS: ksft_test_result_pass(msg);
- else if (err == KSFT_FAIL)
break;
- case KSFT_FAIL: ksft_test_result_fail(msg);
break;
- case KSFT_SKIP:
ksft_test_result_skip(msg);
break;
- default:
ksft_test_result_error("Unknown return code %d from %s",
err, msg);
break;
- } }
static inline int check_allocated_memory(void *ptr, size_t size,
Nice. Thank you for doing this.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
Help people figure out problems by printing a diagnostic when we get an unexpected asynchronous fault.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/mte/mte_common_util.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 0328a1e08f65..24b0c14203cb 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -37,6 +37,8 @@ void mte_default_handler(int signum, siginfo_t *si, void *uc) if (si->si_code == SEGV_MTEAERR) { if (cur_mte_cxt.trig_si_code == si->si_code) cur_mte_cxt.fault_valid = true; + else + ksft_print_msg("Got unexpected SEGV_MTEAERR\n"); return; } /* Compare the context for precise error */
On 3/10/22 7:43 AM, Mark Brown wrote:
Help people figure out problems by printing a diagnostic when we get an unexpected asynchronous fault.
Signed-off-by: Mark Brown broonie@kernel.org
tools/testing/selftests/arm64/mte/mte_common_util.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 0328a1e08f65..24b0c14203cb 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -37,6 +37,8 @@ void mte_default_handler(int signum, siginfo_t *si, void *uc) if (si->si_code == SEGV_MTEAERR) { if (cur_mte_cxt.trig_si_code == si->si_code) cur_mte_cxt.fault_valid = true;
else
ksft_print_msg("Got unexpected SEGV_MTEAERR\n");
This is good. Would it make sense to add more info. - I see this in the doc? Would it make sense to also check si_addr?
- *Asynchronous* - The kernel raises a ``SIGSEGV``, in the offending thread, asynchronously following one or multiple tag check faults, with ``.si_code = SEGV_MTEAERR`` and ``.si_addr = 0`` (the faulting address is unknown).
return; } /* Compare the context for precise error */
Looks good to me as such.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
Currently we just have a big if statement with a non-specific diagnostic checking both the mode and the tag. Since we'll need to dynamically check for asymmetric mode support in the system and to improve debugability split these checks out.
Signed-off-by: Mark Brown broonie@kernel.org --- .../testing/selftests/arm64/mte/mte_common_util.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 24b0c14203cb..9b4529ef2b29 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -271,9 +271,18 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask) { unsigned long en = 0;
- if (!(mte_option == MTE_SYNC_ERR || mte_option == MTE_ASYNC_ERR || - mte_option == MTE_NONE_ERR || incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) { - ksft_print_msg("FAIL: Invalid mte config option\n"); + switch (mte_option) { + case MTE_NONE_ERR: + case MTE_SYNC_ERR: + case MTE_ASYNC_ERR: + break; + default: + ksft_print_msg("FAIL: Invalid MTE option %x\n", mte_option); + return -EINVAL; + } + + if (!(incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) { + ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask); return -EINVAL; } en = PR_TAGGED_ADDR_ENABLE;
On 3/10/22 7:43 AM, Mark Brown wrote:
Currently we just have a big if statement with a non-specific diagnostic checking both the mode and the tag. Since we'll need to dynamically check for asymmetric mode support in the system and to improve debugability split these checks out.
Signed-off-by: Mark Brown broonie@kernel.org
.../testing/selftests/arm64/mte/mte_common_util.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/mte_common_util.c b/tools/testing/selftests/arm64/mte/mte_common_util.c index 24b0c14203cb..9b4529ef2b29 100644 --- a/tools/testing/selftests/arm64/mte/mte_common_util.c +++ b/tools/testing/selftests/arm64/mte/mte_common_util.c @@ -271,9 +271,18 @@ int mte_switch_mode(int mte_option, unsigned long incl_mask) { unsigned long en = 0;
- if (!(mte_option == MTE_SYNC_ERR || mte_option == MTE_ASYNC_ERR ||
mte_option == MTE_NONE_ERR || incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) {
ksft_print_msg("FAIL: Invalid mte config option\n");
- switch (mte_option) {
- case MTE_NONE_ERR:
- case MTE_SYNC_ERR:
- case MTE_ASYNC_ERR:
break;
- default:
ksft_print_msg("FAIL: Invalid MTE option %x\n", mte_option);
return -EINVAL;
- }
- if (!(incl_mask <= MTE_ALLOW_NON_ZERO_TAG)) {
return -EINVAL; } en = PR_TAGGED_ADDR_ENABLE;ksft_print_msg("FAIL: Invalid incl_mask %lx\n", incl_mask);
Looks good to me.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
The current tests use the prctls for various things but there's no coverage of the edges of the interface so add some basics. This isn't hugely useful as it is (it originally had some coverage for the combinations with asymmetric mode but we removed the prctl() for that) but it might be a helpful starting point for future work, for example covering error handling.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/arm64/mte/.gitignore | 1 + .../testing/selftests/arm64/mte/check_prctl.c | 123 ++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tools/testing/selftests/arm64/mte/check_prctl.c
diff --git a/tools/testing/selftests/arm64/mte/.gitignore b/tools/testing/selftests/arm64/mte/.gitignore index d1fe4ddf1669..052d0f9f92b3 100644 --- a/tools/testing/selftests/arm64/mte/.gitignore +++ b/tools/testing/selftests/arm64/mte/.gitignore @@ -3,5 +3,6 @@ check_gcr_el1_cswitch check_tags_inclusion check_child_memory check_mmap_options +check_prctl check_ksm_options check_user_mem diff --git a/tools/testing/selftests/arm64/mte/check_prctl.c b/tools/testing/selftests/arm64/mte/check_prctl.c new file mode 100644 index 000000000000..500aefc5d7cd --- /dev/null +++ b/tools/testing/selftests/arm64/mte/check_prctl.c @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2022 ARM Limited + +#include <stdbool.h> +#include <stdio.h> +#include <string.h> + +#include <sys/auxv.h> +#include <sys/prctl.h> + +#include <asm/hwcap.h> + +#include "kselftest.h" + +static bool system_has_mte; + +static int set_tagged_addr_ctrl(int val) +{ + int ret; + + ret = prctl(PR_SET_TAGGED_ADDR_CTRL, val, 0, 0, 0); + if (ret < 0) + ksft_print_msg("PR_SET_TAGGED_ADDR_CTRL: failed %d %d (%s)\n", + ret, errno, strerror(errno)); + return ret; +} + +static int get_tagged_addr_ctrl(void) +{ + int ret; + + ret = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0); + if (ret < 0) + ksft_print_msg("PR_GET_TAGGED_ADDR_CTRL failed: %d %d (%s)\n", + ret, errno, strerror(errno)); + return ret; +} + +/* + * Read the current mode without having done any configuration, should + * run first. + */ +void check_basic_read(void) +{ + int ret; + + ret = get_tagged_addr_ctrl(); + if (ret < 0) { + ksft_test_result_fail("check_basic_read\n"); + return; + } + + if (ret & PR_MTE_TCF_SYNC) + ksft_print_msg("SYNC enabled\n"); + if (ret & PR_MTE_TCF_ASYNC) + ksft_print_msg("ASYNC enabled\n"); + + /* Any configuration is valid */ + ksft_test_result_pass("check_basic_read\n"); +} + +/* + * Attempt to set a specified combination of modes. + */ +void set_mode_test(const char *name, int hwcap2, int mask) +{ + int ret; + + if ((getauxval(AT_HWCAP2) & hwcap2) != hwcap2) { + ksft_test_result_skip("%s\n", name); + return; + } + + ret = set_tagged_addr_ctrl(mask); + if (ret < 0) { + ksft_test_result_fail("%s\n", name); + return; + } + + ret = get_tagged_addr_ctrl(); + if (ret < 0) { + ksft_test_result_fail("%s\n", name); + return; + } + + if ((ret & PR_MTE_TCF_MASK) == mask) { + ksft_test_result_pass("%s\n", name); + } else { + ksft_print_msg("Got %x, expected %x\n", + (ret & PR_MTE_TCF_MASK), mask); + ksft_test_result_fail("%s\n", name); + } +} + +struct mte_mode { + int mask; + int hwcap2; + const char *name; +} mte_modes[] = { + { PR_MTE_TCF_NONE, 0, "NONE" }, + { PR_MTE_TCF_SYNC, HWCAP2_MTE, "SYNC" }, + { PR_MTE_TCF_ASYNC, HWCAP2_MTE, "ASYNC" }, + { PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC, HWCAP2_MTE, "SYNC+ASYNC" }, +}; + +int main(void) +{ + int i; + + system_has_mte = getauxval(AT_HWCAP2) & HWCAP2_MTE; + + ksft_print_header(); + ksft_set_plan(5); + + check_basic_read(); + for (i = 0; i < ARRAY_SIZE(mte_modes); i++) + set_mode_test(mte_modes[i].name, mte_modes[i].hwcap2, + mte_modes[i].mask); + + ksft_print_cnts(); + + return 0; +}
Hi Mark,
On Thu, Mar 10, 2022 at 02:43:35PM +0000, Mark Brown wrote:
The current tests use the prctls for various things but there's no coverage of the edges of the interface so add some basics. This isn't hugely useful as it is (it originally had some coverage for the combinations with asymmetric mode but we removed the prctl() for that) but it might be a helpful starting point for future work, for example covering error handling.
Signed-off-by: Mark Brown broonie@kernel.org
[..]
diff --git a/tools/testing/selftests/arm64/mte/check_prctl.c b/tools/testing/selftests/arm64/mte/check_prctl.c new file mode 100644 index 000000000000..500aefc5d7cd --- /dev/null +++ b/tools/testing/selftests/arm64/mte/check_prctl.c @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2022 ARM Limited
+#include <stdbool.h> +#include <stdio.h> +#include <string.h>
+#include <sys/auxv.h> +#include <sys/prctl.h>
+#include <asm/hwcap.h>
+#include "kselftest.h"
+static bool system_has_mte;
This looks unused (apart from being set in main()).
Thanks, Joey
On Thu, Mar 10, 2022 at 03:33:06PM +0000, Joey Gouly wrote:
On Thu, Mar 10, 2022 at 02:43:35PM +0000, Mark Brown wrote:
+static bool system_has_mte;
This looks unused (apart from being set in main()).
It is, it was used in the asymmetric bits of the test.
On 3/10/22 7:43 AM, Mark Brown wrote:
The current tests use the prctls for various things but there's no coverage of the edges of the interface so add some basics. This isn't hugely useful as it is (it originally had some coverage for the combinations with asymmetric mode but we removed the prctl() for that) but it might be a helpful starting point for future work, for example covering error handling.
Signed-off-by: Mark Brown broonie@kernel.org
tools/testing/selftests/arm64/mte/.gitignore | 1 + .../testing/selftests/arm64/mte/check_prctl.c | 123 ++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tools/testing/selftests/arm64/mte/check_prctl.c
Looks good to me.
Reviewed-by: Shuah Khan skhan@linuxfoundation.org
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org