Re: RISCV kernel struct sigcontext expansion for V regs and potential glibc ABI break (was Re: [RFC patch 2/5] RISC-V: Reserve about 5K space in mcontext_t to support future ISA expansion.)

[Vineet Gupta <vineetg@rivosinc.com>]

+CC Greentime and Andy

On 12/8/22 19:39, Vineet Gupta wrote:
> Hi Florian,
>
> P.S. Since I'm revisiting a year old thread with some new CC 
> recipients, here's the link to original patch/thread [1]
>
> On 9/17/21 20:04, Vincent Chen wrote:
>> On Thu, Sep 16, 2021 at 4:14 PM Florian Weimer <fweimer@redhat.com> 
>> wrote:
>>>>>> This changes the size of struct ucontext_t, which is an ABI break
>>>>>> (getcontext callers are supposed to provide their own object).
>>>>>>
>>>> The riscv vector registers are all caller-saved registers except for
>>>> VCSR. Therefore, the struct mcontext_t needs to reserve a space for
>>>> it. In addition, RISCV ISA is growing, so I also hope the struct
>>>> mcontext_t has a space for future expansion. Based on the above ideas,
>>>> I reserved a 5K space here.
>>> You have reserved space in ucontext_t that you could use for this.
>>>
>> Sorry, I cannot really understand what you mean. The following is the
>> contents of ucontext_t
>> typedef struct ucontext_t
>>    {
>>      unsigned long int  __uc_flags;
>>      struct ucontext_t *uc_link;
>>      stack_t            uc_stack;
>>      sigset_t           uc_sigmask;
>>      /* There's some padding here to allow sigset_t to be expanded in 
>> the
>>         future.  Though this is unlikely, other architectures put 
>> uc_sigmask
>>         at the end of this structure and explicitly state it can be
>>         expanded, so we didn't want to box ourselves in here. */
>>      char               __glibc_reserved[1024 / 8 - sizeof (sigset_t)];
>>      /* We can't put uc_sigmask at the end of this structure because 
>> we need
>>         to be able to expand sigcontext in the future.  For example, the
>>         vector ISA extension will almost certainly add ISA state.  We 
>> want
>>         to ensure all user-visible ISA state can be saved and 
>> restored via a
>>         ucontext, so we're putting this at the end in order to allow for
>>         infinite extensibility.  Since we know this will be extended 
>> and we
>>         assume sigset_t won't be extended an extreme amount, we're
>>         prioritizing this.  */
>>      mcontext_t uc_mcontext;
>>    } ucontext_t;
>>
>> Currently, we only reserve a space, __glibc_reserved[], for the future
>> expansion of sigset_t.
>> Do you mean I could use __glibc_reserved[] to for future expansion of
>> ISA as well?
>
> Given unlikely sigset expansion, we could in theory use some of those 
> reserved fields to store pointers (offsets) to actual V state, but not 
> for actual V state which is way too large for non-embedded machines 
> with typical 128 or even wider V regs.
>
>
>>
>>>>>> This shouldn't be necessary if the additional vector registers are
>>>>>> caller-saved.
>>>> Here I am a little confused about the usage of struct mcontext_t. As
>>>> far as I know, the struct mcontext_t is used to save the
>>>> machine-specific information in user context operation. Therefore, in
>>>> this case, the struct mcontext_t is allowed to reserve the space only
>>>> for saving caller-saved registers. However, in the signal handler, the
>>>> user seems to be allowed to use uc_mcontext whose data type is struct
>>>> mcontext_t to access the content of the signal context. In this case,
>>>> the struct mcontext_t may need to be the same as the struct sigcontext
>>>> defined at kernel. However, it will have a conflict with your
>>>> suggestion because the struct sigcontext cannot just reserve a space
>>>> for saving caller-saved registers. Could you help me point out my
>>>> misunderstanding? Thank you.
>
> I think the confusion comes from apparent equivalence of kernel struct 
> sigcontext and glibc mcontext_t as they appear in respective struct 
> ucontext definitions.
> I've enumerated the actual RV structs below to keep them handy in one 
> place for discussion.
>
>>> struct sigcontext is allocated by the kernel, so you can have pointers
>>> in reserved fields to out-of-line start, or after struct sigcontext.
>
> In this scheme, would the actual V regfile contents (at the 
> out-of-line location w.r.t kernel sigcontext) be anonymous for glibc 
> i.e. do we not need to expose them to glibc userspace ABI ?
>
>
>>> I don't know how the kernel implements this, but there is considerable
>>> flexibility and extensibility.  The main issues comes from small stacks
>>> which are incompatible with large register files.
>
> Simplistically, Linux kernel needs to preserve the V regfile across 
> task switch. The necessary evil that follows is preserving V across 
> signal-handling (sigaction/sigreturn).
>
> In RV kernel we have following:
>
> struct rt_sigframe {
>   struct siginfo info;
>   struct ucontext uc;
> };
>
> struct ucontext {
>    unsigned long uc_flags;
>    struct ucontext *uc_link;
>    stack_t uc_stack;
>    sigset_t uc_sigmask;
>    __u8 __unused[1024 / 8 - sizeof(sigset_t)];     // this is for 
> sigset_t expansion
>    struct sigcontext uc_mcontext;
> };
>
> struct sigcontext {
>    struct user_regs_struct sc_regs;
>    union __riscv_fp_state sc_fpregs;
> +  __u8 sc_extn[4096+128] __attribute__((__aligned__(16)));   // 
> handle 128B V regs
> };
>
> The sc_extn[] would have V state (regfile + control state) in kernel 
> defined format.
>
> As I understand it, you are suggesting to prevent ABI break, we should 
> not add anything to kernel struct sigcontext i.e. do something like this
>
> struct rt_sigframe {
>   struct siginfo info;
>   struct ucontext uc;
> +__u8 sc_extn[4096+128] __attribute__((__aligned__(16)));
> }
>
> So kernel sig handling can continue to save/restore the V regfile on 
> user stack, w/o making it part of actual struct sigcontext.
> So they are not explicitly visible to userspace at all - is that 
> feasible ? I know that SA_SIGINFO handlers can access the scalar/fp 
> regs, they won't do it V.
> Is there a POSIX req for SA_SIGINFO handlers being able to access all 
> machine regs saved by signal handling.
>
> An alternate approach is what Vincent did originally, to add sc_exn to 
> struct sigcontext. Here to prevent ABI breakage, we can choose to not 
> reflect this in the glibc sigcontext. But the question remains, is 
> that OK ?
>
> The other topic is changing glibc mcontext_t to add V-regs. It would 
> seem one has to as mcontext is "visually equivalent" to struct 
> sigcontext in the respective ucontext structs. But in unserspace 
> *context routine semantics only require callee-regs to be saved, which 
> V regs are not per psABI [2]. So looks like this can be avoided which 
> is what Vincent did in v2 series [3]
>
>
> [1] 
> https://sourceware.org/pipermail/libc-alpha/2021-September/130899.html
> [2] 
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc
> [3] https://sourceware.org/pipermail/libc-alpha/2022-January/135416.html