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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <209a959b-b971-c06b-1731-6356908c623a@linux.ibm.com>
Date:   Mon, 27 Jun 2022 15:16:30 +0200
From:   Pierre Morel <pmorel@...ux.ibm.com>
To:     Nico Boehr <nrb@...ux.ibm.com>, kvm@...r.kernel.org
Cc:     linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        borntraeger@...ibm.com, frankja@...ux.ibm.com, cohuck@...hat.com,
        david@...hat.com, thuth@...hat.com, imbrenda@...ux.ibm.com,
        hca@...ux.ibm.com, gor@...ux.ibm.com, wintera@...ux.ibm.com,
        seiden@...ux.ibm.com
Subject: Re: [PATCH v10 2/3] KVM: s390: guest support for topology function



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@...ux.ibm.com>
> 

-- 
Pierre Morel
IBM Lab Boeblingen