Re: [Qemu-devel] s390: Fix cpu shutdown for KVM

[Alexander Graf <agraf@suse.de>]

On 10/04/2011 01:28 PM, Christian Borntraeger wrote:
> qemu/kvm on s390 currently hangs on panic (doesnt exit on disabled
> wait) and also shuts down on cpu hot-unplug (SIGP stop).
> This patch tries to fix these simple cases.
> On s390 a shutdown is the state of all CPUs being either stopped
> or disabled (for interrupts) waiting. We have to track this number
> to call the shutdown sequence accordingly. This patch implements
> the counting and shutdown handling for the kvm path in qemu.
>
> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
> ---
>   hw/s390-virtio.c   |    4 ++++
>   target-s390x/kvm.c |   29 +++++++++++++++++++++++++----
>   2 files changed, 29 insertions(+), 4 deletions(-)
>
> Index: b/hw/s390-virtio.c
> ===================================================================
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -130,6 +130,9 @@ int s390_virtio_hypercall(CPUState *env,
>       return r;
>   }
>
> +/* defined in target-s390x/kvm.c */
> +extern int s390_running_cpus;

This breaks non-kvm builds. Please put it into s390-virtio.c and export 
functions that get called from target-s390x/kvm.c which add/remove 
running CPUs from that counter.


Alex

> +
>   /* PC hardware initialisation */
>   static void s390_init(ram_addr_t my_ram_size,
>                         const char *boot_device,
> @@ -189,6 +192,7 @@ static void s390_init(ram_addr_t my_ram_
>
>       env->halted = 0;
>       env->exception_index = 0;
> +    s390_running_cpus = 1;
>
>       if (kernel_filename) {
>           kernel_size = load_image(kernel_filename, qemu_get_ram_ptr(0));
> Index: b/target-s390x/kvm.c
> ===================================================================
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -63,6 +63,14 @@
>   #define SCLP_CMDW_READ_SCP_INFO         0x00020001
>   #define SCLP_CMDW_READ_SCP_INFO_FORCED  0x00120001
>
> +/*
> + * The number of running CPUs. On s390 a shutdown is the state of all CPUs
> + * being either stopped or disabled (for interrupts) waiting. We have to
> + * track this number to call the shutdown sequence accordingly. This
> + * number is modified either on startup or while holding the big qemu lock.
> + */
> +int s390_running_cpus;
> +
>   const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>       KVM_CAP_LAST_INFO
>   };
> @@ -185,6 +193,12 @@ void kvm_s390_interrupt_internal(CPUStat
>           return;
>       }
>
> +    /*
> +     * We can only deliver interrupts to (interrupt) enabled CPUs.
> +     * We dont modify s390_running_cpus here, since CPUs in enabled wait
> +     * will wait inside the kernel (no exit). Therefore, the targeted
> +     * CPUs was neither disabled waiting or stopped for qemu.
> +     */
>       env->halted = 0;
>       env->exception_index = -1;
>       qemu_cpu_kick(env);
> @@ -301,6 +315,7 @@ static int s390_cpu_restart(CPUState *en
>       kvm_s390_interrupt(env, KVM_S390_RESTART, 0);
>       env->halted = 0;
>       env->exception_index = -1;
> +    s390_running_cpus++;
>       qemu_cpu_kick(env);
>       dprintf("DONE: SIGP cpu restart: %p\n", env);
>       return 0;
> @@ -425,16 +440,24 @@ static int handle_intercept(CPUState *en
>               r = handle_instruction(env, run);
>               break;
>           case ICPT_WAITPSW:
> -            /* XXX What to do on system shutdown? */
> +            if (--s390_running_cpus == 0) {
> +                qemu_system_shutdown_request();
> +            }
>               env->halted = 1;
>               env->exception_index = EXCP_HLT;
> +            r = EXCP_HALTED;
>               break;
>           case ICPT_SOFT_INTERCEPT:
>               fprintf(stderr, "KVM unimplemented icpt SOFT\n");
>               exit(1);
>               break;
>           case ICPT_CPU_STOP:
> -            qemu_system_shutdown_request();
> +            if (--s390_running_cpus == 0) {
> +                qemu_system_shutdown_request();
> +            }
> +            env->halted = 1;
> +            env->exception_index = EXCP_HLT;
> +            r = EXCP_HALTED;
>               break;
>           case ICPT_IO:
>               fprintf(stderr, "KVM unimplemented icpt IO\n");
> @@ -468,8 +491,6 @@ int kvm_arch_handle_exit(CPUState *env,
>
>       if (ret == 0) {
>           ret = EXCP_INTERRUPT;
> -    } else if (ret>  0) {
> -        ret = 0;
>       }
>       return ret;
>   }