Re: [PATCH v4 28/36] KVM: s390: protvirt: Report CPU state to Ultravisor

David Hildenbrand
2020-02-24T19:06:04Z

On 24.02.20 12:40, Christian Borntraeger wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
>
> VCPU states have to be reported to the ultravisor for SIGP
> interpretation, kdump, kexec and reboot.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> [borntraeger@de.ibm.com: patch merging, splitting, fixing]
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>

[...]

>
> -void kvm_s390_vcpu_stop(struct kvm_vcpu *vcpu)
> +int kvm_s390_vcpu_stop(struct kvm_vcpu *vcpu)
> {
> - int i, online_vcpus, started_vcpus = 0;
> + int i, online_vcpus, r = 0, started_vcpus = 0;
> struct kvm_vcpu *started_vcpu = NULL;
>
> if (is_vcpu_stopped(vcpu))
> - return;
> + return 0;
>
> trace_kvm_s390_vcpu_start_stop(vcpu->vcpu_id, 0);
> /* Only one cpu at a time may enter/leave the STOPPED state. */
> @@ -4501,6 +4509,9 @@ void kvm_s390_vcpu_stop(struct kvm_vcpu *vcpu)
> kvm_s390_clear_stop_irq(vcpu);
>
> kvm_s390_set_cpuflags(vcpu, CPUSTAT_STOPPED);
> + /* Let's tell the UV that we successfully stopped the vcpu */
> + if (kvm_s390_pv_cpu_is_protected(vcpu))
> + r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_STP);
> __disable_ibs_on_vcpu(vcpu);
>
> for (i = 0; i < online_vcpus; i++) {
> @@ -4519,7 +4530,7 @@ void kvm_s390_vcpu_stop(struct kvm_vcpu *vcpu)
> }
>
> spin_unlock(&vcpu->kvm->arch.start_stop_lock);
> - return;
> + return r;
> }

So... you stopped the CPU, the UV call failed, and you'll return an
error. But you did stop the CPU. What is user space expected to do with
that error?

After all, it can't retrigger a STOP, due to if (is_vcpu_stopped(vcpu)).
Same applies to the start path.

--
Thanks,

David / dhildenb