Message ID | 20201128081817.15463-4-huangpei@loongson.cn |
---|---|
State | New |
Headers | show |
Series | [1/3] mips: add hp-timing support for MIPS R2 | expand |
On 28/11/2020 05:18, Huang Pei wrote: > Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka > "li v0, #sys_number") right precedes "syscall", so the kernel syscall > restart sequence can use CP0 EPC - 4 to restart the syscall, because > kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled > this restriction. > > See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h} > > Since glibc-2.24 the minimum kernel version is 3.2(much higer than > 2.6.36), I think it is OK to remove the ugly register spill in > syscall.S just because of the old convention > > Signed-off-by: Huang Pei <huangpei@loongson.cn> We do not use SCO, but rather Copyright assignment. The rest of the patch looks ok. > --- > sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 14 ++------------ > 1 file changed, 2 insertions(+), 12 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S > index aab1f389aa..089524a40b 100644 > --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S > +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S > @@ -27,14 +27,9 @@ > > .text > NESTED (syscall, SZREG, ra) > - .mask 0x00010000, -2 * SZREG > + .mask 0x00000000, 0 > .fmask 0x00000000, 0 > - PTR_ADDIU sp, -2 * SZREG > - cfi_adjust_cfa_offset (2 * SZREG) > - REG_S s0, (sp) > - cfi_rel_offset (s0, 0) > - > - move s0, a0 > + move v0, a0 > move a0, a1 /* shift arg1 - arg7. */ > move a1, a2 > move a2, a3 > @@ -43,13 +38,8 @@ NESTED (syscall, SZREG, ra) > move a5, a6 > move a6, a7 > > - move v0, s0 /* Syscall number -> v0 */ > syscall /* Do the system call. */ > > - REG_L s0, (sp) > - cfi_restore (s0) > - PTR_ADDIU sp, 2 * SZREG > - cfi_adjust_cfa_offset (-2 * 2 * SZREG) > bne a3, zero, L(error) > > ret >
hi, On Mon, Nov 30, 2020 at 11:22:03AM -0300, Adhemerval Zanella wrote: > > > On 28/11/2020 05:18, Huang Pei wrote: > > Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka > > "li v0, #sys_number") right precedes "syscall", so the kernel syscall > > restart sequence can use CP0 EPC - 4 to restart the syscall, because > > kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled > > this restriction. > > > > See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h} > > > > Since glibc-2.24 the minimum kernel version is 3.2(much higer than > > 2.6.36), I think it is OK to remove the ugly register spill in > > syscall.S just because of the old convention > > > > Signed-off-by: Huang Pei <huangpei@loongson.cn> > > We do not use SCO, but rather Copyright assignment. > > The rest of the patch looks ok. > I would like to combine the Patch 2/3 and Patch 3/3 into one patch, since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall restart convention, any advice? > > --- > > sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 14 ++------------ > > 1 file changed, 2 insertions(+), 12 deletions(-) > > > > diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S > > index aab1f389aa..089524a40b 100644 > > --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S > > +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S > > @@ -27,14 +27,9 @@ > > > > .text > > NESTED (syscall, SZREG, ra) > > - .mask 0x00010000, -2 * SZREG > > + .mask 0x00000000, 0 > > .fmask 0x00000000, 0 > > - PTR_ADDIU sp, -2 * SZREG > > - cfi_adjust_cfa_offset (2 * SZREG) > > - REG_S s0, (sp) > > - cfi_rel_offset (s0, 0) > > - > > - move s0, a0 > > + move v0, a0 > > move a0, a1 /* shift arg1 - arg7. */ > > move a1, a2 > > move a2, a3 > > @@ -43,13 +38,8 @@ NESTED (syscall, SZREG, ra) > > move a5, a6 > > move a6, a7 > > > > - move v0, s0 /* Syscall number -> v0 */ > > syscall /* Do the system call. */ > > > > - REG_L s0, (sp) > > - cfi_restore (s0) > > - PTR_ADDIU sp, 2 * SZREG > > - cfi_adjust_cfa_offset (-2 * 2 * SZREG) > > bne a3, zero, L(error) > > > > ret > >
On 01/12/2020 06:39, Huang Pei wrote: > hi, > > On Mon, Nov 30, 2020 at 11:22:03AM -0300, Adhemerval Zanella wrote: >> >> >> On 28/11/2020 05:18, Huang Pei wrote: >>> Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka >>> "li v0, #sys_number") right precedes "syscall", so the kernel syscall >>> restart sequence can use CP0 EPC - 4 to restart the syscall, because >>> kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled >>> this restriction. >>> >>> See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h} >>> >>> Since glibc-2.24 the minimum kernel version is 3.2(much higer than >>> 2.6.36), I think it is OK to remove the ugly register spill in >>> syscall.S just because of the old convention >>> >>> Signed-off-by: Huang Pei <huangpei@loongson.cn> >> >> We do not use SCO, but rather Copyright assignment. >> >> The rest of the patch looks ok. >> > > I would like to combine the Patch 2/3 and Patch 3/3 into one patch, > since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall > restart convention, any advice? Send a v2 with the patches merged and the hp-inline fix.
On Tue, 1 Dec 2020, Adhemerval Zanella via Libc-alpha wrote: > > I would like to combine the Patch 2/3 and Patch 3/3 into one patch, > > since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall > > restart convention, any advice? > > Send a v2 with the patches merged and the hp-inline fix. I would defer the removal of the old syscall restart convention in this single place only. I don't think we need to introduce such a mess. Instead an audit should be made and all the places adjusted with a single change. That would include at least MIPS16 wrappers, and IIRC some microMIPS support code, and then the comment I referred previously needs to be adjusted accordingly (maybe removed altogether even). Maciej
On 04/12/2020 08:03, Maciej W. Rozycki wrote: > On Tue, 1 Dec 2020, Adhemerval Zanella via Libc-alpha wrote: > >>> I would like to combine the Patch 2/3 and Patch 3/3 into one patch, >>> since Patch 2/3 only fix sp alignment, but the key point is folllowing syscall >>> restart convention, any advice? >> >> Send a v2 with the patches merged and the hp-inline fix. > > I would defer the removal of the old syscall restart convention in this > single place only. I don't think we need to introduce such a mess. > > Instead an audit should be made and all the places adjusted with a single > change. That would include at least MIPS16 wrappers, and IIRC some > microMIPS support code, and then the comment I referred previously needs > to be adjusted accordingly (maybe removed altogether even). > > Maciej Alright, it seems better indeed.
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S index aab1f389aa..089524a40b 100644 --- a/sysdeps/unix/sysv/linux/mips/mips64/syscall.S +++ b/sysdeps/unix/sysv/linux/mips/mips64/syscall.S @@ -27,14 +27,9 @@ .text NESTED (syscall, SZREG, ra) - .mask 0x00010000, -2 * SZREG + .mask 0x00000000, 0 .fmask 0x00000000, 0 - PTR_ADDIU sp, -2 * SZREG - cfi_adjust_cfa_offset (2 * SZREG) - REG_S s0, (sp) - cfi_rel_offset (s0, 0) - - move s0, a0 + move v0, a0 move a0, a1 /* shift arg1 - arg7. */ move a1, a2 move a2, a3 @@ -43,13 +38,8 @@ NESTED (syscall, SZREG, ra) move a5, a6 move a6, a7 - move v0, s0 /* Syscall number -> v0 */ syscall /* Do the system call. */ - REG_L s0, (sp) - cfi_restore (s0) - PTR_ADDIU sp, 2 * SZREG - cfi_adjust_cfa_offset (-2 * 2 * SZREG) bne a3, zero, L(error) ret
Before Linux/MIPS 2.6.36, kernel expected setting syscall number(aka "li v0, #sys_number") right precedes "syscall", so the kernel syscall restart sequence can use CP0 EPC - 4 to restart the syscall, because kernel DID NOT save v0 during syscall handling. Linux 2.6.36 canceled this restriction. See sysdeps/unix/sysv/linux/mips/{mips32/sysdep.h,mips64/sysdep.h,sysdep.h} Since glibc-2.24 the minimum kernel version is 3.2(much higer than 2.6.36), I think it is OK to remove the ugly register spill in syscall.S just because of the old convention Signed-off-by: Huang Pei <huangpei@loongson.cn> --- sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)