* [PATCH v2 0/2] RISC-V: Add vector ISA support @ 2022-01-18 4:31 Vincent Chen 2022-01-18 4:31 ` [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h Vincent Chen 2022-01-18 4:31 ` [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC Vincent Chen 0 siblings, 2 replies; 18+ messages in thread From: Vincent Chen @ 2022-01-18 4:31 UTC (permalink / raw) To: libc-alpha, palmer, darius, andrew, dj Cc: kito.cheng, greentime.hu, Vincent Chen According to the feedback for the version 1 patch set, only the "RISC-V: Remove riscv-specific sigcontext.h" patch remains in this version patch set. It means that MINSIGSTKSZ, SIGSTKSZ, and PTHREAD_STACK_MIN are not changed after introducing the V-extension support. Therefore, the current definition of the above stack size is insufficient to backup all vector registers. In this circumstance, users have to use the mechanisms submitted by H.J. Lu https://sourceware.org/git/?p=glibc.git;a=commit;h=6c57d320484988e87e446e2e60ce42816bf51d53 and https://sourceware.org/git/?p=glibc.git;a=commit;h=5d98a7dae955bafa6740c26eaba9c86060ae0344 to obtain the appropriate size of the current system setting. Besides, a new calling convention using vector registers to transfer argument or return value probably be proposed in the feature. It may cause the resolved functions and audit functions to corrupt the content of the vector registers, which are used as argument registers and address return registers. To avoid this problem, this patch set includes Hsiangkai Wang's patch to enable the Glibc dynamic loader to directly resolve the function symbols whose calling convention is incompatible with the standard calling convention. The corresponding implementation in Binutils can be found in https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=8155b8539b55bca87378129e02009cd8907d8c8c. Hsiangkai Wang (1): riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. Vincent Chen (1): RISC-V: remove riscv-specific sigcontext.h elf/elf.h | 7 +++++ manual/platform.texi | 6 +++++ .../sigcontext.h => riscv/dl-dtprocnum.h} | 22 +++++----------- sysdeps/riscv/dl-machine.h | 26 +++++++++++++++++++ 4 files changed, 45 insertions(+), 16 deletions(-) rename sysdeps/{unix/sysv/linux/riscv/bits/sigcontext.h => riscv/dl-dtprocnum.h} (55%) -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-01-18 4:31 [PATCH v2 0/2] RISC-V: Add vector ISA support Vincent Chen @ 2022-01-18 4:31 ` Vincent Chen 2022-01-20 2:36 ` Palmer Dabbelt 2022-01-18 4:31 ` [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC Vincent Chen 1 sibling, 1 reply; 18+ messages in thread From: Vincent Chen @ 2022-01-18 4:31 UTC (permalink / raw) To: libc-alpha, palmer, darius, andrew, dj Cc: kito.cheng, greentime.hu, Vincent Chen Remove riscv-specific sigcontext.h so that Glibc can directly use sigcontext.h provided by the kernel to reduce synchronization work when new extension support is introduced. --- .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- 1 file changed, 31 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h deleted file mode 100644 index b6e15b5f62..0000000000 --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h +++ /dev/null @@ -1,31 +0,0 @@ -/* Machine-dependent signal context structure for Linux. RISC-V version. - Copyright (C) 1996-2022 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_SIGCONTEXT_H -#define _BITS_SIGCONTEXT_H 1 - -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." -#endif - -struct sigcontext { - /* gregs[0] holds the program counter. */ - unsigned long int gregs[32]; - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); -}; - -#endif -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-01-18 4:31 ` [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h Vincent Chen @ 2022-01-20 2:36 ` Palmer Dabbelt 2022-01-20 2:47 ` Kito Cheng 0 siblings, 1 reply; 18+ messages in thread From: Palmer Dabbelt @ 2022-01-20 2:36 UTC (permalink / raw) To: vincent.chen Cc: libc-alpha, Darius Rad, Andrew Waterman, dj, kito.cheng, greentime.hu, vincent.chen On Mon, 17 Jan 2022 20:31:58 PST (-0800), vincent.chen@sifive.com wrote: > Remove riscv-specific sigcontext.h so that Glibc can directly use > sigcontext.h provided by the kernel to reduce synchronization work > when new extension support is introduced. > --- > .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- > 1 file changed, 31 deletions(-) > delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > deleted file mode 100644 > index b6e15b5f62..0000000000 > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > +++ /dev/null > @@ -1,31 +0,0 @@ > -/* Machine-dependent signal context structure for Linux. RISC-V version. > - Copyright (C) 1996-2022 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_SIGCONTEXT_H > -#define _BITS_SIGCONTEXT_H 1 > - > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." > -#endif > - > -struct sigcontext { > - /* gregs[0] holds the program counter. */ > - unsigned long int gregs[32]; > - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); > -}; > - > -#endif This will definitely break API compatibility (the fields have different names) but should be fine for ABI compatibility. IIUC that's within the rules, but I'm not sure it's a desirable outcome. Probably would have been better to get this right the first time around, but I'm not sure it's worth fixing -- essentially we're making a bunch of users change things so we don't have to. That said, it's pretty ugly to have two different definitions of a structure with the same name and layout. Maybe there's some sort of macro-related trick we can use? ie, provide the current definition unless users opt into the Linux one (presumably so they can talk about the V state). There's going to be some hoops to jump through there to maintain ABI compatibility either way, so it's possible we could tie these two together? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-01-20 2:36 ` Palmer Dabbelt @ 2022-01-20 2:47 ` Kito Cheng 2022-01-21 1:29 ` Vincent Chen 0 siblings, 1 reply; 18+ messages in thread From: Kito Cheng @ 2022-01-20 2:47 UTC (permalink / raw) To: Palmer Dabbelt Cc: Vincent Chen, libc-alpha, Darius Rad, Andrew Waterman, DJ Delorie, Greentime Hu Just provide a tricky workaround from GCC side: +#ifdef _ASM_RISCV_SIGCONTEXT_H +#define SIGCONTEXT_PC(SC) (SC)->sc_regs.pc +#else +#define SIGCONTEXT_PC(SC) (SC)->gregs[0] +#endif https://gcc.gnu.org/pipermail/gcc-patches/2022-January/588682.html On Thu, Jan 20, 2022 at 10:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Mon, 17 Jan 2022 20:31:58 PST (-0800), vincent.chen@sifive.com wrote: > > Remove riscv-specific sigcontext.h so that Glibc can directly use > > sigcontext.h provided by the kernel to reduce synchronization work > > when new extension support is introduced. > > --- > > .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- > > 1 file changed, 31 deletions(-) > > delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > deleted file mode 100644 > > index b6e15b5f62..0000000000 > > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > +++ /dev/null > > @@ -1,31 +0,0 @@ > > -/* Machine-dependent signal context structure for Linux. RISC-V version. > > - Copyright (C) 1996-2022 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_SIGCONTEXT_H > > -#define _BITS_SIGCONTEXT_H 1 > > - > > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H > > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." > > -#endif > > - > > -struct sigcontext { > > - /* gregs[0] holds the program counter. */ > > - unsigned long int gregs[32]; > > - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); > > -}; > > - > > -#endif > > This will definitely break API compatibility (the fields have > different names) but should be fine for ABI compatibility. IIUC that's > within the rules, but I'm not sure it's a desirable outcome. Probably > would have been better to get this right the first time around, but I'm > not sure it's worth fixing -- essentially we're making a bunch of users > change things so we don't have to. That said, it's pretty ugly to have > two different definitions of a structure with the same name and layout. > > Maybe there's some sort of macro-related trick we can use? ie, provide > the current definition unless users opt into the Linux one (presumably > so they can talk about the V state). There's going to be some hoops to > jump through there to maintain ABI compatibility either way, so it's > possible we could tie these two together? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-01-20 2:47 ` Kito Cheng @ 2022-01-21 1:29 ` Vincent Chen 2022-01-24 9:42 ` Vincent Chen 2022-02-24 20:56 ` Palmer Dabbelt 0 siblings, 2 replies; 18+ messages in thread From: Vincent Chen @ 2022-01-21 1:29 UTC (permalink / raw) To: Kito Cheng Cc: Palmer Dabbelt, GNU C Library, Darius Rad, Andrew Waterman, DJ Delorie, Greentime Hu On Thu, Jan 20, 2022 at 10:47 AM Kito Cheng <kito.cheng@sifive.com> wrote: > > Just provide a tricky workaround from GCC side: > > +#ifdef _ASM_RISCV_SIGCONTEXT_H > +#define SIGCONTEXT_PC(SC) (SC)->sc_regs.pc > +#else > +#define SIGCONTEXT_PC(SC) (SC)->gregs[0] > +#endif > > https://gcc.gnu.org/pipermail/gcc-patches/2022-January/588682.html > > > On Thu, Jan 20, 2022 at 10:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > > > On Mon, 17 Jan 2022 20:31:58 PST (-0800), vincent.chen@sifive.com wrote: > > > Remove riscv-specific sigcontext.h so that Glibc can directly use > > > sigcontext.h provided by the kernel to reduce synchronization work > > > when new extension support is introduced. > > > --- > > > .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- > > > 1 file changed, 31 deletions(-) > > > delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > deleted file mode 100644 > > > index b6e15b5f62..0000000000 > > > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > +++ /dev/null > > > @@ -1,31 +0,0 @@ > > > -/* Machine-dependent signal context structure for Linux. RISC-V version. > > > - Copyright (C) 1996-2022 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_SIGCONTEXT_H > > > -#define _BITS_SIGCONTEXT_H 1 > > > - > > > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H > > > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." > > > -#endif > > > - > > > -struct sigcontext { > > > - /* gregs[0] holds the program counter. */ > > > - unsigned long int gregs[32]; > > > - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); > > > -}; > > > - > > > -#endif > > > > This will definitely break API compatibility (the fields have > > different names) but should be fine for ABI compatibility. IIUC that's > > within the rules, but I'm not sure it's a desirable outcome. Probably > > would have been better to get this right the first time around, but I'm > > not sure it's worth fixing -- essentially we're making a bunch of users > > change things so we don't have to. That said, it's pretty ugly to have > > two different definitions of a structure with the same name and layout. > > > > Maybe there's some sort of macro-related trick we can use? ie, provide > > the current definition unless users opt into the Linux one (presumably > > so they can talk about the V state). There's going to be some hoops to > > jump through there to maintain ABI compatibility either way, so it's > > possible we could tie these two together? I can understand what you are worried about. Therefore, I also tried to build multiple Yocto images, such as core-image-full-cmdline, core-image-x11, core-image-sato, and core-image-base, to evaluate the impact. After applying Kito's solution to GCC, I didn't get any build errors. According to the results, maybe we can have a quick conclusion about the impact of this patch. The new version Glibc is not compatible with the old version GCC (The old Glibc is still compatible with the new version GCC due to Kito's patch) Some public programs that use struct sigcontext are not covered by this test. (If someone can tell me which program I'm missing, I'm willing to fix it) Some in-house programs use struct sigcontext_t to access signal stack. IMO, the impact seems not severe at this moment. Directly using the kernel's sigcontext can get us away from the pain if we need to add new registers to the signal context for a new extension or vendor customized extension. In addition, I was keeping to find a suitable solution to solve it as you described. But if I still cannot come up with a solution, do you mind that bits/sigcontext.h has a duplicate data struct related to V extension? Thank you ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-01-21 1:29 ` Vincent Chen @ 2022-01-24 9:42 ` Vincent Chen 2022-02-24 20:56 ` Palmer Dabbelt 1 sibling, 0 replies; 18+ messages in thread From: Vincent Chen @ 2022-01-24 9:42 UTC (permalink / raw) To: Kito Cheng Cc: Palmer Dabbelt, GNU C Library, Darius Rad, Andrew Waterman, DJ Delorie, Greentime Hu On Fri, Jan 21, 2022 at 9:29 AM Vincent Chen <vincent.chen@sifive.com> wrote: > > On Thu, Jan 20, 2022 at 10:47 AM Kito Cheng <kito.cheng@sifive.com> wrote: > > > > Just provide a tricky workaround from GCC side: > > > > +#ifdef _ASM_RISCV_SIGCONTEXT_H > > +#define SIGCONTEXT_PC(SC) (SC)->sc_regs.pc > > +#else > > +#define SIGCONTEXT_PC(SC) (SC)->gregs[0] > > +#endif > > > > https://gcc.gnu.org/pipermail/gcc-patches/2022-January/588682.html > > > > > > On Thu, Jan 20, 2022 at 10:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > > > > > On Mon, 17 Jan 2022 20:31:58 PST (-0800), vincent.chen@sifive.com wrote: > > > > Remove riscv-specific sigcontext.h so that Glibc can directly use > > > > sigcontext.h provided by the kernel to reduce synchronization work > > > > when new extension support is introduced. > > > > --- > > > > .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- > > > > 1 file changed, 31 deletions(-) > > > > delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > > > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > > deleted file mode 100644 > > > > index b6e15b5f62..0000000000 > > > > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > > > > +++ /dev/null > > > > @@ -1,31 +0,0 @@ > > > > -/* Machine-dependent signal context structure for Linux. RISC-V version. > > > > - Copyright (C) 1996-2022 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_SIGCONTEXT_H > > > > -#define _BITS_SIGCONTEXT_H 1 > > > > - > > > > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H > > > > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." > > > > -#endif > > > > - > > > > -struct sigcontext { > > > > - /* gregs[0] holds the program counter. */ > > > > - unsigned long int gregs[32]; > > > > - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); > > > > -}; > > > > - > > > > -#endif > > > > > > This will definitely break API compatibility (the fields have > > > different names) but should be fine for ABI compatibility. IIUC that's > > > within the rules, but I'm not sure it's a desirable outcome. Probably > > > would have been better to get this right the first time around, but I'm > > > not sure it's worth fixing -- essentially we're making a bunch of users > > > change things so we don't have to. That said, it's pretty ugly to have > > > two different definitions of a structure with the same name and layout. > > > > > > Maybe there's some sort of macro-related trick we can use? ie, provide > > > the current definition unless users opt into the Linux one (presumably > > > so they can talk about the V state). There's going to be some hoops to > > > jump through there to maintain ABI compatibility either way, so it's > > > possible we could tie these two together? > > I can understand what you are worried about. Therefore, I also tried > to build multiple Yocto images, such as core-image-full-cmdline, > core-image-x11, core-image-sato, and core-image-base, to evaluate the > impact. After applying Kito's solution to GCC, I didn't get any build > errors. According to the results, maybe we can have a quick conclusion > about the impact of this patch. > > The new version Glibc is not compatible with the old version GCC (The > old Glibc is still compatible with the new version GCC due to Kito's > patch) > Some public programs that use struct sigcontext are not covered by > this test. (If someone can tell me which program I'm missing, I'm > willing to fix it) > Some in-house programs use struct sigcontext_t to access signal stack. > > IMO, the impact seems not severe at this moment. Directly using the > kernel's sigcontext can get us away from the pain if we need to add > new registers to the signal context for a new extension or vendor > customized extension. > > In addition, I was keeping to find a suitable solution to solve it as > you described. But if I still cannot come up with a solution, do you > mind that bits/sigcontext.h has a duplicate data struct related to V > extension? Thank you I have some findings these days. Please allow me to update some descriptions. For the evaluation of deprecating the Glibc self-defined sigcontext.h (riscv/bits/sigcontext.h), thanks to David Abdurachmanov's suggestion, I used "world" instead of a particular image. It makes me able to build every single package available in all the layers. It effectively extends the test coverage. Fortunately, I didn't encounter any compile errors due to sigcontext.h deprecation. In addition, I found using "anonymous union" may suffice to avoid API breakage. According to Greentime's vector patch(https://lore.kernel.org/lkml/202111112102.qLSAjr3Q-lkp@intel.com/T/), the following is associated modification in Glibc sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h +++ b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h @@ -22,10 +22,22 @@ # error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." #endif +#define sigcontext kernel_sigcontext +#include <asm/sigcontext.h> +#undef sigcontext + struct sigcontext { /* gregs[0] holds the program counter. */ - unsigned long int gregs[32]; - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); + __extension__ union { + unsigned long int gregs[32]; + struct user_regs_struct sc_regs; + }; + __extension__ union { + unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); + union __riscv_fp_state sc_fpregs; + }; + __u8 __reserved[4224] __attribute__((__aligned__(16))); }; #endif In this modification, users do not need to specify which sigcontext.h he wants to choose. Through the anonymous union, the current member of struct sigcontext, gregs and fpregs, declared in riscv/bits/sigcontext.h is still available. Besides, by redefining struct sigcontext as struct kernel_sigcontext before including asm/sigcontext.h, riscv/bits/sigcontext.h can directly include asm/sigcontext.h to introduce the register data structure of new extension without duplicating the same definitions in Glibc. The modifications can pass the Glibc test suite without additional errors. By the way, I found that the definition of struct sigcontext in riscv/bits/sigcontext.h and asm/sigcontext.h is a bit different now. The memory size of fpregs[66] in riscv/bits/sigcontext.h equals to __riscv_fp_state sc_fpregs in asm/sigcontext.h, but their memory layout is different. Users cannot always use fpregs[x] to get the associated x-th FPU registers from the signal stack in all situations. It may be a minus case, but it points out the potential synchronization issue. Therefore, if possible, I still hope to deprecate the riscv/bits/sigcontext.h. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-01-21 1:29 ` Vincent Chen 2022-01-24 9:42 ` Vincent Chen @ 2022-02-24 20:56 ` Palmer Dabbelt 2022-02-25 0:32 ` Vincent Chen 1 sibling, 1 reply; 18+ messages in thread From: Palmer Dabbelt @ 2022-02-24 20:56 UTC (permalink / raw) To: vincent.chen Cc: kito.cheng, libc-alpha, Darius Rad, Andrew Waterman, dj, greentime.hu On Thu, 20 Jan 2022 17:29:20 PST (-0800), vincent.chen@sifive.com wrote: > On Thu, Jan 20, 2022 at 10:47 AM Kito Cheng <kito.cheng@sifive.com> wrote: >> >> Just provide a tricky workaround from GCC side: >> >> +#ifdef _ASM_RISCV_SIGCONTEXT_H >> +#define SIGCONTEXT_PC(SC) (SC)->sc_regs.pc >> +#else >> +#define SIGCONTEXT_PC(SC) (SC)->gregs[0] >> +#endif >> >> https://gcc.gnu.org/pipermail/gcc-patches/2022-January/588682.html >> >> >> On Thu, Jan 20, 2022 at 10:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> > >> > On Mon, 17 Jan 2022 20:31:58 PST (-0800), vincent.chen@sifive.com wrote: >> > > Remove riscv-specific sigcontext.h so that Glibc can directly use >> > > sigcontext.h provided by the kernel to reduce synchronization work >> > > when new extension support is introduced. >> > > --- >> > > .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- >> > > 1 file changed, 31 deletions(-) >> > > delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h >> > > >> > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h >> > > deleted file mode 100644 >> > > index b6e15b5f62..0000000000 >> > > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h >> > > +++ /dev/null >> > > @@ -1,31 +0,0 @@ >> > > -/* Machine-dependent signal context structure for Linux. RISC-V version. >> > > - Copyright (C) 1996-2022 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_SIGCONTEXT_H >> > > -#define _BITS_SIGCONTEXT_H 1 >> > > - >> > > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H >> > > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." >> > > -#endif >> > > - >> > > -struct sigcontext { >> > > - /* gregs[0] holds the program counter. */ >> > > - unsigned long int gregs[32]; >> > > - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); >> > > -}; >> > > - >> > > -#endif >> > >> > This will definitely break API compatibility (the fields have >> > different names) but should be fine for ABI compatibility. IIUC that's >> > within the rules, but I'm not sure it's a desirable outcome. Probably >> > would have been better to get this right the first time around, but I'm >> > not sure it's worth fixing -- essentially we're making a bunch of users >> > change things so we don't have to. That said, it's pretty ugly to have >> > two different definitions of a structure with the same name and layout. >> > >> > Maybe there's some sort of macro-related trick we can use? ie, provide >> > the current definition unless users opt into the Linux one (presumably >> > so they can talk about the V state). There's going to be some hoops to >> > jump through there to maintain ABI compatibility either way, so it's >> > possible we could tie these two together? > > I can understand what you are worried about. Therefore, I also tried > to build multiple Yocto images, such as core-image-full-cmdline, > core-image-x11, core-image-sato, and core-image-base, to evaluate the > impact. After applying Kito's solution to GCC, I didn't get any build > errors. According to the results, maybe we can have a quick conclusion > about the impact of this patch. > > The new version Glibc is not compatible with the old version GCC (The > old Glibc is still compatible with the new version GCC due to Kito's > patch) > Some public programs that use struct sigcontext are not covered by > this test. (If someone can tell me which program I'm missing, I'm > willing to fix it) > Some in-house programs use struct sigcontext_t to access signal stack. > > IMO, the impact seems not severe at this moment. Directly using the > kernel's sigcontext can get us away from the pain if we need to add > new registers to the signal context for a new extension or vendor > customized extension. > > In addition, I was keeping to find a suitable solution to solve it as > you described. But if I still cannot come up with a solution, do you > mind that bits/sigcontext.h has a duplicate data struct related to V > extension? Thank you I was talking about putting the macros into glibc, so we don't force users into picking up the kernel's sigcontext but instead give them the option of moving over. So something like this: diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h index b6e15b5f62..d07d690d1b 100644 --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h +++ b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h @@ -22,10 +22,18 @@ # error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." #endif +#ifdef __USE_KERNEL_SIGCONTEXT +# include <asm/sigcontext.h> + +/* The Linux kernel headers redefine NULL wrongly, so cleanup afterwards. */ +# define __need_NULL +# include <stddef.h> +#else struct sigcontext { /* gregs[0] holds the program counter. */ unsigned long int gregs[32]; unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); }; +#endif #endif ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h 2022-02-24 20:56 ` Palmer Dabbelt @ 2022-02-25 0:32 ` Vincent Chen 0 siblings, 0 replies; 18+ messages in thread From: Vincent Chen @ 2022-02-25 0:32 UTC (permalink / raw) To: Palmer Dabbelt Cc: Kito Cheng, GNU C Library, Darius Rad, Andrew Waterman, DJ Delorie, Greentime Hu On Fri, Feb 25, 2022 at 4:56 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Thu, 20 Jan 2022 17:29:20 PST (-0800), vincent.chen@sifive.com wrote: > > On Thu, Jan 20, 2022 at 10:47 AM Kito Cheng <kito.cheng@sifive.com> wrote: > >> > >> Just provide a tricky workaround from GCC side: > >> > >> +#ifdef _ASM_RISCV_SIGCONTEXT_H > >> +#define SIGCONTEXT_PC(SC) (SC)->sc_regs.pc > >> +#else > >> +#define SIGCONTEXT_PC(SC) (SC)->gregs[0] > >> +#endif > >> > >> https://gcc.gnu.org/pipermail/gcc-patches/2022-January/588682.html > >> > >> > >> On Thu, Jan 20, 2022 at 10:36 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > >> > > >> > On Mon, 17 Jan 2022 20:31:58 PST (-0800), vincent.chen@sifive.com wrote: > >> > > Remove riscv-specific sigcontext.h so that Glibc can directly use > >> > > sigcontext.h provided by the kernel to reduce synchronization work > >> > > when new extension support is introduced. > >> > > --- > >> > > .../unix/sysv/linux/riscv/bits/sigcontext.h | 31 ------------------- > >> > > 1 file changed, 31 deletions(-) > >> > > delete mode 100644 sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > >> > > > >> > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > >> > > deleted file mode 100644 > >> > > index b6e15b5f62..0000000000 > >> > > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > >> > > +++ /dev/null > >> > > @@ -1,31 +0,0 @@ > >> > > -/* Machine-dependent signal context structure for Linux. RISC-V version. > >> > > - Copyright (C) 1996-2022 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_SIGCONTEXT_H > >> > > -#define _BITS_SIGCONTEXT_H 1 > >> > > - > >> > > -#if !defined _SIGNAL_H && !defined _SYS_UCONTEXT_H > >> > > -# error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." > >> > > -#endif > >> > > - > >> > > -struct sigcontext { > >> > > - /* gregs[0] holds the program counter. */ > >> > > - unsigned long int gregs[32]; > >> > > - unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); > >> > > -}; > >> > > - > >> > > -#endif > >> > > >> > This will definitely break API compatibility (the fields have > >> > different names) but should be fine for ABI compatibility. IIUC that's > >> > within the rules, but I'm not sure it's a desirable outcome. Probably > >> > would have been better to get this right the first time around, but I'm > >> > not sure it's worth fixing -- essentially we're making a bunch of users > >> > change things so we don't have to. That said, it's pretty ugly to have > >> > two different definitions of a structure with the same name and layout. > >> > > >> > Maybe there's some sort of macro-related trick we can use? ie, provide > >> > the current definition unless users opt into the Linux one (presumably > >> > so they can talk about the V state). There's going to be some hoops to > >> > jump through there to maintain ABI compatibility either way, so it's > >> > possible we could tie these two together? > > > > I can understand what you are worried about. Therefore, I also tried > > to build multiple Yocto images, such as core-image-full-cmdline, > > core-image-x11, core-image-sato, and core-image-base, to evaluate the > > impact. After applying Kito's solution to GCC, I didn't get any build > > errors. According to the results, maybe we can have a quick conclusion > > about the impact of this patch. > > > > The new version Glibc is not compatible with the old version GCC (The > > old Glibc is still compatible with the new version GCC due to Kito's > > patch) > > Some public programs that use struct sigcontext are not covered by > > this test. (If someone can tell me which program I'm missing, I'm > > willing to fix it) > > Some in-house programs use struct sigcontext_t to access signal stack. > > > > IMO, the impact seems not severe at this moment. Directly using the > > kernel's sigcontext can get us away from the pain if we need to add > > new registers to the signal context for a new extension or vendor > > customized extension. > > > > In addition, I was keeping to find a suitable solution to solve it as > > you described. But if I still cannot come up with a solution, do you > > mind that bits/sigcontext.h has a duplicate data struct related to V > > extension? Thank you > > I was talking about putting the macros into glibc, so we don't force > users into picking up the kernel's sigcontext but instead give them the > option of moving over. So something like this: > > diff --git a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > index b6e15b5f62..d07d690d1b 100644 > --- a/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > +++ b/sysdeps/unix/sysv/linux/riscv/bits/sigcontext.h > @@ -22,10 +22,18 @@ > # error "Never use <bits/sigcontext.h> directly; include <signal.h> instead." > #endif > > +#ifdef __USE_KERNEL_SIGCONTEXT > +# include <asm/sigcontext.h> > + > +/* The Linux kernel headers redefine NULL wrongly, so cleanup afterwards. */ > +# define __need_NULL > +# include <stddef.h> > +#else > struct sigcontext { > /* gregs[0] holds the program counter. */ > unsigned long int gregs[32]; > unsigned long long int fpregs[66] __attribute__ ((__aligned__ (16))); > }; > +#endif > > #endif Hi Palmer, Thank you for your explanation. In my latest reply to this thread( or you also can find it in https://sourceware.org/pipermail/libc-alpha/2022-January/135610.html), I purposed a new solution. This solution allows users to transparently use the kernel sigcontext.h without breaking API compatibility. Could you help me review it and give me some suggestions? Thank you. Best regards, Vincent ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-18 4:31 [PATCH v2 0/2] RISC-V: Add vector ISA support Vincent Chen 2022-01-18 4:31 ` [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h Vincent Chen @ 2022-01-18 4:31 ` Vincent Chen 2022-01-20 2:21 ` Palmer Dabbelt 2022-12-09 4:11 ` Vineet Gupta 1 sibling, 2 replies; 18+ messages in thread From: Vincent Chen @ 2022-01-18 4:31 UTC (permalink / raw) To: libc-alpha, palmer, darius, andrew, dj Cc: kito.cheng, greentime.hu, Hsiangkai Wang, Vincent Chen From: Hsiangkai Wang <kai.wang@sifive.com> In some cases, we do not want to go through the resolver for function calls. For example, functions with vector arguments will use vector registers to pass arguments. In the resolver, we do not save/restore the vector argument registers for lazy binding efficiency. To avoid ruining the vector arguments, functions with vector arguments will not go through the resolver. To achieve the goal, we will annotate the function symbols with STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic section. In the first pass on PLT relocations, we do not set up to call _dl_runtime_resolve. Instead, we resolve the functions directly. Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> Signed-off-by: Vincent Chen <vincent.chen@sifive.com> --- elf/elf.h | 7 +++++++ manual/platform.texi | 6 ++++++ sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ 4 files changed, 60 insertions(+) create mode 100644 sysdeps/riscv/dl-dtprocnum.h diff --git a/elf/elf.h b/elf/elf.h index 0735f6b579..9c95544050 100644 --- a/elf/elf.h +++ b/elf/elf.h @@ -3911,6 +3911,13 @@ enum #define R_TILEGX_NUM 130 +/* RISC-V specific values for the Dyn d_tag field. */ +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) +#define DT_RISCV_NUM 2 + +/* RISC-V specific values for the st_other field. */ +#define STO_RISCV_VARIANT_CC 0x80 + /* RISC-V ELF Flags */ #define EF_RISCV_RVC 0x0001 #define EF_RISCV_FLOAT_ABI 0x0006 diff --git a/manual/platform.texi b/manual/platform.texi index d5fdc5bd05..a1a740f381 100644 --- a/manual/platform.texi +++ b/manual/platform.texi @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. @node RISC-V @appendixsec RISC-V-specific Facilities +Functions that are lazily bound must be compatible with the standard calling +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means +this function is not compatible with the standard calling convention. The +dynamic linker will directly resolve it instead of using the lazy binding +mechanism. + Cache management facilities specific to RISC-V systems that implement the Linux ABI are declared in @file{sys/cachectl.h}. diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h new file mode 100644 index 0000000000..f189fd700a --- /dev/null +++ b/sysdeps/riscv/dl-dtprocnum.h @@ -0,0 +1,21 @@ +/* Configuration of lookup functions. RISC-V version. + Copyright (C) 2019-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/>. */ + +/* Number of extra dynamic section entries for this architecture. By + default there are none. */ +#define DT_THISPROCNUM DT_RISCV_NUM diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h index 1d3e2e588c..cdbaca6533 100644 --- a/sysdeps/riscv/dl-machine.h +++ b/sysdeps/riscv/dl-machine.h @@ -53,6 +53,9 @@ || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) +//* Translate a processor specific dynamic tag to the index in l_info array. */ +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) + /* Return nonzero iff ELF header is compatible with the running host. */ static inline int __attribute_used__ elf_machine_matches_host (const ElfW(Ehdr) *ehdr) @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], /* Check for unexpected PLT reloc type. */ if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) { + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) + { + /* Check the symbol table for variant CC symbols. */ + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); + const ElfW(Sym) *symtab = + (const void *)D_PTR (map, l_info[DT_SYMTAB]); + const ElfW(Sym) *sym = &symtab[symndx]; + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) + { + /* Avoid lazy resolution of variant CC symbols. */ + const struct r_found_version *version = NULL; + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) + { + const ElfW(Half) *vernum = + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); + version = &map->l_versions[vernum[symndx] & 0x7fff]; + } + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, + skip_ifunc); + return; + } + } + if (__glibc_unlikely (map->l_mach.plt == 0)) { if (l_addr) -- 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-18 4:31 ` [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC Vincent Chen @ 2022-01-20 2:21 ` Palmer Dabbelt 2022-01-20 2:38 ` H.J. Lu 2022-01-21 1:43 ` Vincent Chen 2022-12-09 4:11 ` Vineet Gupta 1 sibling, 2 replies; 18+ messages in thread From: Palmer Dabbelt @ 2022-01-20 2:21 UTC (permalink / raw) To: vincent.chen Cc: libc-alpha, Darius Rad, Andrew Waterman, dj, kito.cheng, greentime.hu, kai.wang, vincent.chen Sorry, I missed the fixed-up patch set (which is why I just sent out a similar bit of documentation). On Mon, 17 Jan 2022 20:31:59 PST (-0800), vincent.chen@sifive.com wrote: > From: Hsiangkai Wang <kai.wang@sifive.com> > > In some cases, we do not want to go through the resolver for function > calls. For example, functions with vector arguments will use vector > registers to pass arguments. In the resolver, we do not save/restore the > vector argument registers for lazy binding efficiency. To avoid ruining > the vector arguments, functions with vector arguments will not go > through the resolver. > > To achieve the goal, we will annotate the function symbols with > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic > section. In the first pass on PLT relocations, we do not set up to call > _dl_runtime_resolve. Instead, we resolve the functions directly. > > Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > elf/elf.h | 7 +++++++ > manual/platform.texi | 6 ++++++ > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ > 4 files changed, 60 insertions(+) > create mode 100644 sysdeps/riscv/dl-dtprocnum.h > > diff --git a/elf/elf.h b/elf/elf.h > index 0735f6b579..9c95544050 100644 > --- a/elf/elf.h > +++ b/elf/elf.h > @@ -3911,6 +3911,13 @@ enum > > #define R_TILEGX_NUM 130 > > +/* RISC-V specific values for the Dyn d_tag field. */ > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > +#define DT_RISCV_NUM 2 > + > +/* RISC-V specific values for the st_other field. */ > +#define STO_RISCV_VARIANT_CC 0x80 > + > /* RISC-V ELF Flags */ > #define EF_RISCV_RVC 0x0001 > #define EF_RISCV_FLOAT_ABI 0x0006 > diff --git a/manual/platform.texi b/manual/platform.texi > index d5fdc5bd05..a1a740f381 100644 > --- a/manual/platform.texi > +++ b/manual/platform.texi > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. > @node RISC-V > @appendixsec RISC-V-specific Facilities > > +Functions that are lazily bound must be compatible with the standard calling > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means > +this function is not compatible with the standard calling convention. The > +dynamic linker will directly resolve it instead of using the lazy binding > +mechanism. IMO this is the wrong way to go: we're essentially re-defining a bit used be the standard ABI to mean something else. I guess we've already defacto forked from the psABI with that "standard calling convention" language, but IMO it'd be prudent to use a different bit to represent this new behavior. In the long term one could imagine trying to get back in line with the psABI, but if we're repurposing two bit patterns it'll be a bit harder than if we're just repurposing one. > + > Cache management facilities specific to RISC-V systems that implement the Linux > ABI are declared in @file{sys/cachectl.h}. > > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > new file mode 100644 > index 0000000000..f189fd700a > --- /dev/null > +++ b/sysdeps/riscv/dl-dtprocnum.h > @@ -0,0 +1,21 @@ > +/* Configuration of lookup functions. RISC-V version. > + Copyright (C) 2019-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/>. */ > + > +/* Number of extra dynamic section entries for this architecture. By > + default there are none. */ > +#define DT_THISPROCNUM DT_RISCV_NUM > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > index 1d3e2e588c..cdbaca6533 100644 > --- a/sysdeps/riscv/dl-machine.h > +++ b/sysdeps/riscv/dl-machine.h > @@ -53,6 +53,9 @@ > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) > > +//* Translate a processor specific dynamic tag to the index in l_info array. */ > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) > + > /* Return nonzero iff ELF header is compatible with the running host. */ > static inline int __attribute_used__ > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], > /* Check for unexpected PLT reloc type. */ > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > { > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > + { > + /* Check the symbol table for variant CC symbols. */ > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > + const ElfW(Sym) *symtab = > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > + const ElfW(Sym) *sym = &symtab[symndx]; > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > + { > + /* Avoid lazy resolution of variant CC symbols. */ > + const struct r_found_version *version = NULL; > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > + { > + const ElfW(Half) *vernum = > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > + } > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, > + skip_ifunc); > + return; > + } > + } > + > if (__glibc_unlikely (map->l_mach.plt == 0)) > { > if (l_addr) Aside from that this one looks fine to me. Given the complexity around this psABI spec deviation and how close we are to release I'd prefer to wait and see if we can come up with a better solution, though -- for example, I'd been kicking around some ideas related to ELF object attributes saying "this follows the psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way we could at least tag binaries that explicitly rely on this new behavior as such, which would give us a shot at eventually getting rid of them. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-20 2:21 ` Palmer Dabbelt @ 2022-01-20 2:38 ` H.J. Lu 2022-01-20 2:43 ` Palmer Dabbelt 2022-01-21 1:43 ` Vincent Chen 1 sibling, 1 reply; 18+ messages in thread From: H.J. Lu @ 2022-01-20 2:38 UTC (permalink / raw) To: Palmer Dabbelt Cc: Vincent Chen, GNU C Library, Andrew Waterman, kai.wang, greentime.hu, kito.cheng On Wed, Jan 19, 2022 at 6:22 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > Sorry, I missed the fixed-up patch set (which is why I just sent out a > similar bit of documentation). > > On Mon, 17 Jan 2022 20:31:59 PST (-0800), vincent.chen@sifive.com wrote: > > From: Hsiangkai Wang <kai.wang@sifive.com> > > > > In some cases, we do not want to go through the resolver for function > > calls. For example, functions with vector arguments will use vector > > registers to pass arguments. In the resolver, we do not save/restore the > > vector argument registers for lazy binding efficiency. To avoid ruining > > the vector arguments, functions with vector arguments will not go > > through the resolver. > > > > To achieve the goal, we will annotate the function symbols with > > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic > > section. In the first pass on PLT relocations, we do not set up to call > > _dl_runtime_resolve. Instead, we resolve the functions directly. > > > > Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > > --- > > elf/elf.h | 7 +++++++ > > manual/platform.texi | 6 ++++++ > > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ > > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ > > 4 files changed, 60 insertions(+) > > create mode 100644 sysdeps/riscv/dl-dtprocnum.h > > > > diff --git a/elf/elf.h b/elf/elf.h > > index 0735f6b579..9c95544050 100644 > > --- a/elf/elf.h > > +++ b/elf/elf.h > > @@ -3911,6 +3911,13 @@ enum > > > > #define R_TILEGX_NUM 130 > > > > +/* RISC-V specific values for the Dyn d_tag field. */ > > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > > +#define DT_RISCV_NUM 2 > > + > > +/* RISC-V specific values for the st_other field. */ > > +#define STO_RISCV_VARIANT_CC 0x80 > > + > > /* RISC-V ELF Flags */ > > #define EF_RISCV_RVC 0x0001 > > #define EF_RISCV_FLOAT_ABI 0x0006 > > diff --git a/manual/platform.texi b/manual/platform.texi > > index d5fdc5bd05..a1a740f381 100644 > > --- a/manual/platform.texi > > +++ b/manual/platform.texi > > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. > > @node RISC-V > > @appendixsec RISC-V-specific Facilities > > > > +Functions that are lazily bound must be compatible with the standard calling > > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means > > +this function is not compatible with the standard calling convention. The > > +dynamic linker will directly resolve it instead of using the lazy binding > > +mechanism. > > IMO this is the wrong way to go: we're essentially re-defining a bit > used be the standard ABI to mean something else. I guess we've already > defacto forked from the psABI with that "standard calling convention" > language, but IMO it'd be prudent to use a different bit to represent > this new behavior. In the long term one could imagine trying to get > back in line with the psABI, but if we're repurposing two bit patterns > it'll be a bit harder than if we're just repurposing one. > > > + > > Cache management facilities specific to RISC-V systems that implement the Linux > > ABI are declared in @file{sys/cachectl.h}. > > > > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > > new file mode 100644 > > index 0000000000..f189fd700a > > --- /dev/null > > +++ b/sysdeps/riscv/dl-dtprocnum.h > > @@ -0,0 +1,21 @@ > > +/* Configuration of lookup functions. RISC-V version. > > + Copyright (C) 2019-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/>. */ > > + > > +/* Number of extra dynamic section entries for this architecture. By > > + default there are none. */ > > +#define DT_THISPROCNUM DT_RISCV_NUM > > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > > index 1d3e2e588c..cdbaca6533 100644 > > --- a/sysdeps/riscv/dl-machine.h > > +++ b/sysdeps/riscv/dl-machine.h > > @@ -53,6 +53,9 @@ > > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ > > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) > > > > +//* Translate a processor specific dynamic tag to the index in l_info array. */ > > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) > > + > > /* Return nonzero iff ELF header is compatible with the running host. */ > > static inline int __attribute_used__ > > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) > > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], > > /* Check for unexpected PLT reloc type. */ > > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > > { > > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > > + { > > + /* Check the symbol table for variant CC symbols. */ > > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > > + const ElfW(Sym) *symtab = > > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > > + const ElfW(Sym) *sym = &symtab[symndx]; > > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > > + { > > + /* Avoid lazy resolution of variant CC symbols. */ > > + const struct r_found_version *version = NULL; > > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > > + { > > + const ElfW(Half) *vernum = > > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > > + } > > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, > > + skip_ifunc); > > + return; > > + } > > + } > > + > > if (__glibc_unlikely (map->l_mach.plt == 0)) > > { > > if (l_addr) > > Aside from that this one looks fine to me. > > Given the complexity around this psABI spec deviation and how close we > are to release I'd prefer to wait and see if we can come up with a > better solution, though -- for example, I'd been kicking around some > ideas related to ELF object attributes saying "this follows the > psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way > we could at least tag binaries that explicitly rely on this new behavior > as such, which would give us a shot at eventually getting rid of them. If you want to go this route, I suggest you use GNU property for this. ld.so supports GNU property. -- H.J. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-20 2:38 ` H.J. Lu @ 2022-01-20 2:43 ` Palmer Dabbelt 0 siblings, 0 replies; 18+ messages in thread From: Palmer Dabbelt @ 2022-01-20 2:43 UTC (permalink / raw) To: H.J. Lu Cc: vincent.chen, libc-alpha, Andrew Waterman, kai.wang, greentime.hu, kito.cheng On Wed, 19 Jan 2022 18:38:35 PST (-0800), H.J. Lu wrote: > On Wed, Jan 19, 2022 at 6:22 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: [snip] >> Given the complexity around this psABI spec deviation and how close we >> are to release I'd prefer to wait and see if we can come up with a >> better solution, though -- for example, I'd been kicking around some >> ideas related to ELF object attributes saying "this follows the >> psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way >> we could at least tag binaries that explicitly rely on this new behavior >> as such, which would give us a shot at eventually getting rid of them. > > If you want to go this route, I suggest you use GNU property for this. > ld.so supports GNU property. Makes sense. I'd been thinking of essentially just defining one bit for each of these incompatibilites, with an absence of them meaning the legacy behavior and then a true/false meaning that users have explicitly opted into to the spec'd or legacy behavior. I hadn't gotten as far as actually figuring out where to put those bits -- I got hung up on the pc-relative vs position-independent one, and that's clearly for after this round of releases so I kind of just put it on the backburner. Given that pretty much all of these are going to need to drive runtime behavior, though, it sounds like a GNU property is a reasonable way to go. There's going to be a lot of edge cases here, though, so happy to hear if anyone has ideas ;) Thanks! ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-20 2:21 ` Palmer Dabbelt 2022-01-20 2:38 ` H.J. Lu @ 2022-01-21 1:43 ` Vincent Chen 2022-02-24 20:56 ` Palmer Dabbelt 1 sibling, 1 reply; 18+ messages in thread From: Vincent Chen @ 2022-01-21 1:43 UTC (permalink / raw) To: Palmer Dabbelt Cc: GNU C Library, Darius Rad, Andrew Waterman, DJ Delorie, Kito Cheng, Greentime Hu, Hsiangkai Wang On Thu, Jan 20, 2022 at 10:21 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > Sorry, I missed the fixed-up patch set (which is why I just sent out a > similar bit of documentation). > > On Mon, 17 Jan 2022 20:31:59 PST (-0800), vincent.chen@sifive.com wrote: > > From: Hsiangkai Wang <kai.wang@sifive.com> > > > > In some cases, we do not want to go through the resolver for function > > calls. For example, functions with vector arguments will use vector > > registers to pass arguments. In the resolver, we do not save/restore the > > vector argument registers for lazy binding efficiency. To avoid ruining > > the vector arguments, functions with vector arguments will not go > > through the resolver. > > > > To achieve the goal, we will annotate the function symbols with > > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic > > section. In the first pass on PLT relocations, we do not set up to call > > _dl_runtime_resolve. Instead, we resolve the functions directly. > > > > Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > > --- > > elf/elf.h | 7 +++++++ > > manual/platform.texi | 6 ++++++ > > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ > > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ > > 4 files changed, 60 insertions(+) > > create mode 100644 sysdeps/riscv/dl-dtprocnum.h > > > > diff --git a/elf/elf.h b/elf/elf.h > > index 0735f6b579..9c95544050 100644 > > --- a/elf/elf.h > > +++ b/elf/elf.h > > @@ -3911,6 +3911,13 @@ enum > > > > #define R_TILEGX_NUM 130 > > > > +/* RISC-V specific values for the Dyn d_tag field. */ > > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > > +#define DT_RISCV_NUM 2 > > + > > +/* RISC-V specific values for the st_other field. */ > > +#define STO_RISCV_VARIANT_CC 0x80 > > + > > /* RISC-V ELF Flags */ > > #define EF_RISCV_RVC 0x0001 > > #define EF_RISCV_FLOAT_ABI 0x0006 > > diff --git a/manual/platform.texi b/manual/platform.texi > > index d5fdc5bd05..a1a740f381 100644 > > --- a/manual/platform.texi > > +++ b/manual/platform.texi > > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. > > @node RISC-V > > @appendixsec RISC-V-specific Facilities > > > > +Functions that are lazily bound must be compatible with the standard calling > > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means > > +this function is not compatible with the standard calling convention. The > > +dynamic linker will directly resolve it instead of using the lazy binding > > +mechanism. > > IMO this is the wrong way to go: we're essentially re-defining a bit > used be the standard ABI to mean something else. I guess we've already > defacto forked from the psABI with that "standard calling convention" > language, but IMO it'd be prudent to use a different bit to represent > this new behavior. In the long term one could imagine trying to get > back in line with the psABI, but if we're repurposing two bit patterns > it'll be a bit harder than if we're just repurposing one. > OK, I understand. I reviewed the psABI spec again and did some modifications. Did you think is it better? Functions that are lazily bound must be compatible with the standard calling convention. Any functions that use additional argument registers must be annotated with STO_RISCV_VARIANT_CC. To prevent these additional argument registers from being corrupted during the lazy binding process, this patch makes such functions be always resolved at load time, not lazily. > > + > > Cache management facilities specific to RISC-V systems that implement the Linux > > ABI are declared in @file{sys/cachectl.h}. > > > > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > > new file mode 100644 > > index 0000000000..f189fd700a > > --- /dev/null > > +++ b/sysdeps/riscv/dl-dtprocnum.h > > @@ -0,0 +1,21 @@ > > +/* Configuration of lookup functions. RISC-V version. > > + Copyright (C) 2019-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/>. */ > > + > > +/* Number of extra dynamic section entries for this architecture. By > > + default there are none. */ > > +#define DT_THISPROCNUM DT_RISCV_NUM > > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > > index 1d3e2e588c..cdbaca6533 100644 > > --- a/sysdeps/riscv/dl-machine.h > > +++ b/sysdeps/riscv/dl-machine.h > > @@ -53,6 +53,9 @@ > > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ > > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) > > > > +//* Translate a processor specific dynamic tag to the index in l_info array. */ > > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) > > + > > /* Return nonzero iff ELF header is compatible with the running host. */ > > static inline int __attribute_used__ > > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) > > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], > > /* Check for unexpected PLT reloc type. */ > > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > > { > > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > > + { > > + /* Check the symbol table for variant CC symbols. */ > > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > > + const ElfW(Sym) *symtab = > > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > > + const ElfW(Sym) *sym = &symtab[symndx]; > > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > > + { > > + /* Avoid lazy resolution of variant CC symbols. */ > > + const struct r_found_version *version = NULL; > > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > > + { > > + const ElfW(Half) *vernum = > > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > > + } > > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, > > + skip_ifunc); > > + return; > > + } > > + } > > + > > if (__glibc_unlikely (map->l_mach.plt == 0)) > > { > > if (l_addr) > > Aside from that this one looks fine to me. > > Given the complexity around this psABI spec deviation and how close we > are to release I'd prefer to wait and see if we can come up with a > better solution, though -- for example, I'd been kicking around some > ideas related to ELF object attributes saying "this follows the > psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way > we could at least tag binaries that explicitly rely on this new behavior > as such, which would give us a shot at eventually getting rid of them. I agree that we don't need to rush to come up with a solution in this release. But, I have a little confused. Even if the ELF object attribute is able to say "this follows the psABI-1.0" vs "this follows the legacy GNU psABI extensions", we still need to use STO_RISCV_VARIANT_CC to tell ld.so whether needs to directly resolve this symbol. Is it correct? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-21 1:43 ` Vincent Chen @ 2022-02-24 20:56 ` Palmer Dabbelt 0 siblings, 0 replies; 18+ messages in thread From: Palmer Dabbelt @ 2022-02-24 20:56 UTC (permalink / raw) To: vincent.chen Cc: libc-alpha, Darius Rad, Andrew Waterman, dj, kito.cheng, greentime.hu, kai.wang On Thu, 20 Jan 2022 17:43:14 PST (-0800), vincent.chen@sifive.com wrote: > On Thu, Jan 20, 2022 at 10:21 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: >> >> Sorry, I missed the fixed-up patch set (which is why I just sent out a >> similar bit of documentation). >> >> On Mon, 17 Jan 2022 20:31:59 PST (-0800), vincent.chen@sifive.com wrote: >> > From: Hsiangkai Wang <kai.wang@sifive.com> >> > >> > In some cases, we do not want to go through the resolver for function >> > calls. For example, functions with vector arguments will use vector >> > registers to pass arguments. In the resolver, we do not save/restore the >> > vector argument registers for lazy binding efficiency. To avoid ruining >> > the vector arguments, functions with vector arguments will not go >> > through the resolver. >> > >> > To achieve the goal, we will annotate the function symbols with >> > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic >> > section. In the first pass on PLT relocations, we do not set up to call >> > _dl_runtime_resolve. Instead, we resolve the functions directly. >> > >> > Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> >> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> >> > --- >> > elf/elf.h | 7 +++++++ >> > manual/platform.texi | 6 ++++++ >> > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ >> > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ >> > 4 files changed, 60 insertions(+) >> > create mode 100644 sysdeps/riscv/dl-dtprocnum.h >> > >> > diff --git a/elf/elf.h b/elf/elf.h >> > index 0735f6b579..9c95544050 100644 >> > --- a/elf/elf.h >> > +++ b/elf/elf.h >> > @@ -3911,6 +3911,13 @@ enum >> > >> > #define R_TILEGX_NUM 130 >> > >> > +/* RISC-V specific values for the Dyn d_tag field. */ >> > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) >> > +#define DT_RISCV_NUM 2 >> > + >> > +/* RISC-V specific values for the st_other field. */ >> > +#define STO_RISCV_VARIANT_CC 0x80 >> > + >> > /* RISC-V ELF Flags */ >> > #define EF_RISCV_RVC 0x0001 >> > #define EF_RISCV_FLOAT_ABI 0x0006 >> > diff --git a/manual/platform.texi b/manual/platform.texi >> > index d5fdc5bd05..a1a740f381 100644 >> > --- a/manual/platform.texi >> > +++ b/manual/platform.texi >> > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. >> > @node RISC-V >> > @appendixsec RISC-V-specific Facilities >> > >> > +Functions that are lazily bound must be compatible with the standard calling >> > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means >> > +this function is not compatible with the standard calling convention. The >> > +dynamic linker will directly resolve it instead of using the lazy binding >> > +mechanism. >> >> IMO this is the wrong way to go: we're essentially re-defining a bit >> used be the standard ABI to mean something else. I guess we've already >> defacto forked from the psABI with that "standard calling convention" >> language, but IMO it'd be prudent to use a different bit to represent >> this new behavior. In the long term one could imagine trying to get >> back in line with the psABI, but if we're repurposing two bit patterns >> it'll be a bit harder than if we're just repurposing one. >> > OK, I understand. I reviewed the psABI spec again and did some > modifications. Did you think is it better? > > Functions that are lazily bound must be compatible with the standard > calling convention. Any functions that use additional argument > registers must be annotated with STO_RISCV_VARIANT_CC. To prevent > these additional argument registers from being corrupted during the > lazy binding process, this patch makes such functions be always > resolved at load time, not lazily. I was trying to suggest using a different bit (with a different name) for the "does not follow the standard calling convention" behavior, rather than re-defining the bit allocated for STO_RISCV_VARIANT_CC in the psABI for that behavior. Maybe it just doesn't matter, given that we're forking, but re-using the same bit will just make things more confusing for everyone in the future. Aside from that the original text looked OK. >> > + >> > Cache management facilities specific to RISC-V systems that implement the Linux >> > ABI are declared in @file{sys/cachectl.h}. >> > >> > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h >> > new file mode 100644 >> > index 0000000000..f189fd700a >> > --- /dev/null >> > +++ b/sysdeps/riscv/dl-dtprocnum.h >> > @@ -0,0 +1,21 @@ >> > +/* Configuration of lookup functions. RISC-V version. >> > + Copyright (C) 2019-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/>. */ >> > + >> > +/* Number of extra dynamic section entries for this architecture. By >> > + default there are none. */ >> > +#define DT_THISPROCNUM DT_RISCV_NUM >> > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h >> > index 1d3e2e588c..cdbaca6533 100644 >> > --- a/sysdeps/riscv/dl-machine.h >> > +++ b/sysdeps/riscv/dl-machine.h >> > @@ -53,6 +53,9 @@ >> > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ >> > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) >> > >> > +//* Translate a processor specific dynamic tag to the index in l_info array. */ >> > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) >> > + >> > /* Return nonzero iff ELF header is compatible with the running host. */ >> > static inline int __attribute_used__ >> > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) >> > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], >> > /* Check for unexpected PLT reloc type. */ >> > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) >> > { >> > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) >> > + { >> > + /* Check the symbol table for variant CC symbols. */ >> > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); >> > + const ElfW(Sym) *symtab = >> > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); >> > + const ElfW(Sym) *sym = &symtab[symndx]; >> > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) >> > + { >> > + /* Avoid lazy resolution of variant CC symbols. */ >> > + const struct r_found_version *version = NULL; >> > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) >> > + { >> > + const ElfW(Half) *vernum = >> > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); >> > + version = &map->l_versions[vernum[symndx] & 0x7fff]; >> > + } >> > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, >> > + skip_ifunc); >> > + return; >> > + } >> > + } >> > + >> > if (__glibc_unlikely (map->l_mach.plt == 0)) >> > { >> > if (l_addr) >> >> Aside from that this one looks fine to me. >> >> Given the complexity around this psABI spec deviation and how close we >> are to release I'd prefer to wait and see if we can come up with a >> better solution, though -- for example, I'd been kicking around some >> ideas related to ELF object attributes saying "this follows the >> psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way >> we could at least tag binaries that explicitly rely on this new behavior >> as such, which would give us a shot at eventually getting rid of them. > > I agree that we don't need to rush to come up with a solution in this > release. But, I have a little confused. Even if the ELF object > attribute is able to say "this follows the psABI-1.0" vs "this follows > the legacy GNU psABI extensions", we still need to use > STO_RISCV_VARIANT_CC to tell ld.so whether needs to directly resolve > this symbol. Is it correct? We need to directly resolve all symbols compatible with psABI-1.0, lazy binding will only be legal for symbols that follow the legacy GNU extensions. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-01-18 4:31 ` [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC Vincent Chen 2022-01-20 2:21 ` Palmer Dabbelt @ 2022-12-09 4:11 ` Vineet Gupta 2022-12-09 4:22 ` Kito Cheng 1 sibling, 1 reply; 18+ messages in thread From: Vineet Gupta @ 2022-12-09 4:11 UTC (permalink / raw) To: Vincent Chen, libc-alpha, palmer, darius, andrew, dj Cc: greentime.hu, kito.cheng, Hsiangkai Wang, Andy Chiu, Björn Töpel, davidlt, Arnd Bergmann, Andrew Waterman, Florian Weimer, Nelson Chu On 1/17/22 20:31, Vincent Chen wrote: > From: Hsiangkai Wang <kai.wang@sifive.com> > > In some cases, we do not want to go through the resolver for function > calls. For example, functions with vector arguments will use vector > registers to pass arguments. In the resolver, we do not save/restore the > vector argument registers for lazy binding efficiency. To avoid ruining > the vector arguments, functions with vector arguments will not go > through the resolver. > > To achieve the goal, we will annotate the function symbols with > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic > section. In the first pass on PLT relocations, we do not set up to call > _dl_runtime_resolve. Instead, we resolve the functions directly. As per the ratified psABI v1.0, the V calling convention doesn't allow use of V reg for functions args, so this is not needed for now. -Vineet > > Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > --- > elf/elf.h | 7 +++++++ > manual/platform.texi | 6 ++++++ > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ > 4 files changed, 60 insertions(+) > create mode 100644 sysdeps/riscv/dl-dtprocnum.h > > diff --git a/elf/elf.h b/elf/elf.h > index 0735f6b579..9c95544050 100644 > --- a/elf/elf.h > +++ b/elf/elf.h > @@ -3911,6 +3911,13 @@ enum > > #define R_TILEGX_NUM 130 > > +/* RISC-V specific values for the Dyn d_tag field. */ > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > +#define DT_RISCV_NUM 2 > + > +/* RISC-V specific values for the st_other field. */ > +#define STO_RISCV_VARIANT_CC 0x80 > + > /* RISC-V ELF Flags */ > #define EF_RISCV_RVC 0x0001 > #define EF_RISCV_FLOAT_ABI 0x0006 > diff --git a/manual/platform.texi b/manual/platform.texi > index d5fdc5bd05..a1a740f381 100644 > --- a/manual/platform.texi > +++ b/manual/platform.texi > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. > @node RISC-V > @appendixsec RISC-V-specific Facilities > > +Functions that are lazily bound must be compatible with the standard calling > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means > +this function is not compatible with the standard calling convention. The > +dynamic linker will directly resolve it instead of using the lazy binding > +mechanism. > + > Cache management facilities specific to RISC-V systems that implement the Linux > ABI are declared in @file{sys/cachectl.h}. > > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > new file mode 100644 > index 0000000000..f189fd700a > --- /dev/null > +++ b/sysdeps/riscv/dl-dtprocnum.h > @@ -0,0 +1,21 @@ > +/* Configuration of lookup functions. RISC-V version. > + Copyright (C) 2019-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/>. */ > + > +/* Number of extra dynamic section entries for this architecture. By > + default there are none. */ > +#define DT_THISPROCNUM DT_RISCV_NUM > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > index 1d3e2e588c..cdbaca6533 100644 > --- a/sysdeps/riscv/dl-machine.h > +++ b/sysdeps/riscv/dl-machine.h > @@ -53,6 +53,9 @@ > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) > > +//* Translate a processor specific dynamic tag to the index in l_info array. */ > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) > + > /* Return nonzero iff ELF header is compatible with the running host. */ > static inline int __attribute_used__ > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], > /* Check for unexpected PLT reloc type. */ > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > { > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > + { > + /* Check the symbol table for variant CC symbols. */ > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > + const ElfW(Sym) *symtab = > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > + const ElfW(Sym) *sym = &symtab[symndx]; > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > + { > + /* Avoid lazy resolution of variant CC symbols. */ > + const struct r_found_version *version = NULL; > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > + { > + const ElfW(Half) *vernum = > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > + } > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, > + skip_ifunc); > + return; > + } > + } > + > if (__glibc_unlikely (map->l_mach.plt == 0)) > { > if (l_addr) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-12-09 4:11 ` Vineet Gupta @ 2022-12-09 4:22 ` Kito Cheng 2022-12-09 4:26 ` Vineet Gupta 0 siblings, 1 reply; 18+ messages in thread From: Kito Cheng @ 2022-12-09 4:22 UTC (permalink / raw) To: Vineet Gupta Cc: Vincent Chen, libc-alpha, palmer, darius, andrew, dj, greentime.hu, Hsiangkai Wang, Andy Chiu, Björn Töpel, davidlt, Arnd Bergmann, Florian Weimer, Nelson Chu >As per the ratified psABI v1.0, the V calling convention doesn't allow > use of V reg for functions args, so this is not needed for now. We don't have one for now, but we could expect the future will have one, so I think we could accept that on upstream first? On Fri, Dec 9, 2022 at 12:11 PM Vineet Gupta <vineetg@rivosinc.com> wrote: > > On 1/17/22 20:31, Vincent Chen wrote: > > From: Hsiangkai Wang <kai.wang@sifive.com> > > > > In some cases, we do not want to go through the resolver for function > > calls. For example, functions with vector arguments will use vector > > registers to pass arguments. In the resolver, we do not save/restore the > > vector argument registers for lazy binding efficiency. To avoid ruining > > the vector arguments, functions with vector arguments will not go > > through the resolver. > > > > To achieve the goal, we will annotate the function symbols with > > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic > > section. In the first pass on PLT relocations, we do not set up to call > > _dl_runtime_resolve. Instead, we resolve the functions directly. > > As per the ratified psABI v1.0, the V calling convention doesn't allow > use of V reg for functions args, so this is not needed for now. > > -Vineet > > > > > Signed-off-by: Hsiangkai Wang <kai.wang@sifive.com> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com> > > --- > > elf/elf.h | 7 +++++++ > > manual/platform.texi | 6 ++++++ > > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ > > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ > > 4 files changed, 60 insertions(+) > > create mode 100644 sysdeps/riscv/dl-dtprocnum.h > > > > diff --git a/elf/elf.h b/elf/elf.h > > index 0735f6b579..9c95544050 100644 > > --- a/elf/elf.h > > +++ b/elf/elf.h > > @@ -3911,6 +3911,13 @@ enum > > > > #define R_TILEGX_NUM 130 > > > > +/* RISC-V specific values for the Dyn d_tag field. */ > > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > > +#define DT_RISCV_NUM 2 > > + > > +/* RISC-V specific values for the st_other field. */ > > +#define STO_RISCV_VARIANT_CC 0x80 > > + > > /* RISC-V ELF Flags */ > > #define EF_RISCV_RVC 0x0001 > > #define EF_RISCV_FLOAT_ABI 0x0006 > > diff --git a/manual/platform.texi b/manual/platform.texi > > index d5fdc5bd05..a1a740f381 100644 > > --- a/manual/platform.texi > > +++ b/manual/platform.texi > > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. > > @node RISC-V > > @appendixsec RISC-V-specific Facilities > > > > +Functions that are lazily bound must be compatible with the standard calling > > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means > > +this function is not compatible with the standard calling convention. The > > +dynamic linker will directly resolve it instead of using the lazy binding > > +mechanism. > > + > > Cache management facilities specific to RISC-V systems that implement the Linux > > ABI are declared in @file{sys/cachectl.h}. > > > > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > > new file mode 100644 > > index 0000000000..f189fd700a > > --- /dev/null > > +++ b/sysdeps/riscv/dl-dtprocnum.h > > @@ -0,0 +1,21 @@ > > +/* Configuration of lookup functions. RISC-V version. > > + Copyright (C) 2019-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/>. */ > > + > > +/* Number of extra dynamic section entries for this architecture. By > > + default there are none. */ > > +#define DT_THISPROCNUM DT_RISCV_NUM > > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > > index 1d3e2e588c..cdbaca6533 100644 > > --- a/sysdeps/riscv/dl-machine.h > > +++ b/sysdeps/riscv/dl-machine.h > > @@ -53,6 +53,9 @@ > > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ > > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) > > > > +//* Translate a processor specific dynamic tag to the index in l_info array. */ > > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) > > + > > /* Return nonzero iff ELF header is compatible with the running host. */ > > static inline int __attribute_used__ > > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) > > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], > > /* Check for unexpected PLT reloc type. */ > > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > > { > > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > > + { > > + /* Check the symbol table for variant CC symbols. */ > > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > > + const ElfW(Sym) *symtab = > > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > > + const ElfW(Sym) *sym = &symtab[symndx]; > > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > > + { > > + /* Avoid lazy resolution of variant CC symbols. */ > > + const struct r_found_version *version = NULL; > > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > > + { > > + const ElfW(Half) *vernum = > > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > > + } > > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, > > + skip_ifunc); > > + return; > > + } > > + } > > + > > if (__glibc_unlikely (map->l_mach.plt == 0)) > > { > > if (l_addr) > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-12-09 4:22 ` Kito Cheng @ 2022-12-09 4:26 ` Vineet Gupta 2022-12-09 4:35 ` Kito Cheng 0 siblings, 1 reply; 18+ messages in thread From: Vineet Gupta @ 2022-12-09 4:26 UTC (permalink / raw) To: Kito Cheng Cc: Vincent Chen, libc-alpha, palmer, darius, andrew, dj, greentime.hu, Hsiangkai Wang, Andy Chiu, Björn Töpel, davidlt, Arnd Bergmann, Florian Weimer, Nelson Chu On 12/8/22 20:22, Kito Cheng wrote: >> As per the ratified psABI v1.0, the V calling convention doesn't allow >> use of V reg for functions args, so this is not needed for now. > We don't have one for now, but we could expect the future will have one, > so I think we could accept that on upstream first? Not sure what you mean. Are you saying that even though the current ABI doesn't require it, we should still add it to glibc, won't it bitrot. I don't feel strongly either ways, but IMO this should be done when the ABI is actually changed. Thx, -Vineet ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. 2022-12-09 4:26 ` Vineet Gupta @ 2022-12-09 4:35 ` Kito Cheng 0 siblings, 0 replies; 18+ messages in thread From: Kito Cheng @ 2022-12-09 4:35 UTC (permalink / raw) To: Vineet Gupta Cc: Vincent Chen, libc-alpha, palmer, darius, andrew, dj, greentime.hu, Andy Chiu, Björn Töpel, davidlt, Arnd Bergmann, Florian Weimer, Nelson Chu > Not sure what you mean. Are you saying that even though the current ABI > doesn't require it, we should still add it to glibc, won't it bitrot. > I don't feel strongly either ways, but IMO this should be done when the > ABI is actually changed. STO_RISCV_VARIANT_CC has already been defined and released in psABI 1.0, and this patch is implementing a feature in released psABI, so I think this is fine to upstream from psABI perspective. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-12-09 4:35 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-18 4:31 [PATCH v2 0/2] RISC-V: Add vector ISA support Vincent Chen 2022-01-18 4:31 ` [PATCH v2 1/2] RISC-V: remove riscv-specific sigcontext.h Vincent Chen 2022-01-20 2:36 ` Palmer Dabbelt 2022-01-20 2:47 ` Kito Cheng 2022-01-21 1:29 ` Vincent Chen 2022-01-24 9:42 ` Vincent Chen 2022-02-24 20:56 ` Palmer Dabbelt 2022-02-25 0:32 ` Vincent Chen 2022-01-18 4:31 ` [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC Vincent Chen 2022-01-20 2:21 ` Palmer Dabbelt 2022-01-20 2:38 ` H.J. Lu 2022-01-20 2:43 ` Palmer Dabbelt 2022-01-21 1:43 ` Vincent Chen 2022-02-24 20:56 ` Palmer Dabbelt 2022-12-09 4:11 ` Vineet Gupta 2022-12-09 4:22 ` Kito Cheng 2022-12-09 4:26 ` Vineet Gupta 2022-12-09 4:35 ` Kito Cheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).