[Qemu-devel] [PATCH 1/1] s390/ipl: fix ipl with -no-reboot

[Qemu-devel] [PATCH 1/1] s390/ipl: fix ipl with -no-reboot

[Christian Borntraeger]

kexec/kdump as well as the bootloader use a subcode of diagnose 308
that is supposed to reset the subsystem but not comprise a full
"reboot". With the latest refactoring this is now broken when
-no-reboot is used. This for example break virt-install from
iso images.

We need to mark these "soft" reboots as ok for rebooting.

Fixes: a30fb811cbe9 (s390x: refactor reset/reipl handling)
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/ipl.c          | 8 +++++++-
 include/sysemu/sysemu.h | 3 +++
 vl.c                    | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 0d67349004..0fab9d2f9d 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -534,8 +534,14 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
              */
             ipl->iplb_valid = s390_gen_initial_iplb(ipl);
         }
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+    } else  if (reset_type == S390_RESET_MODIFIED_CLEAR ||
+                reset_type == S390_RESET_LOAD_NORMAL) {
+        /* ignore -no-reboot */
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET_FORCE);
+    } else {
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     }
-    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
     /* as this is triggered by a CPU, make sure to exit the loop */
     if (tcg_enabled()) {
         cpu_loop_exit(cs);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index e893f72f3b..345369d924 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -44,6 +44,9 @@ typedef enum ShutdownCause {
                                      turns that into a shutdown */
     SHUTDOWN_CAUSE_GUEST_PANIC,   /* Guest panicked, and command line turns
                                      that into a shutdown */
+    SHUTDOWN_CAUSE_GUEST_RESET_FORCE,/* Guest reset that should ignore
+                                        --no-reboot. This is useful for reset
+                                        like actions as s390 kexec/kdump */
     SHUTDOWN_CAUSE__MAX,
 } ShutdownCause;
 
diff --git a/vl.c b/vl.c
index b3426e03d0..722c3b9963 100644
--- a/vl.c
+++ b/vl.c
@@ -1674,7 +1674,7 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
 
 void qemu_system_reset_request(ShutdownCause reason)
 {
-    if (no_reboot) {
+    if (no_reboot && reason != SHUTDOWN_CAUSE_GUEST_RESET_FORCE) {
         shutdown_requested = reason;
     } else {
         reset_requested = reason;
-- 
2.17.0

Re: [Qemu-devel] [PATCH 1/1] s390/ipl: fix ipl with -no-reboot

[David Hildenbrand]

On 21.06.2018 18:25, Christian Borntraeger wrote:
> kexec/kdump as well as the bootloader use a subcode of diagnose 308
> that is supposed to reset the subsystem but not comprise a full
> "reboot". With the latest refactoring this is now broken when
> -no-reboot is used. This for example break virt-install from

breaks

> iso images.
> 
> We need to mark these "soft" reboots as ok for rebooting.
> 
> Fixes: a30fb811cbe9 (s390x: refactor reset/reipl handling)
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/ipl.c          | 8 +++++++-
>  include/sysemu/sysemu.h | 3 +++
>  vl.c                    | 2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d67349004..0fab9d2f9d 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -534,8 +534,14 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type)
>               */
>              ipl->iplb_valid = s390_gen_initial_iplb(ipl);
>          }
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> +    } else  if (reset_type == S390_RESET_MODIFIED_CLEAR ||
> +                reset_type == S390_RESET_LOAD_NORMAL) {
> +        /* ignore -no-reboot */
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET_FORCE);
> +    } else {
> +        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);

If you avoid the "else if", then you only need one instance of
SHUTDOWN_CAUSE_GUEST_RESET


if (reset_type == S390_RESET_MODIFIED_CLEAR ||
    reset_type == S390_RESET_LOAD_NORMAL) {
...
} else {
...
}

Apart from that looks good to me

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

>      }
> -    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>      /* as this is triggered by a CPU, make sure to exit the loop */
>      if (tcg_enabled()) {
>          cpu_loop_exit(cs);
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index e893f72f3b..345369d924 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -44,6 +44,9 @@ typedef enum ShutdownCause {
>                                       turns that into a shutdown */
>      SHUTDOWN_CAUSE_GUEST_PANIC,   /* Guest panicked, and command line turns
>                                       that into a shutdown */
> +    SHUTDOWN_CAUSE_GUEST_RESET_FORCE,/* Guest reset that should ignore
> +                                        --no-reboot. This is useful for reset
> +                                        like actions as s390 kexec/kdump */
>      SHUTDOWN_CAUSE__MAX,
>  } ShutdownCause;
>  
> diff --git a/vl.c b/vl.c
> index b3426e03d0..722c3b9963 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1674,7 +1674,7 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>  
>  void qemu_system_reset_request(ShutdownCause reason)
>  {
> -    if (no_reboot) {
> +    if (no_reboot && reason != SHUTDOWN_CAUSE_GUEST_RESET_FORCE) {
>          shutdown_requested = reason;
>      } else {
>          reset_requested = reason;
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 1/1] s390/ipl: fix ipl with -no-reboot

[Paolo Bonzini]

On 21/06/2018 18:31, David Hildenbrand wrote:
> +    SHUTDOWN_CAUSE_GUEST_RESET_FORCE,/* Guest reset that should ignore
> +                                        --no-reboot. This is useful for reset
> +                                        like actions as s390 kexec/kdump */
>      SHUTDOWN_CAUSE__MAX,
>  } ShutdownCause;
>  
> diff --git a/vl.c b/vl.c
> index b3426e03d0..722c3b9963 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1674,7 +1674,7 @@ void qemu_system_guest_panicked(GuestPanicInformation *info)
>  
>  void qemu_system_reset_request(ShutdownCause reason)
>  {
> -    if (no_reboot) {
> +    if (no_reboot && reason != SHUTDOWN_CAUSE_GUEST_RESET_FORCE) {
>          shutdown_requested = reason;
>      } else {
>          reset_requested = reason;

The only comment is that I'd drop the event too.  It's not possible to
use SHUTDOWN_CAUSE_NONE directly of course, but you can special case it
in qemu_system_reset.

Also I wouldn't be worried of picking an s390-specific name like
SHUTDOWN_CAUSE_GUEST_(RE)?IPL.  At least it explains exactly what it is
for and it makes sense that it doesn't obey -no-reboot and generate the
event.

Paolo