On 21.06.2018 17:49, Christian Borntraeger wrote:
>
>
> On 04/24/2018 12:18 PM, David Hildenbrand wrote:
>> Calling pause_all_vcpus()/resume_all_vcpus() from a VCPU thread might
>> not be the best idea. As pause_all_vcpus() temporarily drops the qemu
>> mutex, two parallel calls to pause_all_vcpus() can be active at a time,
>> resulting in a deadlock. (either by two VCPUs or by the main thread and a
>> VCPU)
>>
>> Let's handle it via the main loop instead, as suggested by Paolo. If we
>> would have two parallel reset requests by two different VCPUs at the
>> same time, the last one would win.
>>
>> We use the existing ipl device to handle it. The nice side effect is
>> that we can get rid of reipl_requested.
>>
>> This change implies that all reset handling now goes via the common
>> path, so "no-reboot" handling is now active for all kinds of reboots.
>
> Ok, this breaks the s390 IPL process when -no-reboot is specified.
> The bios does a diagnose 308 subcode 1 to jump to the final image while
> at the same time resetting all devices. This is now blocked with -no-reboot
> (although it is actually the boot)
Indeed, one corner case I missed, thanks for looking into it.
I wonder if we could turn that into a allow "one
S390_RESET_MODIFIED_CLEAR ignoring -no-reboot" if booting from bios
using a simple flag.
Then s/SHUTDOWN_CAUSE_GUEST_RESET/SHUTDOWN_CAUSE_BIOS_RESET/
>
>
> I have noticed that with virt-install on iso images since virt-install
> specifies -no-reboot.
>
> Something like this seems to help but it is not a nice solution.
>
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d67349004..7b32698eaa 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..e9b11fd6cb 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
> + useful for the subsystem resets on s390
> done
> + for kdump and kexec */
I don't think it is that ugly.
> SHUTDOWN_CAUSE__MAX,
> } ShutdownCause;
>
> diff --git a/vl.c b/vl.c
> index b3426e03d0..18f379e833 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1674,7 +1674,9 @@ void qemu_system_guest_panicked(GuestPanicInformation
> *info)
>
> void qemu_system_reset_request(ShutdownCause reason)
> {
> - if (no_reboot) {
> + if (reason == SHUTDOWN_CAUSE_GUEST_RESET_FORCE) {
> + reset_requested = SHUTDOWN_CAUSE_GUEST_RESET;
> + } else if (no_reboot) {
> shutdown_requested = reason;
> } else {
> reset_requested = reason;
>
--
Thanks,
David / dhildenb
