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)) {
--
2.9.4
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 -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaubitz@debian.org `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
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 -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaubitz@debian.org `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
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 -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaubitz@debian.org `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
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 -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaubitz@debian.org `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
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) {
kernel also checks PC < gUSA region end point,
try this:
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 1e716a9..4e1e4f0 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -3477,7 +3477,8 @@ static abi_ulong get_sigframe(struct
target_sigaction *ka,
static void unwind_gusa(CPUSH4State *regs)
{
/* If the stack pointer is sufficiently negative... */
- if ((regs->gregs[15] & 0xc0000000u) == 0xc0000000u) {
+ if ((regs->gregs[15] & 0xc0000000u) == 0xc0000000u &&
+ regs->pc < regs->gregs[0]) {
/* 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. */
Laurent
© 2016 - 2025 Red Hat, Inc.