Re: [PATCH v10 2/3] KVM: s390: guest support for topology function

[Pierre Morel]

On 6/24/22 08:56, Nico Boehr wrote:
> Quoting Pierre Morel (2022-06-20 14:54:36)
> [...]
> > diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> > index 766028d54a3e..bb54196d4ed6 100644
> > --- a/arch/s390/include/asm/kvm_host.h
> > +++ b/arch/s390/include/asm/kvm_host.h
> [...]
> > @@ -3403,6 +3426,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
> >          rc = kvm_s390_vcpu_setup(vcpu);
> >          if (rc)
> >                  goto out_ucontrol_uninit;
> > +
> > +       kvm_s390_sca_set_mtcr(vcpu->kvm);
>
> We set the MTCR in the vcpu create. Does it also make sense to set it in kvm_arch_vcpu_destroy?

I think you are right.

Even we only destroy vCPU when we destroy the VM and I think that it is 
not currently needed, it would be more logical to do so, and we will be 
ready for the day we can unplug vCPUs.

Unless somebody has another opinion I add the entry.

> [...]
> > diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
> > index 12c464c7cddf..77a692238585 100644
> > --- a/arch/s390/kvm/priv.c
> > +++ b/arch/s390/kvm/priv.c
> > @@ -873,10 +873,13 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
> [...]
> > +       case 15:
> > +               trace_kvm_s390_handle_stsi(vcpu, fc, sel1, sel2, operand2);
> > +               insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2);
> > +               return -EREMOTE;
>
> Maybe the API documentation should clearly note that once you turn on KVM_CAP_S390_CPU_TOPOLOGY, you will get exits to userspace for STSI 15.x.y, regardless of whether KVM_CAP_S390_USER_STSI is on or off.
>
> Other than that, looks good, hence:
>
> Reviewed-by: Nico Boehr <nrb@xxxxxxxxxxxxx>

--
Pierre Morel
IBM Lab Boeblingen