Re: [PATCH] RISC-V: Clobber V registers on syscalls

[Drew Fustini <drew@pdp7.com>]

On Mon, Jun 16, 2025 at 03:30:47PM -0700, Drew Fustini wrote:
> On Mon, Jun 19, 2023 at 12:05:43PM -0700, Palmer Dabbelt wrote:
> > On Mon, 19 Jun 2023 12:01:20 PDT (-0700), bjorn@kernel.org wrote:
> > > Palmer Dabbelt <palmer@rivosinc.com> writes:
> > > 
> > > [...]
> > > 
> > > > > > +		riscv_v_vstate_off(regs);
> > > > > > +
> > > > > 
> > > > > Not off, right? Isn't it __riscv_v_vstate_clean() that you'd like to
> > > > > call? Something like:
> > > > > 
> > > > > static void vstate_discard(struct pt_regs *regs)
> > > > > {
> > > > >        if ((regs->status & SR_VS) == SR_VS_DIRTY)
> > > > >                __riscv_v_vstate_clean(regs);
> > > > > }
> > > > > 
> > > > > Complemented by a !V config variant.
> > > > 
> > > > I think it's just a question of what we're trying to do here: clean
> > > > avoids the kernel V state save, but unless the kernel decides to use
> > > > V during the syscall the register contents will still be usable by
> > > > userspace.  Maybe that's fine and we can just rely on the ISA spec,
> > > > though?  I sent another patch to just document it in Linux, even if
> > > > it's in the ISA spec it seems worth having in the kernel as well.
> > > > 
> > > > That said, I think the right thing to do here might be to zero the V
> > > > register state and set it to initial: that way we can prevent
> > > > userspace from accidentally relying on the state save, but we can
> > > > also avoid the trap that would come from turning it off.  That lets
> > > > us give the hardware a nice clean indication when the V state isn't
> > > > in use, which will hopefully help us avoid the save/restore
> > > > performance issues that other ports have hit.
> > > 
> > > FWIW, I think that's a much better idea than turning V off. I also like
> > > that it'll preventing userland to rely on pre-ecall state.
> > 
> > OK, anyone else opposed?
> > 
> > We're kind of in the weeds on performance, I think we'd need HW to know for
> > sure if either is an issue.  Seems best to just play it safe WRT the uABI
> > for now, we can always deal with any performance issues if the exist.
> 
> I've tested the impact of riscv_v_vstate_discard() on the SiFive X280
> cores [1] in the Tenstorrent Blackhole SoC [2]. The results from the
> Blackhole P100 [3] card show that discarding the vector registers
> increases null syscall latency by 28%.
> 
> The null syscall program [4] executes the vsetvli vector instruction and
> then calls getppid() in a loop for 1 million iterations. The average
> duration of the syscall is 201 ns with a branch based on v6.16-rc1 [5].
> This is with the current upstream behavior where do_trap_ecall_u() calls
> riscv_v_vstate_discard().
> 
> I then created a new branch [6] which disables riscv_v_vstate_discard().
> The average duration of the syscall drops to 143 ns.
> 
> Would some sort of tunable be acceptable to allow the user to opt out
> of the v state discard? Maybe a kernel cmdline argument?
> 
> Thanks,
> Drew
> 
> [1] https://www.sifive.com/document-file/x280-datasheet
> [2] https://tenstorrent.com/en/hardware/blackhole
> [3] https://github.com/tenstorrent/tt-bh-linux
> [4] https://gist.github.com/tt-fustini/fa793a35c34f07059d8a7427e1cd8e84
> [5] https://github.com/tenstorrent/linux/tree/tt-blackhole-v6.16-rc1
> [6] https://github.com/tenstorrent/linux/tree/tt-blackhole-v6.16-rc1_no_vstate_discard

Adding Palmer's current email address.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv