From patchwork Thu Sep 26 04:05:52 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: 11161849 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C99BE14ED for ; Thu, 26 Sep 2019 04:07:53 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 9825721E6F for ; Thu, 26 Sep 2019 04:07:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="nEykw1h8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9825721E6F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=invisiblethingslab.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDL2n-0005el-ML; Thu, 26 Sep 2019 04:06:17 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iDL2m-0005eg-AN for xen-devel@lists.xenproject.org; Thu, 26 Sep 2019 04:06:16 +0000 X-Inumbo-ID: fbc88046-e012-11e9-9641-12813bfff9fa Received: from new1-smtp.messagingengine.com (unknown [66.111.4.221]) by localhost (Halon) with ESMTPS id fbc88046-e012-11e9-9641-12813bfff9fa; Thu, 26 Sep 2019 04:06:15 +0000 (UTC) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id C8FA42CBD; Thu, 26 Sep 2019 00:06:14 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Thu, 26 Sep 2019 00:06:14 -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=UdghN1TsXpBNyHBdZ97o9G2tt2ZjjViWNoaOE4tuB 0Q=; b=nEykw1h8u5K10fcJrPnsvh+MU5LGVQgVCVoFQeBXWhaygseVr61U6YWWO cibEsueWpMxYBOa4eDqkTVc3HkDeEpAlM2qJNls/CknTEVvbKW8/Ay2tMqtxi1K2 W3qzHrqwODqJP1UJtev47TVcJCfwlh5FLutmETDb8MQXh4lrAHIEVg2/3tHzj1V8 mBPdT62rZZ4HzB4Pzb2ft8i5Q0Ru/Zhq70K59BDJul9263a3sr7VyOKzH7IylPRL xRUQnX6+o6FT5CYXRQi/+qa2mNHx4JjLIMzGy8Vsp9y+Ytp5RlFf5BwdFK4TqBms I8AdY2/69u+oYiNT34dDRhdNhK74A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrfeefgdejkecutefuodetggdotefrodftvf 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 52F5BD6005A; Thu, 26 Sep 2019 00:06:11 -0400 (EDT) From: =?utf-8?q?Marek_Marczykowski-G=C3=B3recki?= To: xen-devel@lists.xenproject.org Date: Thu, 26 Sep 2019 06:05:52 +0200 Message-Id: X-Mailer: git-send-email 2.20.1 In-Reply-To: <7d011094eed3f5c3cf6971cc8760874fd56ca443.1569379186.git-series.marmarek@invisiblethingslab.com> References: <7d011094eed3f5c3cf6971cc8760874fd56ca443.1569379186.git-series.marmarek@invisiblethingslab.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH v7.1 4/4] 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" 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. Give the stubdomain enough permissions over the mapped interrupt in order to bind it successfully to it's target domain. 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 lives there. Modify create_irq() to take additional parameter, whether to grant permissions to the domain creating the IRQ, which may be dom0 or a stubdomain. Do this instead of granting access always to hardware_domain. 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 checking if that domain does have permission over the IRQ being destroyed. Then, adjust all callers to provide the parameter. In case of Xen internal allocations, set it to false, but for domain accessible interrupt set it to true. 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 Reviewed-by: Roger Pau Monné Acked-by: Jan Beulich --- 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() Changes in v6: - do not give permission over hpet irq to hardware_domain - move creator_domid to arch_irq_desc - fix creator_domid initialization - always give current->domain permission instead of using get_dm_domain() helper. Analysis of all its use cases tells that it is the only value it returns. - drop unrelated change Changes in v7: - Code style improvements (spaces, use %pd etc) - use bool parameter to create_irq, as it's only getting current->domain or NULL - remove redundant irq_access_permitted() Changes in v7.1: - adjust comments, merge if - update commit message --- xen/arch/x86/hpet.c | 3 +- xen/arch/x86/irq.c | 42 +++++++++++++++++-------- xen/drivers/char/ns16550.c | 2 +- xen/drivers/passthrough/amd/iommu_init.c | 2 +- xen/drivers/passthrough/vtd/iommu.c | 3 +- xen/include/asm-x86/irq.h | 11 ++++++- 6 files changed, 45 insertions(+), 18 deletions(-) diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c index 4b08488..57f68fa 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, false)) < 0 ) return irq; ch->msi.irq = irq; diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c index 0ee3346..4304896 100644 --- a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c @@ -254,7 +254,8 @@ void __init clear_irq_vector(int irq) /* * Dynamic irq allocate and deallocation for MSI */ -int create_irq(nodeid_t node) + +int create_irq(nodeid_t node, bool grant_access) { int irq, ret; struct irq_desc *desc; @@ -282,18 +283,23 @@ int create_irq(nodeid_t node) } ret = assign_irq_vector(irq, mask); } + + ASSERT(desc->arch.creator_domid == DOMID_INVALID); + if (ret < 0) { desc->arch.used = IRQ_UNUSED; irq = ret; } - else if ( hardware_domain ) + else if ( grant_access ) { - ret = irq_permit_access(hardware_domain, irq); + ret = irq_permit_access(current->domain, irq); if ( ret ) printk(XENLOG_G_ERR - "Could not grant Dom0 access to IRQ%d (error %d)\n", - irq, ret); + "Could not grant %pd access to IRQ%d (error %d)\n", + current->domain, irq, ret); + else + desc->arch.creator_domid = current->domain->domain_id; } return irq; @@ -307,14 +313,23 @@ void destroy_irq(unsigned int irq) BUG_ON(!MSI_IRQ(irq)); - if ( hardware_domain ) + if ( desc->arch.creator_domid != DOMID_INVALID ) { - int err = irq_deny_access(hardware_domain, irq); + struct domain *d = get_domain_by_id(desc->arch.creator_domid); - if ( err ) - printk(XENLOG_G_ERR - "Could not revoke Dom0 access to IRQ%u (error %d)\n", - irq, err); + if ( d ) + { + int err = irq_deny_access(d, irq); + if ( err ) + printk(XENLOG_G_ERR + "Could not revoke %pd access to IRQ%u (error %d)\n", + d, irq, err); + + put_domain(d); + + } + + desc->arch.creator_domid = DOMID_INVALID; } spin_lock_irqsave(&desc->lock, flags); @@ -381,6 +396,7 @@ int arch_init_one_irq_desc(struct irq_desc *desc) desc->arch.vector = IRQ_VECTOR_UNASSIGNED; desc->arch.old_vector = IRQ_VECTOR_UNASSIGNED; + desc->arch.creator_domid = DOMID_INVALID; return 0; } @@ -2133,7 +2149,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, true); ret = irq >= 0 ? prepare_domain_irq_pirq(d, irq, pirq + nr, &info) : irq; if ( ret < 0 ) @@ -2818,7 +2834,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, true); } if ( irq < nr_irqs_gsi || irq >= nr_irqs ) diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index 8667de6..fcd3979 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -722,7 +722,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, false); #endif } diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index bb9f33e..233a8ae 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -765,7 +765,7 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu) { int irq, ret; - irq = create_irq(NUMA_NO_NODE); + irq = create_irq(NUMA_NO_NODE, false); if ( irq <= 0 ) { dprintk(XENLOG_ERR, "IOMMU: no irqs\n"); diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 5d72270..24a1e92 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, + false); 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 bc0c0c1..a75c054 100644 --- a/xen/include/asm-x86/irq.h +++ b/xen/include/asm-x86/irq.h @@ -45,6 +45,11 @@ struct arch_irq_desc { unsigned move_cleanup_count; u8 move_in_progress : 1; s8 used; + /* + * Weak reference to domain having permission over this IRQ (which can + * be different from the domain actually having the IRQ assigned) + */ + domid_t creator_domid; }; /* For use with irq_desc.arch.used */ @@ -161,7 +166,11 @@ int init_irq_data(void); void clear_irq_vector(int irq); int irq_to_vector(int irq); -int create_irq(nodeid_t node); +/* + * If grant_access is set the current domain is given permissions over + * the created IRQ. + */ +int create_irq(nodeid_t node, bool grant_access); void destroy_irq(unsigned int irq); int assign_irq_vector(int irq, const cpumask_t *);