From patchwork Thu Jul 20 00:32:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Volodymyr Babchuk X-Patchwork-Id: 13319658 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 30E3CC04E69 for ; Thu, 20 Jul 2023 00:33:03 +0000 (UTC) Received: from list by lists.xenproject.org with outflank-mailman.566255.884843 (Exim 4.92) (envelope-from ) id 1qMHb3-0001ad-6C; Thu, 20 Jul 2023 00:32:45 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 566255.884843; Thu, 20 Jul 2023 00:32:45 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qMHb2-0001Zq-Va; Thu, 20 Jul 2023 00:32:44 +0000 Received: by outflank-mailman (input) for mailman id 566255; Thu, 20 Jul 2023 00:32:42 +0000 Received: from se1-gles-sth1-in.inumbo.com ([159.253.27.254] helo=se1-gles-sth1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1qMHb0-0001JI-NT for xen-devel@lists.xenproject.org; Thu, 20 Jul 2023 00:32:42 +0000 Received: from mx0a-0039f301.pphosted.com (mx0a-0039f301.pphosted.com [148.163.133.242]) by se1-gles-sth1.inumbo.com (Halon) with ESMTPS id ef2c5212-2694-11ee-b23a-6b7b168915f2; Thu, 20 Jul 2023 02:32:41 +0200 (CEST) Received: from pps.filterd (m0174678.ppops.net [127.0.0.1]) by mx0a-0039f301.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 36JMeiB6017190; Thu, 20 Jul 2023 00:32:38 GMT Received: from eur04-he1-obe.outbound.protection.outlook.com (mail-he1eur04lp2057.outbound.protection.outlook.com [104.47.13.57]) by mx0a-0039f301.pphosted.com (PPS) with ESMTPS id 3rxgyx9k43-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Jul 2023 00:32:38 +0000 Received: from VI1PR03MB3710.eurprd03.prod.outlook.com (2603:10a6:803:31::18) by AS8PR03MB7783.eurprd03.prod.outlook.com (2603:10a6:20b:407::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6588.33; Thu, 20 Jul 2023 00:32:32 +0000 Received: from VI1PR03MB3710.eurprd03.prod.outlook.com ([fe80::68d2:d90f:ac32:7c85]) by VI1PR03MB3710.eurprd03.prod.outlook.com ([fe80::68d2:d90f:ac32:7c85%3]) with mapi id 15.20.6588.035; Thu, 20 Jul 2023 00:32:32 +0000 X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: ef2c5212-2694-11ee-b23a-6b7b168915f2 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TxH1RSkGK+hRVuJxMTWcF/mwPOL8tedwFBXn1EVCfoz8eQYQ2veAjpLGgiftfacCcm6ymLNn+gtHTD9Buk4/hhD8zdcN8CDWv3sh4uIGZtxDzO92zGH8CQditvGpwQkkne7UywWjaW2dfFDBTKo6BJaFoMLDGVaZsefNqYMqJpLm9AMjYuWVUjwWeh/sWmAi0hXo6AtZxT0herAEd7snRPKnoaE9/MfPZraQAFYZ35JeryOJgVGBdXlrQIQnFq9nrkcM7R9cIGdqjFa7l49q/92CoF3TtyUwic3y0EOj+xNpsS48ixcFRqc3HIEWOowiubtXLpYtk0D05g2KU+XV7Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=abxFPIgNa+QJErFwy718sKh+mXFlflc8O/+7z9aj10M=; b=XlLL/AbwfjZ3YhwkQfAWXJF5vTdB8dcOJEjgbNS+tKfJ3BwjaXdkfp1gNHo/So9O30J96gZJYTKzB+9kYx6Euj4O02R5C9c3Y3BWIMSk9SdTLd5M983c0J47GLciuKXjdSXAse9+ODudlLTuKe4ts/6Y8HK6sr8zAF2met39raZBpTM3Zsqlr/Wq15EYstpgEh1iWFlfUGEL3n9FN/cqS+NsJoYDYKgElCBPj8J93MG5pBieg5Ne0fQGKbFQhshVWR6z0H8be3Cs8RaiKbiz0wO/FPQ9Zz31TaUpdLQTufEOE64exGihU5b7Pp/K66NyW56TxETSs5MWuByQ7wjLlg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=epam.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=abxFPIgNa+QJErFwy718sKh+mXFlflc8O/+7z9aj10M=; b=TyyKgI2SR5Uc4Kkx937N+OftwGWHvc5rC7erAlz8hx7+8OKuSqnR6x/lEroi0WRE6MgnRtdW3/x4XSIhHJ3s5JJBOZQTvdOcQ6aqViZnpQBLH4TIaWx3y2DWgr5boQMEtdE1sLZitllESNzSWTQNr0iCHRZuiwWVo0JufN5X13bOQ0b9tpjTIbBmIzXM92jzOpwEqvO9/tPK+S/Ij6ZPN8fQMtLGVVMozUhrTwMQ05C7upDUy/PhHdIwf78sh9K8l91c4bs37TBNwbhXZmRFUgbZNzUq3+lW3EkEGACMaxovHgHQSnNE79CS8DCmshQG010loZ3tM+0bMn64bMnOhQ== From: Volodymyr Babchuk To: "xen-devel@lists.xenproject.org" CC: Oleksandr Andrushchenko , =?utf-8?q?Ro?= =?utf-8?q?ger_Pau_Monn=C3=A9?= , Jan Beulich , Volodymyr Babchuk Subject: [PATCH v8 02/13] vpci: use per-domain PCI lock to protect vpci structure Thread-Topic: [PATCH v8 02/13] vpci: use per-domain PCI lock to protect vpci structure Thread-Index: AQHZuqGrLMJPz1ipBEu4urExXp8yiw== Date: Thu, 20 Jul 2023 00:32:31 +0000 Message-ID: <20230720003205.1828537-3-volodymyr_babchuk@epam.com> References: <20230720003205.1828537-1-volodymyr_babchuk@epam.com> In-Reply-To: <20230720003205.1828537-1-volodymyr_babchuk@epam.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-mailer: git-send-email 2.41.0 x-ms-publictraffictype: Email x-ms-traffictypediagnostic: VI1PR03MB3710:EE_|AS8PR03MB7783:EE_ x-ms-office365-filtering-correlation-id: 581027c8-a6bc-453c-1cb2-08db88b8ce92 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 8XLhK+cI/Y9/osOHRZyqh+db/Z5my1ZRXPzSGCw7gyTueyTcI6v+cs7vPCHs6a8Bte9fqou3G8uPYHWhcXtagPsIGQT/vcDEPirj31Bi23z6+B89gh4cPNvfVQaVeGSVQZYpTOibaQJatablAGot6FtGj2GtloDDlzMH8lhG1RtIdERw+rta5Sj4433dTEwr9+1wpCuBRPN6ft9DI4lYnea+PtSogtXQshYivv+eluSu4anKjGxmKA8QVLlEvZNa/Q1Pp4eQQjwfcWMoLZxwZH0MkdPB3v5mo9SScecv6THFWL9PFNEWj4KKcvbc4Yt+Hw3sE8LIRXyEk+2Hya+isMZFsb1M5YGpYwFo+DP/hWD5kq/87kESs6JNe0IpIlsMK7ykVCLNzAnj9vxEyRJ06B6hP3MCTNB4/bodePEJuWH8solmaOqkNTSkHkkBDqij6y04XAbFI9cbS1GrBD8ieqUw9qrySgbKV/D8mrxLeXGWgRporTI8YJ3y1lXbyAcQCQXLxGlAgH1bW0iBnPeeQiDKMWCUP/LD//airW2f+Pl8odR+7nVxXpI0xKh+HYftXHkqQdb8swtuEQwhZUaI1gAhMzVpxS2yO/S+WjyEuia0lYubAc7APMIrDr4O2ibgkjgLnWLOHky1+boroXcElw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR03MB3710.eurprd03.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(136003)(39860400002)(346002)(366004)(396003)(376002)(451199021)(55236004)(26005)(1076003)(6506007)(41300700001)(316002)(83380400001)(2616005)(107886003)(186003)(6512007)(6486002)(122000001)(478600001)(66946007)(54906003)(71200400001)(4326008)(64756008)(38070700005)(66556008)(76116006)(66476007)(91956017)(6916009)(38100700002)(86362001)(5660300002)(66446008)(8936002)(8676002)(36756003)(30864003)(2906002)(309714004);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?q?vUfqebxAuyOkAC3xYlQDxkLQ5/NQ?= =?utf-8?q?j00d+6oDa4w8IBEc8bOLVidLNx0F+bOvGewUH0OkVnJuuJC9XlQXr//YMe2SaWM3e?= =?utf-8?q?qaerebhMsSkL75LLxdCqszqB9/9naHW766xGoS45zqcnKUzkdyvpAcfPoQ42sWYPh?= =?utf-8?q?4SO8m31Wt5qIzYld3N2LcfJmAcyRFAJpSz7xxVhpxiOIJegwlQTdmGuQ2iOkwHMS/?= =?utf-8?q?YtU+ocjculYYO5jPMVPSEjSJANFwUz/4EKCz1lYv73KnbjqToW9iG0artqU2iH89o?= =?utf-8?q?CKFS+eLHgWyla9F4j4tGnoPrdej444/lbmjwnCPDlunVxV/2jGbquopc2qUykBc3+?= =?utf-8?q?Rzox1y6q5E+kovLAUbKDb5dVS3cj95HEBZP0s5tu6QYSVGwQEjVm925puMKP+0YlI?= =?utf-8?q?kjz2DYOwJRpcY89WhrFWkro4f1JteEBl1dfpbksT2Fs58CX6qJlmtRgzkCfEuZ9qv?= =?utf-8?q?RqzEahYbLTozfMm8385+4agBjYTPYXn5eBmod8oELkuX9QO+avZT7IB4zr3uD1nyd?= =?utf-8?q?WZmTEjfK1wdEfpvHRvY3pvoNUb5QqiH3/H0QQa2zn+mI5RTLEht7NsSdEmbI5Zk1g?= =?utf-8?q?MFWBbHbDr4L4FBAwi0SAad1Vt+wBi7Xcl9f1ubDkk6SUX9UY3E/tVbSMaFsBWitXI?= =?utf-8?q?DcGlcEU49FYZn0OsFtxLDo3OcYRII+8DZx6Z0YhJl9JMEMPRpUWouKNoOYQUinFgK?= =?utf-8?q?3bUNSniySVtNCXZ/elb47v99yS49OxhzyocCCxi+eXEd1griYCjpvdifDrgGslfb6?= =?utf-8?q?naHIiDXsNu3UvaN+PdD3SUeWp9ktEfbv42DQzsHInBoKZcn78oh4b8Zd5wCOxz8RL?= =?utf-8?q?M/3Wj0N8v7pq1kcjpk2c7gsfwokMDGWQtrQa2Xq9LNxse94NB+p8PJrecbnJD+8sw?= =?utf-8?q?fgf9xdvWSb/bqw+P4MslUKTg5I1wGplvcjZ/I8CBELWumN852yK97aMNPNRoBG1fU?= =?utf-8?q?NNVveBAAc14ulLy4ul4ABrkncRAUDlPoRqp/ksH3mOVbFK/27BYBznXA3olLuAUhK?= =?utf-8?q?xIhK1NwCRAcsyGppUmkD8JylVqNskHjF7dEXQgXYbh4iBKcyKOfLVdRpFKsSym9zW?= =?utf-8?q?6Kde6T1zJgRv0pYnVF0Qmm1Ns34ExHjTix5dwnrcU4n4xNqV0tl5QFYZmCfpP/auY?= =?utf-8?q?W4LtEdsJLoP8BaFPU7L6PxCCNJ5yo6Jwev0xkSowmOiYmLzncSgL2ATTv9XQsOXI8?= =?utf-8?q?0ZPZzM8FjQNXYb7Jm6ZslbNUdULin9thbQDyflXyF85Q4xf5rwqxb5OoPkGGx2Alk?= =?utf-8?q?Q8EGD9ze2TjkC746DZ9cji3w8XgLdeWjb/qUbCZSE3xP+9E0/x/6QHNciBOmSb5i+?= =?utf-8?q?h62b9PKhN6/7zdioEQaHcqd7d3ymUSgt9HYXeGr713XmZiYfvG0lCD2tDPpsSHuuT?= =?utf-8?q?Tsc/IArhtvT/3mkU3IIhJ8ALxmRSID487iCDswqL2Tot9bQWio4N+bxyR1Yiz5b9Q?= =?utf-8?q?UxpnYfTauLEzMLB8hM+9a09CTi/px0lVGYWIW8Tv0GfWVyw4girwr5Q+mnkusP6Wb?= =?utf-8?q?AyfcPbWTiJ/bVCPAkuSK6ETegsDEFHYp0A=3D=3D?= Content-ID: MIME-Version: 1.0 X-OriginatorOrg: epam.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: VI1PR03MB3710.eurprd03.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 581027c8-a6bc-453c-1cb2-08db88b8ce92 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jul 2023 00:32:31.3755 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b41b72d0-4e9f-4c26-8a69-f949f367c91d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: BkrJQVEi0+vUpEXg8coxK4v4webME8P0jKt7mpCq1pDwZMW3s3L/0XE2lry/IS9VL37hbKLR8fCDjGh30BJFJEqJWpQCVUZoCx5rhhalqQk= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR03MB7783 X-Proofpoint-ORIG-GUID: wS0UN25VQYO8CBIh5be9I2KXNyp6YnQE X-Proofpoint-GUID: wS0UN25VQYO8CBIh5be9I2KXNyp6YnQE X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-07-19_16,2023-07-19_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 impostorscore=0 priorityscore=1501 bulkscore=0 spamscore=0 adultscore=0 clxscore=1015 suspectscore=0 phishscore=0 mlxlogscore=999 malwarescore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2307200002 From: Oleksandr Andrushchenko Use a previously introduced per-domain read/write lock to check whether vpci is present, so we are sure there are no accesses to the contents of the vpci struct if not. This lock can be used (and in a few cases is used right away) so that vpci removal can be performed while holding the lock in write mode. Previously such removal could race with vpci_read for example. 1. Per-domain's pci_rwlock is used to protect pdev->vpci structure from being removed. 2. Writing the command register and ROM BAR register may trigger modify_bars to run, which in turn may access multiple pdevs while checking for the existing BAR's overlap. The overlapping check, if done under the read lock, requires vpci->lock to be acquired on both devices being compared, which may produce a deadlock. It is not possible to upgrade read lock to write lock in such a case. So, in order to prevent the deadlock, use d->pci_lock instead. To prevent deadlock while locking both hwdom->pci_lock and dom_xen->pci_lock, always lock hwdom first. All other code, which doesn't lead to pdev->vpci destruction and does not access multiple pdevs at the same time, can still use a combination of the read lock and pdev->vpci->lock. 3. Drop const qualifier where the new rwlock is used and this is appropriate. 4. Do not call process_pending_softirqs with any locks held. For that unlock prior the call and re-acquire the locks after. After re-acquiring the lock there is no need to check if pdev->vpci exists: - in apply_map because of the context it is called (no race condition possible) - for MSI/MSI-X debug code because it is called at the end of pdev->vpci access and no further access to pdev->vpci is made 5. Introduce pcidevs_trylock, so there is a possibility to try locking the pcidev's lock. 6. Use d->pci_lock around for_each_pdev and pci_get_pdev_by_domain while accessing pdevs in vpci code. Suggested-by: Roger Pau Monné Suggested-by: Jan Beulich Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Volodymyr Babchuk --- Changes in v8: - changed d->vpci_lock to d->pci_lock - introducing d->pci_lock in a separate patch - extended locked region in vpci_process_pending - removed pcidevs_lockis vpci_dump_msi() - removed some changes as they are not needed with the new locking scheme - added handling for hwdom && dom_xen case --- xen/arch/x86/hvm/vmsi.c | 4 +++ xen/drivers/passthrough/pci.c | 7 +++++ xen/drivers/vpci/header.c | 18 ++++++++++++ xen/drivers/vpci/msi.c | 14 ++++++++-- xen/drivers/vpci/msix.c | 52 ++++++++++++++++++++++++++++++----- xen/drivers/vpci/vpci.c | 46 +++++++++++++++++++++++++++++-- xen/include/xen/pci.h | 1 + 7 files changed, 129 insertions(+), 13 deletions(-) diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c index 3cd4923060..8c1bd66b9c 100644 --- a/xen/arch/x86/hvm/vmsi.c +++ b/xen/arch/x86/hvm/vmsi.c @@ -895,6 +895,8 @@ int vpci_msix_arch_print(const struct vpci_msix *msix) { unsigned int i; + ASSERT(rw_is_locked(&msix->pdev->domain->pci_lock)); + for ( i = 0; i < msix->max_entries; i++ ) { const struct vpci_msix_entry *entry = &msix->entries[i]; @@ -913,7 +915,9 @@ int vpci_msix_arch_print(const struct vpci_msix *msix) struct pci_dev *pdev = msix->pdev; spin_unlock(&msix->pdev->vpci->lock); + read_unlock(&pdev->domain->pci_lock); process_pending_softirqs(); + read_lock(&pdev->domain->pci_lock); /* NB: we assume that pdev cannot go away for an alive domain. */ if ( !pdev->vpci || !spin_trylock(&pdev->vpci->lock) ) return -EBUSY; diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 5b4632ead2..6f8692cd9c 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -57,6 +57,11 @@ void pcidevs_lock(void) spin_lock_recursive(&_pcidevs_lock); } +int pcidevs_trylock(void) +{ + return spin_trylock_recursive(&_pcidevs_lock); +} + void pcidevs_unlock(void) { spin_unlock_recursive(&_pcidevs_lock); @@ -1144,7 +1149,9 @@ static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt, } while ( devfn != pdev->devfn && PCI_SLOT(devfn) == PCI_SLOT(pdev->devfn) ); + write_lock(&ctxt->d->pci_lock); err = vpci_add_handlers(pdev); + write_unlock(&ctxt->d->pci_lock); if ( err ) printk(XENLOG_ERR "setup of vPCI for d%d failed: %d\n", ctxt->d->domain_id, err); diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index b41556d007..2780fcae72 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -152,6 +152,7 @@ bool vpci_process_pending(struct vcpu *v) if ( rc == -ERESTART ) return true; + write_lock(&v->domain->pci_lock); spin_lock(&v->vpci.pdev->vpci->lock); /* Disable memory decoding unconditionally on failure. */ modify_decoding(v->vpci.pdev, @@ -170,6 +171,7 @@ bool vpci_process_pending(struct vcpu *v) * failure. */ vpci_remove_device(v->vpci.pdev); + write_unlock(&v->domain->pci_lock); } return false; @@ -181,8 +183,20 @@ static int __init apply_map(struct domain *d, const struct pci_dev *pdev, struct map_data data = { .d = d, .map = true }; int rc; + ASSERT(rw_is_locked(&d->pci_lock)); + while ( (rc = rangeset_consume_ranges(mem, map_range, &data)) == -ERESTART ) + { + /* + * It's safe to drop and reacquire the lock in this context + * without risking pdev disappearing because devices cannot be + * removed until the initial domain has been started. + */ + read_unlock(&d->pci_lock); process_pending_softirqs(); + read_lock(&d->pci_lock); + } + rangeset_destroy(mem); if ( !rc ) modify_decoding(pdev, cmd, false); @@ -223,6 +237,8 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only) unsigned int i; int rc; + ASSERT(rw_is_locked(&pdev->domain->pci_lock)); + if ( !mem ) return -ENOMEM; @@ -502,6 +518,8 @@ static int cf_check init_bars(struct pci_dev *pdev) struct vpci_bar *bars = header->bars; int rc; + ASSERT(rw_is_locked(&pdev->domain->pci_lock)); + switch ( pci_conf_read8(pdev->sbdf, PCI_HEADER_TYPE) & 0x7f ) { case PCI_HEADER_TYPE_NORMAL: diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c index 8f2b59e61a..e63152c224 100644 --- a/xen/drivers/vpci/msi.c +++ b/xen/drivers/vpci/msi.c @@ -190,6 +190,8 @@ static int cf_check init_msi(struct pci_dev *pdev) uint16_t control; int ret; + ASSERT(rw_is_write_locked(&pdev->domain->pci_lock)); + if ( !pos ) return 0; @@ -265,7 +267,7 @@ REGISTER_VPCI_INIT(init_msi, VPCI_PRIORITY_LOW); void vpci_dump_msi(void) { - const struct domain *d; + struct domain *d; rcu_read_lock(&domlist_read_lock); for_each_domain ( d ) @@ -277,6 +279,9 @@ void vpci_dump_msi(void) printk("vPCI MSI/MSI-X d%d\n", d->domain_id); + if ( !read_trylock(&d->pci_lock) ) + continue; + for_each_pdev ( d, pdev ) { const struct vpci_msi *msi; @@ -318,14 +323,17 @@ void vpci_dump_msi(void) * holding the lock. */ printk("unable to print all MSI-X entries: %d\n", rc); - process_pending_softirqs(); - continue; + goto pdev_done; } } spin_unlock(&pdev->vpci->lock); + pdev_done: + read_unlock(&d->pci_lock); process_pending_softirqs(); + read_lock(&d->pci_lock); } + read_unlock(&d->pci_lock); } rcu_read_unlock(&domlist_read_lock); } diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c index 25bde77586..9481274579 100644 --- a/xen/drivers/vpci/msix.c +++ b/xen/drivers/vpci/msix.c @@ -147,6 +147,8 @@ static struct vpci_msix *msix_find(const struct domain *d, unsigned long addr) { struct vpci_msix *msix; + ASSERT(rw_is_locked(&d->pci_lock)); + list_for_each_entry ( msix, &d->arch.hvm.msix_tables, next ) { const struct vpci_bar *bars = msix->pdev->vpci->header.bars; @@ -163,7 +165,13 @@ static struct vpci_msix *msix_find(const struct domain *d, unsigned long addr) static int cf_check msix_accept(struct vcpu *v, unsigned long addr) { - return !!msix_find(v->domain, addr); + int rc; + + read_lock(&v->domain->pci_lock); + rc = !!msix_find(v->domain, addr); + read_unlock(&v->domain->pci_lock); + + return rc; } static bool access_allowed(const struct pci_dev *pdev, unsigned long addr, @@ -358,21 +366,34 @@ static int adjacent_read(const struct domain *d, const struct vpci_msix *msix, static int cf_check msix_read( struct vcpu *v, unsigned long addr, unsigned int len, unsigned long *data) { - const struct domain *d = v->domain; - struct vpci_msix *msix = msix_find(d, addr); + struct domain *d = v->domain; + struct vpci_msix *msix; const struct vpci_msix_entry *entry; unsigned int offset; *data = ~0ul; + read_lock(&d->pci_lock); + + msix = msix_find(d, addr); if ( !msix ) + { + read_unlock(&d->pci_lock); return X86EMUL_RETRY; + } if ( adjacent_handle(msix, addr) ) - return adjacent_read(d, msix, addr, len, data); + { + int rc = adjacent_read(d, msix, addr, len, data); + read_unlock(&d->pci_lock); + return rc; + } if ( !access_allowed(msix->pdev, addr, len) ) + { + read_unlock(&d->pci_lock); return X86EMUL_OKAY; + } spin_lock(&msix->pdev->vpci->lock); entry = get_entry(msix, addr); @@ -404,6 +425,7 @@ static int cf_check msix_read( break; } spin_unlock(&msix->pdev->vpci->lock); + read_unlock(&d->pci_lock); return X86EMUL_OKAY; } @@ -491,19 +513,32 @@ static int adjacent_write(const struct domain *d, const struct vpci_msix *msix, static int cf_check msix_write( struct vcpu *v, unsigned long addr, unsigned int len, unsigned long data) { - const struct domain *d = v->domain; - struct vpci_msix *msix = msix_find(d, addr); + struct domain *d = v->domain; + struct vpci_msix *msix; struct vpci_msix_entry *entry; unsigned int offset; + read_lock(&d->pci_lock); + + msix = msix_find(d, addr); if ( !msix ) + { + read_unlock(&d->pci_lock); return X86EMUL_RETRY; + } if ( adjacent_handle(msix, addr) ) - return adjacent_write(d, msix, addr, len, data); + { + int rc = adjacent_write(d, msix, addr, len, data); + read_unlock(&d->pci_lock); + return rc; + } if ( !access_allowed(msix->pdev, addr, len) ) + { + read_unlock(&d->pci_lock); return X86EMUL_OKAY; + } spin_lock(&msix->pdev->vpci->lock); entry = get_entry(msix, addr); @@ -579,6 +614,7 @@ static int cf_check msix_write( break; } spin_unlock(&msix->pdev->vpci->lock); + read_unlock(&d->pci_lock); return X86EMUL_OKAY; } @@ -665,6 +701,8 @@ static int cf_check init_msix(struct pci_dev *pdev) struct vpci_msix *msix; int rc; + ASSERT(rw_is_write_locked(&pdev->domain->pci_lock)); + msix_offset = pci_find_cap_offset(pdev->seg, pdev->bus, slot, func, PCI_CAP_ID_MSIX); if ( !msix_offset ) diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c index d73fa76302..f22cbf2112 100644 --- a/xen/drivers/vpci/vpci.c +++ b/xen/drivers/vpci/vpci.c @@ -38,6 +38,8 @@ extern vpci_register_init_t *const __end_vpci_array[]; void vpci_remove_device(struct pci_dev *pdev) { + ASSERT(rw_is_write_locked(&pdev->domain->pci_lock)); + if ( !has_vpci(pdev->domain) || !pdev->vpci ) return; @@ -73,6 +75,8 @@ int vpci_add_handlers(struct pci_dev *pdev) const unsigned long *ro_map; int rc = 0; + ASSERT(rw_is_write_locked(&pdev->domain->pci_lock)); + if ( !has_vpci(pdev->domain) ) return 0; @@ -326,11 +330,12 @@ static uint32_t merge_result(uint32_t data, uint32_t new, unsigned int size, uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int size) { - const struct domain *d = current->domain; + struct domain *d = current->domain; const struct pci_dev *pdev; const struct vpci_register *r; unsigned int data_offset = 0; uint32_t data = ~(uint32_t)0; + rwlock_t *lock; if ( !size ) { @@ -342,11 +347,21 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int size) * Find the PCI dev matching the address, which for hwdom also requires * consulting DomXEN. Passthrough everything that's not trapped. */ + lock = &d->pci_lock; + read_lock(lock); pdev = pci_get_pdev(d, sbdf); if ( !pdev && is_hardware_domain(d) ) + { + read_unlock(lock); + lock = &dom_xen->pci_lock; + read_lock(lock); pdev = pci_get_pdev(dom_xen, sbdf); + } if ( !pdev || !pdev->vpci ) + { + read_unlock(lock); return vpci_read_hw(sbdf, reg, size); + } spin_lock(&pdev->vpci->lock); @@ -392,6 +407,7 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int size) ASSERT(data_offset < size); } spin_unlock(&pdev->vpci->lock); + read_unlock(lock); if ( data_offset < size ) { @@ -431,10 +447,23 @@ static void vpci_write_helper(const struct pci_dev *pdev, r->private); } +/* Helper function to unlock locks taken by vpci_write in proper order */ +static void unlock_locks(struct domain *d) +{ + ASSERT(rw_is_locked(&d->pci_lock)); + + if ( is_hardware_domain(d) ) + { + ASSERT(rw_is_locked(&d->pci_lock)); + read_unlock(&dom_xen->pci_lock); + } + read_unlock(&d->pci_lock); +} + void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size, uint32_t data) { - const struct domain *d = current->domain; + struct domain *d = current->domain; const struct pci_dev *pdev; const struct vpci_register *r; unsigned int data_offset = 0; @@ -447,8 +476,16 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size, /* * Find the PCI dev matching the address, which for hwdom also requires - * consulting DomXEN. Passthrough everything that's not trapped. + * consulting DomXEN. Passthrough everything that's not trapped. + * If this is hwdom, we need to hold locks for both domain in case if + * modify_bars is called() */ + read_lock(&d->pci_lock); + + /* dom_xen->pci_lock always should be taken second to prevent deadlock */ + if ( is_hardware_domain(d) ) + read_lock(&dom_xen->pci_lock); + pdev = pci_get_pdev(d, sbdf); if ( !pdev && is_hardware_domain(d) ) pdev = pci_get_pdev(dom_xen, sbdf); @@ -459,6 +496,8 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size, if ( !ro_map || !test_bit(sbdf.bdf, ro_map) ) vpci_write_hw(sbdf, reg, size, data); + + unlock_locks(d); return; } @@ -498,6 +537,7 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size, ASSERT(data_offset < size); } spin_unlock(&pdev->vpci->lock); + unlock_locks(d); if ( data_offset < size ) /* Tailing gap, write the remaining. */ diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h index 5975ca2f30..4512910dca 100644 --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -157,6 +157,7 @@ struct pci_dev { */ void pcidevs_lock(void); +int pcidevs_trylock(void); void pcidevs_unlock(void); bool_t __must_check pcidevs_locked(void);