From: Drew Fustini <fustini@kernel.org>
To: Vivian Wang <wangruikang@iscas.ac.cn>, Darius Rad <darius@bluespec.com>
Cc: "Paul Walmsley" <paul.walmsley@sifive.com>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Alexandre Ghiti" <alex@ghiti.fr>,
"Samuel Holland" <samuel.holland@sifive.com>,
"Björn Töpel" <bjorn@rivosinc.com>,
"Andy Chiu" <andybnac@gmail.com>,
"Conor Dooley" <conor.dooley@microchip.com>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
"Drew Fustini" <dfustini@tenstorrent.com>
Subject: Re: [PATCH v2] riscv: Add sysctl to control discard of vstate during syscall
Date: Sat, 9 Aug 2025 00:54:42 -0700 [thread overview]
Message-ID: <aJb+wifbIAsit+me@x1> (raw)
In-Reply-To: <e4d21516-23b5-404b-a7da-cf6ebfd02dbc@iscas.ac.cn>
On Sat, Aug 09, 2025 at 11:58:24AM +0800, Vivian Wang wrote:
> My previous comment on v1 on prefering clobbering with VS = Initial
> handling aside...
I found that in the discard vector state patch discussion 2 years ago
that Andy and Bjorn discussed how Initial could cause a problem [1]:
It's not a racy, but you're correct that setting the state to Initial,
will cause issues. When get/set_regs is called, the tracee will be
stopped, and a schedule() has been done.
In the v3 series, Bjorn notes [2]:
Set state to Dirty after discard, for proper ptrace() handling (Andy)
Also, I would like the ability to have the ability to switch off
__riscv_v_vstate_discard() and not loose any cycles to it, so I think
this sysctl is a good fit for that.
>
> On 8/8/25 20:36, Darius Rad wrote:
> > On Wed, Aug 06, 2025 at 07:03:28AM -0700, Drew Fustini wrote:
> > [...]
> >> diff --git a/Documentation/arch/riscv/vector.rst b/Documentation/arch/riscv/vector.rst
> >> index 3987f5f76a9deb0824e53a72df4c3bf90ac2bee1..b702c00351617165a4d8897c7df68eadcd2d562e 100644
> >> --- a/Documentation/arch/riscv/vector.rst
> >> +++ b/Documentation/arch/riscv/vector.rst
> >> @@ -134,7 +134,25 @@ processes in form of sysctl knob:
> >> 3. Vector Register State Across System Calls
> >> ---------------------------------------------
> >>
> >> -As indicated by version 1.0 of the V extension [1], vector registers are
> >> -clobbered by system calls.
> >> +Linux adopts the syscall ABI proposed by version 1.0 of the V extension [1],
> >> +where vector registers are clobbered by system calls. Specifically:
> >> +
> >> + Executing a system call causes all caller-saved vector registers
> >> + (v0-v31, vl, vtype) and vstart to become unspecied.
> >> +
> > Perhaps:
> >
> > Clobbering the vector registers may prevent leaking information to user
>
> No... Not clobbering does not "leak" anything. If you find that it leaks
> information, please report - that's a bug.
Thanks Darius and Vivian for your comments. I think it is a good idea
for me to write about the possible advantages of mandatory clobbering on
syscall entry. However, I am also uncertain how clobbering on syscall
entry helps prevent leaking information.
> > space and aid in debugging, but can significantly increase system call
> > latency for some implementations. [...]
I think that is a good idea for me to call out that this is can be
useful for debugging and testing.
> >
> >> +However, clobbering the vector registers can significantly increase system call
> >> +latency for some implementations. To mitigate this performance impact, a sysctl
> >> +knob is provided that controls whether vector state is always discarded in the
> >> +syscall path:
> >> +
> >> +* /proc/sys/abi/riscv_v_vstate_discard
> >> +
> >> + Valid values are:
> >> +
> >> + * 0: Vector state is not always clobbered in all syscalls
> >> + * 1: Mandatory clobbering of vector state in all syscalls
> >> +
> >> + Reading this file returns the current discard behavior. The initial state is
> >> + controlled by CONFIG_RISCV_ISA_V_VSTATE_DISCARD.
> >>
> >> 1: https://github.com/riscv/riscv-v-spec/blob/master/calling-convention.adoc
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index 36061f4732b7496a9c68a9a10f9959849dc2a95c..7bb8a8513135cbc105bd94d273012486a886f724 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -656,6 +656,16 @@ config RISCV_ISA_V_DEFAULT_ENABLE
> >>
> >> If you don't know what to do here, say Y.
> >>
> >> +config RISCV_ISA_V_VSTATE_DISCARD
> >> + bool "Enable Vector state discard by default"
> >> + depends on RISCV_ISA_V
> >> + default n
> >> + help
> > Perhaps add the following paragraph:
> >
> > Discarding vector state is more robust, but has negative performance
> > implications in certain implementations.
>
> "Robust" is too vague... I don't think this word is helpful for anyone
> trying to understand what this does.
I agree that I should add more description to the Kconfig option as I
think what I wrote assumes too much prior knowledge of the code. Maybe
something like this:
Discarding vector state on syscall entry can help identify userpace
programs that are mistakenly relying on vector state being preserved
across syscalls. This can be useful for debugging and test suites.
However, this behavior can negatively impact performance on some
RISC-V implementations.
Say Y here if you want mandatory clobbering of vector state before
entering all syscalls. If you select N, then userspace can still
eanble it via the abi.riscv_v_vstate_discard sysctl knob.
If you don't know what to do here, then select N.
Thanks,
Drew
[1] https://lore.kernel.org/linux-riscv/87r0pug6hb.fsf@all.your.base.are.belong.to.us/
[2] https://lore.kernel.org/linux-riscv/20230629062730.985184-1-bjorn@kernel.org/
next prev parent reply other threads:[~2025-08-09 7:54 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-06 14:03 [PATCH v2] riscv: Add sysctl to control discard of vstate during syscall Drew Fustini
2025-08-08 12:36 ` Darius Rad
2025-08-09 3:58 ` Vivian Wang
2025-08-09 7:54 ` Drew Fustini [this message]
2025-08-10 9:52 ` Vivian Wang
2025-08-12 5:46 ` Drew Fustini
2025-08-09 13:23 ` Darius Rad
2025-08-09 8:40 ` Florian Weimer
2025-08-09 21:23 ` Drew Fustini
2025-08-10 7:45 ` Florian Weimer
2025-08-11 20:36 ` Drew Fustini
2025-08-12 8:03 ` Florian Weimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aJb+wifbIAsit+me@x1 \
--to=fustini@kernel.org \
--cc=alex@ghiti.fr \
--cc=andybnac@gmail.com \
--cc=bjorn@rivosinc.com \
--cc=conor.dooley@microchip.com \
--cc=darius@bluespec.com \
--cc=dfustini@tenstorrent.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=samuel.holland@sifive.com \
--cc=wangruikang@iscas.ac.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox