Date:   Tue, 14 Feb 2023 17:50:51 +0100
From:   Björn Töpel <bjorn@...nel.org>
To:     Andy Chiu <andy.chiu@...ive.com>
Cc:     Vineet Gupta <vineetg@...osinc.com>,
        linux-riscv@...ts.infradead.org, palmer@...belt.com,
        anup@...infault.org, atishp@...shpatra.org,
        kvm-riscv@...ts.infradead.org, kvm@...r.kernel.org,
        greentime.hu@...ive.com, guoren@...ux.alibaba.com,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Heiko Stuebner <heiko.stuebner@...ll.eu>,
        Andrew Jones <ajones@...tanamicro.com>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Conor Dooley <conor.dooley@...rochip.com>,
        Jisheng Zhang <jszhang@...nel.org>,
        Vincent Chen <vincent.chen@...ive.com>,
        Guo Ren <guoren@...nel.org>,
        Li Zhengyu <lizhengyu3@...wei.com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Richard Henderson <richard.henderson@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH -next v13 10/19] riscv: Allocate user's vector context
 in the first-use trap

Andy Chiu <andy.chiu@...ive.com> writes:

> Hey Björn,
>
> On Tue, Feb 14, 2023 at 2:43 PM Björn Töpel <bjorn@...nel.org> 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)?

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. 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?

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.

>>
>> 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!


Björn

Powered by blists - more mailing lists