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

From: Drew Fustini <pdp7pdp7@gmail.com>
To: Palmer Dabbelt <palmer@rivosinc.com>
Cc: bjorn@kernel.org, linux-riscv@lists.infradead.org
Subject: Re: [PATCH] RISC-V: Clobber V registers on syscalls
Date: Mon, 16 Jun 2025 15:30:47 -0700	[thread overview]
Message-ID: <aFCbF/g+wxOwI3af@x1> (raw)
In-Reply-To: <mhng-6bb8dac2-33c3-410f-82ce-36554e497e9c@palmer-ri-x1c9>

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

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