public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v 0/2] Optimize the allocation of vector regset
@ 2025-10-01 11:14 Yong-Xuan Wang
  2025-10-01 11:14 ` [PATCH v 1/2] riscv: ptrace: " Yong-Xuan Wang
  2025-10-01 11:14 ` [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface Yong-Xuan Wang
  0 siblings, 2 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-10-01 11:14 UTC (permalink / raw)
  To: linux-kernel, linux-riscv
  Cc: greentime.hu, vincent.chen, andybnac, Yong-Xuan Wang,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti

The vector regset uses the maximum possible vlenb 8192 to allocate a
2^18 bytes buffer to copy the vector register. But most platforms
don’t support the largest vlenb.

The regset has 2 users, ptrace syscall and coredump. When handling the
PTRACE_GETREGSET requests from ptrace syscall, Linux will prepare a
kernel buffer which size is min(user buffer size, limit). A malicious
user process might overwhelm a memory-constrainted system when the
buffer limit is very large. The coredump uses regset_get_alloc() to
get the context of vector register. But this API allocates buffer
before checking whether the target process uses vector extension, this
wastes time to prepare a large memory buffer. 

The buffer limit can be determined after getting platform vlenb in the
early boot stage, this can let the regset buffer match real hardware
limits. Also add .active callbacks to let the coredump skip vector part
when target process doesn't use it.

After this patchset, userspace process needs 2 ptrace syscalls to
retrieve the vector regset with PTRACE_GETREGSET. The first ptrace call
only reads the header to get the vlenb information. Then prepare a
suitable buffer to get the register context. The new vector ptrace
kselftest demonstrates it.

Yong-Xuan Wang (2):
  riscv: ptrace: Optimize the allocation of vector regset
  selftests: riscv: Add test for the Vector ptrace interface

 arch/riscv/include/asm/vector.h               |   1 +
 arch/riscv/kernel/ptrace.c                    |  24 +++-
 arch/riscv/kernel/vector.c                    |   2 +
 tools/testing/selftests/riscv/vector/Makefile |   5 +-
 .../selftests/riscv/vector/vstate_ptrace.c    | 132 ++++++++++++++++++
 5 files changed, 160 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v 1/2] riscv: ptrace: Optimize the allocation of vector regset
  2025-10-01 11:14 [PATCH v 0/2] Optimize the allocation of vector regset Yong-Xuan Wang
@ 2025-10-01 11:14 ` Yong-Xuan Wang
  2025-10-01 14:50   ` Andy Chiu
  2025-10-01 11:14 ` [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface Yong-Xuan Wang
  1 sibling, 1 reply; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-10-01 11:14 UTC (permalink / raw)
  To: linux-kernel, linux-riscv
  Cc: greentime.hu, vincent.chen, andybnac, Yong-Xuan Wang,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Oleg Nesterov, Charlie Jenkins, Jesse Taube, Han Gao,
	Conor Dooley, Thomas Gleixner, Bill O'Donnell, Joel Granados

The vector regset uses the maximum possible vlen value to estimate the
.n field. But not all the hardwares support the maximum vlen. Linux
might wastes time to prepare a large memory buffer(about 2^6 pages) for
the vector regset.

The regset can only copy vector registers when the process are using
vector. Add .active callback and determine the n field of vector regset
in riscv_v_setup_ctx_cache() doesn't affect the ptrace syscall and
coredump. It can avoid oversized allocations and better matches real
hardware limits.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Reviewed-by: Greentime Hu <greentime.hu@sifive.com>
---
 arch/riscv/include/asm/vector.h |  1 +
 arch/riscv/kernel/ptrace.c      | 24 +++++++++++++++++++++---
 arch/riscv/kernel/vector.c      |  2 ++
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index b61786d43c20..e7aa449368ad 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -51,6 +51,7 @@ void put_cpu_vector_context(void);
 void riscv_v_thread_free(struct task_struct *tsk);
 void __init riscv_v_setup_ctx_cache(void);
 void riscv_v_thread_alloc(struct task_struct *tsk);
+void __init update_regset_vector_info(unsigned long size);
 
 static inline u32 riscv_v_flags(void)
 {
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 8e86305831ea..e6272d74572f 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -153,6 +153,17 @@ static int riscv_vr_set(struct task_struct *target,
 				 0, riscv_v_vsize);
 	return ret;
 }
+
+static int riscv_vr_active(struct task_struct *target, const struct user_regset *regset)
+{
+	if (!(has_vector() || has_xtheadvector()))
+		return -ENODEV;
+
+	if (!riscv_v_vstate_query(task_pt_regs(target)))
+		return 0;
+
+	return regset->n;
+}
 #endif
 
 #ifdef CONFIG_RISCV_ISA_SUPM
@@ -184,7 +195,7 @@ static int tagged_addr_ctrl_set(struct task_struct *target,
 }
 #endif
 
-static const struct user_regset riscv_user_regset[] = {
+static struct user_regset riscv_user_regset[] __ro_after_init = {
 	[REGSET_X] = {
 		USER_REGSET_NOTE_TYPE(PRSTATUS),
 		.n = ELF_NGREG,
@@ -207,11 +218,10 @@ static const struct user_regset riscv_user_regset[] = {
 	[REGSET_V] = {
 		USER_REGSET_NOTE_TYPE(RISCV_VECTOR),
 		.align = 16,
-		.n = ((32 * RISCV_MAX_VLENB) +
-		      sizeof(struct __riscv_v_regset_state)) / sizeof(__u32),
 		.size = sizeof(__u32),
 		.regset_get = riscv_vr_get,
 		.set = riscv_vr_set,
+		.active = riscv_vr_active,
 	},
 #endif
 #ifdef CONFIG_RISCV_ISA_SUPM
@@ -233,6 +243,14 @@ static const struct user_regset_view riscv_user_native_view = {
 	.n = ARRAY_SIZE(riscv_user_regset),
 };
 
+#ifdef CONFIG_RISCV_ISA_V
+void __init update_regset_vector_info(unsigned long size)
+{
+	riscv_user_regset[REGSET_V].n = (size + sizeof(struct __riscv_v_regset_state)) /
+					sizeof(__u32);
+}
+#endif
+
 struct pt_regs_offset {
 	const char *name;
 	int offset;
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 901e67adf576..3ed071dab9d8 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -66,6 +66,8 @@ void __init riscv_v_setup_ctx_cache(void)
 	if (!(has_vector() || has_xtheadvector()))
 		return;
 
+	update_regset_vector_info(riscv_v_vsize);
+
 	riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
 							 riscv_v_vsize, 16, SLAB_PANIC,
 							 0, riscv_v_vsize, NULL);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface
  2025-10-01 11:14 [PATCH v 0/2] Optimize the allocation of vector regset Yong-Xuan Wang
  2025-10-01 11:14 ` [PATCH v 1/2] riscv: ptrace: " Yong-Xuan Wang
@ 2025-10-01 11:14 ` Yong-Xuan Wang
  2025-10-01 15:12   ` Andy Chiu
  2025-10-02  5:53   ` Andy Chiu
  1 sibling, 2 replies; 7+ messages in thread
From: Yong-Xuan Wang @ 2025-10-01 11:14 UTC (permalink / raw)
  To: linux-kernel, linux-riscv
  Cc: greentime.hu, vincent.chen, andybnac, Yong-Xuan Wang, Shuah Khan,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Charlie Jenkins, linux-kselftest

Add a test case that does some basic verification of the Vector ptrace
interface. This forks a child process then using ptrace to inspect and
manipulate the v31 register of the child.

Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
---
 tools/testing/selftests/riscv/vector/Makefile |   5 +-
 .../selftests/riscv/vector/vstate_ptrace.c    | 132 ++++++++++++++++++
 2 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c

diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
index 6f7497f4e7b3..45f25e9dd264 100644
--- a/tools/testing/selftests/riscv/vector/Makefile
+++ b/tools/testing/selftests/riscv/vector/Makefile
@@ -2,7 +2,7 @@
 # Copyright (C) 2021 ARM Limited
 # Originally tools/testing/arm64/abi/Makefile
 
-TEST_GEN_PROGS := v_initval vstate_prctl
+TEST_GEN_PROGS := v_initval vstate_prctl vsate_ptrace
 TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc
 
 include ../../lib.mk
@@ -26,3 +26,6 @@ $(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
 $(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c
 	$(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
 		-Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
+
+$(OUTPUT)/vstate_ptrace: vstate_ptrace.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
+	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/vector/vstate_ptrace.c b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
new file mode 100644
index 000000000000..8a7bcf318e59
--- /dev/null
+++ b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <stdio.h>
+#include <stdlib.h>
+#include <asm/ptrace.h>
+#include <linux/elf.h>
+#include <sys/ptrace.h>
+#include <sys/uio.h>
+#include <sys/wait.h>
+#include "../../kselftest.h"
+#include "v_helpers.h"
+
+int parent_set_val, child_set_val;
+
+static long do_ptrace(enum __ptrace_request op, pid_t pid, long type, size_t size, void *data)
+{
+	struct iovec v_iovec = {
+		.iov_len = size,
+		.iov_base = data
+	};
+
+	return ptrace(op, pid, type, &v_iovec);
+}
+
+static int do_child(void)
+{
+	int out;
+
+	if (ptrace(PTRACE_TRACEME, -1, NULL, NULL)) {
+		ksft_perror("PTRACE_TRACEME failed\n");
+		return EXIT_FAILURE;
+	}
+
+	asm volatile (".option push\n\t"
+		".option	arch, +v\n\t"
+		"vsetivli	x0, 1, e32, m1, ta, ma\n\t"
+		"vmv.s.x	v31, %[in]\n\t"
+		"ebreak\n\t"
+		"vmv.x.s	%[out], v31\n\t"
+		".option pop\n\t"
+		: [out] "=r" (out)
+		: [in] "r" (child_set_val));
+
+	if (out != parent_set_val)
+		return EXIT_FAILURE;
+
+	return EXIT_SUCCESS;
+}
+
+static void do_parent(pid_t child)
+{
+	int status;
+	void *data = NULL;
+
+	/* Attach to the child */
+	while (waitpid(child, &status, 0)) {
+		if (WIFEXITED(status)) {
+			ksft_test_result(WEXITSTATUS(status) == 0, "SETREGSET vector\n");
+			goto out;
+		} else if (WIFSTOPPED(status) && (WSTOPSIG(status) == SIGTRAP)) {
+			size_t size, t;
+			void *data, *v31;
+			struct __riscv_v_regset_state *v_regset_hdr;
+			struct user_regs_struct *gpreg;
+
+			size = sizeof(*v_regset_hdr);
+			data = malloc(size);
+			if (!data)
+				goto out;
+			v_regset_hdr = (struct __riscv_v_regset_state *)data;
+
+			if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
+				goto out;
+
+			ksft_print_msg("vlenb %ld\n", v_regset_hdr->vlenb);
+			data = realloc(data, size + v_regset_hdr->vlenb * 32);
+			if (!data)
+				goto out;
+			v31 = (void *)(data + size + v_regset_hdr->vlenb * 31);
+			size += v_regset_hdr->vlenb * 32;
+
+			if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
+				goto out;
+
+			ksft_test_result(*(int *)v31 == child_set_val, "GETREGSET vector\n");
+
+			*(int *)v31 = parent_set_val;
+			if (do_ptrace(PTRACE_SETREGSET, child, NT_RISCV_VECTOR, size, data))
+				goto out;
+
+			/* move the pc forward */
+			size = sizeof(*gpreg);
+			data = realloc(data, size);
+			gpreg = (struct user_regs_struct *)data;
+
+			if (do_ptrace(PTRACE_GETREGSET, child, NT_PRSTATUS, size, data))
+				goto out;
+
+			gpreg->pc += 2;
+			if (do_ptrace(PTRACE_SETREGSET, child, NT_PRSTATUS, size, data))
+				goto out;
+		}
+
+		ptrace(PTRACE_CONT, child, NULL, NULL);
+	}
+
+out:
+	free(data);
+}
+
+int main(void)
+{
+	pid_t child;
+
+	ksft_set_plan(2);
+	if (!is_vector_supported() && !is_xtheadvector_supported())
+		ksft_exit_skip("Vector not supported\n");
+
+	srandom(getpid());
+	parent_set_val = rand();
+	child_set_val = rand();
+
+	child = fork();
+	if (child < 0)
+		ksft_exit_fail_msg("Fork failed %d\n", child);
+
+	if (!child)
+		return do_child();
+
+	do_parent(child);
+
+	ksft_finished();
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v 1/2] riscv: ptrace: Optimize the allocation of vector regset
  2025-10-01 11:14 ` [PATCH v 1/2] riscv: ptrace: " Yong-Xuan Wang
@ 2025-10-01 14:50   ` Andy Chiu
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Chiu @ 2025-10-01 14:50 UTC (permalink / raw)
  To: Yong-Xuan Wang
  Cc: linux-kernel, linux-riscv, greentime.hu, vincent.chen,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Oleg Nesterov, Charlie Jenkins, Jesse Taube, Han Gao,
	Conor Dooley, Thomas Gleixner, Bill O'Donnell, Joel Granados

Hi Yong-Xuan,

On Wed, Oct 1, 2025 at 6:15 AM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote:
>
> The vector regset uses the maximum possible vlen value to estimate the
> .n field. But not all the hardwares support the maximum vlen. Linux
> might wastes time to prepare a large memory buffer(about 2^6 pages) for
> the vector regset.
>
> The regset can only copy vector registers when the process are using
> vector. Add .active callback and determine the n field of vector regset
> in riscv_v_setup_ctx_cache() doesn't affect the ptrace syscall and
> coredump. It can avoid oversized allocations and better matches real
> hardware limits.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> Reviewed-by: Greentime Hu <greentime.hu@sifive.com>

Reviewed-by: Andy Chiu <andybnac@gmail.com>

Thanks,
Andy

> ---
>  arch/riscv/include/asm/vector.h |  1 +
>  arch/riscv/kernel/ptrace.c      | 24 +++++++++++++++++++++---
>  arch/riscv/kernel/vector.c      |  2 ++
>  3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
> index b61786d43c20..e7aa449368ad 100644
> --- a/arch/riscv/include/asm/vector.h
> +++ b/arch/riscv/include/asm/vector.h
> @@ -51,6 +51,7 @@ void put_cpu_vector_context(void);
>  void riscv_v_thread_free(struct task_struct *tsk);
>  void __init riscv_v_setup_ctx_cache(void);
>  void riscv_v_thread_alloc(struct task_struct *tsk);
> +void __init update_regset_vector_info(unsigned long size);
>
>  static inline u32 riscv_v_flags(void)
>  {
> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index 8e86305831ea..e6272d74572f 100644
> --- a/arch/riscv/kernel/ptrace.c
> +++ b/arch/riscv/kernel/ptrace.c
> @@ -153,6 +153,17 @@ static int riscv_vr_set(struct task_struct *target,
>                                  0, riscv_v_vsize);
>         return ret;
>  }
> +
> +static int riscv_vr_active(struct task_struct *target, const struct user_regset *regset)
> +{
> +       if (!(has_vector() || has_xtheadvector()))
> +               return -ENODEV;
> +
> +       if (!riscv_v_vstate_query(task_pt_regs(target)))
> +               return 0;
> +
> +       return regset->n;
> +}
>  #endif
>
>  #ifdef CONFIG_RISCV_ISA_SUPM
> @@ -184,7 +195,7 @@ static int tagged_addr_ctrl_set(struct task_struct *target,
>  }
>  #endif
>
> -static const struct user_regset riscv_user_regset[] = {
> +static struct user_regset riscv_user_regset[] __ro_after_init = {
>         [REGSET_X] = {
>                 USER_REGSET_NOTE_TYPE(PRSTATUS),
>                 .n = ELF_NGREG,
> @@ -207,11 +218,10 @@ static const struct user_regset riscv_user_regset[] = {
>         [REGSET_V] = {
>                 USER_REGSET_NOTE_TYPE(RISCV_VECTOR),
>                 .align = 16,
> -               .n = ((32 * RISCV_MAX_VLENB) +
> -                     sizeof(struct __riscv_v_regset_state)) / sizeof(__u32),
>                 .size = sizeof(__u32),
>                 .regset_get = riscv_vr_get,
>                 .set = riscv_vr_set,
> +               .active = riscv_vr_active,
>         },
>  #endif
>  #ifdef CONFIG_RISCV_ISA_SUPM
> @@ -233,6 +243,14 @@ static const struct user_regset_view riscv_user_native_view = {
>         .n = ARRAY_SIZE(riscv_user_regset),
>  };
>
> +#ifdef CONFIG_RISCV_ISA_V
> +void __init update_regset_vector_info(unsigned long size)
> +{
> +       riscv_user_regset[REGSET_V].n = (size + sizeof(struct __riscv_v_regset_state)) /
> +                                       sizeof(__u32);
> +}
> +#endif
> +
>  struct pt_regs_offset {
>         const char *name;
>         int offset;
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 901e67adf576..3ed071dab9d8 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -66,6 +66,8 @@ void __init riscv_v_setup_ctx_cache(void)
>         if (!(has_vector() || has_xtheadvector()))
>                 return;
>
> +       update_regset_vector_info(riscv_v_vsize);
> +
>         riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
>                                                          riscv_v_vsize, 16, SLAB_PANIC,
>                                                          0, riscv_v_vsize, NULL);
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface
  2025-10-01 11:14 ` [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface Yong-Xuan Wang
@ 2025-10-01 15:12   ` Andy Chiu
  2025-10-02  5:53   ` Andy Chiu
  1 sibling, 0 replies; 7+ messages in thread
From: Andy Chiu @ 2025-10-01 15:12 UTC (permalink / raw)
  To: Yong-Xuan Wang
  Cc: linux-kernel, linux-riscv, greentime.hu, vincent.chen, Shuah Khan,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Charlie Jenkins, linux-kselftest

Hi Yong-Xuan,

On Wed, Oct 1, 2025 at 6:15 AM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote:
>
> Add a test case that does some basic verification of the Vector ptrace
> interface. This forks a child process then using ptrace to inspect and
> manipulate the v31 register of the child.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  tools/testing/selftests/riscv/vector/Makefile |   5 +-
>  .../selftests/riscv/vector/vstate_ptrace.c    | 132 ++++++++++++++++++
>  2 files changed, 136 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c
>
> diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
> index 6f7497f4e7b3..45f25e9dd264 100644
> --- a/tools/testing/selftests/riscv/vector/Makefile
> +++ b/tools/testing/selftests/riscv/vector/Makefile
> @@ -2,7 +2,7 @@
>  # Copyright (C) 2021 ARM Limited
>  # Originally tools/testing/arm64/abi/Makefile
>
> -TEST_GEN_PROGS := v_initval vstate_prctl
> +TEST_GEN_PROGS := v_initval vstate_prctl vsate_ptrace
>  TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc
>
>  include ../../lib.mk
> @@ -26,3 +26,6 @@ $(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
>  $(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c
>         $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
>                 -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
> +
> +$(OUTPUT)/vstate_ptrace: vstate_ptrace.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
> +       $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> diff --git a/tools/testing/selftests/riscv/vector/vstate_ptrace.c b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
> new file mode 100644
> index 000000000000..8a7bcf318e59
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <asm/ptrace.h>
> +#include <linux/elf.h>
> +#include <sys/ptrace.h>
> +#include <sys/uio.h>
> +#include <sys/wait.h>
> +#include "../../kselftest.h"
> +#include "v_helpers.h"
> +
> +int parent_set_val, child_set_val;
> +
> +static long do_ptrace(enum __ptrace_request op, pid_t pid, long type, size_t size, void *data)
> +{
> +       struct iovec v_iovec = {
> +               .iov_len = size,
> +               .iov_base = data
> +       };
> +
> +       return ptrace(op, pid, type, &v_iovec);
> +}
> +
> +static int do_child(void)
> +{
> +       int out;
> +
> +       if (ptrace(PTRACE_TRACEME, -1, NULL, NULL)) {
> +               ksft_perror("PTRACE_TRACEME failed\n");
> +               return EXIT_FAILURE;
> +       }
> +
> +       asm volatile (".option push\n\t"
> +               ".option        arch, +v\n\t"
> +               "vsetivli       x0, 1, e32, m1, ta, ma\n\t"
> +               "vmv.s.x        v31, %[in]\n\t"
> +               "ebreak\n\t"
> +               "vmv.x.s        %[out], v31\n\t"
> +               ".option pop\n\t"
> +               : [out] "=r" (out)
> +               : [in] "r" (child_set_val));
> +
> +       if (out != parent_set_val)
> +               return EXIT_FAILURE;
> +
> +       return EXIT_SUCCESS;
> +}
> +
> +static void do_parent(pid_t child)
> +{
> +       int status;
> +       void *data = NULL;
> +
> +       /* Attach to the child */
> +       while (waitpid(child, &status, 0)) {
> +               if (WIFEXITED(status)) {
> +                       ksft_test_result(WEXITSTATUS(status) == 0, "SETREGSET vector\n");
> +                       goto out;
> +               } else if (WIFSTOPPED(status) && (WSTOPSIG(status) == SIGTRAP)) {
> +                       size_t size, t;
> +                       void *data, *v31;
> +                       struct __riscv_v_regset_state *v_regset_hdr;
> +                       struct user_regs_struct *gpreg;
> +
> +                       size = sizeof(*v_regset_hdr);
> +                       data = malloc(size);
> +                       if (!data)
> +                               goto out;
> +                       v_regset_hdr = (struct __riscv_v_regset_state *)data;
> +
> +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
> +                               goto out;
> +
> +                       ksft_print_msg("vlenb %ld\n", v_regset_hdr->vlenb);
> +                       data = realloc(data, size + v_regset_hdr->vlenb * 32);
> +                       if (!data)
> +                               goto out;
> +                       v31 = (void *)(data + size + v_regset_hdr->vlenb * 31);
> +                       size += v_regset_hdr->vlenb * 32;
> +
> +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
> +                               goto out;
> +
> +                       ksft_test_result(*(int *)v31 == child_set_val, "GETREGSET vector\n");
> +
> +                       *(int *)v31 = parent_set_val;
> +                       if (do_ptrace(PTRACE_SETREGSET, child, NT_RISCV_VECTOR, size, data))
> +                               goto out;
> +
> +                       /* move the pc forward */
> +                       size = sizeof(*gpreg);
> +                       data = realloc(data, size);
> +                       gpreg = (struct user_regs_struct *)data;
> +
> +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_PRSTATUS, size, data))
> +                               goto out;
> +
> +                       gpreg->pc += 2;

Just nitpicking here, simply adding 2 may fail if the program is not
compiled with C. You may either +=4 and use ".option norvc" in the asm
or determine the size of ebreak by decoding it.

with or without the fix,

Reviewed-by: Andy Chiu <andybnac@gmail.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface
  2025-10-01 11:14 ` [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface Yong-Xuan Wang
  2025-10-01 15:12   ` Andy Chiu
@ 2025-10-02  5:53   ` Andy Chiu
  2025-10-02  5:59     ` Charlie Jenkins
  1 sibling, 1 reply; 7+ messages in thread
From: Andy Chiu @ 2025-10-02  5:53 UTC (permalink / raw)
  To: Yong-Xuan Wang
  Cc: linux-kernel, linux-riscv, greentime.hu, vincent.chen, Shuah Khan,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Charlie Jenkins, linux-kselftest

Hi Yong-Xuan,

I found some issues which deserve a re-roll:

On Wed, Oct 1, 2025 at 6:15 AM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote:
>
> Add a test case that does some basic verification of the Vector ptrace
> interface. This forks a child process then using ptrace to inspect and
> manipulate the v31 register of the child.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> ---
>  tools/testing/selftests/riscv/vector/Makefile |   5 +-
>  .../selftests/riscv/vector/vstate_ptrace.c    | 132 ++++++++++++++++++
>  2 files changed, 136 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c
>
> diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
> index 6f7497f4e7b3..45f25e9dd264 100644
> --- a/tools/testing/selftests/riscv/vector/Makefile
> +++ b/tools/testing/selftests/riscv/vector/Makefile
> @@ -2,7 +2,7 @@
>  # Copyright (C) 2021 ARM Limited
>  # Originally tools/testing/arm64/abi/Makefile
>
> -TEST_GEN_PROGS := v_initval vstate_prctl
> +TEST_GEN_PROGS := v_initval vstate_prctl vsate_ptrace

Please s/vsate_ptrace/vstate_ptrace

Otherwise we will not get the program compiled

>  TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc
>
>  include ../../lib.mk
> @@ -26,3 +26,6 @@ $(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
>  $(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c
>         $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
>                 -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
> +
> +$(OUTPUT)/vstate_ptrace: vstate_ptrace.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
> +       $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> diff --git a/tools/testing/selftests/riscv/vector/vstate_ptrace.c b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
> new file mode 100644
> index 000000000000..8a7bcf318e59
> --- /dev/null
> +++ b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <asm/ptrace.h>
> +#include <linux/elf.h>
> +#include <sys/ptrace.h>
> +#include <sys/uio.h>
> +#include <sys/wait.h>
> +#include "../../kselftest.h"
> +#include "v_helpers.h"
> +
> +int parent_set_val, child_set_val;
> +
> +static long do_ptrace(enum __ptrace_request op, pid_t pid, long type, size_t size, void *data)
> +{
> +       struct iovec v_iovec = {
> +               .iov_len = size,
> +               .iov_base = data
> +       };
> +
> +       return ptrace(op, pid, type, &v_iovec);
> +}
> +
> +static int do_child(void)
> +{
> +       int out;
> +
> +       if (ptrace(PTRACE_TRACEME, -1, NULL, NULL)) {
> +               ksft_perror("PTRACE_TRACEME failed\n");
> +               return EXIT_FAILURE;
> +       }
> +
> +       asm volatile (".option push\n\t"
> +               ".option        arch, +v\n\t"

As mentioned before, please use ".option arch, +v,-c\n\t" or ".option
norvc\n\t" and +=4 when advancing the pc

> +               "vsetivli       x0, 1, e32, m1, ta, ma\n\t"
> +               "vmv.s.x        v31, %[in]\n\t"
> +               "ebreak\n\t"
> +               "vmv.x.s        %[out], v31\n\t"
> +               ".option pop\n\t"
> +               : [out] "=r" (out)
> +               : [in] "r" (child_set_val));
> +
> +       if (out != parent_set_val)
> +               return EXIT_FAILURE;
> +
> +       return EXIT_SUCCESS;
> +}
> +
> +static void do_parent(pid_t child)
> +{
> +       int status;
> +       void *data = NULL;
> +
> +       /* Attach to the child */
> +       while (waitpid(child, &status, 0)) {
> +               if (WIFEXITED(status)) {
> +                       ksft_test_result(WEXITSTATUS(status) == 0, "SETREGSET vector\n");
> +                       goto out;
> +               } else if (WIFSTOPPED(status) && (WSTOPSIG(status) == SIGTRAP)) {
> +                       size_t size, t;

unused variable t

> +                       void *data, *v31;
> +                       struct __riscv_v_regset_state *v_regset_hdr;
> +                       struct user_regs_struct *gpreg;
> +
> +                       size = sizeof(*v_regset_hdr);
> +                       data = malloc(size);
> +                       if (!data)
> +                               goto out;
> +                       v_regset_hdr = (struct __riscv_v_regset_state *)data;
> +
> +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
> +                               goto out;
> +
> +                       ksft_print_msg("vlenb %ld\n", v_regset_hdr->vlenb);
> +                       data = realloc(data, size + v_regset_hdr->vlenb * 32);

realloc may give a new pointer so v_regset_hdr has to be updated here
before the next use

> +                       if (!data)
> +                               goto out;
> +                       v31 = (void *)(data + size + v_regset_hdr->vlenb * 31);
> +                       size += v_regset_hdr->vlenb * 32;
> +
> +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
> +                               goto out;
> +
> +                       ksft_test_result(*(int *)v31 == child_set_val, "GETREGSET vector\n");
> +
> +                       *(int *)v31 = parent_set_val;
> +                       if (do_ptrace(PTRACE_SETREGSET, child, NT_RISCV_VECTOR, size, data))
> +                               goto out;
> +
> +                       /* move the pc forward */
> +                       size = sizeof(*gpreg);
> +                       data = realloc(data, size);
> +                       gpreg = (struct user_regs_struct *)data;
> +
> +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_PRSTATUS, size, data))
> +                               goto out;
> +
> +                       gpreg->pc += 2;
> +                       if (do_ptrace(PTRACE_SETREGSET, child, NT_PRSTATUS, size, data))
> +                               goto out;
> +               }
> +
> +               ptrace(PTRACE_CONT, child, NULL, NULL);
> +       }
> +
> +out:
> +       free(data);
> +}
> +
> +int main(void)
> +{
> +       pid_t child;
> +
> +       ksft_set_plan(2);
> +       if (!is_vector_supported() && !is_xtheadvector_supported())
> +               ksft_exit_skip("Vector not supported\n");
> +
> +       srandom(getpid());
> +       parent_set_val = rand();
> +       child_set_val = rand();
> +
> +       child = fork();
> +       if (child < 0)
> +               ksft_exit_fail_msg("Fork failed %d\n", child);
> +
> +       if (!child)
> +               return do_child();
> +
> +       do_parent(child);
> +
> +       ksft_finished();
> +}
> --
> 2.43.0
>

Thanks,
Andy

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface
  2025-10-02  5:53   ` Andy Chiu
@ 2025-10-02  5:59     ` Charlie Jenkins
  0 siblings, 0 replies; 7+ messages in thread
From: Charlie Jenkins @ 2025-10-02  5:59 UTC (permalink / raw)
  To: Andy Chiu
  Cc: Yong-Xuan Wang, linux-kernel, linux-riscv, greentime.hu,
	vincent.chen, Shuah Khan, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexandre Ghiti, linux-kselftest

On Thu, Oct 02, 2025 at 12:53:13AM -0500, Andy Chiu wrote:
> Hi Yong-Xuan,
> 
> I found some issues which deserve a re-roll:
> 
> On Wed, Oct 1, 2025 at 6:15 AM Yong-Xuan Wang <yongxuan.wang@sifive.com> wrote:
> >
> > Add a test case that does some basic verification of the Vector ptrace
> > interface. This forks a child process then using ptrace to inspect and
> > manipulate the v31 register of the child.
> >
> > Signed-off-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
> > ---
> >  tools/testing/selftests/riscv/vector/Makefile |   5 +-
> >  .../selftests/riscv/vector/vstate_ptrace.c    | 132 ++++++++++++++++++
> >  2 files changed, 136 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/testing/selftests/riscv/vector/vstate_ptrace.c
> >
> > diff --git a/tools/testing/selftests/riscv/vector/Makefile b/tools/testing/selftests/riscv/vector/Makefile
> > index 6f7497f4e7b3..45f25e9dd264 100644
> > --- a/tools/testing/selftests/riscv/vector/Makefile
> > +++ b/tools/testing/selftests/riscv/vector/Makefile
> > @@ -2,7 +2,7 @@
> >  # Copyright (C) 2021 ARM Limited
> >  # Originally tools/testing/arm64/abi/Makefile
> >
> > -TEST_GEN_PROGS := v_initval vstate_prctl
> > +TEST_GEN_PROGS := v_initval vstate_prctl vsate_ptrace
> 
> Please s/vsate_ptrace/vstate_ptrace
> 
> Otherwise we will not get the program compiled
> 
> >  TEST_GEN_PROGS_EXTENDED := vstate_exec_nolibc v_exec_initval_nolibc
> >
> >  include ../../lib.mk
> > @@ -26,3 +26,6 @@ $(OUTPUT)/v_initval: v_initval.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
> >  $(OUTPUT)/v_exec_initval_nolibc: v_exec_initval_nolibc.c
> >         $(CC) -nostdlib -static -include ../../../../include/nolibc/nolibc.h \
> >                 -Wall $(CFLAGS) $(LDFLAGS) $^ -o $@ -lgcc
> > +
> > +$(OUTPUT)/vstate_ptrace: vstate_ptrace.c $(OUTPUT)/sys_hwprobe.o $(OUTPUT)/v_helpers.o
> > +       $(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
> > diff --git a/tools/testing/selftests/riscv/vector/vstate_ptrace.c b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
> > new file mode 100644
> > index 000000000000..8a7bcf318e59
> > --- /dev/null
> > +++ b/tools/testing/selftests/riscv/vector/vstate_ptrace.c
> > @@ -0,0 +1,132 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <asm/ptrace.h>
> > +#include <linux/elf.h>
> > +#include <sys/ptrace.h>
> > +#include <sys/uio.h>
> > +#include <sys/wait.h>
> > +#include "../../kselftest.h"
> > +#include "v_helpers.h"
> > +
> > +int parent_set_val, child_set_val;
> > +
> > +static long do_ptrace(enum __ptrace_request op, pid_t pid, long type, size_t size, void *data)
> > +{
> > +       struct iovec v_iovec = {
> > +               .iov_len = size,
> > +               .iov_base = data
> > +       };
> > +
> > +       return ptrace(op, pid, type, &v_iovec);
> > +}
> > +
> > +static int do_child(void)
> > +{
> > +       int out;
> > +
> > +       if (ptrace(PTRACE_TRACEME, -1, NULL, NULL)) {
> > +               ksft_perror("PTRACE_TRACEME failed\n");
> > +               return EXIT_FAILURE;
> > +       }
> > +
> > +       asm volatile (".option push\n\t"
> > +               ".option        arch, +v\n\t"
> 
> As mentioned before, please use ".option arch, +v,-c\n\t" or ".option
> norvc\n\t" and +=4 when advancing the pc

arch -c should be avoided, there are cases when it does not always avoid
using all compressed instructions. norvc should always do the right
thing though. There is discussion at [1] about deprecating it (along with
all variants of -ext).

[1] https://inbox.sourceware.org/binutils/7ecdc846-0822-4666-957f-ff818786fb44@iscas.ac.cn/T/#t

- Charlie

> 
> > +               "vsetivli       x0, 1, e32, m1, ta, ma\n\t"
> > +               "vmv.s.x        v31, %[in]\n\t"
> > +               "ebreak\n\t"
> > +               "vmv.x.s        %[out], v31\n\t"
> > +               ".option pop\n\t"
> > +               : [out] "=r" (out)
> > +               : [in] "r" (child_set_val));
> > +
> > +       if (out != parent_set_val)
> > +               return EXIT_FAILURE;
> > +
> > +       return EXIT_SUCCESS;
> > +}
> > +
> > +static void do_parent(pid_t child)
> > +{
> > +       int status;
> > +       void *data = NULL;
> > +
> > +       /* Attach to the child */
> > +       while (waitpid(child, &status, 0)) {
> > +               if (WIFEXITED(status)) {
> > +                       ksft_test_result(WEXITSTATUS(status) == 0, "SETREGSET vector\n");
> > +                       goto out;
> > +               } else if (WIFSTOPPED(status) && (WSTOPSIG(status) == SIGTRAP)) {
> > +                       size_t size, t;
> 
> unused variable t
> 
> > +                       void *data, *v31;
> > +                       struct __riscv_v_regset_state *v_regset_hdr;
> > +                       struct user_regs_struct *gpreg;
> > +
> > +                       size = sizeof(*v_regset_hdr);
> > +                       data = malloc(size);
> > +                       if (!data)
> > +                               goto out;
> > +                       v_regset_hdr = (struct __riscv_v_regset_state *)data;
> > +
> > +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
> > +                               goto out;
> > +
> > +                       ksft_print_msg("vlenb %ld\n", v_regset_hdr->vlenb);
> > +                       data = realloc(data, size + v_regset_hdr->vlenb * 32);
> 
> realloc may give a new pointer so v_regset_hdr has to be updated here
> before the next use
> 
> > +                       if (!data)
> > +                               goto out;
> > +                       v31 = (void *)(data + size + v_regset_hdr->vlenb * 31);
> > +                       size += v_regset_hdr->vlenb * 32;
> > +
> > +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_RISCV_VECTOR, size, data))
> > +                               goto out;
> > +
> > +                       ksft_test_result(*(int *)v31 == child_set_val, "GETREGSET vector\n");
> > +
> > +                       *(int *)v31 = parent_set_val;
> > +                       if (do_ptrace(PTRACE_SETREGSET, child, NT_RISCV_VECTOR, size, data))
> > +                               goto out;
> > +
> > +                       /* move the pc forward */
> > +                       size = sizeof(*gpreg);
> > +                       data = realloc(data, size);
> > +                       gpreg = (struct user_regs_struct *)data;
> > +
> > +                       if (do_ptrace(PTRACE_GETREGSET, child, NT_PRSTATUS, size, data))
> > +                               goto out;
> > +
> > +                       gpreg->pc += 2;
> > +                       if (do_ptrace(PTRACE_SETREGSET, child, NT_PRSTATUS, size, data))
> > +                               goto out;
> > +               }
> > +
> > +               ptrace(PTRACE_CONT, child, NULL, NULL);
> > +       }
> > +
> > +out:
> > +       free(data);
> > +}
> > +
> > +int main(void)
> > +{
> > +       pid_t child;
> > +
> > +       ksft_set_plan(2);
> > +       if (!is_vector_supported() && !is_xtheadvector_supported())
> > +               ksft_exit_skip("Vector not supported\n");
> > +
> > +       srandom(getpid());
> > +       parent_set_val = rand();
> > +       child_set_val = rand();
> > +
> > +       child = fork();
> > +       if (child < 0)
> > +               ksft_exit_fail_msg("Fork failed %d\n", child);
> > +
> > +       if (!child)
> > +               return do_child();
> > +
> > +       do_parent(child);
> > +
> > +       ksft_finished();
> > +}
> > --
> > 2.43.0
> >
> 
> Thanks,
> Andy

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-02  5:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-01 11:14 [PATCH v 0/2] Optimize the allocation of vector regset Yong-Xuan Wang
2025-10-01 11:14 ` [PATCH v 1/2] riscv: ptrace: " Yong-Xuan Wang
2025-10-01 14:50   ` Andy Chiu
2025-10-01 11:14 ` [PATCH v 2/2] selftests: riscv: Add test for the Vector ptrace interface Yong-Xuan Wang
2025-10-01 15:12   ` Andy Chiu
2025-10-02  5:53   ` Andy Chiu
2025-10-02  5:59     ` Charlie Jenkins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox