From 8e41bd54317b04f2bf03012a4ca8ab7360c9beef Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Thu, 4 Oct 2018 14:42:43 +0200 Subject: [PATCH 1/4] KVM: s390: fix locking for crypto setting error path We need to unlock the kvm->lock mutex in the error case. Reported-by: smatch Fixes: 37940fb0b6a2c4bf101 ("KVM: s390: device attrs to enable/disable AP interpretation") Signed-off-by: Christian Borntraeger Reviewed-by: Janosch Frank Reviewed-by: Pierre Morel Reviewed-by: David Hildenbrand Reviewed-by: Cornelia Huck Signed-off-by: Christian Borntraeger --- arch/s390/kvm/kvm-s390.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index a6230b00c1df..734d87d88eb3 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -859,8 +859,10 @@ static int kvm_s390_vm_set_crypto(struct kvm *kvm, struct kvm_device_attr *attr) mutex_lock(&kvm->lock); switch (attr->attr) { case KVM_S390_VM_CRYPTO_ENABLE_AES_KW: - if (!test_kvm_facility(kvm, 76)) + if (!test_kvm_facility(kvm, 76)) { + mutex_unlock(&kvm->lock); return -EINVAL; + } get_random_bytes( kvm->arch.crypto.crycb->aes_wrapping_key_mask, sizeof(kvm->arch.crypto.crycb->aes_wrapping_key_mask)); @@ -868,8 +870,10 @@ static int kvm_s390_vm_set_crypto(struct kvm *kvm, struct kvm_device_attr *attr) VM_EVENT(kvm, 3, "%s", "ENABLE: AES keywrapping support"); break; case KVM_S390_VM_CRYPTO_ENABLE_DEA_KW: - if (!test_kvm_facility(kvm, 76)) + if (!test_kvm_facility(kvm, 76)) { + mutex_unlock(&kvm->lock); return -EINVAL; + } get_random_bytes( kvm->arch.crypto.crycb->dea_wrapping_key_mask, sizeof(kvm->arch.crypto.crycb->dea_wrapping_key_mask)); @@ -877,16 +881,20 @@ static int kvm_s390_vm_set_crypto(struct kvm *kvm, struct kvm_device_attr *attr) VM_EVENT(kvm, 3, "%s", "ENABLE: DEA keywrapping support"); break; case KVM_S390_VM_CRYPTO_DISABLE_AES_KW: - if (!test_kvm_facility(kvm, 76)) + if (!test_kvm_facility(kvm, 76)) { + mutex_unlock(&kvm->lock); return -EINVAL; + } kvm->arch.crypto.aes_kw = 0; memset(kvm->arch.crypto.crycb->aes_wrapping_key_mask, 0, sizeof(kvm->arch.crypto.crycb->aes_wrapping_key_mask)); VM_EVENT(kvm, 3, "%s", "DISABLE: AES keywrapping support"); break; case KVM_S390_VM_CRYPTO_DISABLE_DEA_KW: - if (!test_kvm_facility(kvm, 76)) + if (!test_kvm_facility(kvm, 76)) { + mutex_unlock(&kvm->lock); return -EINVAL; + } kvm->arch.crypto.dea_kw = 0; memset(kvm->arch.crypto.crycb->dea_wrapping_key_mask, 0, sizeof(kvm->arch.crypto.crycb->dea_wrapping_key_mask)); From 0e237e44699465139c07f969b051f83066a2ec1d Mon Sep 17 00:00:00 2001 From: Pierre Morel Date: Fri, 5 Oct 2018 10:31:09 +0200 Subject: [PATCH 2/4] KVM: s390: Tracing APCB changes kvm_arch_crypto_set_masks is a new function to centralize the setup the APCB masks inside the CRYCB SIE satellite. To trace APCB mask changes, we add KVM_EVENT() tracing to both kvm_arch_crypto_set_masks and kvm_arch_crypto_clear_masks. Signed-off-by: Pierre Morel Message-Id: <1538728270-10340-2-git-send-email-pmorel@linux.ibm.com> Reviewed-by: Cornelia Huck Signed-off-by: Christian Borntraeger --- arch/s390/include/asm/kvm_host.h | 2 ++ arch/s390/kvm/kvm-s390.c | 41 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h index 36d35313e840..22aa4da91f7a 100644 --- a/arch/s390/include/asm/kvm_host.h +++ b/arch/s390/include/asm/kvm_host.h @@ -861,6 +861,8 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu, struct kvm_async_pf *work); void kvm_arch_crypto_clear_masks(struct kvm *kvm); +void kvm_arch_crypto_set_masks(struct kvm *kvm, unsigned long *apm, + unsigned long *aqm, unsigned long *adm); 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 734d87d88eb3..22a320a9a00d 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2064,6 +2064,46 @@ static void kvm_s390_set_crycb_format(struct kvm *kvm) kvm->arch.crypto.crycbd |= CRYCB_FORMAT1; } +void kvm_arch_crypto_set_masks(struct kvm *kvm, unsigned long *apm, + unsigned long *aqm, unsigned long *adm) +{ + struct kvm_s390_crypto_cb *crycb = kvm->arch.crypto.crycb; + + mutex_lock(&kvm->lock); + kvm_s390_vcpu_block_all(kvm); + + switch (kvm->arch.crypto.crycbd & CRYCB_FORMAT_MASK) { + case CRYCB_FORMAT2: /* APCB1 use 256 bits */ + memcpy(crycb->apcb1.apm, apm, 32); + VM_EVENT(kvm, 3, "SET CRYCB: apm %016lx %016lx %016lx %016lx", + apm[0], apm[1], apm[2], apm[3]); + memcpy(crycb->apcb1.aqm, aqm, 32); + VM_EVENT(kvm, 3, "SET CRYCB: aqm %016lx %016lx %016lx %016lx", + aqm[0], aqm[1], aqm[2], aqm[3]); + memcpy(crycb->apcb1.adm, adm, 32); + VM_EVENT(kvm, 3, "SET CRYCB: adm %016lx %016lx %016lx %016lx", + adm[0], adm[1], adm[2], adm[3]); + break; + case CRYCB_FORMAT1: + case CRYCB_FORMAT0: /* Fall through both use APCB0 */ + memcpy(crycb->apcb0.apm, apm, 8); + memcpy(crycb->apcb0.aqm, aqm, 2); + memcpy(crycb->apcb0.adm, adm, 2); + VM_EVENT(kvm, 3, "SET CRYCB: apm %016lx aqm %04x adm %04x", + apm[0], *((unsigned short *)aqm), + *((unsigned short *)adm)); + break; + default: /* Can not happen */ + break; + } + + /* recreate the shadow crycb for each vcpu */ + kvm_s390_sync_request_broadcast(kvm, KVM_REQ_VSIE_RESTART); + kvm_s390_vcpu_unblock_all(kvm); + mutex_unlock(&kvm->lock); +} +EXPORT_SYMBOL_GPL(kvm_arch_crypto_set_masks); + void kvm_arch_crypto_clear_masks(struct kvm *kvm) { mutex_lock(&kvm->lock); @@ -2074,6 +2114,7 @@ void kvm_arch_crypto_clear_masks(struct kvm *kvm) memset(&kvm->arch.crypto.crycb->apcb1, 0, sizeof(kvm->arch.crypto.crycb->apcb1)); + VM_EVENT(kvm, 3, "%s", "CLR CRYCB:"); /* recreate the shadow crycb for each vcpu */ kvm_s390_sync_request_broadcast(kvm, KVM_REQ_VSIE_RESTART); kvm_s390_vcpu_unblock_all(kvm); From 76c7829f5b8c7691b18929cdedd6d2e79db3c2b9 Mon Sep 17 00:00:00 2001 From: Pierre Morel Date: Fri, 5 Oct 2018 10:31:10 +0200 Subject: [PATCH 3/4] s390: vfio-ap: setup APCB mask using KVM dedicated function We replace the vfio_ap_mdev_copy_masks() by the new kvm_arch_crypto_set_masks() to be able to use the standard KVM tracing system. Signed-off-by: Pierre Morel Message-Id: <1538728270-10340-3-git-send-email-pmorel@linux.ibm.com> Reviewed-by: Cornelia Huck Reviewed-by: David Hildenbrand Signed-off-by: Christian Borntraeger --- drivers/s390/crypto/vfio_ap_ops.c | 35 +++---------------------------- 1 file changed, 3 insertions(+), 32 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index d3d9eb72b0f1..ea99165d1045 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -727,37 +727,6 @@ static const struct attribute_group *vfio_ap_mdev_attr_groups[] = { NULL }; -static void vfio_ap_mdev_copy_masks(struct ap_matrix_mdev *matrix_mdev) -{ - int nbytes; - unsigned long *apm, *aqm, *adm; - struct kvm_s390_crypto_cb *crycb = matrix_mdev->kvm->arch.crypto.crycb; - - switch (matrix_mdev->kvm->arch.crypto.crycbd & CRYCB_FORMAT_MASK) { - case CRYCB_FORMAT2: - apm = (unsigned long *)crycb->apcb1.apm; - aqm = (unsigned long *)crycb->apcb1.aqm; - adm = (unsigned long *)crycb->apcb1.adm; - break; - case CRYCB_FORMAT1: - case CRYCB_FORMAT0: - apm = (unsigned long *)crycb->apcb0.apm; - aqm = (unsigned long *)crycb->apcb0.aqm; - adm = (unsigned long *)crycb->apcb0.adm; - break; - default: - /* cannot happen */ - return; - } - - nbytes = DIV_ROUND_UP(matrix_mdev->matrix.apm_max + 1, BITS_PER_BYTE); - memcpy(apm, matrix_mdev->matrix.apm, nbytes); - nbytes = DIV_ROUND_UP(matrix_mdev->matrix.aqm_max + 1, BITS_PER_BYTE); - memcpy(aqm, matrix_mdev->matrix.aqm, nbytes); - nbytes = DIV_ROUND_UP(matrix_mdev->matrix.adm_max + 1, BITS_PER_BYTE); - memcpy(adm, matrix_mdev->matrix.adm, nbytes); -} - /** * vfio_ap_mdev_set_kvm * @@ -814,7 +783,9 @@ static int vfio_ap_mdev_group_notifier(struct notifier_block *nb, if (!matrix_mdev->kvm->arch.crypto.crycbd) return NOTIFY_DONE; - vfio_ap_mdev_copy_masks(matrix_mdev); + kvm_arch_crypto_set_masks(matrix_mdev->kvm, matrix_mdev->matrix.apm, + matrix_mdev->matrix.aqm, + matrix_mdev->matrix.adm); return NOTIFY_OK; } From 46623ab3194a3f37ea12da2964bca3b35fdd20ed Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Fri, 5 Oct 2018 19:22:38 +0200 Subject: [PATCH 4/4] s390: vfio-ap: make local functions and data static no functional change, just hygiene. Signed-off-by: Christian Borntraeger Reviewed-by: Cornelia Huck Reviewed-by: David Hildenbrand --- drivers/s390/crypto/vfio_ap_drv.c | 4 ++-- drivers/s390/crypto/vfio_ap_ops.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c index 8b51821d9bf7..7667b38728f0 100644 --- a/drivers/s390/crypto/vfio_ap_drv.c +++ b/drivers/s390/crypto/vfio_ap_drv.c @@ -112,7 +112,7 @@ static void vfio_ap_matrix_dev_destroy(void) root_device_unregister(matrix_dev->device.parent); } -int __init vfio_ap_init(void) +static int __init vfio_ap_init(void) { int ret; @@ -146,7 +146,7 @@ int __init vfio_ap_init(void) return 0; } -void __exit vfio_ap_exit(void) +static void __exit vfio_ap_exit(void) { vfio_ap_mdev_unregister(); ap_driver_unregister(&vfio_ap_drv); diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index ea99165d1045..272ef427dcc0 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -77,7 +77,7 @@ static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf) return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT); } -MDEV_TYPE_ATTR_RO(name); +static MDEV_TYPE_ATTR_RO(name); static ssize_t available_instances_show(struct kobject *kobj, struct device *dev, char *buf) @@ -86,7 +86,7 @@ static ssize_t available_instances_show(struct kobject *kobj, atomic_read(&matrix_dev->available_instances)); } -MDEV_TYPE_ATTR_RO(available_instances); +static MDEV_TYPE_ATTR_RO(available_instances); static ssize_t device_api_show(struct kobject *kobj, struct device *dev, char *buf) @@ -94,7 +94,7 @@ static ssize_t device_api_show(struct kobject *kobj, struct device *dev, return sprintf(buf, "%s\n", VFIO_DEVICE_API_AP_STRING); } -MDEV_TYPE_ATTR_RO(device_api); +static MDEV_TYPE_ATTR_RO(device_api); static struct attribute *vfio_ap_mdev_type_attrs[] = { &mdev_type_attr_name.attr, @@ -395,7 +395,7 @@ static ssize_t unassign_adapter_store(struct device *dev, return count; } -DEVICE_ATTR_WO(unassign_adapter); +static DEVICE_ATTR_WO(unassign_adapter); static int vfio_ap_mdev_verify_queues_reserved_for_apqi(struct ap_matrix_mdev *matrix_mdev, @@ -491,7 +491,7 @@ done: return ret; } -DEVICE_ATTR_WO(assign_domain); +static DEVICE_ATTR_WO(assign_domain); /** @@ -537,7 +537,7 @@ static ssize_t unassign_domain_store(struct device *dev, return count; } -DEVICE_ATTR_WO(unassign_domain); +static DEVICE_ATTR_WO(unassign_domain); /** * assign_control_domain_store @@ -586,7 +586,7 @@ static ssize_t assign_control_domain_store(struct device *dev, return count; } -DEVICE_ATTR_WO(assign_control_domain); +static DEVICE_ATTR_WO(assign_control_domain); /** * unassign_control_domain_store @@ -630,7 +630,7 @@ static ssize_t unassign_control_domain_store(struct device *dev, return count; } -DEVICE_ATTR_WO(unassign_control_domain); +static DEVICE_ATTR_WO(unassign_control_domain); static ssize_t control_domains_show(struct device *dev, struct device_attribute *dev_attr, @@ -654,7 +654,7 @@ static ssize_t control_domains_show(struct device *dev, return nchars; } -DEVICE_ATTR_RO(control_domains); +static DEVICE_ATTR_RO(control_domains); static ssize_t matrix_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -704,7 +704,7 @@ static ssize_t matrix_show(struct device *dev, struct device_attribute *attr, return nchars; } -DEVICE_ATTR_RO(matrix); +static DEVICE_ATTR_RO(matrix); static struct attribute *vfio_ap_mdev_attrs[] = { &dev_attr_assign_adapter.attr,