| Message ID | 20201114064200.1214-4-huangpei@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] mips: add hp-timing support for MIPS R2 | expand |
On Sat, 14 Nov 2020, Huang Pei wrote:
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
Please be a bit more verbose in the change description. This used to be
an implementation of the old syscall restart convention, where $v0 had to
be (re)loaded by the instruction immediately preceding SYSCALL, obviously
from a source that does not get clobbered in syscall processing (i.e.
either memory or a static register). I imagine a good change description
would say at least actually why this convention is no longer relevant.
Refer to the comment in sysdeps/unix/sysv/linux/mips/mips32/sysdep.h or
sysdeps/unix/sysv/linux/mips/mips64/sysdep.h for further details.
Also I am not sure if it makes sense to remove this single instance only,
possibly for the least used syscall in user code, and leave all the other
places unchanged.
Maciej
On Fri, Dec 04, 2020 at 11:24:24AM +0000, Maciej W. Rozycki wrote: > Hi Huang Pei, > > Apologies for my late reply, I've had a hectic time recently and some GCC > stuff had to take priority due to the end of Stage 1 there. > > > I must admit that I did not realize that Patch 3 would cause compatibiliy > > issue, that is Linux/MIPS kernel before 2.6.36 depended on > > "move a0, s0 ' just preceding "syscall" to make SYSCALL Restart > > work, while Linux/MIPS after 2.6.36 do not > > Or `move v0, s0' (in this case) to be correct. Yes > > Yes, this has been quite obscure and used to be hardly documented up to > the point that breakage happened while the old scheme was still supported. > And given that syscall restarts are not at all that frequent any symptom > would be limited to an occasional program crash or other odd behaviour as > the wrong syscall was dispatched upon a restart. > > This is why I added the explanation along with fixes for BZ #15054 made > with commit b82ba2f011fc ("MIPS: Respect the legacy syscall restart > convention."). Before that we only had Linux sources as a reference. > > NB the old scheme was removed from Linux with commit 8f5a00eb422e ("MIPS: > Sanitize restart logics"). > > > since glibc 2.24，the minimum kernel version is 3.2, is it ok to add Patch > > 3 for glibc after version 2.24? If so, I would send V3, which merge Patch > > 2 and Patch 3 and rewrite the change description with minimium kernel > > version requirement. > > As I noted elsewhere I object such a change if made on its own, as it > would only cause a mess. This one was still a mess, since the ".set noreorder" is within b82ba2f011fc, but missing here(I hate '.set noreorder', which bite me too much) I came to this one accidentally when I addressed a unaligned access issue, but the fix is not the key point(see Patch V1 2/3), and Patch V1 3/3 without touching ".set noreorder"(I focus on) is. PS: I think it would be mutch better without both ".set reorder" and ".set noreorder", and with 8f5a00eb422e, we can remove more ".set noreorder" > > You can review my commit referred above, and also commit 43301bd3c281 > ("Add support for building as MIPS16 code.") to see what has to be done to > remove bits for the old feature in a consistent manner. > > Maciej Now it is OK to remove the mess as a whole, I will send V3 to get rid of it all
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
Signed-off-by: Huang Pei <huangpei@loongson.cn> --- sysdeps/unix/sysv/linux/mips/mips64/syscall.S | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)