linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jon Derrick <jonathan.derrick@intel.com>
To: Joerg Roedel <joro@8bytes.org>, <iommu@lists.linux-foundation.org>
Cc: Bjorn Helgaas <helgaas@kernel.org>, <linux-pci@vger.kernel.org>,
	Lu Baolu <baolu.lu@linux.intel.com>,
	Daniel Drake <drake@endlessm.com>, <linux@endlessm.com>,
	Jon Derrick <jonathan.derrick@intel.com>
Subject: [PATCH 1/1] iommu/vt-d: use DMA domain for real DMA devices and subdevices
Date: Thu,  9 Apr 2020 15:17:36 -0400	[thread overview]
Message-ID: <20200409191736.6233-2-jonathan.derrick@intel.com> (raw)
In-Reply-To: <20200409191736.6233-1-jonathan.derrick@intel.com>

The PCI devices handled by intel-iommu may have a DMA requester on another bus,
such as VMD subdevices needing to use the VMD endpoint.

The real DMA device is now used for the DMA mapping, but one case was missed
earlier: if the VMD device (and hence subdevices too) are under
IOMMU_DOMAIN_IDENTITY, mappings do not work.

Codepaths like intel_map_page() handle the IOMMU_DOMAIN_DMA case by creating an
iommu DMA mapping, and fall back on dma_direct_map_page() for the
IOMMU_DOMAIN_IDENTITY case. However, handling of the IDENTITY case is broken
when intel_page_page() handles a subdevice.

We observe that at iommu attach time, dmar_insert_one_dev_info() for the
subdevices will never set dev->archdata.iommu. This is because that function
uses find_domain() to check if there is already an IOMMU for the device, and
find_domain() then defers to the real DMA device which does have one. Thus
dmar_insert_one_dev_info() returns without assigning dev->archdata.iommu.

Then, later:

1. intel_map_page() checks if an IOMMU mapping is needed by calling
   iommu_need_mapping() on the subdevice. identity_mapping() returns
   false because dev->archdata.iommu is NULL, so this function
   returns false indicating that mapping is needed.
2. __intel_map_single() is called to create the mapping.
3. __intel_map_single() calls find_domain(). This function now returns
   the IDENTITY domain corresponding to the real DMA device.
4. __intel_map_single() calls domain_get_iommu() on this "real" domain.
   A failure is hit and the entire operation is aborted, because this
   codepath is not intended to handle IDENTITY mappings:
       if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
                   return NULL;

This becomes problematic if the real DMA device and the subdevices have
different addressing capabilities and some require translation. Instead we can
put the real DMA dev and any subdevices on the DMA domain. This change assigns
subdevices to the DMA domain, and moves the real DMA device to the DMA domain
if necessary.

Reported-by: Daniel Drake <drake@endlessm.com>
Fixes: b0140c69637e ("iommu/vt-d: Use pci_real_dma_dev() for mapping")
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
Signed-off-by: Daniel Drake <drake@endlessm.com>
---
 drivers/iommu/intel-iommu.c | 56 ++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ef0a5246700e..b4844a502499 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3049,6 +3049,9 @@ static int device_def_domain_type(struct device *dev)
 		if ((iommu_identity_mapping & IDENTMAP_GFX) && IS_GFX_DEVICE(pdev))
 			return IOMMU_DOMAIN_IDENTITY;
 
+		if (pci_real_dma_dev(pdev) != pdev)
+			return IOMMU_DOMAIN_DMA;
+
 		/*
 		 * We want to start off with all devices in the 1:1 domain, and
 		 * take them out later if we find they can't access all of memory.
@@ -5781,12 +5784,32 @@ static bool intel_iommu_capable(enum iommu_cap cap)
 	return false;
 }
 
+static int intel_iommu_request_dma_domain_for_dev(struct device *dev,
+                                                  struct dmar_domain *domain)
+{
+	int ret;
+
+	ret = iommu_request_dma_domain_for_dev(dev);
+	if (ret) {
+		dmar_remove_one_dev_info(dev);
+		domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
+		if (!get_private_domain_for_dev(dev)) {
+			dev_warn(dev,
+				 "Failed to get a private domain.\n");
+				return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
 static int intel_iommu_add_device(struct device *dev)
 {
 	struct dmar_domain *dmar_domain;
 	struct iommu_domain *domain;
 	struct intel_iommu *iommu;
 	struct iommu_group *group;
+	struct device *real_dev = dev;
 	u8 bus, devfn;
 	int ret;
 
@@ -5810,6 +5833,21 @@ static int intel_iommu_add_device(struct device *dev)
 
 	domain = iommu_get_domain_for_dev(dev);
 	dmar_domain = to_dmar_domain(domain);
+
+	if (dev_is_pci(dev))
+		real_dev = &pci_real_dma_dev(to_pci_dev(dev))->dev;
+
+	if (real_dev != dev) {
+		domain = iommu_get_domain_for_dev(real_dev);
+		if (domain->type != IOMMU_DOMAIN_DMA) {
+			dmar_remove_one_dev_info(real_dev);
+
+			ret = intel_iommu_request_dma_domain_for_dev(real_dev, dmar_domain);
+			if (ret)
+				goto unlink;
+		}
+	}
+
 	if (domain->type == IOMMU_DOMAIN_DMA) {
 		if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
 			ret = iommu_request_dm_for_dev(dev);
@@ -5823,20 +5861,12 @@ static int intel_iommu_add_device(struct device *dev)
 		}
 	} else {
 		if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
-			ret = iommu_request_dma_domain_for_dev(dev);
-			if (ret) {
-				dmar_remove_one_dev_info(dev);
-				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
-				if (!get_private_domain_for_dev(dev)) {
-					dev_warn(dev,
-						 "Failed to get a private domain.\n");
-					ret = -ENOMEM;
-					goto unlink;
-				}
+			ret = intel_iommu_request_dma_domain_for_dev(dev, dmar_domain);
+			if (ret)
+				goto unlink;
 
-				dev_info(dev,
-					 "Device uses a private dma domain.\n");
-			}
+			dev_info(dev,
+				 "Device uses a private dma domain.\n");
 		}
 	}
 
-- 
2.18.1


  reply	other threads:[~2020-04-09 19:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-09 19:17 [PATCH 0/1] Real DMA dev DMA domain patch Jon Derrick
2020-04-09 19:17 ` Jon Derrick [this message]
2020-04-10  1:22   ` [PATCH 1/1] iommu/vt-d: use DMA domain for real DMA devices and subdevices Lu Baolu
2020-04-13  2:25     ` Daniel Drake
2020-04-13  2:48       ` Lu Baolu
2020-04-13 16:08         ` Derrick, Jonathan
2020-04-18 12:13         ` Joerg Roedel
2020-04-12  3:50   ` Daniel Drake
2020-04-12  7:35     ` Christoph Hellwig
2020-04-13 16:08     ` Derrick, Jonathan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200409191736.6233-2-jonathan.derrick@intel.com \
    --to=jonathan.derrick@intel.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=drake@endlessm.com \
    --cc=helgaas@kernel.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@endlessm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).