Re: [RFC patch 2/5] RISC-V: Reserve about 5K space in mcontext_t to support future ISA expansion.

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

On Fri, Sep 17, 2021 at 7:56 AM Ben Woodard <woodard@redhat.com> wrote:
>
> I know this patch set mostly deals with signal handling but don’t forget LD_AUDIT. It has a similar issue for the plt_enter and exit functions.
>
> -ben

Hi Ben,
I am not familiar with the mechanism of LD_AUDIT, so I actually do not
know if this modification may have any effect on LD_AUDIT. If
possible, could you briefly introduce the issues for me? Thank you
very much.

Regards,
Vincent
>
> > On Sep 16, 2021, at 1:03 AM, Vincent Chen <vincent.chen@sifive.com> wrote:
> >
> > On Mon, Sep 13, 2021 at 9:52 PM Rich Felker <dalias@libc.org> wrote:
> >>
> >>> On Mon, Sep 13, 2021 at 03:44:09PM +0200, Florian Weimer via Libc-alpha wrote:
> >>> * Vincent Chen:
> >>>
> >>>> Following the changes of struct sigcontext in Linux to reserve about 5K space
> >>>> to support future ISA expansion.
> >>>> ---
> >>>> sysdeps/unix/sysv/linux/riscv/sys/ucontext.h | 2 ++
> >>>> 1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/sysdeps/unix/sysv/linux/riscv/sys/ucontext.h b/sysdeps/unix/sysv/linux/riscv/sys/ucontext.h
> >>>> index cfafa44..80caf07 100644
> >>>> --- a/sysdeps/unix/sysv/linux/riscv/sys/ucontext.h
> >>>> +++ b/sysdeps/unix/sysv/linux/riscv/sys/ucontext.h
> >>>> @@ -82,6 +82,8 @@ typedef struct mcontext_t
> >>>>   {
> >>>>     __riscv_mc_gp_state __gregs;
> >>>>     union  __riscv_mc_fp_state __fpregs;
> >>>> +    /* 5K + 256 reserved for vector state and future expansion.  */
> >>>> +    unsigned char __reserved[5376] __attribute__ ((__aligned__ (16)));
> >>>>   } mcontext_t;
> >>>
> > Hi Florian and Rich,
> > Sorry for the late reply and thank you for reminding me the
> > modification will cause ABI break.
> >
> >>> 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.
> >
> >>> 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.
> >
> >> Indeed, that was my first thought when I saw this too. Any late
> >> additions to the register file must be call-clobbered or else they are
> >> a new ABI. And mcontext_t does not need to represent any
> >> call-clobbered state.
> >>
> >> Rich
> >
>