From patchwork Wed Jun 29 05:59:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Quan Xu X-Patchwork-Id: 9204343 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 547086075F for ; Wed, 29 Jun 2016 06:07:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 42927285B9 for ; Wed, 29 Jun 2016 06:07:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 371D528641; Wed, 29 Jun 2016 06:07:02 +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 5637E285B9 for ; Wed, 29 Jun 2016 06:07:01 +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 1bI8bx-0007kd-Us; Wed, 29 Jun 2016 06:04:33 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bI8bw-0007kF-5K for xen-devel@lists.xen.org; Wed, 29 Jun 2016 06:04:32 +0000 Received: from [193.109.254.147] by server-6.bemta-14.messagelabs.com id 68/C7-30934-FE463775; Wed, 29 Jun 2016 06:04:31 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrFLMWRWlGSWpSXmKPExsXS1tYhofsmpTj cYP0XAYslHxezODB6HN39mymAMYo1My8pvyKBNeP5zMmsBV9qKn7uW8PSwHgopouRg0NIoFLi +S+vLkZODgkBXokjy2awgoQlBPwlzsy1h6iokTjxxQ6kgk1ARWJG8zt2EFtEQFri2ufLjF2MX BzMAtcZJaY2HGUCSQgLeEqsOXSRBcRmEVCVWLJtISuIzSvgKDFzzg1WiFUKEsu+rGUGsTkFnC S27JkMVi8EVPP41Qn2CYy8CxgZVjGqF6cWlaUW6ZrqJRVlpmeU5CZm5ugaGpro5aYWFyemp+Y kJhXrJefnbmIEhgEDEOxgXLfY+RCjJAeTkijvtODicCG+pPyUyozE4oz4otKc1OJDjDIcHEoS vHuTgXKCRanpqRVpmTnAgIRJS3DwKInw/gVJ8xYXJOYWZ6ZDpE4xKkqJ89aAJARAEhmleXBts Ci4xCgrJczLCHSIEE9BalFuZgmq/CtGcQ5GJWHeeyBTeDLzSuCmvwJazAS0mLU6H2RxSSJCSq qB0Tr3fdiswBCBW+V1beqNfUWLLizhVVyza7nhhelzlcP/2X5K8df1f1ErrpUdl7Br2Ty11d+ bv55erpZafrY4lDeiJ1Fj5mGhvoQ/pY65hxSyVK5kZ/tunex7Yvk/xxklip3sWSIpqd62ZYs5 WBelLTXJMmaKrf3/uNSk6aO8ldWM02GpBnOVWIozEg21mIuKEwGma+3cfQIAAA== X-Env-Sender: quan.xu@intel.com X-Msg-Ref: server-7.tower-27.messagelabs.com!1467180263!46065112!2 X-Originating-IP: [134.134.136.24] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTM0LjEzNC4xMzYuMjQgPT4gMzkwOTcx\n X-StarScan-Received: X-StarScan-Version: 8.46; banners=-,-,- X-VirusChecked: Checked Received: (qmail 61038 invoked from network); 29 Jun 2016 06:04:27 -0000 Received: from mga09.intel.com (HELO mga09.intel.com) (134.134.136.24) by server-7.tower-27.messagelabs.com with SMTP; 29 Jun 2016 06:04:27 -0000 Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP; 28 Jun 2016 23:04:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,545,1459839600"; d="scan'208";a="130375084" Received: from xen-commits.sh.intel.com ([10.239.82.178]) by fmsmga004.fm.intel.com with ESMTP; 28 Jun 2016 23:04:22 -0700 From: "Xu, Quan" To: xen-devel@lists.xen.org Date: Wed, 29 Jun 2016 13:59:32 +0800 Message-Id: <1467179974-57317-2-git-send-email-quan.xu@intel.com> X-Mailer: git-send-email 1.8.3.2 In-Reply-To: <1467179974-57317-1-git-send-email-quan.xu@intel.com> References: <1467179974-57317-1-git-send-email-quan.xu@intel.com> Cc: Kevin Tian , Feng Wu , Jan Beulich , dario.faggioli@citrix.com, Suravee Suthikulpanit , Quan Xu Subject: [Xen-devel] [PATCH v13 1/3] IOMMU/x86: use a struct pci_dev* instead of SBDF 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 A struct pci_dev* instead of SBDF is stored inside struct pci_ats_dev and parameter to *_ats_device(). Also use ats_dev for "struct pci_ats_dev" variable, while pdev (_pdev, if there is already a pdev) for "struct pci_dev" in the scope of IOMMU. Signed-off-by: Quan Xu CC: Jan Beulich CC: Kevin Tian CC: Feng Wu CC: Suravee Suthikulpanit Acked-by: Kevin Tian , with one minor comment: --- v13: 1. use ats_dev for "struct pci_ats_dev" variable, while pdev (_pdev, if there is already a pdev) for "struct pci_dev" in the scope of IOMMU. 2. add local variables seg, bus, and devfn, which will greatly reduce the number of changes in *_ats_device(). 3. convert SBDF into struct pci_dev* to disable_ats_device() get_ats_device() as well. ( enable_ats_device() and disable_ats_device() can't have the const added to 'struct pci_dev *pdev', otherwise then compilation fails. also Afaict that would in turn eliminate the need for some of the changes further up. but get_ats_device() can. ) --- xen/drivers/passthrough/amd/iommu_cmd.c | 13 +++--- xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +- xen/drivers/passthrough/ats.h | 10 ++--- xen/drivers/passthrough/vtd/intremap.c | 8 ++-- xen/drivers/passthrough/vtd/iommu.c | 14 +++--- xen/drivers/passthrough/vtd/x86/ats.c | 24 +++++++---- xen/drivers/passthrough/x86/ats.c | 67 ++++++++++++++++++----------- 7 files changed, 81 insertions(+), 59 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c index 7c9d9be..934977a 100644 --- a/xen/drivers/passthrough/amd/iommu_cmd.c +++ b/xen/drivers/passthrough/amd/iommu_cmd.c @@ -294,28 +294,27 @@ void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev, if ( !ats_enabled ) return; - ats_pdev = get_ats_device(pdev->seg, pdev->bus, pdev->devfn); + ats_pdev = get_ats_device(pdev); if ( ats_pdev == NULL ) return; - if ( !pci_ats_enabled(ats_pdev->seg, ats_pdev->bus, ats_pdev->devfn) ) + if ( !pci_ats_enabled(pdev->seg, pdev->bus, pdev->devfn) ) return; - iommu = find_iommu_for_device(ats_pdev->seg, - PCI_BDF2(ats_pdev->bus, ats_pdev->devfn)); + iommu = find_iommu_for_device(pdev->seg, PCI_BDF2(pdev->bus, pdev->devfn)); if ( !iommu ) { AMD_IOMMU_DEBUG("%s: Can't find iommu for %04x:%02x:%02x.%u\n", - __func__, ats_pdev->seg, ats_pdev->bus, - PCI_SLOT(ats_pdev->devfn), PCI_FUNC(ats_pdev->devfn)); + __func__, pdev->seg, pdev->bus, + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); return; } if ( !iommu_has_cap(iommu, PCI_CAP_IOTLB_SHIFT) ) return; - req_id = get_dma_requestor_id(iommu->seg, PCI_BDF2(ats_pdev->bus, devfn)); + req_id = get_dma_requestor_id(iommu->seg, PCI_BDF2(pdev->bus, pdev->devfn)); queueid = req_id; maxpend = ats_pdev->ats_queue_depth & 0xff; diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c index 7761241..dad4a71 100644 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c @@ -162,7 +162,7 @@ static void amd_iommu_setup_domain_device( !pci_ats_enabled(iommu->seg, bus, pdev->devfn) ) { if ( devfn == pdev->devfn ) - enable_ats_device(iommu->seg, bus, devfn, iommu); + enable_ats_device(iommu, pdev); amd_iommu_flush_iotlb(devfn, pdev, INV_IOMMU_ALL_PAGES_ADDRESS, 0); } @@ -356,7 +356,7 @@ void amd_iommu_disable_domain_device(struct domain *domain, if ( devfn == pdev->devfn && pci_ats_device(iommu->seg, bus, devfn) && pci_ats_enabled(iommu->seg, bus, devfn) ) - disable_ats_device(iommu->seg, bus, devfn); + disable_ats_device(pdev); } static int reassign_device(struct domain *source, struct domain *target, diff --git a/xen/drivers/passthrough/ats.h b/xen/drivers/passthrough/ats.h index 5c91572..47ff22d 100644 --- a/xen/drivers/passthrough/ats.h +++ b/xen/drivers/passthrough/ats.h @@ -19,9 +19,7 @@ struct pci_ats_dev { struct list_head list; - u16 seg; - u8 bus; - u8 devfn; + struct pci_dev *pdev; u16 ats_queue_depth; /* ATS device invalidation queue depth */ const void *iommu; /* No common IOMMU struct so use void pointer */ }; @@ -34,9 +32,9 @@ struct pci_ats_dev { extern struct list_head ats_devices; extern bool_t ats_enabled; -int enable_ats_device(int seg, int bus, int devfn, const void *iommu); -void disable_ats_device(int seg, int bus, int devfn); -struct pci_ats_dev *get_ats_device(int seg, int bus, int devfn); +int enable_ats_device(const void *iommu, struct pci_dev *pdev); +void disable_ats_device(struct pci_dev *pdev); +struct pci_ats_dev *get_ats_device(const struct pci_dev *pdev); static inline int pci_ats_enabled(int seg, int bus, int devfn) { diff --git a/xen/drivers/passthrough/vtd/intremap.c b/xen/drivers/passthrough/vtd/intremap.c index 7c11565..28e3cf4 100644 --- a/xen/drivers/passthrough/vtd/intremap.c +++ b/xen/drivers/passthrough/vtd/intremap.c @@ -952,7 +952,7 @@ int pi_update_irte(const struct vcpu *v, const struct pirq *pirq, const struct msi_desc *msi_desc; int remap_index; int rc = 0; - const struct pci_dev *pci_dev; + const struct pci_dev *pdev; const struct acpi_drhd_unit *drhd; struct iommu *iommu; struct ir_ctrl *ir_ctrl; @@ -972,8 +972,8 @@ int pi_update_irte(const struct vcpu *v, const struct pirq *pirq, goto unlock_out; } - pci_dev = msi_desc->dev; - if ( !pci_dev ) + pdev = msi_desc->dev; + if ( !pdev ) { rc = -ENODEV; goto unlock_out; @@ -990,7 +990,7 @@ int pi_update_irte(const struct vcpu *v, const struct pirq *pirq, * 'struct msi_desc' in some other place, so we don't need to waste * time searching it here. */ - drhd = acpi_find_matched_drhd_unit(pci_dev); + drhd = acpi_find_matched_drhd_unit(pdev); if ( !drhd ) return -ENODEV; diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index f010612..cc34497 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1461,8 +1461,8 @@ int domain_context_mapping_one( return rc; } -static int domain_context_mapping( - struct domain *domain, u8 devfn, const struct pci_dev *pdev) +static int domain_context_mapping(struct domain *domain, u8 devfn, + struct pci_dev *pdev) { struct acpi_drhd_unit *drhd; int ret = 0; @@ -1498,7 +1498,7 @@ static int domain_context_mapping( ret = domain_context_mapping_one(domain, drhd->iommu, bus, devfn, pdev); if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 ) - enable_ats_device(seg, bus, devfn, drhd->iommu); + enable_ats_device(drhd->iommu, pdev); break; @@ -1611,8 +1611,8 @@ int domain_context_unmap_one( return rc; } -static int domain_context_unmap( - struct domain *domain, u8 devfn, const struct pci_dev *pdev) +static int domain_context_unmap(struct domain *domain, u8 devfn, + struct pci_dev *pdev) { struct acpi_drhd_unit *drhd; struct iommu *iommu; @@ -1648,7 +1648,7 @@ static int domain_context_unmap( PCI_SLOT(devfn), PCI_FUNC(devfn)); ret = domain_context_unmap_one(domain, iommu, bus, devfn); if ( !ret && devfn == pdev->devfn && ats_device(pdev, drhd) > 0 ) - disable_ats_device(seg, bus, devfn); + disable_ats_device(pdev); break; @@ -1994,7 +1994,7 @@ static int intel_iommu_enable_device(struct pci_dev *pdev) if ( ret <= 0 ) return ret; - ret = enable_ats_device(pdev->seg, pdev->bus, pdev->devfn, drhd->iommu); + ret = enable_ats_device(drhd->iommu, pdev); return ret >= 0 ? 0 : ret; } diff --git a/xen/drivers/passthrough/vtd/x86/ats.c b/xen/drivers/passthrough/vtd/x86/ats.c index dfa4d30..11fe9bb 100644 --- a/xen/drivers/passthrough/vtd/x86/ats.c +++ b/xen/drivers/passthrough/vtd/x86/ats.c @@ -71,12 +71,14 @@ int ats_device(const struct pci_dev *pdev, const struct acpi_drhd_unit *drhd) return pos; } -static int device_in_domain(struct iommu *iommu, struct pci_ats_dev *pdev, u16 did) +static int device_in_domain(struct iommu *iommu, struct pci_ats_dev *ats_dev, u16 did) { struct root_entry *root_entry = NULL; struct context_entry *ctxt_entry = NULL; int tt, found = 0; + struct pci_dev *pdev = ats_dev->pdev; + ASSERT(pdev); root_entry = (struct root_entry *) map_vtd_domain_page(iommu->root_maddr); if ( !root_entry || !root_present(root_entry[pdev->bus]) ) goto out; @@ -108,37 +110,41 @@ out: int dev_invalidate_iotlb(struct iommu *iommu, u16 did, u64 addr, unsigned int size_order, u64 type) { - struct pci_ats_dev *pdev; + struct pci_ats_dev *ats_dev; int ret = 0; if ( !ecap_dev_iotlb(iommu->ecap) ) return ret; - list_for_each_entry( pdev, &ats_devices, list ) + list_for_each_entry( ats_dev, &ats_devices, list ) { - u16 sid = PCI_BDF2(pdev->bus, pdev->devfn); + struct pci_dev *pdev = ats_dev->pdev; + u16 sid; bool_t sbit; int rc = 0; + ASSERT(pdev); + sid = PCI_BDF2(pdev->bus, pdev->devfn); + /* Only invalidate devices that belong to this IOMMU */ - if ( pdev->iommu != iommu ) + if ( ats_dev->iommu != iommu ) continue; switch ( type ) { case DMA_TLB_DSI_FLUSH: - if ( !device_in_domain(iommu, pdev, did) ) + if ( !device_in_domain(iommu, ats_dev, did) ) break; /* fall through if DSI condition met */ case DMA_TLB_GLOBAL_FLUSH: /* invalidate all translations: sbit=1,bit_63=0,bit[62:12]=1 */ sbit = 1; addr = (~0UL << PAGE_SHIFT_4K) & 0x7FFFFFFFFFFFFFFF; - rc = qinval_device_iotlb_sync(iommu, pdev->ats_queue_depth, + rc = qinval_device_iotlb_sync(iommu, ats_dev->ats_queue_depth, sid, sbit, addr); break; case DMA_TLB_PSI_FLUSH: - if ( !device_in_domain(iommu, pdev, did) ) + if ( !device_in_domain(iommu, ats_dev, did) ) break; /* if size <= 4K, set sbit = 0, else set sbit = 1 */ @@ -154,7 +160,7 @@ int dev_invalidate_iotlb(struct iommu *iommu, u16 did, addr |= (((u64)1 << (size_order - 1)) - 1) << PAGE_SHIFT_4K; } - rc = qinval_device_iotlb_sync(iommu, pdev->ats_queue_depth, + rc = qinval_device_iotlb_sync(iommu, ats_dev->ats_queue_depth, sid, sbit, addr); break; default: diff --git a/xen/drivers/passthrough/x86/ats.c b/xen/drivers/passthrough/x86/ats.c index 40c9f40..627b9e3 100644 --- a/xen/drivers/passthrough/x86/ats.c +++ b/xen/drivers/passthrough/x86/ats.c @@ -22,10 +22,12 @@ LIST_HEAD(ats_devices); bool_t __read_mostly ats_enabled = 0; boolean_param("ats", ats_enabled); -int enable_ats_device(int seg, int bus, int devfn, const void *iommu) +int enable_ats_device(const void *iommu, struct pci_dev *pdev) { - struct pci_ats_dev *pdev = NULL; + struct pci_ats_dev *ats_dev = NULL; u32 value; + u16 seg = pdev->seg; + u8 bus = pdev->bus, devfn = pdev->devfn; int pos; pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS); @@ -39,9 +41,14 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu) PCI_FUNC(devfn), pos + ATS_REG_CTL); if ( value & ATS_ENABLE ) { - list_for_each_entry ( pdev, &ats_devices, list ) + list_for_each_entry ( ats_dev, &ats_devices, list ) { - if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn ) + struct pci_dev *_pdev = ats_dev->pdev; + + ASSERT(_pdev); + if ( _pdev->seg == seg && + _pdev->bus == bus && + _pdev->devfn == devfn ) { pos = 0; break; @@ -49,8 +56,8 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu) } } if ( pos ) - pdev = xmalloc(struct pci_ats_dev); - if ( !pdev ) + ats_dev = xmalloc(struct pci_ats_dev); + if ( !ats_dev ) return -ENOMEM; if ( !(value & ATS_ENABLE) ) @@ -62,15 +69,13 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu) if ( pos ) { - pdev->seg = seg; - pdev->bus = bus; - pdev->devfn = devfn; - pdev->iommu = iommu; + ats_dev->pdev = pdev; + ats_dev->iommu = iommu; value = pci_conf_read16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos + ATS_REG_CAP); - pdev->ats_queue_depth = value & ATS_QUEUE_DEPTH_MASK ?: - ATS_QUEUE_DEPTH_MASK + 1; - list_add(&pdev->list, &ats_devices); + ats_dev->ats_queue_depth = value & ATS_QUEUE_DEPTH_MASK ?: + ATS_QUEUE_DEPTH_MASK + 1; + list_add(&ats_dev->list, &ats_devices); } if ( iommu_verbose ) @@ -81,10 +86,12 @@ int enable_ats_device(int seg, int bus, int devfn, const void *iommu) return pos; } -void disable_ats_device(int seg, int bus, int devfn) +void disable_ats_device(struct pci_dev *pdev) { - struct pci_ats_dev *pdev; + struct pci_ats_dev *ats_dev; u32 value; + u16 seg = pdev->seg; + u8 bus = pdev->bus, devfn = pdev->devfn; int pos; pos = pci_find_ext_capability(seg, bus, devfn, PCI_EXT_CAP_ID_ATS); @@ -96,12 +103,17 @@ void disable_ats_device(int seg, int bus, int devfn) pci_conf_write16(seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), pos + ATS_REG_CTL, value); - list_for_each_entry ( pdev, &ats_devices, list ) + list_for_each_entry ( ats_dev, &ats_devices, list ) { - if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn ) + struct pci_dev *_pdev = ats_dev->pdev; + + ASSERT(_pdev); + if ( _pdev->seg == seg && + _pdev->bus == bus && + _pdev->devfn == devfn ) { - list_del(&pdev->list); - xfree(pdev); + list_del(&ats_dev->list); + xfree(ats_dev); break; } } @@ -111,17 +123,24 @@ void disable_ats_device(int seg, int bus, int devfn) seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn)); } -struct pci_ats_dev *get_ats_device(int seg, int bus, int devfn) +struct pci_ats_dev *get_ats_device(const struct pci_dev *pdev) { - struct pci_ats_dev *pdev; + struct pci_ats_dev *ats_dev; + u16 seg = pdev->seg; + u8 bus = pdev->bus, devfn = pdev->devfn; if ( !pci_ats_device(seg, bus, devfn) ) return NULL; - list_for_each_entry ( pdev, &ats_devices, list ) + list_for_each_entry ( ats_dev, &ats_devices, list ) { - if ( pdev->seg == seg && pdev->bus == bus && pdev->devfn == devfn ) - return pdev; + struct pci_dev *_pdev = ats_dev->pdev; + + ASSERT(_pdev); + if ( _pdev->seg == seg && + _pdev->bus == bus && + _pdev->devfn == devfn ) + return ats_dev; } return NULL;