Received: by 2002:a05:6a10:1287:0:0:0:0 with SMTP id d7csp12237pxv; Wed, 21 Jul 2021 14:05:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyQedN9oW30dm6VwvzZnpy2SN3LMVStCSHmo/wqjNHD3fyxBho0P0RVmnwhL5AU45M5+Gja X-Received: by 2002:a5d:9e4f:: with SMTP id i15mr25427864ioi.58.1626901521883; Wed, 21 Jul 2021 14:05:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1626901521; cv=none; d=google.com; s=arc-20160816; b=cBvPtAR3ZSKn3RBpLCbGw5mXBTT4sHl4xLmvziuuZaWXuLnMnmp//K3zrHm2cqIeU1 nY2bgputu5YiYIJz+oo5w0waJPAkHcwcN5rqsSZymjbGRos8nqlJn0qRWCxGR0OPPJqv YFL0E/3Xpcgu6OnDGe93iMYMT8m6GScUYWva8rzYE6fU8dGxeMeWC3L1HOJ2uwUmJ0AV AKnoIcIZA1nn185G7AkyPWgNmIGbPCLlEwE2ynu2lbBEeY4CkYAMxaGR10+h7M3w8cg0 rAIPfETF3DfC0K7G5ZKrDIwU/M3kx+aWqgLad5G8eJTMbUwUPqcJumcnCmhfTqsSuYKn z6BQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:content-language :in-reply-to:mime-version:user-agent:date:message-id:organization :from:references:cc:to:subject:reply-to:dkim-signature; bh=AzHhZKFYYciWklt6q5wFtY8MduXCET7U8bwUZdVRyHA=; b=Q7wwcjwMPLBa98kIT1egUrool9HZzqj3CMjrh5sqJtfDbIpUoqKzOsaUc55+zp7Yol oL72A/CTczi2fO6sa1WS59/6LYhUrhWmTcR+xHan5GNG0cvM8Te5GPk4QQPocTZaZKaJ QsDpv0xufID76KfD7dbPNmdE9Rplza1XOd8IrF7GCcxo1EW/0LCCwcnzmkEnSNi5cjAb oql6ob9D0EY1QqbMav7E3P9qcJpW7SH7OLi/VM8kFbPIe7/kWg5nxNXYAyAXRBhg6qll ptgGiM3CvNY7ANhdqyf7j7lTSaGPvi/T5KXBwu/gq7B4usLz4K++55bb1oqUYVRSDcFO aLFw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=WRKtsHuk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=ibm.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id k19si29289537ion.69.2021.07.21.14.05.09; Wed, 21 Jul 2021 14:05:21 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=WRKtsHuk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=ibm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232087AbhGUS5P (ORCPT + 99 others); Wed, 21 Jul 2021 14:57:15 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:56356 "EHLO mx0b-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231218AbhGUS5P (ORCPT ); Wed, 21 Jul 2021 14:57:15 -0400 Received: from pps.filterd (m0127361.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 16LJXRcP015721; Wed, 21 Jul 2021 15:37:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=reply-to : subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=AzHhZKFYYciWklt6q5wFtY8MduXCET7U8bwUZdVRyHA=; b=WRKtsHukCX5C700Xgl+MzSO7BmnCedQTLRj4lXmyerTOzLHPS+i+E4MMN3PQYhM3kEZt QLMRnKi/e9eNEZN1R0w0NUs8W3ECJYy1900PPq3ntNDhYdSSJxlXU5N/mq2Dr1lpG6Mo NQKlxmujkUaoKsYPpSK3PCB8OOTJqBKdgAU2cF8RDRnUNZU0zUCL/daIhfdHV8Cq5Zaa /ZvHHmtjjLNS/ooirCwp4ZJ/kk4jNP/Ln7ySpOIv89jLtRw+lV+FcXL2Og/wmDrGxril BO1Ps/DcuwcwG/JuFaTnMheuR+FE9mcfeZf5vnQLyeH6/BDfHqo4u5EBekpftGEEG5gu 3A== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 39xsbarvfg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Jul 2021 15:37:49 -0400 Received: from m0127361.ppops.net (m0127361.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 16LJXu5i016435; Wed, 21 Jul 2021 15:37:49 -0400 Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 39xsbarvey-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Jul 2021 15:37:49 -0400 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 16LJbguY015847; Wed, 21 Jul 2021 19:37:48 GMT Received: from b01cxnp23033.gho.pok.ibm.com (b01cxnp23033.gho.pok.ibm.com [9.57.198.28]) by ppma04wdc.us.ibm.com with ESMTP id 39upucfybh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 21 Jul 2021 19:37:48 +0000 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp23033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 16LJblHI35783042 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 Jul 2021 19:37:47 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6814912405A; Wed, 21 Jul 2021 19:37:47 +0000 (GMT) Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0DE30124058; Wed, 21 Jul 2021 19:37:47 +0000 (GMT) Received: from [9.85.196.19] (unknown [9.85.196.19]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTP; Wed, 21 Jul 2021 19:37:46 +0000 (GMT) Reply-To: jjherne@linux.ibm.com Subject: Re: [PATCH 2/2] s390/vfio-ap: replace open coded locks for VFIO_GROUP_NOTIFY_SET_KVM notification To: Tony Krowiak , linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org Cc: borntraeger@de.ibm.com, cohuck@redhat.com, pasic@linux.vnet.ibm.com, jgg@nvidia.com, alex.williamson@redhat.com, kwankhede@nvidia.com, david@redhat.com References: <20210719193503.793910-1-akrowiak@linux.ibm.com> <20210719193503.793910-3-akrowiak@linux.ibm.com> From: "Jason J. Herne" Organization: IBM Message-ID: <6c7244b5-be7b-1566-f406-4c4c37f06fd7@linux.ibm.com> Date: Wed, 21 Jul 2021 15:37:46 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.0 MIME-Version: 1.0 In-Reply-To: <20210719193503.793910-3-akrowiak@linux.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: wVRSy_cQFyMzWp19k-xCBl9_vW6GLn_7 X-Proofpoint-GUID: XGeCKfeXCAd2uDkKHH70TBoa1M0kFP7P X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790 definitions=2021-07-21_10:2021-07-21,2021-07-21 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 malwarescore=0 impostorscore=0 clxscore=1011 mlxlogscore=999 bulkscore=0 suspectscore=0 spamscore=0 priorityscore=1501 mlxscore=0 adultscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000 definitions=main-2107210115 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/19/21 3:35 PM, Tony Krowiak wrote: > It was pointed out during an unrelated patch review that locks should not > be open coded - i.e., writing the algorithm of a standard lock in a > function instead of using a lock from the standard library. The setting and > testing of a busy flag and sleeping on a wait_event is the same thing > a lock does. The open coded locks are invisible to lockdep, so potential > locking problems are not detected. > > This patch removes the open coded locks used during > VFIO_GROUP_NOTIFY_SET_KVM notification. The busy flag > and wait queue were introduced to resolve a possible circular locking > dependency reported by lockdep when starting a secure execution guest > configured with AP adapters and domains. Reversing the order in which > the kvm->lock mutex and matrix_dev->lock mutex are locked resolves the > issue reported by lockdep, thus enabling the removal of the open coded > locks. > > Signed-off-by: Tony Krowiak > --- > arch/s390/kvm/kvm-s390.c | 27 +++++- > drivers/s390/crypto/vfio_ap_ops.c | 132 ++++++++------------------ > drivers/s390/crypto/vfio_ap_private.h | 2 - > 3 files changed, 63 insertions(+), 98 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index a08f242a9f27..4d2ef3a3286e 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -2559,12 +2559,24 @@ static void kvm_s390_set_crycb_format(struct kvm *kvm) > kvm->arch.crypto.crycbd |= CRYCB_FORMAT1; > } > > +/* > + * kvm_arch_crypto_set_masks > + * > + * @kvm: a pointer to the object containing the crypto masks This should probably say "a pointer to the target guest's KVM struct" or something to that effect. Same comment for the comment above kvm_arch_crypto_clear_masks. > + * @apm: the mask identifying the accessible AP adapters > + * @aqm: the mask identifying the accessible AP domains > + * @adm: the mask identifying the accessible AP control domains > + * > + * Set the masks that identify the adapters, domains and control domains to > + * which the KVM guest is granted access. > + * > + * Note: The kvm->lock mutex must be locked by the caller. > + */ > 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) { > @@ -2595,13 +2607,21 @@ void kvm_arch_crypto_set_masks(struct kvm *kvm, unsigned long *apm, > /* 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); > > +/* > + * kvm_arch_crypto_set_masks Copy/paste error here. Rename to kvm_arch_crypto_CLEAR_masks I did not find anything else in my review. However, I'm still very new to this code, so take that with a grain of salt :). Also, I could not apply this to master. If there is a next version do you mind rebasing? Seeing the patch in full context would be helpful. -- -- Jason J. Herne (jjherne@linux.ibm.com)