From nobody Fri Apr 19 08:13:11 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=citrix.com ARC-Seal: i=1; a=rsa-sha256; t=1572632952; cv=none; d=zoho.com; s=zohoarc; b=EieUn73AzXnkrQ7c8tWcOUvbkiAO46EG37RsllYOqVFv1pZxPgPl7S4xwsLFdS3Da941KYPWlkV++yUx2njG+3tbS0Ron7Bi3xIBOBIVmUeGhCbYtNc1izNO8dEVzT8y5Nh7WLE80XVPlwcL+hb7MTbVkkqR2qES/ZN1hIg+sCA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1572632952; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=TosYjnHBAmlkHXNmq4SJp5bAOdikQmipSCavn8f25hE=; b=UGLq8UBqEymKZvyMlGqaFYpMxpTBmhg2jK2KEwwTyzXV+vA3PMhRmlp1V/YkMCXZUnygc6xu/DoS3toG8Mf2GVflFogzHLn4vAtHkDGKzHWaAhtgYxJBrznSwFF9zVwUOUbm5b47m9l/89WUf8Ol8y4fZE6zrN9txT7/AK5RfoE= ARC-Authentication-Results: i=1; mx.zoho.com; dkim=fail; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 157263295231946.14731638892795; Fri, 1 Nov 2019 11:29:12 -0700 (PDT) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iQbea-00071f-Pa; Fri, 01 Nov 2019 18:28:08 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iQbeZ-00071U-EN for xen-devel@lists.xenproject.org; Fri, 01 Nov 2019 18:28:07 +0000 Received: from esa2.hc3370-68.iphmx.com (unknown [216.71.145.153]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 58b1234c-fcd5-11e9-a703-bc764e2007e4; Fri, 01 Nov 2019 18:28:06 +0000 (UTC) X-Inumbo-ID: 58b1234c-fcd5-11e9-a703-bc764e2007e4 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1572632887; h=from:to:cc:subject:date:message-id:mime-version; bh=rPaTw/eP8TcQJr1aOIDRIGwoUfa20/D+ax5qWDcPvrk=; b=WjFOzPy+pR3/ykRt2JsmqqAb0g9V14roi3hJ15DWgvGbtH1PNbCXFuka mIp1Ajb/zihMg06LQ69OOV8MSJ0Rb2GXZB57E2ps63sVN0Jf2Ntq7QXsJ wMU36Lymq5c2mYEMvadLbK5Fn8vz5dm9j/85DyefWxmEFSsk/7bgrJmBB U=; Authentication-Results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=igor.druzhinin@citrix.com; spf=Pass smtp.mailfrom=igor.druzhinin@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Received-SPF: None (esa2.hc3370-68.iphmx.com: no sender authenticity information available from domain of igor.druzhinin@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa2.hc3370-68.iphmx.com; envelope-from="igor.druzhinin@citrix.com"; x-sender="igor.druzhinin@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa2.hc3370-68.iphmx.com: domain of igor.druzhinin@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa2.hc3370-68.iphmx.com; envelope-from="igor.druzhinin@citrix.com"; x-sender="igor.druzhinin@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ip4:168.245.78.127 ~all" Received-SPF: None (esa2.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa2.hc3370-68.iphmx.com; envelope-from="igor.druzhinin@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: UvNqXOSfJ6UjCjr6kOKR3HRxjs156NXO0gLuzPU27V7v0XNmHvZz449ybV+fwHPAjRSetFfc1x Lvaq7REGiRlDFFQ3VmiOADK7HthNvDP/IMWY2dl7RCW5hl9Hgpp2Gabm+3X0JVlj6eEUForqp3 XUu3rzCgaZHDFXFH+5AxBZnRePr1po5BPCTlyJuBujXi141VaJj/dDZdW1OX25r8Uc1tIemWpa EaNZt1YaewrZ0L9LNh+aesdzL+WQAomeRJaJPo67VCpOnANyiNzE47TlEzEV+HewO6MTOoQiV8 8UQ= X-SBRS: 2.7 X-MesageID: 7741650 X-Ironport-Server: esa2.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.68,256,1569297600"; d="scan'208";a="7741650" From: Igor Druzhinin To: Date: Fri, 1 Nov 2019 18:28:01 +0000 Message-ID: <1572632881-9050-1-git-send-email-igor.druzhinin@citrix.com> X-Mailer: git-send-email 2.7.4 MIME-Version: 1.0 Subject: [Xen-devel] [PATCH for-4.13 v2] passthrough: simplify locking and logging X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: jgross@suse.com, pdurrant@amazon.com, jbeulich@suse.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) From: Paul Durrant Dropping the pcidevs lock between calling device_assigned() and assign_device() means that the latter has to do the same check as the former for no obvious gain. Also, since long running operations under pcidevs lock already drop the lock and return -ERESTART periodically there is little point in immediately failing an assignment operation with -ERESTART just because the pcidevs lock could not be acquired (for the second time, having already blocked on acquiring the lock in device_assigned()). This patch instead acquires the lock once for assignment (or test assign) operations directly in iommu_do_pci_domctl() and thus can remove the duplicate domain ownership check in assign_device(). Whilst in the neighbourhood, the patch also removes some debug logging from assign_device() and deassign_device() and replaces it with proper error logging, which allows error logging in iommu_do_pci_domctl() to be removed. Also, since device_assigned() can tell the difference between a guest assigned device and a non-existent one, log the actual error condition rather then being ambiguous for the sake a few extra lines of code. Signed-off-by: Paul Durrant --- This is XSA-302 followup and contains some changes important for XenServer. Juergen, could this be considered for 4.13 inclusion? v2: updated Paul's email address --- xen/drivers/passthrough/pci.c | 101 ++++++++++++++++++--------------------= ---- 1 file changed, 42 insertions(+), 59 deletions(-) diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index e64666d..ea0770d 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -932,30 +932,27 @@ static int deassign_device(struct domain *d, uint16_t= seg, uint8_t bus, break; ret =3D hd->platform_ops->reassign_device(d, target, devfn, pci_to_dev(pdev)); - if ( !ret ) - continue; - - printk(XENLOG_G_ERR "%pd: deassign %04x:%02x:%02x.%u failed (%d)\n= ", - d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret); - return ret; + if ( ret ) + goto out; } =20 devfn =3D pdev->devfn; ret =3D hd->platform_ops->reassign_device(d, target, devfn, pci_to_dev(pdev)); if ( ret ) - { - dprintk(XENLOG_G_ERR, - "%pd: deassign device (%04x:%02x:%02x.%u) failed\n", - d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); - return ret; - } + goto out; =20 if ( pdev->domain =3D=3D hardware_domain ) pdev->quarantine =3D false; =20 pdev->fault.count =3D 0; =20 +out: + if ( ret ) + printk(XENLOG_G_ERR + "%pd: deassign device (%04x:%02x:%02x.%u) failed (%d)\n", d, + seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret); + return ret; } =20 @@ -976,10 +973,7 @@ int pci_release_devices(struct domain *d) { bus =3D pdev->bus; devfn =3D pdev->devfn; - if ( deassign_device(d, pdev->seg, bus, devfn) ) - printk("domain %d: deassign device (%04x:%02x:%02x.%u) failed!= \n", - d->domain_id, pdev->seg, bus, - PCI_SLOT(devfn), PCI_FUNC(devfn)); + deassign_device(d, pdev->seg, bus, devfn); } pcidevs_unlock(); =20 @@ -1534,8 +1528,7 @@ static int device_assigned(u16 seg, u8 bus, u8 devfn) struct pci_dev *pdev; int rc =3D 0; =20 - pcidevs_lock(); - + ASSERT(pcidevs_locked()); pdev =3D pci_get_pdev(seg, bus, devfn); =20 if ( !pdev ) @@ -1549,11 +1542,10 @@ static int device_assigned(u16 seg, u8 bus, u8 devf= n) pdev->domain !=3D dom_io ) rc =3D -EBUSY; =20 - pcidevs_unlock(); - return rc; } =20 +/* caller should hold the pcidevs_lock */ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 = flag) { const struct domain_iommu *hd =3D dom_iommu(d); @@ -1571,23 +1563,11 @@ static int assign_device(struct domain *d, u16 seg,= u8 bus, u8 devfn, u32 flag) vm_event_check_ring(d->vm_event_paging)) ) return -EXDEV; =20 - if ( !pcidevs_trylock() ) - return -ERESTART; - + /* device_assigned() should already have cleared the device for assign= ment */ + ASSERT(pcidevs_locked()); pdev =3D pci_get_pdev(seg, bus, devfn); - - rc =3D -ENODEV; - if ( !pdev ) - goto done; - - rc =3D 0; - if ( d =3D=3D pdev->domain ) - goto done; - - rc =3D -EBUSY; - if ( pdev->domain !=3D hardware_domain && - pdev->domain !=3D dom_io ) - goto done; + ASSERT(pdev && (pdev->domain =3D=3D hardware_domain || + pdev->domain =3D=3D dom_io)); =20 if ( pdev->msix ) { @@ -1608,19 +1588,17 @@ static int assign_device(struct domain *d, u16 seg,= u8 bus, u8 devfn, u32 flag) if ( PCI_SLOT(devfn) !=3D PCI_SLOT(pdev->devfn) ) break; rc =3D hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev),= flag); - if ( rc ) - printk(XENLOG_G_WARNING "d%d: assign %04x:%02x:%02x.%u failed = (%d)\n", - d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn= ), - rc); } =20 done: + if ( rc ) + printk(XENLOG_G_ERR + "%pd: assign device (%04x:%02x:%02x.%u) failed (%d)\n", d, + seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), rc); /* The device is assigned to dom_io so mark it as quarantined */ - if ( !rc && d =3D=3D dom_io ) + else if ( d =3D=3D dom_io ) pdev->quarantine =3D true; =20 - pcidevs_unlock(); - return rc; } =20 @@ -1776,29 +1754,40 @@ int iommu_do_pci_domctl( bus =3D PCI_BUS(machine_sbdf); devfn =3D PCI_DEVFN2(machine_sbdf); =20 + pcidevs_lock(); ret =3D device_assigned(seg, bus, devfn); if ( domctl->cmd =3D=3D XEN_DOMCTL_test_assign_device ) { - if ( ret ) + switch ( ret ) { + case 0: + break; + + case -ENODEV: printk(XENLOG_G_INFO - "%04x:%02x:%02x.%u already assigned, or non-existen= t\n", + "%04x:%02x:%02x.%u non-existent\n", seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); ret =3D -EINVAL; + break; + + case -EBUSY: + printk(XENLOG_G_INFO + "%04x:%02x:%02x.%u already assigned\n", + seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); + ret =3D -EINVAL; + break; + + default: + ret =3D -EINVAL; + break; } - break; } - if ( !ret ) + else if ( !ret ) ret =3D assign_device(d, seg, bus, devfn, flags); + pcidevs_unlock(); if ( ret =3D=3D -ERESTART ) ret =3D hypercall_create_continuation(__HYPERVISOR_domctl, "h", u_domctl); - else if ( ret ) - printk(XENLOG_G_ERR - "assign %04x:%02x:%02x.%u to dom%d failed (%d)\n", - seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), - d->domain_id, ret); - break; =20 case XEN_DOMCTL_deassign_device: @@ -1830,12 +1819,6 @@ int iommu_do_pci_domctl( pcidevs_lock(); ret =3D deassign_device(d, seg, bus, devfn); pcidevs_unlock(); - if ( ret ) - printk(XENLOG_G_ERR - "deassign %04x:%02x:%02x.%u from dom%d failed (%d)\n", - seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), - d->domain_id, ret); - break; =20 default: --=20 2.7.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel