[PATCH v2] kselftest/arm64: signal: fix/refactor SVE vector length enumeration

[Andre Przywara]

On Mon, 19 Aug 2024 13:57:21 +0100
Andre Przywara <andre.przywara at arm.com> wrote:

Hi,

sorry, just realised I missed one include...

> Currently a number of SVE/SME related tests have almost identical
> functions to enumerate all supported vector lengths. However over time
> the copy&pasted code has diverged, allowing some bugs to creep in:
> - fake_sigreturn_sme_change_vl reports a failure, not a SKIP if only
>   one vector length is supported (but the SVE version is fine)
> - fake_sigreturn_sme_change_vl tries to set the SVE vector length, not
>   the SME one (but the other SME tests are fine)
> - za_no_regs keeps iterating forever if only one vector length is
>   supported (but za_regs is correct)
> 
> Since those bugs seem to be mostly copy&paste ones, let's consolidate
> the enumeration loop into one shared function, and just call that from
> each test. That should fix the above bugs, and prevent similar issues
> from happening again.
> 
> Fixes: 4963aeb35a9e ("kselftest/arm64: signal: Add SME signal handling tests")
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
> Hi,
> 
> a small update, making the SME VL loop check more generic, and adding a
> comment about the reason we have that in the first place.
> 
> Cheers,
> Andre
> 
>  tools/testing/selftests/arm64/signal/Makefile |  2 +-
>  .../selftests/arm64/signal/sve_helpers.c      | 56 +++++++++++++++++++
>  .../selftests/arm64/signal/sve_helpers.h      | 21 +++++++
>  .../testcases/fake_sigreturn_sme_change_vl.c  | 31 +++-------
>  .../testcases/fake_sigreturn_sve_change_vl.c  | 30 ++--------
>  .../arm64/signal/testcases/ssve_regs.c        | 36 +++---------
>  .../arm64/signal/testcases/ssve_za_regs.c     | 36 +++---------
>  .../arm64/signal/testcases/sve_regs.c         | 32 +++--------
>  .../arm64/signal/testcases/za_no_regs.c       | 32 +++--------
>  .../arm64/signal/testcases/za_regs.c          | 36 +++---------
>  10 files changed, 131 insertions(+), 181 deletions(-)
>  create mode 100644 tools/testing/selftests/arm64/signal/sve_helpers.c
>  create mode 100644 tools/testing/selftests/arm64/signal/sve_helpers.h
> 
> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
> index 37c8207b99cfc..1381039fb36f9 100644
> --- a/tools/testing/selftests/arm64/signal/Makefile
> +++ b/tools/testing/selftests/arm64/signal/Makefile
> @@ -23,7 +23,7 @@ $(TEST_GEN_PROGS): $(PROGS)
>  # Common test-unit targets to build common-layout test-cases executables
>  # Needs secondary expansion to properly include the testcase c-file in pre-reqs
>  COMMON_SOURCES := test_signals.c test_signals_utils.c testcases/testcases.c \
> -	signals.S
> +	signals.S sve_helpers.c
>  COMMON_HEADERS := test_signals.h test_signals_utils.h testcases/testcases.h
>  
>  .SECONDEXPANSION:
> diff --git a/tools/testing/selftests/arm64/signal/sve_helpers.c b/tools/testing/selftests/arm64/signal/sve_helpers.c
> new file mode 100644
> index 0000000000000..f57265354eaed
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/sve_helpers.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Common helper functions for SVE and SME functionality.
> + */
> +
> +#include <stdbool.h>
> +#include <kselftest.h>
> +#include <asm/sigcontext.h>
> +#include <sys/prctl.h>
> +
> +unsigned int vls[SVE_VQ_MAX];
> +unsigned int nvls;
> +
> +int sve_fill_vls(bool use_sme, int min_vls)
> +{
> +	int vq, vl;
> +	int pr_set_vl = use_sme ? PR_SME_SET_VL : PR_SVE_SET_VL;
> +	int len_mask = use_sme ? PR_SME_VL_LEN_MASK : PR_SVE_VL_LEN_MASK;
> +
> +	/*
> +	 * Enumerate up to SVE_VQ_MAX vector lengths
> +	 */
> +	for (vq = SVE_VQ_MAX; vq > 0; --vq) {
> +		vl = prctl(pr_set_vl, vq * 16);
> +		if (vl == -1)
> +			return KSFT_FAIL;
> +
> +		vl &= len_mask;
> +
> +		/*
> +		 * Unlike SVE, SME does not require the minimum vector length
> +		 * to be implemented, or the VLs to be consecutive, so any call
> +		 * to the prctl might return the single implemented VL, which
> +		 * might be larger than 16. So to avoid this loop never
> +		 * terminating,  bail out here when we find a higher VL than
> +		 * we asked for.
> +		 * See the ARM ARM, DDI 0487K.a, B1.4.2: I_QQRNR and I_NWYBP.
> +		 */
> +		if (vq < sve_vq_from_vl(vl))
> +			break;
> +
> +		/* Skip missing VLs */
> +		vq = sve_vq_from_vl(vl);
> +
> +		vls[nvls++] = vl;
> +	}
> +
> +	if (nvls < min_vls) {
> +		fprintf(stderr, "Only %d VL supported\n", nvls);
> +		return KSFT_SKIP;
> +	}
> +
> +	return KSFT_PASS;
> +}
> diff --git a/tools/testing/selftests/arm64/signal/sve_helpers.h b/tools/testing/selftests/arm64/signal/sve_helpers.h
> new file mode 100644
> index 0000000000000..50948ce471cc6
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/sve_helpers.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Common helper functions for SVE and SME functionality.
> + */
> +
> +#ifndef __SVE_HELPERS_H__
> +#define __SVE_HELPERS_H__
> +
> +#include <stdbool.h>
> +
> +#define VLS_USE_SVE	false
> +#define VLS_USE_SME	true
> +
> +extern unsigned int vls[];
> +extern unsigned int nvls;
> +
> +int sve_fill_vls(bool use_sme, int min_vls);
> +
> +#endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_sme_change_vl.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_sme_change_vl.c
> index ebd5815b54bba..bc10585062d57 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_sme_change_vl.c
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_sme_change_vl.c
> @@ -11,39 +11,22 @@
>  #include <sys/prctl.h>

We need an "#include <kselftest.h>" here.
Sorry, but that slipped through because the kselftest build throws quite
some warnings and errors for me (some of fixed with the other series).

Will send a v3 ASAP.

Cheers,
Andre.

>  
>  #include "test_signals_utils.h"
> +#include "sve_helpers.h"
>  #include "testcases.h"
>  

[ ... ]