From patchwork Fri Feb 13 05:27:02 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yu Zhao X-Patchwork-Id: 6948 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n1D5RCYT019975 for ; Fri, 13 Feb 2009 05:27:12 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750768AbZBMF1I (ORCPT ); Fri, 13 Feb 2009 00:27:08 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750747AbZBMF1I (ORCPT ); Fri, 13 Feb 2009 00:27:08 -0500 Received: from mga02.intel.com ([134.134.136.20]:13499 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750714AbZBMF1H (ORCPT ); Fri, 13 Feb 2009 00:27:07 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 12 Feb 2009 21:22:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.38,200,1233561600"; d="scan'208";a="386486108" Received: from shxpwyzhao12a.ccr.corp.intel.com (HELO [10.239.13.56]) ([10.239.13.56]) by orsmga002.jf.intel.com with ESMTP; 12 Feb 2009 21:35:55 -0800 Message-ID: <499504A6.3050107@intel.com> Date: Fri, 13 Feb 2009 13:27:02 +0800 From: "Zhao, Yu" User-Agent: Thunderbird 2.0.0.17 (Windows/20080914) MIME-Version: 1.0 To: Matthew Wilcox CC: "jbarnes@virtuousgeek.org" , "dwmw2@infradead.org" , "linux-pci@vger.kernel.org" , "iommu@lists.linux-foundation.org" , "kvm@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 0/6] ATS capability support for Intel IOMMU References: <1234443038-15437-1-git-send-email-yu.zhao@intel.com> <20090213034457.GM3624@parisc-linux.org> In-Reply-To: <20090213034457.GM3624@parisc-linux.org> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Matthew Wilcox wrote: > On Thu, Feb 12, 2009 at 08:50:32PM +0800, Yu Zhao wrote: >> 2, avoid using pci_find_ext_capability every time when reading ATS >> Invalidate Queue Depth (Matthew Wilcox) > > er ... I didn't say it was a problem. I said I couldn't tell if it was a > problem. There's no point in taking up an extra 4 bytes per pci_dev if > it's not a performance problem. How often do we query the queue depth? > Is this something a device driver will call once per device and then > remember for itself, or only use at setup? Or is it something we call > every millisecond? > The query function is called only once per device in v2 patch series. The queue depth is cached in a VT-d private data structure, and it's used when device driver calls pci_unmap_single or pci_unmap_sg. My concern was that packing everything into `pci_dev' would make it grows very fast. For v3, the queue depth is removed from the VT-d `device_domain_info'. Instead, it's cached in `pci_dev', and the query function is called every time when the queue depth is needed. It would be easier to write the IOMMU-specific ATS code for AMD, IBM and other vendors' IOMMUs, if they have same requirement as Intel's (needs to query the queue depth every time when invalidating the IOMMU TLB cache inside the device). So it looks having the queue depth in `pci_dev' is reasonable, but I'm not sure. Following is the logics I used in v2. + info->iommu = iommu; + info->qdep = pci_ats_qdep(info->dev); + if (!info->qdep) + return NULL; + list_for_each_entry(info, &domain->devices, link) { + if (!info->dev || !pci_ats_enabled(info->dev)) + continue; + + sid = info->bus << 8 | info->devfn; + rc = qi_flush_dev_iotlb(info->iommu, sid, + info->qdep, addr, mask); + if (rc) + printk(KERN_ERR "IOMMU: flush device IOTLB failed\n"); + } + spin_unlock_irqrestore(&device_domain_lock, flags); +} --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c index 261b6bd..a7ff7cb 100644 --- a/drivers/pci/intel-iommu.c +++ b/drivers/pci/intel-iommu.c @@ -240,6 +241,8 @@ struct device_domain_info { struct list_head global; /* link to global list */ u8 bus; /* PCI bus numer */ u8 devfn; /* PCI devfn number */ + int qdep; /* invalidate queue depth */ + struct intel_iommu *iommu; /* IOMMU used by this device */ struct pci_dev *dev; /* it's NULL for PCIE-to-PCI bridge */ struct dmar_domain *domain; /* pointer to domain */ };