[PATCHv3] s390: Fix cpu shutdown for KVM

On s390 a shutdown is the state of all CPUs being either stopped
or disabled (for interrupts) waiting. We have to track the overall
number of running CPUs to call the shutdown sequence accordingly.
This patch implements the counting and shutdown handling for the 
kvm path in qemu.
Lets also wrap changes to env->halted and env->exception_index.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390-virtio.c   |   32 ++++++++++++++++++++++++++++++--
 target-s390x/cpu.h |    2 ++
 target-s390x/kvm.c |   19 +++++++------------
 3 files changed, 39 insertions(+), 14 deletions(-)

On 10/04/2011 05:20 PM, Christian Borntraeger wrote:
> On s390 a shutdown is the state of all CPUs being either stopped
> or disabled (for interrupts) waiting. We have to track the overall
> number of running CPUs to call the shutdown sequence accordingly.
> This patch implements the counting and shutdown handling for the
> kvm path in qemu.
> Lets also wrap changes to env->halted and env->exception_index.
>
> Signed-off-by: Christian Borntraeger<borntraeger@de.ibm.com>
> ---
>   hw/s390-virtio.c   |   32 ++++++++++++++++++++++++++++++--
>   target-s390x/cpu.h |    2 ++
>   target-s390x/kvm.c |   19 +++++++------------
>   3 files changed, 39 insertions(+), 14 deletions(-)
>
> Index: b/hw/s390-virtio.c
> ===================================================================
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -130,6 +130,34 @@ int s390_virtio_hypercall(CPUState *env,
>       return r;
>   }
>
> +/*
> + * 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.
> + */
> +static unsigned s390_running_cpus;
> +
> +void s390_add_running_cpu(CPUState *env)
> +{
> +    if (env->halted) {
> +        s390_running_cpus++;
> +        env->halted = 0;
> +        env->exception_index = -1;
> +    }
> +}
> +
> +unsigned s390_del_running_cpu(CPUState *env)
> +{
> +    if (env->halted == 0) {
> +        assert(s390_running_cpus>= 1);
> +        s390_running_cpus--;
> +        env->halted = 1;
> +        env->exception_index = EXCP_HLT;
> +    }
> +    return s390_running_cpus;
> +}
> +
>   /* PC hardware initialisation */
>   static void s390_init(ram_addr_t my_ram_size,
>                         const char *boot_device,
> @@ -187,8 +215,8 @@ static void s390_init(ram_addr_t my_ram_
>           tmp_env->storage_keys = storage_keys;
>       }
>
> -    env->halted = 0;
> -    env->exception_index = 0;
> +    /* One CPU has to run */
> +    s390_add_running_cpu(env);
>
>       if (kernel_filename) {
>           kernel_size = load_image(kernel_filename, qemu_get_ram_ptr(0));
> Index: b/target-s390x/cpu.h
> ===================================================================
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -309,6 +309,8 @@ static inline void kvm_s390_interrupt_in
>   }
>   #endif
>   CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
> +void s390_add_running_cpu(CPUState *env);
> +unsigned s390_del_running_cpu(CPUState *env);
>
>   /* from s390-virtio-bus */
>   extern const target_phys_addr_t virtio_size;
> Index: b/target-s390x/kvm.c
> ===================================================================
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -185,8 +185,7 @@ void kvm_s390_interrupt_internal(CPUStat
>           return;
>       }
>
> -    env->halted = 0;
> -    env->exception_index = -1;
> +    s390_add_running_cpu(env);
>       qemu_cpu_kick(env);
>
>       kvmint.type = type;
> @@ -299,8 +298,7 @@ static int handle_diag(CPUState *env, st
>   static int s390_cpu_restart(CPUState *env)
>   {
>       kvm_s390_interrupt(env, KVM_S390_RESTART, 0);
> -    env->halted = 0;
> -    env->exception_index = -1;
> +    s390_add_running_cpu(env);
>       qemu_cpu_kick(env);
>       dprintf("DONE: SIGP cpu restart: %p\n", env);
>       return 0;
> @@ -425,17 +423,16 @@ static int handle_intercept(CPUState *en
>               r = handle_instruction(env, run);
>               break;
>           case ICPT_WAITPSW:
> -            /* XXX What to do on system shutdown? */
> -            env->halted = 1;
> -            env->exception_index = EXCP_HLT;
> +        case ICPT_CPU_STOP:
> +            if (s390_del_running_cpu(env) == 0) {
> +                qemu_system_shutdown_request();
> +            }
> +            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();
> -            break;
>           case ICPT_IO:
>               fprintf(stderr, "KVM unimplemented icpt IO\n");
>               exit(1);
> @@ -468,8 +465,6 @@ int kvm_arch_handle_exit(CPUState *env,
>
>       if (ret == 0) {
>           ret = EXCP_INTERRUPT;
> -    } else if (ret>  0) {
> -        ret = 0;

Hrm. Are you sure that this doesn't break anything? The rest looks good.


Alex

> Hrm. Are you sure that this doesn't break anything? The rest looks good.

Works on my kvm box. Unfortunately the non-kvm mode doesnt work (with and 
without the patch) so I cant test that one.

Christian

On 04.10.2011, at 17:20, Christian Borntraeger wrote:

> On s390 a shutdown is the state of all CPUs being either stopped
> or disabled (for interrupts) waiting. We have to track the overall
> number of running CPUs to call the shutdown sequence accordingly.
> This patch implements the counting and shutdown handling for the 
> kvm path in qemu.
> Lets also wrap changes to env->halted and env->exception_index.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

Thanks, applied to s390-next.

Alex