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

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