* [PATCH 0/2] linux-user/s390x: Fix single-stepping SVC
@ 2023-05-10 23:02 Ilya Leoshkevich
2023-05-10 23:02 ` [PATCH 1/2] " Ilya Leoshkevich
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2023-05-10 23:02 UTC (permalink / raw)
To: Laurent Vivier, Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x, Ilya Leoshkevich
Hi,
I noticed that single-stepping SVC runs two instructions instead of
one. The reason is that EXCP_SVC masks EXCP_DEBUG.
Patch 1 fixes this problem, patch 2 adds a test.
Btw, there is at least one more problem in that area, namely
single-stepping instructions that cause e.g. SIGILL. Using the
existing signals-s390x test as an example:
(gdb) x/i $pc
=> 0x1001740 <illegal_op>: .long 0x000007fe
(gdb) si
Program received signal SIGILL, Illegal instruction.
(gdb) x/i $pc
=> 0x1001742 <after_illegal_op>: br %r14
# So far so good.
(gdb) si
(gdb) x/i $pc
=> 0x10017b6 <handle_signal+6>: lay %r15,-344(%r15)
# Missed the first signal handler instruction!
I'm not sure what to do about it - the trivial fix to add
gdb_handlesig(cpu, 0) to the end of handle_pending_signal() caused GDB
to hang, and I haven't looked further yet.
Best regards,
Ilya
Ilya Leoshkevich (2):
linux-user/s390x: Fix single-stepping SVC
tests/tcg/s390x: Test single-stepping SVC
linux-user/s390x/cpu_loop.c | 9 ++++
tests/tcg/s390x/Makefile.target | 11 ++++-
tests/tcg/s390x/gdbstub/test-svc.py | 64 +++++++++++++++++++++++++++++
tests/tcg/s390x/hello-s390x-asm.S | 20 +++++++++
4 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/s390x/gdbstub/test-svc.py
create mode 100644 tests/tcg/s390x/hello-s390x-asm.S
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
2023-05-10 23:02 [PATCH 0/2] linux-user/s390x: Fix single-stepping SVC Ilya Leoshkevich
@ 2023-05-10 23:02 ` Ilya Leoshkevich
2023-05-11 10:55 ` Michael Tokarev
2023-05-10 23:02 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
2023-05-31 14:37 ` [PATCH 0/2] linux-user/s390x: Fix " Thomas Huth
2 siblings, 1 reply; 11+ messages in thread
From: Ilya Leoshkevich @ 2023-05-10 23:02 UTC (permalink / raw)
To: Laurent Vivier, Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x, Ilya Leoshkevich
Currently single-stepping SVC executes two instructions. The reason is
that EXCP_DEBUG for the SVC instruction itself is masked by EXCP_SVC.
Fix by re-raising EXCP_DEBUG.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
linux-user/s390x/cpu_loop.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index 285bc60071a..8b7ac2879ef 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -86,6 +86,15 @@ void cpu_loop(CPUS390XState *env)
} else if (ret != -QEMU_ESIGRETURN) {
env->regs[2] = ret;
}
+
+ if (unlikely(cs->singlestep_enabled)) {
+ /*
+ * cpu_tb_exec() did not raise EXCP_DEBUG, because it has seen
+ * that EXCP_SVC was already pending.
+ */
+ cs->exception_index = EXCP_DEBUG;
+ }
+
break;
case EXCP_DEBUG:
--
2.40.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] tests/tcg/s390x: Test single-stepping SVC
2023-05-10 23:02 [PATCH 0/2] linux-user/s390x: Fix single-stepping SVC Ilya Leoshkevich
2023-05-10 23:02 ` [PATCH 1/2] " Ilya Leoshkevich
@ 2023-05-10 23:02 ` Ilya Leoshkevich
2023-05-11 10:51 ` Alex Bennée
2023-07-07 8:37 ` Thomas Huth
2023-05-31 14:37 ` [PATCH 0/2] linux-user/s390x: Fix " Thomas Huth
2 siblings, 2 replies; 11+ messages in thread
From: Ilya Leoshkevich @ 2023-05-10 23:02 UTC (permalink / raw)
To: Laurent Vivier, Richard Henderson, David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x, Ilya Leoshkevich
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/s390x/Makefile.target | 11 ++++-
tests/tcg/s390x/gdbstub/test-svc.py | 64 +++++++++++++++++++++++++++++
tests/tcg/s390x/hello-s390x-asm.S | 20 +++++++++
3 files changed, 94 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/s390x/gdbstub/test-svc.py
create mode 100644 tests/tcg/s390x/hello-s390x-asm.S
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 0031868b136..bd435e4dd63 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -74,7 +74,16 @@ run-gdbstub-signals-s390x: signals-s390x
--bin $< --test $(S390X_SRC)/gdbstub/test-signals-s390x.py, \
mixing signals and debugging)
-EXTRA_RUNS += run-gdbstub-signals-s390x
+hello-s390x-asm: CFLAGS+=-nostdlib
+
+run-gdbstub-svc: hello-s390x-asm
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(S390X_SRC)/gdbstub/test-svc.py, \
+ single-stepping svc)
+
+EXTRA_RUNS += run-gdbstub-signals-s390x run-gdbstub-svc
endif
# MVX versions of sha512
diff --git a/tests/tcg/s390x/gdbstub/test-svc.py b/tests/tcg/s390x/gdbstub/test-svc.py
new file mode 100644
index 00000000000..7851ca72846
--- /dev/null
+++ b/tests/tcg/s390x/gdbstub/test-svc.py
@@ -0,0 +1,64 @@
+"""Test single-stepping SVC.
+
+This runs as a sourced script (via -x, via run-test.py)."""
+from __future__ import print_function
+import gdb
+import sys
+
+
+n_failures = 0
+
+
+def report(cond, msg):
+ """Report success/fail of a test"""
+ if cond:
+ print("PASS: {}".format(msg))
+ else:
+ print("FAIL: {}".format(msg))
+ global n_failures
+ n_failures += 1
+
+
+def run_test():
+ """Run through the tests one by one"""
+ report("lghi\t" in gdb.execute("x/i $pc", False, True), "insn #1")
+ gdb.execute("si")
+ report("larl\t" in gdb.execute("x/i $pc", False, True), "insn #2")
+ gdb.execute("si")
+ report("lghi\t" in gdb.execute("x/i $pc", False, True), "insn #3")
+ gdb.execute("si")
+ report("svc\t" in gdb.execute("x/i $pc", False, True), "insn #4")
+ gdb.execute("si")
+ report("xgr\t" in gdb.execute("x/i $pc", False, True), "insn #5")
+ gdb.execute("si")
+ report("svc\t" in gdb.execute("x/i $pc", False, True), "insn #6")
+ gdb.execute("si")
+
+
+def main():
+ """Prepare the environment and run through the tests"""
+ try:
+ inferior = gdb.selected_inferior()
+ print("ATTACHED: {}".format(inferior.architecture().name()))
+ except (gdb.error, AttributeError):
+ print("SKIPPING (not connected)")
+ exit(0)
+
+ if gdb.parse_and_eval('$pc') == 0:
+ print("SKIP: PC not set")
+ exit(0)
+
+ try:
+ # These are not very useful in scripts
+ gdb.execute("set pagination off")
+ gdb.execute("set confirm off")
+
+ # Run the actual tests
+ run_test()
+ except gdb.error:
+ report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
+ print("All tests complete: %d failures" % n_failures)
+ exit(n_failures)
+
+
+main()
diff --git a/tests/tcg/s390x/hello-s390x-asm.S b/tests/tcg/s390x/hello-s390x-asm.S
new file mode 100644
index 00000000000..2e9faa16047
--- /dev/null
+++ b/tests/tcg/s390x/hello-s390x-asm.S
@@ -0,0 +1,20 @@
+/*
+ * Hello, World! in assembly.
+ */
+
+.globl _start
+_start:
+
+/* puts("Hello, World!"); */
+lghi %r2,1
+larl %r3,foo
+lghi %r4,foo_end-foo
+svc 4
+
+/* exit(0); */
+xgr %r2,%r2
+svc 1
+
+.align 2
+foo: .asciz "Hello, World!\n"
+foo_end:
--
2.40.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] tests/tcg/s390x: Test single-stepping SVC
2023-05-10 23:02 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
@ 2023-05-11 10:51 ` Alex Bennée
2023-07-07 8:37 ` Thomas Huth
1 sibling, 0 replies; 11+ messages in thread
From: Alex Bennée @ 2023-05-11 10:51 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Laurent Vivier, Richard Henderson, David Hildenbrand,
Thomas Huth, qemu-s390x, qemu-devel
Ilya Leoshkevich <iii@linux.ibm.com> writes:
> Add a small test to prevent regressions.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
2023-05-10 23:02 ` [PATCH 1/2] " Ilya Leoshkevich
@ 2023-05-11 10:55 ` Michael Tokarev
2023-05-11 11:20 ` Ilya Leoshkevich
0 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2023-05-11 10:55 UTC (permalink / raw)
To: Ilya Leoshkevich, Laurent Vivier, Richard Henderson,
David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x
11.05.2023 02:02, Ilya Leoshkevich wrote:
> Currently single-stepping SVC executes two instructions. The reason is
> that EXCP_DEBUG for the SVC instruction itself is masked by EXCP_SVC.
> Fix by re-raising EXCP_DEBUG.
Is it a -stable material?
/mjt
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
2023-05-11 10:55 ` Michael Tokarev
@ 2023-05-11 11:20 ` Ilya Leoshkevich
2023-05-11 12:32 ` Michael Tokarev
0 siblings, 1 reply; 11+ messages in thread
From: Ilya Leoshkevich @ 2023-05-11 11:20 UTC (permalink / raw)
To: Michael Tokarev, Laurent Vivier, Richard Henderson,
David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x
On Thu, 2023-05-11 at 13:55 +0300, Michael Tokarev wrote:
> 11.05.2023 02:02, Ilya Leoshkevich wrote:
> > Currently single-stepping SVC executes two instructions. The reason
> > is
> > that EXCP_DEBUG for the SVC instruction itself is masked by
> > EXCP_SVC.
> > Fix by re-raising EXCP_DEBUG.
>
> Is it a -stable material?
>
> /mjt
While I would personally love to see this in -stable, I don't think it
fits the official criteria - it's not a security fix and it's not a
regression.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
2023-05-11 11:20 ` Ilya Leoshkevich
@ 2023-05-11 12:32 ` Michael Tokarev
2023-05-11 13:45 ` Ilya Leoshkevich
0 siblings, 1 reply; 11+ messages in thread
From: Michael Tokarev @ 2023-05-11 12:32 UTC (permalink / raw)
To: Ilya Leoshkevich, Laurent Vivier, Richard Henderson,
David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x
11.05.2023 14:20, Ilya Leoshkevich wrote:
> On Thu, 2023-05-11 at 13:55 +0300, Michael Tokarev wrote:
>> Is it a -stable material?
> While I would personally love to see this in -stable, I don't think it
> fits the official criteria - it's not a security fix and it's not a
> regression.
I'm not sure I follow. It's definitely okay to include a bug fix into
-stable, this has been done countless times in the past..
/mjt
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
2023-05-11 12:32 ` Michael Tokarev
@ 2023-05-11 13:45 ` Ilya Leoshkevich
2023-05-11 13:50 ` Michael Tokarev
0 siblings, 1 reply; 11+ messages in thread
From: Ilya Leoshkevich @ 2023-05-11 13:45 UTC (permalink / raw)
To: Michael Tokarev, Laurent Vivier, Richard Henderson,
David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x
On Thu, 2023-05-11 at 15:32 +0300, Michael Tokarev wrote:
> 11.05.2023 14:20, Ilya Leoshkevich wrote:
> > On Thu, 2023-05-11 at 13:55 +0300, Michael Tokarev wrote:
>
> > > Is it a -stable material?
>
> > While I would personally love to see this in -stable, I don't think
> > it
> > fits the official criteria - it's not a security fix and it's not a
> > regression.
>
> I'm not sure I follow. It's definitely okay to include a bug fix into
> -stable, this has been done countless times in the past..
>
> /mjt
Okay, then let's include it into -stable.
It's just that I'm not too familiar with the QEMU -stable process, so
I read [1], and it sounded quite strict.
[1] https://www.qemu.org/docs/master/devel/stable-process.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
2023-05-11 13:45 ` Ilya Leoshkevich
@ 2023-05-11 13:50 ` Michael Tokarev
0 siblings, 0 replies; 11+ messages in thread
From: Michael Tokarev @ 2023-05-11 13:50 UTC (permalink / raw)
To: Ilya Leoshkevich, Laurent Vivier, Richard Henderson,
David Hildenbrand, Thomas Huth
Cc: qemu-devel, qemu-s390x
11.05.2023 16:45, Ilya Leoshkevich wrote:
>> 11.05.2023 14:20, Ilya Leoshkevich wrote:
>>> While I would personally love to see this in -stable, I don't think it
>>> fits the official criteria - it's not a security fix and it's not a
>>> regression.
> Okay, then let's include it into -stable.
>
> It's just that I'm not too familiar with the QEMU -stable process, so
> I read [1], and it sounded quite strict.
>
> [1] https://www.qemu.org/docs/master/devel/stable-process.html
The text there reads:
If you think the patch would be important for users of
the current release (or for a distribution picking fixes),
it is usually a good candidate for stable.
:)
Please Cc: qemu-stable@nongnu.org the next time you think
something is good to have there.
/mjt
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] linux-user/s390x: Fix single-stepping SVC
2023-05-10 23:02 [PATCH 0/2] linux-user/s390x: Fix single-stepping SVC Ilya Leoshkevich
2023-05-10 23:02 ` [PATCH 1/2] " Ilya Leoshkevich
2023-05-10 23:02 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
@ 2023-05-31 14:37 ` Thomas Huth
2 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2023-05-31 14:37 UTC (permalink / raw)
To: Ilya Leoshkevich, Laurent Vivier, Richard Henderson, David Hildenbrand
Cc: qemu-devel, qemu-s390x
On 11/05/2023 01.02, Ilya Leoshkevich wrote:
> Hi,
>
> I noticed that single-stepping SVC runs two instructions instead of
> one. The reason is that EXCP_SVC masks EXCP_DEBUG.
> Patch 1 fixes this problem, patch 2 adds a test.
>
> Btw, there is at least one more problem in that area, namely
> single-stepping instructions that cause e.g. SIGILL. Using the
> existing signals-s390x test as an example:
>
> (gdb) x/i $pc
> => 0x1001740 <illegal_op>: .long 0x000007fe
>
> (gdb) si
> Program received signal SIGILL, Illegal instruction.
> (gdb) x/i $pc
> => 0x1001742 <after_illegal_op>: br %r14
> # So far so good.
>
> (gdb) si
> (gdb) x/i $pc
> => 0x10017b6 <handle_signal+6>: lay %r15,-344(%r15)
> # Missed the first signal handler instruction!
>
> I'm not sure what to do about it - the trivial fix to add
> gdb_handlesig(cpu, 0) to the end of handle_pending_signal() caused GDB
> to hang, and I haven't looked further yet.
>
> Best regards,
> Ilya
>
> Ilya Leoshkevich (2):
> linux-user/s390x: Fix single-stepping SVC
> tests/tcg/s390x: Test single-stepping SVC
If there are no disagreements, I can take this through my s390x tree. Queued
it for my next pull request now.
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] tests/tcg/s390x: Test single-stepping SVC
2023-05-10 23:02 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
2023-05-11 10:51 ` Alex Bennée
@ 2023-07-07 8:37 ` Thomas Huth
1 sibling, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2023-07-07 8:37 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: qemu-devel, qemu-s390x, Laurent Vivier, Richard Henderson,
David Hildenbrand, Michael Tokarev, Andreas Krebbel
On 11/05/2023 01.02, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
...
> diff --git a/tests/tcg/s390x/hello-s390x-asm.S b/tests/tcg/s390x/hello-s390x-asm.S
> new file mode 100644
> index 00000000000..2e9faa16047
> --- /dev/null
> +++ b/tests/tcg/s390x/hello-s390x-asm.S
> @@ -0,0 +1,20 @@
> +/*
> + * Hello, World! in assembly.
> + */
> +
> +.globl _start
> +_start:
> +
> +/* puts("Hello, World!"); */
> +lghi %r2,1
> +larl %r3,foo
> +lghi %r4,foo_end-foo
Hi Ilya!
While testing your other currently pending s390x TCG patches, I noticed that
this is failing with Clang (v16.0.1):
tests/tcg/s390x/hello-s390x-asm.S:11:10: error: invalid operand for instruction
lghi %r4,foo_end-foo
^
make[1]: *** [Makefile:121: hello-s390x-asm] Error 1
Any ideas how to fix this?
Thomas
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-07-07 8:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 23:02 [PATCH 0/2] linux-user/s390x: Fix single-stepping SVC Ilya Leoshkevich
2023-05-10 23:02 ` [PATCH 1/2] " Ilya Leoshkevich
2023-05-11 10:55 ` Michael Tokarev
2023-05-11 11:20 ` Ilya Leoshkevich
2023-05-11 12:32 ` Michael Tokarev
2023-05-11 13:45 ` Ilya Leoshkevich
2023-05-11 13:50 ` Michael Tokarev
2023-05-10 23:02 ` [PATCH 2/2] tests/tcg/s390x: Test " Ilya Leoshkevich
2023-05-11 10:51 ` Alex Bennée
2023-07-07 8:37 ` Thomas Huth
2023-05-31 14:37 ` [PATCH 0/2] linux-user/s390x: Fix " Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.