Re: [PATCH v11 39/40] kselftest/arm64: Add coverage for the ZA ptrace interface

[Shuah Khan <skhan@linuxfoundation.org>]

On 2/7/22 8:21 AM, Mark Brown wrote:
> Add some basic coverage for the ZA ptrace interface, including walking
> through all the vector lengths supported in the system.  Unlike SVE
> doing syscalls does not discard the ZA state so when we set data in ZA
> we run the child process briefly, having it add one to each byte in ZA
> in order to validate that both the vector size and data are being read
> and written as expected when the process runs.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>   tools/testing/selftests/arm64/fp/.gitignore  |   1 +
>   tools/testing/selftests/arm64/fp/Makefile    |   3 +-
>   tools/testing/selftests/arm64/fp/za-ptrace.c | 354 +++++++++++++++++++
>   3 files changed, 357 insertions(+), 1 deletion(-)
>   create mode 100644 tools/testing/selftests/arm64/fp/za-ptrace.c
> 
> diff --git a/tools/testing/selftests/arm64/fp/.gitignore b/tools/testing/selftests/arm64/fp/.gitignore
> index ead3197e720b..d98d3d48b504 100644
> --- a/tools/testing/selftests/arm64/fp/.gitignore
> +++ b/tools/testing/selftests/arm64/fp/.gitignore
> @@ -8,4 +8,5 @@ sve-test
>   ssve-test
>   vec-syscfg
>   vlset
> +za-ptrace
>   za-test
> diff --git a/tools/testing/selftests/arm64/fp/Makefile b/tools/testing/selftests/arm64/fp/Makefile
> index 38d2d0d5a0eb..807a8faf8d57 100644
> --- a/tools/testing/selftests/arm64/fp/Makefile
> +++ b/tools/testing/selftests/arm64/fp/Makefile
> @@ -1,7 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
>   CFLAGS += -I../../../../../usr/include/
> -TEST_GEN_PROGS := sve-ptrace sve-probe-vls vec-syscfg
> +TEST_GEN_PROGS := sve-ptrace sve-probe-vls vec-syscfg za-ptrace
>   TEST_PROGS_EXTENDED := fp-pidbench fpsimd-test fpsimd-stress \
>   	rdvl-sme rdvl-sve \
>   	sve-test sve-stress \
> @@ -27,5 +27,6 @@ vec-syscfg: vec-syscfg.o rdvl.o
>   vlset: vlset.o
>   za-test: za-test.o asm-utils.o
>   	$(CC) -nostdlib $^ -o $@
> +za-ptrace: za-ptrace.o
>   
>   include ../../lib.mk
> diff --git a/tools/testing/selftests/arm64/fp/za-ptrace.c b/tools/testing/selftests/arm64/fp/za-ptrace.c
> new file mode 100644
> index 000000000000..692c82624855
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/fp/za-ptrace.c
> @@ -0,0 +1,354 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 ARM Limited.
> + */
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/auxv.h>
> +#include <sys/prctl.h>
> +#include <sys/ptrace.h>
> +#include <sys/types.h>
> +#include <sys/uio.h>
> +#include <sys/wait.h>
> +#include <asm/sigcontext.h>
> +#include <asm/ptrace.h>
> +
> +#include "../../kselftest.h"
> +
> +/* <linux/elf.h> and <sys/auxv.h> don't like each other, so: */
> +#ifndef NT_ARM_ZA
> +#define NT_ARM_ZA 0x40c
> +#endif
> +
> +#define EXPECTED_TESTS (((SVE_VQ_MAX - SVE_VQ_MIN) + 1) * 3)
> +
> +static void fill_buf(char *buf, size_t size)
> +{
> +	int i;
> +
> +	for (i = 0; i < size; i++)
> +		buf[i] = random();
> +}
> +
> +static int do_child(void)
> +{
> +	if (ptrace(PTRACE_TRACEME, -1, NULL, NULL))
> +		ksft_exit_fail_msg("PTRACE_TRACEME", strerror(errno));
> +
> +	if (raise(SIGSTOP))
> +		ksft_exit_fail_msg("raise(SIGSTOP)", strerror(errno));
> +
> +	return EXIT_SUCCESS;
> +}
> +
> +static struct user_za_header *get_za(pid_t pid, void **buf, size_t *size)
> +{
> +	struct user_za_header *za;
> +	void *p;
> +	size_t sz = sizeof(*za);
> +	struct iovec iov;
> +
> +	while (1) {
> +		if (*size < sz) {
> +			p = realloc(*buf, sz);
> +			if (!p) {
> +				errno = ENOMEM;
> +				goto error;

Do we free buf in error path? Didn't see it in error handling below.

> +			}
> +
> +			*buf = p;
> +			*size = sz;
> +		}
> +
> +		iov.iov_base = *buf;
> +		iov.iov_len = sz;
> +		if (ptrace(PTRACE_GETREGSET, pid, NT_ARM_ZA, &iov))
> +			goto error;
> +
> +		za = *buf;
> +		if (za->size <= sz)
> +			break;
> +
> +		sz = za->size;
> +	}
> +
> +	return za;
> +
> +error:

Free buf?

> +	return NULL;
> +}
> +

Rest looked okay.

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm