diff mbox series

[v10,10/26] KVM: s390: interfaces to clear CRYCB masks

Message ID 1536781396-13601-11-git-send-email-akrowiak@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show
Series guest dedicated crypto adapters | expand

Commit Message

Tony Krowiak Sept. 12, 2018, 7:43 p.m. UTC
From: Tony Krowiak <akrowiak@linux.ibm.com>

Introduces two new KVM interface to clear the APM, AQM and ADM masks in
the guest's CRYCB.  The VCPUs are taken out of SIE to ensure the VCPUs do
not get out of sync.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
Acked-by: Halil Pasic <pasic@linux.ibm.com>
Tested-by: Michael Mueller <mimu@linux.ibm.com>
Tested-by: Farhan Ali <alifm@linux.ibm.com>
Tested-by: Pierre Morel <pmorel@linux.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |    2 ++
 arch/s390/kvm/kvm-s390.c         |   15 +++++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

Comments

Cornelia Huck Sept. 24, 2018, 11:01 a.m. UTC | #1
On Wed, 12 Sep 2018 15:43:00 -0400
Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:

> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> Introduces two new KVM interface to clear the APM, AQM and ADM masks in
> the guest's CRYCB.  The VCPUs are taken out of SIE to ensure the VCPUs do
> not get out of sync.

Hm, that patch description does not quite match what the patch actually
does...

> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> Acked-by: Halil Pasic <pasic@linux.ibm.com>
> Tested-by: Michael Mueller <mimu@linux.ibm.com>
> Tested-by: Farhan Ali <alifm@linux.ibm.com>
> Tested-by: Pierre Morel <pmorel@linux.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/include/asm/kvm_host.h |    2 ++
>  arch/s390/kvm/kvm-s390.c         |   15 +++++++++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 423cce7..1e758fe 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -858,6 +858,8 @@ void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>  void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>  				 struct kvm_async_pf *work);
>  
> +void kvm_arch_crypto_clear_masks(struct kvm *kvm);
> +
>  extern int sie64a(struct kvm_s390_sie_block *, u64 *);
>  extern char sie_exit;
>  
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index d717041..ac4c93f 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2033,6 +2033,21 @@ static void kvm_s390_set_crycb_format(struct kvm *kvm)
>  		kvm->arch.crypto.crycbd |= CRYCB_FORMAT1;
>  }
>  
> +void kvm_arch_crypto_clear_masks(struct kvm *kvm)
> +{
> +	mutex_lock(&kvm->lock);
> +	kvm_s390_vcpu_block_all(kvm);
> +
> +	memset(&kvm->arch.crypto.crycb->apcb0, 0,
> +	       sizeof(kvm->arch.crypto.crycb->apcb0));
> +	memset(&kvm->arch.crypto.crycb->apcb1, 0,
> +	       sizeof(kvm->arch.crypto.crycb->apcb1));
> +
> +	kvm_s390_vcpu_unblock_all(kvm);
> +	mutex_unlock(&kvm->lock);
> +}
> +EXPORT_SYMBOL_GPL(kvm_arch_crypto_clear_masks);

...although this function looks fine.

> +
>  static u64 kvm_s390_get_initial_cpuid(void)
>  {
>  	struct cpuid cpuid;
Halil Pasic Sept. 24, 2018, 11:50 a.m. UTC | #2
On 09/24/2018 01:01 PM, Cornelia Huck wrote:
> On Wed, 12 Sep 2018 15:43:00 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> 
>> From: Tony Krowiak <akrowiak@linux.ibm.com>
>>
>> Introduces two new KVM interface to clear the APM, AQM and ADM masks in
>> the guest's CRYCB.  The VCPUs are taken out of SIE to ensure the VCPUs do
>> not get out of sync.
> 
> Hm, that patch description does not quite match what the patch actually
> does...
> 

You mean it is not obvious where the masks get cleared? Well the APCB is
defined to contain exactly the three masks. As far as I can tell the patch
description is correct.

Regards,
Halil

>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> Acked-by: Halil Pasic <pasic@linux.ibm.com>
>> Tested-by: Michael Mueller <mimu@linux.ibm.com>
>> Tested-by: Farhan Ali <alifm@linux.ibm.com>
>> Tested-by: Pierre Morel <pmorel@linux.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  arch/s390/include/asm/kvm_host.h |    2 ++
>>  arch/s390/kvm/kvm-s390.c         |   15 +++++++++++++++
>>  2 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index 423cce7..1e758fe 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -858,6 +858,8 @@ void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>>  void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>>  				 struct kvm_async_pf *work);
>>  
>> +void kvm_arch_crypto_clear_masks(struct kvm *kvm);
>> +
>>  extern int sie64a(struct kvm_s390_sie_block *, u64 *);
>>  extern char sie_exit;
>>  
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index d717041..ac4c93f 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2033,6 +2033,21 @@ static void kvm_s390_set_crycb_format(struct kvm *kvm)
>>  		kvm->arch.crypto.crycbd |= CRYCB_FORMAT1;
>>  }
>>  
>> +void kvm_arch_crypto_clear_masks(struct kvm *kvm)
>> +{
>> +	mutex_lock(&kvm->lock);
>> +	kvm_s390_vcpu_block_all(kvm);
>> +
>> +	memset(&kvm->arch.crypto.crycb->apcb0, 0,
>> +	       sizeof(kvm->arch.crypto.crycb->apcb0));
>> +	memset(&kvm->arch.crypto.crycb->apcb1, 0,
>> +	       sizeof(kvm->arch.crypto.crycb->apcb1));
>> +
>> +	kvm_s390_vcpu_unblock_all(kvm);
>> +	mutex_unlock(&kvm->lock);
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_arch_crypto_clear_masks);
> 
> ...although this function looks fine.
> 
>> +
>>  static u64 kvm_s390_get_initial_cpuid(void)
>>  {
>>  	struct cpuid cpuid;
>
Cornelia Huck Sept. 24, 2018, 12:01 p.m. UTC | #3
On Mon, 24 Sep 2018 13:50:36 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:

> On 09/24/2018 01:01 PM, Cornelia Huck wrote:
> > On Wed, 12 Sep 2018 15:43:00 -0400
> > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> >   
> >> From: Tony Krowiak <akrowiak@linux.ibm.com>
> >>
> >> Introduces two new KVM interface to clear the APM, AQM and ADM masks in
> >> the guest's CRYCB.  The VCPUs are taken out of SIE to ensure the VCPUs do
> >> not get out of sync.  
> > 
> > Hm, that patch description does not quite match what the patch actually
> > does...
> >   
> 
> You mean it is not obvious where the masks get cleared? Well the APCB is
> defined to contain exactly the three masks. As far as I can tell the patch
> description is correct.

No, I was referring to "two new KVM interface[s]". The patch just
introduces a new function that clears the masks (well, maybe you can
call this an "interface"). So

s/two new KVM interface/a function in KVM/

and I'd be happy :) (sorry for being too vague)
Anthony Krowiak Sept. 24, 2018, 2:49 p.m. UTC | #4
On 09/24/2018 07:01 AM, Cornelia Huck wrote:
> On Wed, 12 Sep 2018 15:43:00 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> 
>> From: Tony Krowiak <akrowiak@linux.ibm.com>
>>
>> Introduces two new KVM interface to clear the APM, AQM and ADM masks in
>> the guest's CRYCB.  The VCPUs are taken out of SIE to ensure the VCPUs do
>> not get out of sync.
> 
> Hm, that patch description does not quite match what the patch actually
> does...

No, it doesn't, does it? I'll fix it.

> 
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
>> Acked-by: Halil Pasic <pasic@linux.ibm.com>
>> Tested-by: Michael Mueller <mimu@linux.ibm.com>
>> Tested-by: Farhan Ali <alifm@linux.ibm.com>
>> Tested-by: Pierre Morel <pmorel@linux.ibm.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>   arch/s390/include/asm/kvm_host.h |    2 ++
>>   arch/s390/kvm/kvm-s390.c         |   15 +++++++++++++++
>>   2 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index 423cce7..1e758fe 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -858,6 +858,8 @@ void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
>>   void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>>   				 struct kvm_async_pf *work);
>>   
>> +void kvm_arch_crypto_clear_masks(struct kvm *kvm);
>> +
>>   extern int sie64a(struct kvm_s390_sie_block *, u64 *);
>>   extern char sie_exit;
>>   
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index d717041..ac4c93f 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2033,6 +2033,21 @@ static void kvm_s390_set_crycb_format(struct kvm *kvm)
>>   		kvm->arch.crypto.crycbd |= CRYCB_FORMAT1;
>>   }
>>   
>> +void kvm_arch_crypto_clear_masks(struct kvm *kvm)
>> +{
>> +	mutex_lock(&kvm->lock);
>> +	kvm_s390_vcpu_block_all(kvm);
>> +
>> +	memset(&kvm->arch.crypto.crycb->apcb0, 0,
>> +	       sizeof(kvm->arch.crypto.crycb->apcb0));
>> +	memset(&kvm->arch.crypto.crycb->apcb1, 0,
>> +	       sizeof(kvm->arch.crypto.crycb->apcb1));
>> +
>> +	kvm_s390_vcpu_unblock_all(kvm);
>> +	mutex_unlock(&kvm->lock);
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_arch_crypto_clear_masks);
> 
> ...although this function looks fine.
> 
>> +
>>   static u64 kvm_s390_get_initial_cpuid(void)
>>   {
>>   	struct cpuid cpuid;
>
Anthony Krowiak Sept. 24, 2018, 3:33 p.m. UTC | #5
On 09/24/2018 08:01 AM, Cornelia Huck wrote:
> On Mon, 24 Sep 2018 13:50:36 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
>> On 09/24/2018 01:01 PM, Cornelia Huck wrote:
>>> On Wed, 12 Sep 2018 15:43:00 -0400
>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>>    
>>>> From: Tony Krowiak <akrowiak@linux.ibm.com>
>>>>
>>>> Introduces two new KVM interface to clear the APM, AQM and ADM masks in
>>>> the guest's CRYCB.  The VCPUs are taken out of SIE to ensure the VCPUs do
>>>> not get out of sync.
>>>
>>> Hm, that patch description does not quite match what the patch actually
>>> does...
>>>    
>>
>> You mean it is not obvious where the masks get cleared? Well the APCB is
>> defined to contain exactly the three masks. As far as I can tell the patch
>> description is correct.
> 
> No, I was referring to "two new KVM interface[s]". The patch just
> introduces a new function that clears the masks (well, maybe you can
> call this an "interface"). So
> 
> s/two new KVM interface/a function in KVM/
> 
> and I'd be happy :) (sorry for being too vague)

I'm not quite sure why this is not an interface, but this is not a hill
I'm willing to die on. And, I agree, there are not two of them. I'll
make the change.

>
diff mbox series

Patch

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 423cce7..1e758fe 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -858,6 +858,8 @@  void kvm_arch_async_page_not_present(struct kvm_vcpu *vcpu,
 void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
 				 struct kvm_async_pf *work);
 
+void kvm_arch_crypto_clear_masks(struct kvm *kvm);
+
 extern int sie64a(struct kvm_s390_sie_block *, u64 *);
 extern char sie_exit;
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d717041..ac4c93f 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2033,6 +2033,21 @@  static void kvm_s390_set_crycb_format(struct kvm *kvm)
 		kvm->arch.crypto.crycbd |= CRYCB_FORMAT1;
 }
 
+void kvm_arch_crypto_clear_masks(struct kvm *kvm)
+{
+	mutex_lock(&kvm->lock);
+	kvm_s390_vcpu_block_all(kvm);
+
+	memset(&kvm->arch.crypto.crycb->apcb0, 0,
+	       sizeof(kvm->arch.crypto.crycb->apcb0));
+	memset(&kvm->arch.crypto.crycb->apcb1, 0,
+	       sizeof(kvm->arch.crypto.crycb->apcb1));
+
+	kvm_s390_vcpu_unblock_all(kvm);
+	mutex_unlock(&kvm->lock);
+}
+EXPORT_SYMBOL_GPL(kvm_arch_crypto_clear_masks);
+
 static u64 kvm_s390_get_initial_cpuid(void)
 {
 	struct cpuid cpuid;