| Message ID | 20170706002401.10507-6-rth@twiddle.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Le 06/07/2017 à 02:23, Richard Henderson a écrit : > We translate gUSA regions atomically in a parallel context. > But in a serial context a gUSA region may be interrupted. > In that case, restart the region as the kernel would. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > linux-user/signal.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/linux-user/signal.c b/linux-user/signal.c > index 3d18d1b..1e716a9 100644 > --- a/linux-user/signal.c > +++ b/linux-user/signal.c > @@ -3471,6 +3471,23 @@ static abi_ulong get_sigframe(struct target_sigaction *ka, > return (sp - frame_size) & -8ul; > } > > +/* Notice when we're in the middle of a gUSA region and reset. > + Note that this will only occur for !parallel_cpus, as we will > + translate such sequences differently in a parallel context. */ > +static void unwind_gusa(CPUSH4State *regs) > +{ > + /* If the stack pointer is sufficiently negative... */ > + if ((regs->gregs[15] & 0xc0000000u) == 0xc0000000u) { > + /* Reset the PC to before the gUSA region, as computed from > + R0 = region end, SP = -(region size), plus one more insn > + that actually sets SP to the region size. */ > + regs->pc = regs->gregs[0] + regs->gregs[15] - 2; > + > + /* Reset the SP to the saved version in R1. */ > + regs->gregs[15] = regs->gregs[1]; > + } > +} > + > static void setup_sigcontext(struct target_sigcontext *sc, > CPUSH4State *regs, unsigned long mask) > { > @@ -3534,6 +3551,8 @@ static void setup_frame(int sig, struct target_sigaction *ka, > abi_ulong frame_addr; > int i; > > + unwind_gusa(regs); > + > frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame)); > trace_user_setup_frame(regs, frame_addr); > if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) { > @@ -3583,6 +3602,8 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka, > abi_ulong frame_addr; > int i; > > + unwind_gusa(regs); > + > frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame)); > trace_user_setup_rt_frame(regs, frame_addr); > if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) { > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Hi! Wow, great to see that there is actually now support for handling gUSA in qemu-user on sh4. I wonder whether this will help resolve the lock-up issues on qemu-sh4-user when building Debian packages for sh4. The lockups occur fairly often when any process uses multi-threading. Would this help here? Was gUSA support recently added to qemu or is this just a fix? I will give this a try soon. Thanks Laurent for letting me know. Adrian
Le 06/07/2017 à 10:10, John Paul Adrian Glaubitz a écrit : > Hi! > > Wow, great to see that there is actually now support for handling gUSA > in qemu-user on sh4. I wonder whether this will help resolve the > lock-up issues on qemu-sh4-user when building Debian packages for > sh4. > > The lockups occur fairly often when any process uses > multi-threading. Would this help here? > > Was gUSA support recently added to qemu or is this just a fix? This is a patch series proposed by Richard, it is not included in qemu at the moment: http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg01196.html I think it would be interesting if you generate a qemu binary with it and use it in your buildds to test it. I guess the series can be pulled directly from Richard's branch: git://github.com/rth7680/qemu.git tgt-sh4 Thanks, Laurent
On Thu, Jul 06, 2017 at 10:35:15AM +0200, Laurent Vivier wrote: > This is a patch series proposed by Richard, it is not included in qemu > at the moment: > > http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg01196.html Aye, awesome! > I think it would be interesting if you generate a qemu binary with it > and use it in your buildds to test it. > > I guess the series can be pulled directly from Richard's branch: > > git://github.com/rth7680/qemu.git tgt-sh4 You bet, I will! Adrian
On Thu, Jul 06, 2017 at 10:35:15AM +0200, Laurent Vivier wrote: > I think it would be interesting if you generate a qemu binary with it > and use it in your buildds to test it. > > I guess the series can be pulled directly from Richard's branch: > > git://github.com/rth7680/qemu.git tgt-sh4 Pull from this tree and built qemu-sh4-user as static from the tgt-sh4 branch. Copied in Debian/sh4 chroot and tried to enter it: root@nofan:/local_scratch/sid-sh4-sbuild> chroot . bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) qemu: uncaught target signal 11 (Segmentation fault) - core dumped qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault root@nofan:/local_scratch/sid-sh4-sbuild> Adrian
Le 06/07/2017 à 11:13, John Paul Adrian Glaubitz a écrit : > On Thu, Jul 06, 2017 at 10:35:15AM +0200, Laurent Vivier wrote: >> I think it would be interesting if you generate a qemu binary with it >> and use it in your buildds to test it. >> >> I guess the series can be pulled directly from Richard's branch: >> >> git://github.com/rth7680/qemu.git tgt-sh4 > > Pull from this tree and built qemu-sh4-user as static from the tgt-sh4 > branch. Copied in Debian/sh4 chroot and tried to enter it: > > root@nofan:/local_scratch/sid-sh4-sbuild> chroot . > bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8) > qemu: uncaught target signal 11 (Segmentation fault) - core dumped > qemu: uncaught target signal 11 (Segmentation fault) - core dumped > Segmentation fault > root@nofan:/local_scratch/sid-sh4-sbuild> Could you try origin/master to see if the regression is introduced by this series or by a previous one? Thanks, Laurent
On Thu, Jul 06, 2017 at 03:09:59AM +0200, Laurent Vivier wrote: > Reviewed-by: Laurent Vivier <laurent@vivier.eu> I have identified this patch as reponsible for the segfaults. Reverting this patch fixes the issue. The crashes are random. Sometimes it crashes directly when entering the chroot, sometimes only after a command like running "apt update". Interestingly, the issue does not reproduce on an older chroot from 2015. I have uploaded two chroots for testing, one from 2015 [1] and one freshly generated [2] which shows the crashes with patch nr. 5 applied. Adrian > [1] https://people.debian.org/~glaubitz/chroots/unstable-sh4-20150315.tar.gz > [2] https://people.debian.org/~glaubitz/chroots/unstable-sh4-20170706.tgz
diff --git a/linux-user/signal.c b/linux-user/signal.c index 3d18d1b..1e716a9 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -3471,6 +3471,23 @@ static abi_ulong get_sigframe(struct target_sigaction *ka, return (sp - frame_size) & -8ul; } +/* Notice when we're in the middle of a gUSA region and reset. + Note that this will only occur for !parallel_cpus, as we will + translate such sequences differently in a parallel context. */ +static void unwind_gusa(CPUSH4State *regs) +{ + /* If the stack pointer is sufficiently negative... */ + if ((regs->gregs[15] & 0xc0000000u) == 0xc0000000u) { + /* Reset the PC to before the gUSA region, as computed from + R0 = region end, SP = -(region size), plus one more insn + that actually sets SP to the region size. */ + regs->pc = regs->gregs[0] + regs->gregs[15] - 2; + + /* Reset the SP to the saved version in R1. */ + regs->gregs[15] = regs->gregs[1]; + } +} + static void setup_sigcontext(struct target_sigcontext *sc, CPUSH4State *regs, unsigned long mask) { @@ -3534,6 +3551,8 @@ static void setup_frame(int sig, struct target_sigaction *ka, abi_ulong frame_addr; int i; + unwind_gusa(regs); + frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame)); trace_user_setup_frame(regs, frame_addr); if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) { @@ -3583,6 +3602,8 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka, abi_ulong frame_addr; int i; + unwind_gusa(regs); + frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame)); trace_user_setup_rt_frame(regs, frame_addr); if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
We translate gUSA regions atomically in a parallel context. But in a serial context a gUSA region may be interrupted. In that case, restart the region as the kernel would. Signed-off-by: Richard Henderson <rth@twiddle.net> --- linux-user/signal.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)