From patchwork Wed Jul 17 01:00:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marek_Marczykowski-G=C3=B3recki?= X-Patchwork-Id: 11046861 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1F2C16C5 for ; Wed, 17 Jul 2019 01:03:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0FA7A286BC for ; Wed, 17 Jul 2019 01:03:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id ECA282872A; Wed, 17 Jul 2019 01:03:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.0 required=2.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C82EB2871E for ; Wed, 17 Jul 2019 01:03:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hnYK2-00054d-Ic; Wed, 17 Jul 2019 01:01:30 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hnYK0-00054G-Vr for xen-devel@lists.xenproject.org; Wed, 17 Jul 2019 01:01:29 +0000 X-Inumbo-ID: 6763abf1-a82e-11e9-8980-bc764e045a96 Received: from new3-smtp.messagingengine.com (unknown [66.111.4.229]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTPS id 6763abf1-a82e-11e9-8980-bc764e045a96; Wed, 17 Jul 2019 01:01:27 +0000 (UTC) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id A45702F83; Tue, 16 Jul 2019 21:01:26 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Tue, 16 Jul 2019 21:01:26 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=AC2Lao6o/QARAgAoAa4syLbv6oH/XwjaYrpSUTCEq wE=; b=Wyfm842xZ1m+dvPR2cxEIrNuoG+0R/3BR/T+XDRutVjK14jVFe6JHuRZd vVlc7OoF9FNaoilMKaELvtQl8ZMnEgXKYzCmkAUGIj4KXc0/nIidXyJjpF5v1pSd fkwRVcGWOOGYbV27jC6zogdKB9AR3VQ92ky6fYTZlUuLCXUjK/16Hz4yQVynQDJk a+TCnv8Mfu0iJKIMH9gsZDhcxOgdJ9QaTUHe7xa8MUK+vm97mT/MGqx0yYC0CNyA w0fTWTo6lJN+nSlb904iKbbkLGfVeLhweqsdWk46xgRRIKXWvVSvEhOIhHmNRdiL Ia0H01oHMxrS6Za2FFb2Lr/EKDHyg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddriedugdegvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkofgjfhggtgfgsehtkeertdertdejnecuhfhrohhmpeforghrvghk ucforghrtgiihihkohifshhkihdqifpkrhgvtghkihcuoehmrghrmhgrrhgvkhesihhnvh hishhisghlvghthhhinhhgshhlrggsrdgtohhmqeenucffohhmrghinhepghhithhhuhgs rdgtohhmnecukfhppeeluddrieehrdefgedrfeefnecurfgrrhgrmhepmhgrihhlfhhroh hmpehmrghrmhgrrhgvkhesihhnvhhishhisghlvghthhhinhhgshhlrggsrdgtohhmnecu vehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from localhost.localdomain (ip5b412221.dynamic.kabel-deutschland.de [91.65.34.33]) by mail.messagingengine.com (Postfix) with ESMTPA id 7F6A0380086; Tue, 16 Jul 2019 21:01:24 -0400 (EDT) From: =?utf-8?q?Marek_Marczykowski-G=C3=B3recki?= To: xen-devel@lists.xenproject.org Date: Wed, 17 Jul 2019 03:00:42 +0200 Message-Id: X-Mailer: git-send-email 2.20.1 In-Reply-To: References: MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v5 4/6] xen/x86: Allow stubdom access to irq created for msi. 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: Kevin Tian , Stefano Stabellini , Suravee Suthikulpanit , Wei Liu , Konrad Rzeszutek Wilk , George Dunlap , Andrew Cooper , Ian Jackson , =?utf-8?q?Marek_Marczykowski-G?= =?utf-8?q?=C3=B3recki?= , Tim Deegan , Simon Gaiser , Julien Grall , Jan Beulich , Brian Woods , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP Stubdomains need to be given sufficient privilege over the guest which it provides emulation for in order for PCI passthrough to work correctly. When a HVM domain try to enable MSI, QEMU in stubdomain calls PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as part of xc_domain_update_msi_irq. Allow for that as part of PHYSDEVOP_map_pirq. This is not needed for PCI INTx, because IRQ in that case is known beforehand and the stubdomain is given permissions over this IRQ by libxl__device_pci_add (there's a do_pci_add against the stubdomain). create_irq() already grant IRQ access to hardware_domain, with assumption the device model (something managing this IRQ) lives there. Modify create_irq() to take additional parameter pointing at device model domain - which may be dom0 or stubdomain. Save ID of the domain given permission, to revoke it in destroy_irq() - easier and cleaner than replaying logic of create_irq() parameter. Use domid instead of actual reference to the domain, because it might get destroyed before destroying IRQ (stubdomain is destroyed before its target domain). And it is not an issue, because IRQ permissions live within domain structure, so destroying a domain also implicitly revoke the permission. Potential domid reuse is detected by by checking if that domain does have permission over the IRQ being destroyed. Then, adjust all callers to provide the parameter. In case of calls not related to stubdomain-initiated allocations, give it either hardware_domain (so the behavior is unchanged there), or NULL for interrupts used by Xen internally. Inspired by https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch by Eric Chanudet . Signed-off-by: Simon Gaiser Signed-off-by: Marek Marczykowski-Górecki --- Changes in v3: - extend commit message Changes in v4: - add missing destroy_irq on error path Changes in v5: - move irq_{grant,revoke}_access() to {create,destroy}_irq(), which basically make it a different patch - add get_dm_domain() helper - do not give hardware_domain permission over IRQs used in Xen internally - rename create_irq argument to just 'd', to avoid confusion when it's called by hardware domain - verify that device is de-assigned before pci_remove_device call - save ID of domain given permission in create_irq(), to revoke it in destroy_irq() - drop domain parameter from destroy_irq() and msi_free_irq() - do not give hardware domain permission over IRQ created in iommu_set_interrupt() --- xen/arch/x86/hpet.c | 3 +- xen/arch/x86/irq.c | 51 ++++++++++++++++++------- xen/common/irq.c | 1 +- xen/drivers/char/ns16550.c | 2 +- xen/drivers/passthrough/amd/iommu_init.c | 2 +- xen/drivers/passthrough/pci.c | 3 +- xen/drivers/passthrough/vtd/iommu.c | 3 +- xen/include/asm-x86/irq.h | 2 +- xen/include/xen/irq.h | 1 +- 9 files changed, 50 insertions(+), 18 deletions(-) diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c index 4b08488..b4854ff 100644 --- a/xen/arch/x86/hpet.c +++ b/xen/arch/x86/hpet.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -368,7 +369,7 @@ static int __init hpet_assign_irq(struct hpet_event_channel *ch) { int irq; - if ( (irq = create_irq(NUMA_NO_NODE)) < 0 ) + if ( (irq = create_irq(NUMA_NO_NODE, hardware_domain)) < 0 ) return irq; ch->msi.irq = irq; diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index 2cac28a..dc5d302 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -164,10 +164,21 @@ int __init bind_irq_vector(int irq, int vector, const cpumask_t *cpu_mask) return ret; } +static struct domain *get_dm_domain(struct domain *d) +{ + return current->domain->target == d ? current->domain : hardware_domain; +} + /* * Dynamic irq allocate and deallocation for MSI */ -int create_irq(nodeid_t node) + +/* + * create_irq - allocate irq for MSI + * @d domain that will get permission over the allocated irq; this permission + * will automatically be revoked on destroy_irq + */ +int create_irq(nodeid_t node, struct domain *d) { int irq, ret; struct irq_desc *desc; @@ -200,18 +211,24 @@ int create_irq(nodeid_t node) desc->arch.used = IRQ_UNUSED; irq = ret; } - else if ( hardware_domain ) + else if ( d ) { - ret = irq_permit_access(hardware_domain, irq); + ASSERT(d == current->domain); + ret = irq_permit_access(d, irq); if ( ret ) printk(XENLOG_G_ERR - "Could not grant Dom0 access to IRQ%d (error %d)\n", - irq, ret); + "Could not grant Dom%u access to IRQ%d (error %d)\n", + d->domain_id, irq, ret); + else + desc->creator_domid = d->domain_id; } return irq; } +/* + * destroy_irq - deallocate irq for MSI + */ void destroy_irq(unsigned int irq) { struct irq_desc *desc = irq_to_desc(irq); @@ -220,14 +237,22 @@ void destroy_irq(unsigned int irq) BUG_ON(!MSI_IRQ(irq)); - if ( hardware_domain ) + if ( desc->creator_domid != DOMID_INVALID ) { - int err = irq_deny_access(hardware_domain, irq); + struct domain *d = get_domain_by_id(desc->creator_domid); - if ( err ) - printk(XENLOG_G_ERR - "Could not revoke Dom0 access to IRQ%u (error %d)\n", - irq, err); + if ( d && irq_access_permitted(d, irq) ) { + int err; + + err = irq_deny_access(d, irq); + if ( err ) + printk(XENLOG_G_ERR + "Could not revoke Dom%u access to IRQ%u (error %d)\n", + d->domain_id, irq, err); + } + + if ( d ) + put_domain(d); } spin_lock_irqsave(&desc->lock, flags); @@ -2058,7 +2083,7 @@ int map_domain_pirq( spin_unlock_irqrestore(&desc->lock, flags); info = NULL; - irq = create_irq(NUMA_NO_NODE); + irq = create_irq(NUMA_NO_NODE, get_dm_domain(d)); ret = irq >= 0 ? prepare_domain_irq_pirq(d, irq, pirq + nr, &info) : irq; if ( ret < 0 ) @@ -2691,7 +2716,7 @@ int allocate_and_map_msi_pirq(struct domain *d, int index, int *pirq_p, if ( irq == -1 ) { case MAP_PIRQ_TYPE_MULTI_MSI: - irq = create_irq(NUMA_NO_NODE); + irq = create_irq(NUMA_NO_NODE, get_dm_domain(d)); } if ( irq < nr_irqs_gsi || irq >= nr_irqs ) diff --git a/xen/common/irq.c b/xen/common/irq.c index f42512d..42b27a9 100644 --- a/xen/common/irq.c +++ b/xen/common/irq.c @@ -16,6 +16,7 @@ int init_one_irq_desc(struct irq_desc *desc) spin_lock_init(&desc->lock); cpumask_setall(desc->affinity); INIT_LIST_HEAD(&desc->rl_link); + desc->creator_domid = DOMID_INVALID; err = arch_init_one_irq_desc(desc); if ( err ) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 189e121..ccc8b04 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -719,7 +719,7 @@ static void __init ns16550_init_irq(struct serial_port *port) struct ns16550 *uart = port->uart; if ( uart->msi ) - uart->irq = create_irq(0); + uart->irq = create_irq(0, NULL); #endif } diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index 4e76b26..50785e0 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -781,7 +781,7 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu) hw_irq_controller *handler; u16 control; - irq = create_irq(NUMA_NO_NODE); + irq = create_irq(NUMA_NO_NODE, NULL); if ( irq <= 0 ) { dprintk(XENLOG_ERR, "IOMMU: no irqs\n"); diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index e886894..507b3d1 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -845,6 +845,9 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn) list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list ) if ( pdev->bus == bus && pdev->devfn == devfn ) { + ret = -EBUSY; + if ( pdev->domain && pdev->domain != hardware_domain ) + break; ret = iommu_remove_device(pdev); if ( pdev->domain ) list_del(&pdev->domain_list); diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 8b27d7e..79f9682 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1138,7 +1138,8 @@ static int __init iommu_set_interrupt(struct acpi_drhd_unit *drhd) struct irq_desc *desc; irq = create_irq(rhsa ? pxm_to_node(rhsa->proximity_domain) - : NUMA_NO_NODE); + : NUMA_NO_NODE, + NULL); if ( irq <= 0 ) { dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no irq available!\n"); diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h index c0c6e7c..5b24428 100644 --- a/xen/include/asm-x86/irq.h +++ b/xen/include/asm-x86/irq.h @@ -155,7 +155,7 @@ int init_irq_data(void); void clear_irq_vector(int irq); int irq_to_vector(int irq); -int create_irq(nodeid_t node); +int create_irq(nodeid_t node, struct domain *d); void destroy_irq(unsigned int irq); int assign_irq_vector(int irq, const cpumask_t *); diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h index 586b783..c7a6a83 100644 --- a/xen/include/xen/irq.h +++ b/xen/include/xen/irq.h @@ -91,6 +91,7 @@ typedef struct irq_desc { spinlock_t lock; struct arch_irq_desc arch; cpumask_var_t affinity; + domid_t creator_domid; /* weak reference to domain handling this IRQ */ /* irq ratelimit */ s_time_t rl_quantum_start;