Re: [RFC patch 4/5] RISC-V: Extend MINSIGSTKSZ and SIGSTKSZ to backup RVV registers

[Vincent Chen <vincent.chen@sifive.com>]

On Mon, Sep 13, 2021 at 9:51 PM Rich Felker <dalias@libc.org> wrote:
>
> On Mon, Sep 13, 2021 at 09:41:17AM +0800, Vincent Chen wrote:
> > As using RVV extension, the original MINSIGSTKSZ is not enough to
> > back up all RVV registers for the normal case. Therefore, the MINSIGSTKSZ
> > is expanded to about 5K and the SIGSTKSZ is expanded to about 16K. This
> > space is enough for the case that the VLENB of a vector register is 128
> > bytes. For the case that VLENB > 128 bytes, users can use
> > sysconf (_SC_MINSIGSTKSZ) and sysconf (_SC_SIGSTKSZ) to get the
> > appropriate signal stack size.
> > ---
> >  sysdeps/unix/sysv/linux/riscv/bits/sigstack.h | 32 +++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >  create mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigstack.h
> >
> > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigstack.h b/sysdeps/unix/sysv/linux/riscv/bits/sigstack.h
> > new file mode 100644
> > index 0000000..c18512f
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/riscv/bits/sigstack.h
> > @@ -0,0 +1,32 @@
> > +/* sigstack, sigaltstack definitions.
> > +   Copyright (C) 2021 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _BITS_SIGSTACK_H
> > +#define _BITS_SIGSTACK_H 1
> > +
> > +#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H
> > +# error "Never include this file directly.  Use <signal.h> instead"
> > +#endif
> > +
> > +/* Minimum stack size (5k+256 bytes) for a signal handler.  */
> > +#define MINSIGSTKSZ  5376
> > +
> > +/* System default stack size.  */
> > +#define SIGSTKSZ     16384
> > +
> > +#endif /* bits/sigstack.h */
> > --
> > 2.7.4
>
> Strictly speaking this is also an ABI change (and what the kernel is
> doing is too). If possible I think there should be an effort to get
> the riscv folks to rethink this. Aside from being breakage, large
> state that has the be saved/restored at context switch time is an
> anti-feature. Any reasonable amount of vector state fits in the
> existing size.
>
> If it is to be changed, I suspect 5376 is too small. IIRC other archs
> that have large (4k or so?) register files used something like 6k as
> the min (1-2k margin for actual execution space).
>
> Rich

Hi Rich
I will share this modification in the RISC-V SW mailing list to
discuss it. Thank you very much for your suggestions.

Vincent