From patchwork Wed Jun 8 08:58:55 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Quan Xu X-Patchwork-Id: 9163745 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 03BF260467 for ; Wed, 8 Jun 2016 09:05:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E6AC428345 for ; Wed, 8 Jun 2016 09:05:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DB26128363; Wed, 8 Jun 2016 09:05:47 +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=-4.2 required=2.0 tests=BAYES_00, 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 AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 22AEA2838F for ; Wed, 8 Jun 2016 09:05:46 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bAZOe-00044C-RN; Wed, 08 Jun 2016 09:03:32 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bAZOd-00043r-Sj for xen-devel@lists.xen.org; Wed, 08 Jun 2016 09:03:32 +0000 Received: from [193.109.254.147] by server-12.bemta-14.messagelabs.com id 1C/A1-19610-36FD7575; Wed, 08 Jun 2016 09:03:31 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBLMWRWlGSWpSXmKPExsVywNwkQjfhfni 4wcff0hZLPi5mcWD0OLr7N1MAYxRrZl5SfkUCa8bMJdeYCxZnV1x7epC5gfGAexcjJ4eQQKVE z+f7bCC2hACvxJFlM1ghbH+JtZ+vM0HU1Eh0HL/CDGKzCahIzGh+xw5iiwhIS1z7fJmxi5GLg 1lgAZPE3b4XYA3CAlESh8/uA2tgEVCVaG77xQhi8wo4SvybM48ZYoGCxLIva4FsDg5OASeJrc dKIXY5Smzb/YRlAiPvAkaGVYzqxalFZalFumZ6SUWZ6RkluYmZObqGhiZ6uanFxYnpqTmJScV 6yfm5mxiBocAABDsY/05wPsQoycGkJMqr6B4eLsSXlJ9SmZFYnBFfVJqTWnyIUYaDQ0mCt/Ye UE6wKDU9tSItMwcYlDBpCQ4eJRFeKZA0b3FBYm5xZjpE6hSjLseWBTfWMgmx5OXnpUqJ84qCF AmAFGWU5sGNgEXIJUZZKWFeRqCjhHgKUotyM0tQ5V8xinMwKgnzhoFM4cnMK4Hb9AroCCagI5 YfATuiJBEhJdXAqKrv+dW7JFDGT0dY28cj0n195vXVs45MrjEOkG+68PdTm9JescleW8QqJq1 yvLrH/eNlthoN5u5Lclvmv1TMXyUh+IAzxdLlhqPZLvGWxO92nbwpYbN1563YepX5dvakL5es Fe7Uaf/YMXu1T7ryHwm+OX7vzk45ptl7ba6dgYDLpv9FawPslViKMxINtZiLihMBw1/aj4sCA AA= X-Env-Sender: quan.xu@intel.com X-Msg-Ref: server-9.tower-27.messagelabs.com!1465376607!46437496!1 X-Originating-IP: [192.55.52.88] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTkyLjU1LjUyLjg4ID0+IDM3NDcyNQ==\n X-StarScan-Received: X-StarScan-Version: 8.46; banners=-,-,- X-VirusChecked: Checked Received: (qmail 27393 invoked from network); 8 Jun 2016 09:03:27 -0000 Received: from mga01.intel.com (HELO mga01.intel.com) (192.55.52.88) by server-9.tower-27.messagelabs.com with SMTP; 8 Jun 2016 09:03:27 -0000 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 08 Jun 2016 02:03:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,438,1459839600"; d="scan'208";a="983223882" Received: from xen-commits.sh.intel.com ([10.239.82.178]) by fmsmga001.fm.intel.com with ESMTP; 08 Jun 2016 02:03:25 -0700 From: "Xu, Quan" To: xen-devel@lists.xen.org Date: Wed, 8 Jun 2016 16:58:55 +0800 Message-Id: <1465376344-28290-3-git-send-email-quan.xu@intel.com> X-Mailer: git-send-email 1.8.3.2 In-Reply-To: <1465376344-28290-1-git-send-email-quan.xu@intel.com> References: <1465376344-28290-1-git-send-email-quan.xu@intel.com> Cc: Kevin Tian , Keir Fraser , Jan Beulich , George Dunlap , Andrew Cooper , dario.faggioli@citrix.com, Jun Nakajima , Quan Xu Subject: [Xen-devel] [PATCH v7 02/11] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Quan Xu When IOMMU mapping is failed, we issue a best effort rollback, stopping IOMMU mapping, unmapping the previous IOMMU maps and then reporting the error up to the call trees. When rollback is not feasible (in early initialization phase or trade-off of complexity) for the hardware domain, we do things on a best effort basis, only throwing out an error message. IOMMU unmapping should perhaps continue despite an error, in an attempt to do best effort cleanup. Signed-off-by: Quan Xu CC: Keir Fraser CC: Jan Beulich CC: Andrew Cooper CC: Jun Nakajima CC: Kevin Tian CC: George Dunlap v7: 1. add __must_check annotation to iommu_map_page() / iommu_unmap_page(). 2. use the same code structure for p2m_pt_set_entry() as I do on the EPT side. 3. fix amd_iommu_hwdom_init() / iommu_hwdom_init() here. 4. enhance print infomation. It is no need to mention "hardware domain" in printk of iommu_hwdom_init(). 5. make the assignment be replaced by its right side becoming the variable's initializer. Reviewed-by: Jan Beulich Acked-by: Kevin Tian --- xen/arch/x86/mm.c | 13 ++++++----- xen/arch/x86/mm/p2m-ept.c | 34 +++++++++++++++++++++++------ xen/arch/x86/mm/p2m-pt.c | 23 ++++++++++++++++--- xen/arch/x86/mm/p2m.c | 18 ++++++++++++--- xen/arch/x86/x86_64/mm.c | 4 +++- xen/drivers/passthrough/amd/pci_amd_iommu.c | 15 +++++++++++-- xen/drivers/passthrough/iommu.c | 13 ++++++++++- xen/drivers/passthrough/vtd/x86/vtd.c | 15 +++++++++++-- xen/include/xen/iommu.h | 6 ++--- 9 files changed, 114 insertions(+), 27 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 8d10a3e..ae7c8ab 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2467,7 +2467,7 @@ static int __get_page_type(struct page_info *page, unsigned long type, int preemptible) { unsigned long nx, x, y = page->u.inuse.type_info; - int rc = 0; + int rc = 0, iommu_ret = 0; ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2))); @@ -2578,11 +2578,11 @@ static int __get_page_type(struct page_info *page, unsigned long type, if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) ) { if ( (x & PGT_type_mask) == PGT_writable_page ) - iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page))); + iommu_ret = iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page))); else if ( type == PGT_writable_page ) - iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)), - page_to_mfn(page), - IOMMUF_readable|IOMMUF_writable); + iommu_ret = iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)), + page_to_mfn(page), + IOMMUF_readable|IOMMUF_writable); } } @@ -2599,6 +2599,9 @@ static int __get_page_type(struct page_info *page, unsigned long type, if ( (x & PGT_partial) && !(nx & PGT_partial) ) put_page(page); + if ( !rc ) + rc = iommu_ret; + return rc; } diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 1ed5b47..5aebc24 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -667,6 +667,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, unsigned long gfn_remainder = gfn; unsigned int i, target = order / EPT_TABLE_ORDER; int ret, rc = 0; + bool_t entry_written = 0; bool_t direct_mmio = (p2mt == p2m_mmio_direct); uint8_t ipat = 0; bool_t need_modify_vtd_table = 1; @@ -812,10 +813,15 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, rc = atomic_write_ept_entry(ept_entry, new_entry, target); if ( unlikely(rc) ) old_entry.epte = 0; - else if ( p2mt != p2m_invalid && - (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) ) - /* Track the highest gfn for which we have ever had a valid mapping */ - p2m->max_mapped_pfn = gfn + (1UL << order) - 1; + else + { + entry_written = 1; + + if ( p2mt != p2m_invalid && + (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) ) + /* Track the highest gfn for which we have ever had a valid mapping */ + p2m->max_mapped_pfn = gfn + (1UL << order) - 1; + } out: if ( needs_sync ) @@ -831,10 +837,24 @@ out: { if ( iommu_flags ) for ( i = 0; i < (1 << order); i++ ) - iommu_map_page(d, gfn + i, mfn_x(mfn) + i, iommu_flags); + { + rc = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, iommu_flags); + if ( unlikely(rc) ) + { + while ( i-- ) + if ( iommu_unmap_page(p2m->domain, gfn + i) ) + continue; + + break; + } + } else for ( i = 0; i < (1 << order); i++ ) - iommu_unmap_page(d, gfn + i); + { + ret = iommu_unmap_page(d, gfn + i); + if ( !rc ) + rc = ret; + } } } @@ -847,7 +867,7 @@ out: if ( is_epte_present(&old_entry) ) ept_free_entry(p2m, &old_entry, target); - if ( rc == 0 && p2m_is_hostp2m(p2m) ) + if ( entry_written && p2m_is_hostp2m(p2m) ) p2m_altp2m_propagate_change(d, _gfn(gfn), mfn, order, p2mt, p2ma); return rc; diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index 3d80612..a3870da 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -673,6 +673,8 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, if ( iommu_enabled && need_iommu(p2m->domain) && (iommu_old_flags != iommu_pte_flags || old_mfn != mfn_x(mfn)) ) { + ASSERT(rc == 0); + if ( iommu_use_hap_pt(p2m->domain) ) { if ( iommu_old_flags ) @@ -680,11 +682,26 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, } else if ( iommu_pte_flags ) for ( i = 0; i < (1UL << page_order); i++ ) - iommu_map_page(p2m->domain, gfn + i, mfn_x(mfn) + i, - iommu_pte_flags); + { + rc = iommu_map_page(p2m->domain, gfn + i, mfn_x(mfn) + i, + iommu_pte_flags); + if ( unlikely(rc) ) + { + while ( i-- ) + if ( iommu_unmap_page(p2m->domain, gfn + i) ) + continue; + + break; + } + } else for ( i = 0; i < (1UL << page_order); i++ ) - iommu_unmap_page(p2m->domain, gfn + i); + { + int ret = iommu_unmap_page(p2m->domain, gfn + i); + + if ( !rc ) + rc = ret; + } } /* diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 9b19769..7ce25d2 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -641,10 +641,20 @@ p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn, if ( !paging_mode_translate(p2m->domain) ) { + int rc = 0; + if ( need_iommu(p2m->domain) ) + { for ( i = 0; i < (1 << page_order); i++ ) - iommu_unmap_page(p2m->domain, mfn + i); - return 0; + { + int ret = iommu_unmap_page(p2m->domain, mfn + i); + + if ( !rc ) + rc = ret; + } + } + + return rc; } ASSERT(gfn_locked_by_me(p2m, gfn)); @@ -700,7 +710,9 @@ guest_physmap_add_entry(struct domain *d, unsigned long gfn, if ( rc != 0 ) { while ( i-- > 0 ) - iommu_unmap_page(d, mfn + i); + if ( iommu_unmap_page(d, mfn + i) ) + continue; + return rc; } } diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c index e07e69e..6dab7ff 100644 --- a/xen/arch/x86/x86_64/mm.c +++ b/xen/arch/x86/x86_64/mm.c @@ -1436,7 +1436,9 @@ int memory_add(unsigned long spfn, unsigned long epfn, unsigned int pxm) if ( i != epfn ) { while (i-- > old_max) - iommu_unmap_page(hardware_domain, i); + if ( iommu_unmap_page(hardware_domain, i) ) + continue; + goto destroy_m2p; } } diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index fce9827..4a860af 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -282,6 +282,8 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain *d) if ( !iommu_passthrough && !need_iommu(d) ) { + int rc = 0; + /* Set up 1:1 page table for dom0 */ for ( i = 0; i < max_pdx; i++ ) { @@ -292,12 +294,21 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain *d) * a pfn_valid() check would seem desirable here. */ if ( mfn_valid(pfn) ) - amd_iommu_map_page(d, pfn, pfn, - IOMMUF_readable|IOMMUF_writable); + { + int ret = amd_iommu_map_page(d, pfn, pfn, + IOMMUF_readable|IOMMUF_writable); + + if ( !rc ) + rc = ret; + } if ( !(i & 0xfffff) ) process_pending_softirqs(); } + + if ( rc ) + AMD_IOMMU_DEBUG("d%d: IOMMU mapping failed: %d\n", + d->domain_id, rc); } for_each_amd_iommu ( iommu ) diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index 673e126..ec85352 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -171,20 +171,31 @@ void __hwdom_init iommu_hwdom_init(struct domain *d) { struct page_info *page; unsigned int i = 0; + int rc = 0; + page_list_for_each ( page, &d->page_list ) { unsigned long mfn = page_to_mfn(page); unsigned long gfn = mfn_to_gmfn(d, mfn); unsigned int mapping = IOMMUF_readable; + int ret; if ( ((page->u.inuse.type_info & PGT_count_mask) == 0) || ((page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page) ) mapping |= IOMMUF_writable; - hd->platform_ops->map_page(d, gfn, mfn, mapping); + + ret = hd->platform_ops->map_page(d, gfn, mfn, mapping); + if ( !rc ) + rc = ret; + if ( !(i++ & 0xfffff) ) process_pending_softirqs(); } + + if ( rc ) + printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n", + d->domain_id, rc); } return hd->platform_ops->hwdom_init(d); diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c index c0d6aab..3d67909 100644 --- a/xen/drivers/passthrough/vtd/x86/vtd.c +++ b/xen/drivers/passthrough/vtd/x86/vtd.c @@ -118,6 +118,8 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d) for ( i = 0; i < top; i++ ) { + int rc = 0; + /* * Set up 1:1 mapping for dom0. Default to use only conventional RAM * areas and let RMRRs include needed reserved regions. When set, the @@ -140,8 +142,17 @@ void __hwdom_init vtd_set_hwdom_mapping(struct domain *d) tmp = 1 << (PAGE_SHIFT - PAGE_SHIFT_4K); for ( j = 0; j < tmp; j++ ) - iommu_map_page(d, pfn * tmp + j, pfn * tmp + j, - IOMMUF_readable|IOMMUF_writable); + { + int ret = iommu_map_page(d, pfn * tmp + j, pfn * tmp + j, + IOMMUF_readable|IOMMUF_writable); + + if( !rc ) + rc = ret; + } + + if ( rc ) + printk(XENLOG_WARNING VTDPREFIX " d%d: IOMMU mapping failed: %d\n", + d->domain_id, rc); if (!(i & (0xfffff >> (PAGE_SHIFT - PAGE_SHIFT_4K)))) process_pending_softirqs(); diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 19ba976..eaa2c77 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -74,9 +74,9 @@ void iommu_teardown(struct domain *d); #define IOMMUF_readable (1u<<_IOMMUF_readable) #define _IOMMUF_writable 1 #define IOMMUF_writable (1u<<_IOMMUF_writable) -int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn, - unsigned int flags); -int iommu_unmap_page(struct domain *d, unsigned long gfn); +int __must_check iommu_map_page(struct domain *d, unsigned long gfn, + unsigned long mfn, unsigned int flags); +int __must_check iommu_unmap_page(struct domain *d, unsigned long gfn); enum iommu_feature {