[PATCH v2 0/2] accel/tcg: Test unaligned stores to s390x low-address-protected lowcore

[PATCH v2 0/2] accel/tcg: Test unaligned stores to s390x low-address-protected lowcore

[Ilya Leoshkevich]

Hi,

This is a follow-up series for [1].

The fix has been committed.

I asked Christian what might be a good alternative for the
mmio-debug-exit device for testing, and he suggested to look into
shutdown/panic actions.

Patch 1 adds a new panic action.
Patch 2 tests unaligned stores to s390x low-address-protected lowcore;
it performs a shutdown on success and panic on failure.

Best regards,
Ilya

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-07/msg01876.html

Ilya Leoshkevich (2):
  qapi: Add exit-failure PanicAction
  tests/tcg/s390x: Test unaligned accesses to lowcore

 include/sysemu/sysemu.h                 |  2 +-
 qapi/run-state.json                     |  4 +++-
 qemu-options.hx                         |  2 +-
 softmmu/main.c                          |  6 ++++--
 softmmu/runstate.c                      | 17 +++++++++++++----
 tests/tcg/s390x/Makefile.softmmu-target |  9 +++++++++
 tests/tcg/s390x/unaligned-lowcore.S     | 19 +++++++++++++++++++
 7 files changed, 50 insertions(+), 9 deletions(-)
 create mode 100644 tests/tcg/s390x/Makefile.softmmu-target
 create mode 100644 tests/tcg/s390x/unaligned-lowcore.S

-- 
2.35.3

[PATCH v2 1/2] qapi: Add exit-failure PanicAction

[Ilya Leoshkevich]

Currently QEMU exits with code 0 on both panic an shutdown. For tests
it is useful to return 1 on panic, so that it counts as a test
failure.

Introduce a new exit-failure PanicAction that makes main() return
EXIT_FAILURE. Tests can use -action panic=exit-failure option to
activate this behavior.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 include/sysemu/sysemu.h |  2 +-
 qapi/run-state.json     |  4 +++-
 qemu-options.hx         |  2 +-
 softmmu/main.c          |  6 ++++--
 softmmu/runstate.c      | 17 +++++++++++++----
 5 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 812f66a31a..31aa45160b 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -103,7 +103,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
 bool defaults_enabled(void);
 
 void qemu_init(int argc, char **argv, char **envp);
-void qemu_main_loop(void);
+int qemu_main_loop(void);
 void qemu_cleanup(void);
 
 extern QemuOptsList qemu_legacy_drive_opts;
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 6e2162d7b3..d42c370c4f 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -364,10 +364,12 @@
 #
 # @shutdown: Shutdown the VM and exit, according to the shutdown action
 #
+# @exit-failure: Shutdown the VM and exit with nonzero status
+#
 # Since: 6.0
 ##
 { 'enum': 'PanicAction',
-  'data': [ 'pause', 'shutdown', 'none' ] }
+  'data': [ 'pause', 'shutdown', 'exit-failure', 'none' ] }
 
 ##
 # @watchdog-set-action:
diff --git a/qemu-options.hx b/qemu-options.hx
index 79e00916a1..8e17c5064a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4239,7 +4239,7 @@ DEF("action", HAS_ARG, QEMU_OPTION_action,
     "                   action when guest reboots [default=reset]\n"
     "-action shutdown=poweroff|pause\n"
     "                   action when guest shuts down [default=poweroff]\n"
-    "-action panic=pause|shutdown|none\n"
+    "-action panic=pause|shutdown|exit-failure|none\n"
     "                   action when guest panics [default=shutdown]\n"
     "-action watchdog=reset|shutdown|poweroff|inject-nmi|pause|debug|none\n"
     "                   action when watchdog fires [default=reset]\n",
diff --git a/softmmu/main.c b/softmmu/main.c
index c00432ff09..1b675a8c03 100644
--- a/softmmu/main.c
+++ b/softmmu/main.c
@@ -32,11 +32,13 @@
 
 int qemu_main(int argc, char **argv, char **envp)
 {
+    int status;
+
     qemu_init(argc, argv, envp);
-    qemu_main_loop();
+    status = qemu_main_loop();
     qemu_cleanup();
 
-    return 0;
+    return status;
 }
 
 #ifndef CONFIG_COCOA
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index 168e1b78a0..1e68680b9d 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -482,7 +482,8 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
         qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE,
                                         !!info, info);
         vm_stop(RUN_STATE_GUEST_PANICKED);
-    } else if (panic_action == PANIC_ACTION_SHUTDOWN) {
+    } else if (panic_action == PANIC_ACTION_SHUTDOWN ||
+               panic_action == PANIC_ACTION_EXIT_FAILURE) {
         qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
                                        !!info, info);
         vm_stop(RUN_STATE_GUEST_PANICKED);
@@ -662,7 +663,7 @@ void qemu_system_debug_request(void)
     qemu_notify_event();
 }
 
-static bool main_loop_should_exit(void)
+static bool main_loop_should_exit(int *status)
 {
     RunState r;
     ShutdownCause request;
@@ -680,6 +681,10 @@ static bool main_loop_should_exit(void)
         if (shutdown_action == SHUTDOWN_ACTION_PAUSE) {
             vm_stop(RUN_STATE_SHUTDOWN);
         } else {
+            if (request == SHUTDOWN_CAUSE_GUEST_PANIC &&
+                panic_action == PANIC_ACTION_EXIT_FAILURE) {
+                *status = EXIT_FAILURE;
+            }
             return true;
         }
     }
@@ -715,12 +720,14 @@ static bool main_loop_should_exit(void)
     return false;
 }
 
-void qemu_main_loop(void)
+int qemu_main_loop(void)
 {
+    int status = EXIT_SUCCESS;
 #ifdef CONFIG_PROFILER
     int64_t ti;
 #endif
-    while (!main_loop_should_exit()) {
+
+    while (!main_loop_should_exit(&status)) {
 #ifdef CONFIG_PROFILER
         ti = profile_getclock();
 #endif
@@ -729,6 +736,8 @@ void qemu_main_loop(void)
         dev_time += profile_getclock() - ti;
 #endif
     }
+
+    return status;
 }
 
 void qemu_add_exit_notifier(Notifier *notify)
-- 
2.35.3

[PATCH v2 2/2] tests/tcg/s390x: Test unaligned accesses to lowcore

[Ilya Leoshkevich]

Add a small test to avoid regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.softmmu-target |  9 +++++++++
 tests/tcg/s390x/unaligned-lowcore.S     | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 tests/tcg/s390x/Makefile.softmmu-target
 create mode 100644 tests/tcg/s390x/unaligned-lowcore.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target
new file mode 100644
index 0000000000..a34fa68473
--- /dev/null
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -0,0 +1,9 @@
+S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
+VPATH+=$(S390X_SRC)
+QEMU_OPTS=-action panic=exit-failure -kernel
+
+%: %.S
+	$(CC) -march=z13 -m64 -nostartfiles -static -Wl,-Ttext=0 \
+		-Wl,--build-id=none $< -o $@
+
+TESTS += unaligned-lowcore
diff --git a/tests/tcg/s390x/unaligned-lowcore.S b/tests/tcg/s390x/unaligned-lowcore.S
new file mode 100644
index 0000000000..246b517f11
--- /dev/null
+++ b/tests/tcg/s390x/unaligned-lowcore.S
@@ -0,0 +1,19 @@
+    .org 0x1D0                         /* program new PSW */
+    .quad 0x2000000000000, 0           /* disabled wait */
+    .org 0x200                         /* lowcore padding */
+
+    .globl _start
+_start:
+    lctlg %c0,%c0,_c0
+    vst %v0,_unaligned
+    lpswe quiesce_psw
+
+    .align 8
+quiesce_psw:
+    .quad 0x2000000000000,0xfff        /* see is_special_wait_psw() */
+_c0:
+    .quad 0x10060000                   /* lowcore protection, AFP, VX */
+
+    .byte 0
+_unaligned:
+    .octa 0
-- 
2.35.3

Re: [PATCH v2 1/2] qapi: Add exit-failure PanicAction

[Richard Henderson]

On 7/23/22 05:06, Ilya Leoshkevich wrote:
> Currently QEMU exits with code 0 on both panic an shutdown. For tests
> it is useful to return 1 on panic, so that it counts as a test
> failure.
> 
> Introduce a new exit-failure PanicAction that makes main() return
> EXIT_FAILURE. Tests can use -action panic=exit-failure option to
> activate this behavior.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

I like it.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> ---
>   include/sysemu/sysemu.h |  2 +-
>   qapi/run-state.json     |  4 +++-
>   qemu-options.hx         |  2 +-
>   softmmu/main.c          |  6 ++++--
>   softmmu/runstate.c      | 17 +++++++++++++----
>   5 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 812f66a31a..31aa45160b 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -103,7 +103,7 @@ void qemu_boot_set(const char *boot_order, Error **errp);
>   bool defaults_enabled(void);
>   
>   void qemu_init(int argc, char **argv, char **envp);
> -void qemu_main_loop(void);
> +int qemu_main_loop(void);
>   void qemu_cleanup(void);
>   
>   extern QemuOptsList qemu_legacy_drive_opts;
> diff --git a/qapi/run-state.json b/qapi/run-state.json
> index 6e2162d7b3..d42c370c4f 100644
> --- a/qapi/run-state.json
> +++ b/qapi/run-state.json
> @@ -364,10 +364,12 @@
>   #
>   # @shutdown: Shutdown the VM and exit, according to the shutdown action
>   #
> +# @exit-failure: Shutdown the VM and exit with nonzero status
> +#
>   # Since: 6.0
>   ##
>   { 'enum': 'PanicAction',
> -  'data': [ 'pause', 'shutdown', 'none' ] }
> +  'data': [ 'pause', 'shutdown', 'exit-failure', 'none' ] }
>   
>   ##
>   # @watchdog-set-action:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 79e00916a1..8e17c5064a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4239,7 +4239,7 @@ DEF("action", HAS_ARG, QEMU_OPTION_action,
>       "                   action when guest reboots [default=reset]\n"
>       "-action shutdown=poweroff|pause\n"
>       "                   action when guest shuts down [default=poweroff]\n"
> -    "-action panic=pause|shutdown|none\n"
> +    "-action panic=pause|shutdown|exit-failure|none\n"
>       "                   action when guest panics [default=shutdown]\n"
>       "-action watchdog=reset|shutdown|poweroff|inject-nmi|pause|debug|none\n"
>       "                   action when watchdog fires [default=reset]\n",
> diff --git a/softmmu/main.c b/softmmu/main.c
> index c00432ff09..1b675a8c03 100644
> --- a/softmmu/main.c
> +++ b/softmmu/main.c
> @@ -32,11 +32,13 @@
>   
>   int qemu_main(int argc, char **argv, char **envp)
>   {
> +    int status;
> +
>       qemu_init(argc, argv, envp);
> -    qemu_main_loop();
> +    status = qemu_main_loop();
>       qemu_cleanup();
>   
> -    return 0;
> +    return status;
>   }
>   
>   #ifndef CONFIG_COCOA
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index 168e1b78a0..1e68680b9d 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -482,7 +482,8 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>           qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_PAUSE,
>                                           !!info, info);
>           vm_stop(RUN_STATE_GUEST_PANICKED);
> -    } else if (panic_action == PANIC_ACTION_SHUTDOWN) {
> +    } else if (panic_action == PANIC_ACTION_SHUTDOWN ||
> +               panic_action == PANIC_ACTION_EXIT_FAILURE) {
>           qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
>                                          !!info, info);
>           vm_stop(RUN_STATE_GUEST_PANICKED);
> @@ -662,7 +663,7 @@ void qemu_system_debug_request(void)
>       qemu_notify_event();
>   }
>   
> -static bool main_loop_should_exit(void)
> +static bool main_loop_should_exit(int *status)
>   {
>       RunState r;
>       ShutdownCause request;
> @@ -680,6 +681,10 @@ static bool main_loop_should_exit(void)
>           if (shutdown_action == SHUTDOWN_ACTION_PAUSE) {
>               vm_stop(RUN_STATE_SHUTDOWN);
>           } else {
> +            if (request == SHUTDOWN_CAUSE_GUEST_PANIC &&
> +                panic_action == PANIC_ACTION_EXIT_FAILURE) {
> +                *status = EXIT_FAILURE;
> +            }
>               return true;
>           }
>       }
> @@ -715,12 +720,14 @@ static bool main_loop_should_exit(void)
>       return false;
>   }
>   
> -void qemu_main_loop(void)
> +int qemu_main_loop(void)
>   {
> +    int status = EXIT_SUCCESS;
>   #ifdef CONFIG_PROFILER
>       int64_t ti;
>   #endif
> -    while (!main_loop_should_exit()) {
> +
> +    while (!main_loop_should_exit(&status)) {
>   #ifdef CONFIG_PROFILER
>           ti = profile_getclock();
>   #endif
> @@ -729,6 +736,8 @@ void qemu_main_loop(void)
>           dev_time += profile_getclock() - ti;
>   #endif
>       }
> +
> +    return status;
>   }
>   
>   void qemu_add_exit_notifier(Notifier *notify)

Re: [PATCH v2 2/2] tests/tcg/s390x: Test unaligned accesses to lowcore

[Richard Henderson]

On 7/23/22 05:06, Ilya Leoshkevich wrote:
> Add a small test to avoid regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~


> ---
>   tests/tcg/s390x/Makefile.softmmu-target |  9 +++++++++
>   tests/tcg/s390x/unaligned-lowcore.S     | 19 +++++++++++++++++++
>   2 files changed, 28 insertions(+)
>   create mode 100644 tests/tcg/s390x/Makefile.softmmu-target
>   create mode 100644 tests/tcg/s390x/unaligned-lowcore.S
> 
> diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target
> new file mode 100644
> index 0000000000..a34fa68473
> --- /dev/null
> +++ b/tests/tcg/s390x/Makefile.softmmu-target
> @@ -0,0 +1,9 @@
> +S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
> +VPATH+=$(S390X_SRC)
> +QEMU_OPTS=-action panic=exit-failure -kernel
> +
> +%: %.S
> +	$(CC) -march=z13 -m64 -nostartfiles -static -Wl,-Ttext=0 \
> +		-Wl,--build-id=none $< -o $@
> +
> +TESTS += unaligned-lowcore
> diff --git a/tests/tcg/s390x/unaligned-lowcore.S b/tests/tcg/s390x/unaligned-lowcore.S
> new file mode 100644
> index 0000000000..246b517f11
> --- /dev/null
> +++ b/tests/tcg/s390x/unaligned-lowcore.S
> @@ -0,0 +1,19 @@
> +    .org 0x1D0                         /* program new PSW */
> +    .quad 0x2000000000000, 0           /* disabled wait */
> +    .org 0x200                         /* lowcore padding */
> +
> +    .globl _start
> +_start:
> +    lctlg %c0,%c0,_c0
> +    vst %v0,_unaligned
> +    lpswe quiesce_psw
> +
> +    .align 8
> +quiesce_psw:
> +    .quad 0x2000000000000,0xfff        /* see is_special_wait_psw() */
> +_c0:
> +    .quad 0x10060000                   /* lowcore protection, AFP, VX */
> +
> +    .byte 0
> +_unaligned:
> +    .octa 0

Re: [PATCH v2 1/2] qapi: Add exit-failure PanicAction

[David Hildenbrand]

On 23.07.22 01:36, Ilya Leoshkevich wrote:
> Currently QEMU exits with code 0 on both panic an shutdown. For tests
> it is useful to return 1 on panic, so that it counts as a test
> failure.
> 
> Introduce a new exit-failure PanicAction that makes main() return
> EXIT_FAILURE. Tests can use -action panic=exit-failure option to
> activate this behavior.

Roughly what I proposed, nice.

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH v2 0/2] accel/tcg: Test unaligned stores to s390x low-address-protected lowcore

[Alex Bennée]

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> Hi,
>
> This is a follow-up series for [1].
>
> The fix has been committed.
>
> I asked Christian what might be a good alternative for the
> mmio-debug-exit device for testing, and he suggested to look into
> shutdown/panic actions.
>
> Patch 1 adds a new panic action.
> Patch 2 tests unaligned stores to s390x low-address-protected lowcore;
> it performs a shutdown on success and panic on failure.
>
> Best regards,
> Ilya

Queued to testing/next, thanks.

-- 
Alex Bennée

Re: [PATCH v2 1/2] qapi: Add exit-failure PanicAction

[Eric Blake]

On Sat, Jul 23, 2022 at 01:36:13AM +0200, Ilya Leoshkevich wrote:
> Currently QEMU exits with code 0 on both panic an shutdown. For tests
> it is useful to return 1 on panic, so that it counts as a test
> failure.
> 
> Introduce a new exit-failure PanicAction that makes main() return
> EXIT_FAILURE. Tests can use -action panic=exit-failure option to
> activate this behavior.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---

> +++ b/qapi/run-state.json
> @@ -364,10 +364,12 @@
>  #
>  # @shutdown: Shutdown the VM and exit, according to the shutdown action
>  #
> +# @exit-failure: Shutdown the VM and exit with nonzero status

Missing a '(since 7.1)' tag.  Otherwise a nice addition.

> +#
>  # Since: 6.0
>  ##
>  { 'enum': 'PanicAction',
> -  'data': [ 'pause', 'shutdown', 'none' ] }
> +  'data': [ 'pause', 'shutdown', 'exit-failure', 'none' ] }

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org