Re: [PATCH] riscv: Add sysctl to control discard of vstate during syscall

[Drew Fustini <fustini@kernel.org>]

On Mon, Jul 21, 2025 at 02:35:38PM +0200, Radim Krčmář wrote:
> 2025-07-18T20:39:13-07:00, Drew Fustini <fustini@kernel.org>:
> > From: Drew Fustini <dfustini@tenstorrent.com>
> >
> > Clobbering the vector registers can significantly increase system call
> > latency for some implementations. To mitigate this performance impact, a
> > policy mechanism is provided to administrators, distro maintainers, and
> > developers to control vector state discard in the form of a sysctl knob:
> >
> > /proc/sys/abi/riscv_v_vstate_discard
> >
> > Valid values are:
> >
> > 0: Do not discard vector state during syscall
> > 1: Discard vector state during syscall
> >
> > The initial state is controlled by CONFIG_RISCV_ISA_V_VSTATE_DISCARD.
> 
> I think it is a bit more complicated to do this nicely...
> Programs don't have to save/restore vector registers around syscalls
> when compiled for riscv_v_vstate_discard=0, so running under
> riscv_v_vstate_discard=1 would break them.

Thanks for your comments. You raise a good point that this sysctl can
lead to the case where a program might be compiled to not save/restore
vector registers around syscalls. That same program would not work
correctly if the sysadmin changes riscv_v_vstate_discard to 1.

> Shouldn't we have a way to prevent riscv_v_vstate_discard=0 executable
> from running with riscv_v_vstate_discard=1?

Yes, this does make me concerned that a program could crash as a result
of this sysctl which would be confusing for the user as they may not
even be aware of this sysctl. I'll have to think more about how such a
protection could work.

> 
> > Fixes: 9657e9b7d253 ("riscv: Discard vector state on syscalls")
> 
> Programs compiled for riscv_v_vstate_discard=1 are compatible with 0, so
> I think it would be simplest to revert that patch, and pretended it
> never happened... (The issues will eventually go away.)

I agree that reverting the existing discard behavior would be the
simplest solution to the peformance issue observed on some
implementations. However, I believe there is also the desire to have a
way to enforce strict clobbering across syscalls to catch any incorrect
behavior while testing. I was hoping a syscall could allow both use
cases to be handled, but you raise good points about compatibility.

> Shouldn't the RISC-V Linux syscall ABI be defined somewhere?
> How come we could have broken it with 9657e9b7d253?

I may have been wrong to use a Fixes tag for 9657e9b7d253. I was trying
to highlight the original discussion that I was trying to address with
this sysctl patch.

> 
> Thanks.
> 
> ---
> I don't think it makes much sense to clobber vector registers on a
> syscall -- a kernel might not even touch vector registers, so they are
> efforlessly preserved in that case.
> If kernel needs to use vector registers in the syscall, then the kernel
> needs to prevent any register leaks to userspace anyway by restoring
> some state into them -- and why not restore the original one?
> 
> I think that main point of clobbering would be to optimize
> context-switches after the userspace is not using vector registers
> anymore, but it's terribly inefficient if the ratio of syscalls to
> context switches is high.
> Linux can also try to detect the situation, and turn to lazy vector
> context-switch, with sstatus.VS=off, instead of eagerly restoring
> clobbered state.
> (A good indicator might be that the userspace hasn't dirtied the vectors
>  since the last context-switch -- kernel didn't need to save the state,
>  so it will restore lazily.)

I think this is an interesting discussion to have. I was hoping this
patch would get people discussing if mandatory vector state cloberring
is really something that should be do in syscalls.

Thanks,
Drew