Re: [PATCH -next v13 10/19] riscv: Allocate user's vector context in the first-use trap

[Vineet Gupta]

On 2/14/23 08:50, Björn Töpel wrote:
> Andy Chiu <andy.chiu@xxxxxxxxxx> writes:
>
> > Hey Björn,
> >
> > On Tue, Feb 14, 2023 at 2:43 PM Björn Töpel <bjorn@xxxxxxxxxx> wrote:
> > > So, two changes:
> > >
> > > 1. Disallow V-enablement if the existing altstack does not fit a V-sized
> > >     frame.
> > This could potentially break old programs (non-V) that load new system
> > libraries (with V), If the program sets a small alt stack and takes
> > the fault in some libraries that use V. However, existing
> > implementation will also kill the process when the signal arrives,
> > finding insufficient stack frame in such cases. I'd choose the second
> > one if we only have these two options, because there is a chance that
> > the signal handler may not even run.
> I think we might have different views here. A process has a pre-V, a and
> post-V state. Is allowing a process to enter V without the correct
> preconditions a good idea? Allow to run with V turned on, but not able
> to correctly handle a signal (the stack is too small)?

The requirement is sane, but the issue is user experience: User trying 
to bring up some V code has no clue that deep in some startup code some 
alt stack had been setup and causing his process to be terminated on 
first V code.

> This was the same argument that the Intel folks had when enabling
> AMX. Sure, AMX requires *explicit* enablement, but same rules should
> apply, no?
>
> > > 2. Sanitize altstack changes when V is enabled.
> > Yes, I'd like to have this. But it may be tricky when it comes to
> > deciding whether V is enabled, due to the first-use trap. If V is
> > commonly used in system libraries then it is likely that V will be
> > enabled before an user set an altstack. Sanitizing this case would be
> > easy and straightforward.

Good. Lets have this in v14 as it seems reasonably easy to implement.

> > But what if the user sets an altstack before
> > enabling V in the first-use trap? This could happen on a statically
> > program that has hand-written V routines. This takes us to the 1st
> > question above, should we fail the user program immediately if the
> > altstack is set too small?

Please lets not cross threads. We discussed this already at top. While 
ideally required, seems tricky so lets start with post-V alt stack check.

> For me it's obvious to fail (always) "if the altstack is too small to
> enable V", because it allows to execute V without proper preconditions.
>
> Personally, I prefer a stricter model. Only enter V if you can, and
> after entering it disallow changing the altstack.
>
> Then again, this is *my* opinion and concern. What do other people
> think? I don't want to stall the series.

I concur that the alt stack checking requirements are sensible in the 
long run. We can add the obvious check for post-V case and see if there 
is a sane way to flag pre-V case to.


> > > Other than the altstack handling, I think the series is a good state! It
> > > would great if we could see a v14 land in -next...
> > Thanks. I am reforming the v14 patch and hoping the same to happen soon too!
> Thank you for your hard work! It would be awesome to *finally* have
> vector support in the kernel!

Indeed we've come a long way, lets push the gear so we can use the 
coming cycle to flesh out any changes for a possible 6.4 inclusion.

Thx,
-Vineet