Re: [PATCH v2 1/4] kselftest/arm64: Provide a helper binary and "library" for SVE RDVL

[Dave Martin <Dave.Martin@arm.com>]

On Wed, Jul 28, 2021 at 05:33:15PM +0100, Mark Brown wrote:
> SVE provides an instruction RDVL which reports the currently configured
> vector length. In order to validate that our vector length configuration
> interfaces are working correctly without having to build the C code for
> our test programs with SVE enabled provide a trivial assembly library
> with a C callable function that executes RDVL. Since these interfaces
> also control behaviour on exec*() provide a trivial wrapper program which
> reports the currently configured vector length on stdout, tests can use
> this to verify that behaviour on exec*() is as expected.
> 
> In preparation for providing similar helper functionality for SME, the
> Scalable Matrix Extension, which allows separately configured vector
> lengths to be read back both the assembler function and wrapper binary
> have SVE included in their name.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Dave Martin <Dave.Martin@arm.com>

(With or without a couple of trivial nits below.)

> ---
>  tools/testing/selftests/arm64/fp/.gitignore |  1 +
>  tools/testing/selftests/arm64/fp/Makefile   |  6 +++++-
>  tools/testing/selftests/arm64/fp/rdvl-sve.c | 14 ++++++++++++++
>  tools/testing/selftests/arm64/fp/rdvl.S     |  9 +++++++++
>  tools/testing/selftests/arm64/fp/rdvl.h     |  8 ++++++++
>  5 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/arm64/fp/rdvl-sve.c
>  create mode 100644 tools/testing/selftests/arm64/fp/rdvl.S
>  create mode 100644 tools/testing/selftests/arm64/fp/rdvl.h
> 
> diff --git a/tools/testing/selftests/arm64/fp/.gitignore b/tools/testing/selftests/arm64/fp/.gitignore
> index d66f76d2a650..6b53a7b60fee 100644
> --- a/tools/testing/selftests/arm64/fp/.gitignore
> +++ b/tools/testing/selftests/arm64/fp/.gitignore
> @@ -1,4 +1,5 @@
>  fpsimd-test
> +rdvl-sve
>  sve-probe-vls
>  sve-ptrace
>  sve-test
> diff --git a/tools/testing/selftests/arm64/fp/Makefile b/tools/testing/selftests/arm64/fp/Makefile
> index a57009d3a0dc..ed62e7003b96 100644
> --- a/tools/testing/selftests/arm64/fp/Makefile
> +++ b/tools/testing/selftests/arm64/fp/Makefile
> @@ -2,12 +2,16 @@
>  
>  CFLAGS += -I../../../../../usr/include/
>  TEST_GEN_PROGS := sve-ptrace sve-probe-vls
> -TEST_PROGS_EXTENDED := fpsimd-test fpsimd-stress sve-test sve-stress vlset
> +TEST_PROGS_EXTENDED := fpsimd-test fpsimd-stress \
> +	rdvl-sve \
> +	sve-test sve-stress \
> +	vlset
>  
>  all: $(TEST_GEN_PROGS) $(TEST_PROGS_EXTENDED)
>  
>  fpsimd-test: fpsimd-test.o
>  	$(CC) -nostdlib $^ -o $@
> +rdvl-sve: rdvl-sve.o rdvl.o
>  sve-ptrace: sve-ptrace.o sve-ptrace-asm.o
>  sve-probe-vls: sve-probe-vls.o
>  sve-test: sve-test.o
> diff --git a/tools/testing/selftests/arm64/fp/rdvl-sve.c b/tools/testing/selftests/arm64/fp/rdvl-sve.c
> new file mode 100644
> index 000000000000..7f8a13a18f5d
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/fp/rdvl-sve.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <stdio.h>
> +
> +#include "rdvl.h"
> +
> +int main(void)
> +{
> +	int vl = rdvl_sve();
> +
> +	printf("%d\n", vl);
> +
> +	return 0;

For consistency with the changes in vec-syscfg, we could use
EXIT_SUCCESS here.

Although it's hard to see what could go wrong I/O-wise that doesn't
involve vec-syscfg itself having gone wrong, it's probably good
practice to do the final error check:

	if (ferror(stdout) || fclose(stdout))
		return EXIT_FAILURE;

	return EXIT_SUCCESS;

(In reality, people rarely seem to bother with this, so I'm not going
to lose sleep if we don't do it...)

> +}
> diff --git a/tools/testing/selftests/arm64/fp/rdvl.S b/tools/testing/selftests/arm64/fp/rdvl.S
> new file mode 100644
> index 000000000000..6e76dd720b87
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/fp/rdvl.S
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (C) 2021 ARM Limited.
> +
> +.arch_extension sve
> +
> +.globl rdvl_sve

Should we stick a

	.type rdvl_sve, @function

here?  This may avoid surprises with future toolchain behaviours.
Probably doesn't matter, but I have bad memories of Thumb-2...

Lacking this annotation is widespread though, as well as being de facto
standard before awkward architectures came along.

If the selftests have access to the ENTRY() macro we could use that, but
I'm guessing that isn't exported for userspace.

> +rdvl_sve:
> +	rdvl	x0, #1
> +	ret
> diff --git a/tools/testing/selftests/arm64/fp/rdvl.h b/tools/testing/selftests/arm64/fp/rdvl.h
> new file mode 100644
> index 000000000000..7c9d953fc9e7
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/fp/rdvl.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef RDVL_H
> +#define RDVL_H
> +
> +int rdvl_sve(void);
> +
> +#endif
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel