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

Cornelia Huck
2020-02-25T13:02:02Z

On Tue, 25 Feb 2020 09:29:42 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 24.02.20 20:05, David Hildenbrand wrote:
> > 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>
> >

> Looks now like:
>
> @@ -4445,18 +4451,27 @@ static void __enable_ibs_on_vcpu(struct kvm_vcpu *vcpu)
> kvm_s390_sync_request(KVM_REQ_ENABLE_IBS, vcpu);
> }
>
> -void kvm_s390_vcpu_start(struct kvm_vcpu *vcpu)
> +int kvm_s390_vcpu_start(struct kvm_vcpu *vcpu)
> {
> - int i, online_vcpus, started_vcpus = 0;
> + int i, online_vcpus, r = 0, started_vcpus = 0;
>
> if (!is_vcpu_stopped(vcpu))
> - return;
> + return 0;
>
> trace_kvm_s390_vcpu_start_stop(vcpu->vcpu_id, 1);
> /* Only one cpu at a time may enter/leave the STOPPED state. */
> spin_lock(&vcpu->kvm->arch.start_stop_lock);
> online_vcpus = atomic_read(&vcpu->kvm->online_vcpus);
>
> + /* Let's tell the UV that we want to change into the operating state */
> + if (kvm_s390_pv_cpu_is_protected(vcpu)) {
> + r = kvm_s390_pv_set_cpu_state(vcpu, PV_CPU_STATE_OPR);
> + if (r) {
> + spin_unlock(&vcpu->kvm->arch.start_stop_lock);
> + return r;
> + }
> + }
> +
> for (i = 0; i < online_vcpus; i++) {
> if (!is_vcpu_stopped(vcpu->kvm->vcpus[i]))
> started_vcpus++;
> @@ -4481,22 +4496,31 @@ void kvm_s390_vcpu_start(struct kvm_vcpu *vcpu)
> */
> kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> spin_unlock(&vcpu->kvm->arch.start_stop_lock);
> - return;
> + return r;
> }
>
>

Hm, this is actually one of the cases where posting a new patch would
be less confusing than not doing so :)