Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model

From: David Hildenbrand <david@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>,
	Janosch Frank <frankja@linux.vnet.ibm.com>
Cc: KVM <kvm@vger.kernel.org>, "Cornelia Huck" <cohuck@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	"Collin Walling" <walling@linux.ibm.com>,
	"Jason J . Herne" <jjherne@linux.ibm.com>
Subject: Re: [PATCH v2 4/7] KVM: s390: enable MSA9 keywrapping functions depending on cpu model
Date: Thu, 18 Apr 2019 09:49:38 +0200	[thread overview]
Message-ID: <c40b3ef2-fbb8-d263-8fab-4ffedd978f55@redhat.com> (raw)
In-Reply-To: <abad6f35-d677-aba3-67b2-0c91d8036009@de.ibm.com>

On 18.04.19 09:35, Christian Borntraeger wrote:
> On 17.04.19 20:29, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Collin Walling <walling@linux.ibm.com>
>> ---
>> v1->v2: - enable vsie
>> 	- also check if the host has the pckmo functions
>>  arch/s390/include/asm/kvm_host.h | 1 +
>>  arch/s390/kvm/kvm-s390.c         | 7 +++++++
>>  arch/s390/kvm/vsie.c             | 5 ++++-
>>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> FWIW, I tested this variant successfully with some printk debugging to 
> check if the settings are good. The only question is: does anybody cares
> about
> 	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
> 	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] & kvm_s390_available_subfunc.pckmo[5] & 0xc0))
> 
> being too long? I find this more readable than 
> 
> 	if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] &
> 	     kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
> 	    (vcpu->kvm->arch.model.subfuncs.pckmo[5] &
> 	     kvm_s390_available_subfunc.pckmo[5] & 0xc0))
> 

Can you just factor that out into a function / makro?

> Christian
> 

-- 

Thanks,

David / dhildenb