lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 13 Feb 2009 13:27:02 +0800
From:	"Zhao, Yu" <yu.zhao@...el.com>
To:	Matthew Wilcox <matthew@....cx>
CC:	"jbarnes@...tuousgeek.org" <jbarnes@...tuousgeek.org>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 0/6] ATS capability support for Intel IOMMU

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.

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 */
 };

<snipped>

+	info->iommu = iommu;
+	info->qdep = pci_ats_qdep(info->dev);
+	if (!info->qdep)
+		return NULL;

<snipped>

+	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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ