[PATCH v4 00/15] iommu/vt-d: Delegate DMA domain to generic iommu

[PATCH v4 00/15] iommu/vt-d: Delegate DMA domain to generic iommu

[Lu Baolu]

Hi,

This patchset delegates the iommu DMA domain management to the
generic iommu layer. It avoids the use of find_or_alloc_domain
whose domain assignment is inconsistent with the iommu grouping
as determined by pci_device_group.

The major change is to permit domains of type IOMMU_DOMAIN_DMA
and IOMMU_DOMAIN_IDENTITY to be allocated via the iommu_ops api.
This allows the default_domain of an iommu group to be set in
iommu.c. This domain will be attached to every device that is
brought up with an iommu group, and the devices reserved regions
will be mapped using regions returned by get_resv_regions.

The default domain implementation defines a default domain type
and a domain of the default domain type will be allocated and
attached to devices which belong to a same group. Unfortunately,
this doesn't work for some quirky devices which is known to only
work with a specific domain type. PATCH 1/15 adds an iommu ops
which allows the IOMMU driver to request a dma domain if the
default identity domain doesn't match the device. Together with
iommu_request_dm_for_dev(), we can handle the mismatching cases
in a nice way.

Other changes are limited within the Intel IOMMU driver. They
mainly allow the driver to adapt to allocating domains, attaching
domains, applying and direct mapping reserved memory regions,
deferred domain attachment, and so on, via the iommu_ops api's.

This patchset was initiated by James Sewart. The v1 and v2 were
posted here [1] [2] for discussion. Lu Baolu took over the work
for testing and bug fixing with permission from James Sewart.

This version of implementation depends on the patchset of "RMRR
related fixes and enhancements". The latest version is under
discussion at [3]. It allows the iommu_alloc_resv_region() to
be called in a non preemptible section.

Reference:
[1] https://lkml.org/lkml/2019/3/4/644
[2] https://lkml.org/lkml/2019/3/14/299
[3] https://lkml.org/lkml/2019/5/16/237

Best regards,
Lu Baolu

Change log:
 v3->v4:
  - Add code to probe the DMA-capable devices through ACPI
    name space.
  - Remove the callbacks for pci hot-plug devices in Intel
    IOMMU driver.
  - Remove the code to prepare static identity map during
    boot.
  - Add iommu_request_dma_domain_for_dev() to request dma
    domain in case the default identity domain doesn't match
    the device.

 v2->v3:
  - https://lkml.org/lkml/2019/4/28/284
  - Add supported default domain type callback.
  - Make the iommu map() callback work even the domain is not
    attached.
  - Add domain deferred attach when iommu is pre-enabled in
    kdump kernel.

 v1->v2:
  - https://lkml.org/lkml/2019/3/14/299
  - Refactored ISA direct mappings to be returned by
    iommu_get_resv_regions.
  - Integrated patch by Lu to defer turning on DMAR until iommu.c
    has mapped reserved regions.
  - Integrated patches by Lu to remove more unused code in cleanup.

 v1:
  -Original post https://lkml.org/lkml/2019/3/4/644

James Sewart (1):
  iommu/vt-d: Implement apply_resv_region iommu ops entry

Lu Baolu (14):
  iommu: Add API to request DMA domain for device
  iommu/vt-d: Expose ISA direct mapping region via
    iommu_get_resv_regions
  iommu/vt-d: Enable DMA remapping after rmrr mapped
  iommu/vt-d: Add device_def_domain_type() helper
  iommu/vt-d: Delegate the identity domain to upper layer
  iommu/vt-d: Delegate the dma domain to upper layer
  iommu/vt-d: Identify default domains replaced with private
  iommu/vt-d: Handle 32bit device with identity default domain
  iommu/vt-d: Probe DMA-capable ACPI name space devices
  iommu/vt-d: Implement is_attach_deferred iommu ops entry
  iommu/vt-d: Cleanup get_valid_domain_for_dev()
  iommu/vt-d: Remove startup parameter from device_def_domain_type()
  iommu/vt-d: Remove duplicated code for device hotplug
  iommu/vt-d: Remove static identity map code

 drivers/iommu/intel-iommu.c | 608 +++++++++++++++++-------------------
 drivers/iommu/iommu.c       |  36 ++-
 include/linux/intel-iommu.h |   1 -
 include/linux/iommu.h       |   6 +
 4 files changed, 324 insertions(+), 327 deletions(-)

-- 
2.17.1

[PATCH v4 01/15] iommu: Add API to request DMA domain for device

[Lu Baolu]

Normally during iommu probing a device, a default doamin will
be allocated and attached to the device. The domain type of
the default domain is statically defined, which results in a
situation where the allocated default domain isn't suitable
for the device due to some limitations. We already have API
iommu_request_dm_for_dev() to replace a DMA domain with an
identity one. This adds iommu_request_dma_domain_for_dev()
to request a dma domain if an allocated identity domain isn't
suitable for the device in question.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 36 +++++++++++++++++++++++++-----------
 include/linux/iommu.h |  6 ++++++
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c09d60401778..03ba8406d1e4 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1910,10 +1910,10 @@ struct iommu_resv_region *iommu_alloc_resv_region(phys_addr_t start,
 	return region;
 }
 
-/* Request that a device is direct mapped by the IOMMU */
-int iommu_request_dm_for_dev(struct device *dev)
+static int
+request_default_domain_for_dev(struct device *dev, unsigned long type)
 {
-	struct iommu_domain *dm_domain;
+	struct iommu_domain *domain;
 	struct iommu_group *group;
 	int ret;
 
@@ -1926,8 +1926,7 @@ int iommu_request_dm_for_dev(struct device *dev)
 
 	/* Check if the default domain is already direct mapped */
 	ret = 0;
-	if (group->default_domain &&
-	    group->default_domain->type == IOMMU_DOMAIN_IDENTITY)
+	if (group->default_domain && group->default_domain->type == type)
 		goto out;
 
 	/* Don't change mappings of existing devices */
@@ -1937,23 +1936,26 @@ int iommu_request_dm_for_dev(struct device *dev)
 
 	/* Allocate a direct mapped domain */
 	ret = -ENOMEM;
-	dm_domain = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_IDENTITY);
-	if (!dm_domain)
+	domain = __iommu_domain_alloc(dev->bus, type);
+	if (!domain)
 		goto out;
 
 	/* Attach the device to the domain */
-	ret = __iommu_attach_group(dm_domain, group);
+	ret = __iommu_attach_group(domain, group);
 	if (ret) {
-		iommu_domain_free(dm_domain);
+		iommu_domain_free(domain);
 		goto out;
 	}
 
+	iommu_group_create_direct_mappings(group, dev);
+
 	/* Make the direct mapped domain the default for this group */
 	if (group->default_domain)
 		iommu_domain_free(group->default_domain);
-	group->default_domain = dm_domain;
+	group->default_domain = domain;
 
-	dev_info(dev, "Using iommu direct mapping\n");
+	dev_info(dev, "Using iommu %s mapping\n",
+		 type == IOMMU_DOMAIN_DMA ? "dma" : "direct");
 
 	ret = 0;
 out:
@@ -1963,6 +1965,18 @@ int iommu_request_dm_for_dev(struct device *dev)
 	return ret;
 }
 
+/* Request that a device is direct mapped by the IOMMU */
+int iommu_request_dm_for_dev(struct device *dev)
+{
+	return request_default_domain_for_dev(dev, IOMMU_DOMAIN_IDENTITY);
+}
+
+/* Request that a device can't be direct mapped by the IOMMU */
+int iommu_request_dma_domain_for_dev(struct device *dev)
+{
+	return request_default_domain_for_dev(dev, IOMMU_DOMAIN_DMA);
+}
+
 const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 {
 	const struct iommu_ops *ops = NULL;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 14a521f85f14..593a3411d860 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -368,6 +368,7 @@ extern void iommu_set_fault_handler(struct iommu_domain *domain,
 extern void iommu_get_resv_regions(struct device *dev, struct list_head *list);
 extern void iommu_put_resv_regions(struct device *dev, struct list_head *list);
 extern int iommu_request_dm_for_dev(struct device *dev);
+extern int iommu_request_dma_domain_for_dev(struct device *dev);
 extern struct iommu_resv_region *
 iommu_alloc_resv_region(phys_addr_t start, size_t length, int prot,
 			enum iommu_resv_type type, gfp_t flags);
@@ -632,6 +633,11 @@ static inline int iommu_request_dm_for_dev(struct device *dev)
 	return -ENODEV;
 }
 
+static inline int iommu_request_dma_domain_for_dev(struct device *dev)
+{
+	return -ENODEV;
+}
+
 static inline int iommu_attach_group(struct iommu_domain *domain,
 				     struct iommu_group *group)
 {
-- 
2.17.1

[PATCH v4 02/15] iommu/vt-d: Implement apply_resv_region iommu ops entry

[Lu Baolu]

From: James Sewart <jamessewart@arista.com>

Used by iommu.c before creating identity mappings for reserved
ranges to ensure dma-ops won't ever remap these ranges.

Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/iommu/intel-iommu.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 5809185750b4..4d0fa3097f5d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5588,6 +5588,19 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev)
 	return ret;
 }
 
+static void intel_iommu_apply_resv_region(struct device *dev,
+					  struct iommu_domain *domain,
+					  struct iommu_resv_region *region)
+{
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	unsigned long start, end;
+
+	start = IOVA_PFN(region->start);
+	end   = IOVA_PFN(region->start + region->length - 1);
+
+	WARN_ON_ONCE(!reserve_iova(&dmar_domain->iovad, start, end));
+}
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
 {
@@ -5753,6 +5766,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.remove_device		= intel_iommu_remove_device,
 	.get_resv_regions	= intel_iommu_get_resv_regions,
 	.put_resv_regions	= intel_iommu_put_resv_regions,
+	.apply_resv_region	= intel_iommu_apply_resv_region,
 	.device_group		= pci_device_group,
 	.dev_has_feat		= intel_iommu_dev_has_feat,
 	.dev_feat_enabled	= intel_iommu_dev_feat_enabled,
-- 
2.17.1

[PATCH v4 03/15] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions

[Lu Baolu]

To support mapping ISA region via iommu_group_create_direct_mappings,
make sure its exposed by iommu_get_resv_regions.

Signed-off-by: James Sewart <jamessewart@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 4d0fa3097f5d..c42317e27b0c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5520,6 +5520,20 @@ static void intel_iommu_get_resv_regions(struct device *device,
 	}
 	rcu_read_unlock();
 
+#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
+	if (dev_is_pci(device)) {
+		struct pci_dev *pdev = to_pci_dev(device);
+
+		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
+			reg = iommu_alloc_resv_region(0, 1UL << 24, 0,
+						      IOMMU_RESV_DIRECT,
+						      GFP_KERNEL);
+			if (reg)
+				list_add_tail(&reg->list, head);
+		}
+	}
+#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */
+
 	reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
 				      IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1,
 				      0, IOMMU_RESV_MSI, GFP_KERNEL);
-- 
2.17.1

[PATCH v4 04/15] iommu/vt-d: Enable DMA remapping after rmrr mapped

[Lu Baolu]

The rmrr devices require identity map of the rmrr regions before
enabling DMA remapping. Otherwise, there will be a window during
which DMA from/to the rmrr regions will be blocked. In order to
alleviate this, we move enabling DMA remapping after all rmrr
regions get mapped.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index c42317e27b0c..bac226dc8360 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3538,11 +3538,6 @@ static int __init init_dmars(void)
 		ret = dmar_set_interrupt(iommu);
 		if (ret)
 			goto free_iommu;
-
-		if (!translation_pre_enabled(iommu))
-			iommu_enable_translation(iommu);
-
-		iommu_disable_protect_mem_regions(iommu);
 	}
 
 	return 0;
@@ -4922,7 +4917,6 @@ int __init intel_iommu_init(void)
 		goto out_free_reserved_range;
 	}
 	up_write(&dmar_global_lock);
-	pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
 
 #if defined(CONFIG_X86) && defined(CONFIG_SWIOTLB)
 	swiotlb = 0;
@@ -4945,6 +4939,16 @@ int __init intel_iommu_init(void)
 		register_memory_notifier(&intel_iommu_memory_nb);
 	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
 			  intel_iommu_cpu_dead);
+
+	/* Finally, we enable the DMA remapping hardware. */
+	for_each_iommu(iommu, drhd) {
+		if (!translation_pre_enabled(iommu))
+			iommu_enable_translation(iommu);
+
+		iommu_disable_protect_mem_regions(iommu);
+	}
+	pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
+
 	intel_iommu_enabled = 1;
 	intel_iommu_debugfs_init();
 
-- 
2.17.1

[PATCH v4 05/15] iommu/vt-d: Add device_def_domain_type() helper

[Lu Baolu]

This helper returns the default domain type that the device
requires.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 40 +++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index bac226dc8360..e1663af4e01b 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2958,29 +2958,37 @@ static bool device_is_rmrr_locked(struct device *dev)
 	return true;
 }
 
-static int iommu_should_identity_map(struct device *dev, int startup)
+/*
+ * Return the required default domain type for a specific device.
+ *
+ * @dev: the device in query
+ * @startup: true if this is during early boot
+ *
+ * Returns:
+ *  - IOMMU_DOMAIN_DMA: device requires a dynamic mapping domain
+ *  - IOMMU_DOMAIN_IDENTITY: device requires an identical mapping domain
+ *  - 0: both identity and dynamic domains work for this device
+ */
+static int device_def_domain_type(struct device *dev, int startup)
 {
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
 
 		if (device_is_rmrr_locked(dev))
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 
 		/*
 		 * Prevent any device marked as untrusted from getting
 		 * placed into the statically identity mapping domain.
 		 */
 		if (pdev->untrusted)
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 
 		if ((iommu_identity_mapping & IDENTMAP_AZALIA) && IS_AZALIA(pdev))
-			return 1;
+			return IOMMU_DOMAIN_IDENTITY;
 
 		if ((iommu_identity_mapping & IDENTMAP_GFX) && IS_GFX_DEVICE(pdev))
-			return 1;
-
-		if (!(iommu_identity_mapping & IDENTMAP_ALL))
-			return 0;
+			return IOMMU_DOMAIN_IDENTITY;
 
 		/*
 		 * We want to start off with all devices in the 1:1 domain, and
@@ -3001,14 +3009,14 @@ static int iommu_should_identity_map(struct device *dev, int startup)
 		 */
 		if (!pci_is_pcie(pdev)) {
 			if (!pci_is_root_bus(pdev->bus))
-				return 0;
+				return IOMMU_DOMAIN_DMA;
 			if (pdev->class >> 8 == PCI_CLASS_BRIDGE_PCI)
-				return 0;
+				return IOMMU_DOMAIN_DMA;
 		} else if (pci_pcie_type(pdev) == PCI_EXP_TYPE_PCI_BRIDGE)
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 	} else {
 		if (device_has_rmrr(dev))
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 	}
 
 	/*
@@ -3030,7 +3038,13 @@ static int iommu_should_identity_map(struct device *dev, int startup)
 		return dma_mask >= dma_get_required_mask(dev);
 	}
 
-	return 1;
+	return (iommu_identity_mapping & IDENTMAP_ALL) ?
+			IOMMU_DOMAIN_IDENTITY : 0;
+}
+
+static inline int iommu_should_identity_map(struct device *dev, int startup)
+{
+	return device_def_domain_type(dev, startup) == IOMMU_DOMAIN_IDENTITY;
 }
 
 static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw)
-- 
2.17.1

[PATCH v4 06/15] iommu/vt-d: Delegate the identity domain to upper layer

[Lu Baolu]

This allows the iommu generic layer to allocate an identity domain
and attach it to a device. Hence, the identity domain is delegated
to upper layer. As a side effect, iommu_identity_mapping can't be
used to check the existence of identity domains any more.

Signed-off-by: James Sewart <jamessewart@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 90 ++++++++++++++++++++++++-------------
 1 file changed, 58 insertions(+), 32 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e1663af4e01b..907f9323921d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -349,6 +349,7 @@ static void domain_context_clear(struct intel_iommu *iommu,
 				 struct device *dev);
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu);
+static bool device_is_rmrr_locked(struct device *dev);
 
 #ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
 int dmar_disabled = 0;
@@ -2835,7 +2836,9 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width);
 
 static int __init si_domain_init(int hw)
 {
-	int nid, ret;
+	struct dmar_rmrr_unit *rmrr;
+	struct device *dev;
+	int i, nid, ret;
 
 	si_domain = alloc_domain(DOMAIN_FLAG_STATIC_IDENTITY);
 	if (!si_domain)
@@ -2846,8 +2849,6 @@ static int __init si_domain_init(int hw)
 		return -EFAULT;
 	}
 
-	pr_debug("Identity mapping domain allocated\n");
-
 	if (hw)
 		return 0;
 
@@ -2863,6 +2864,31 @@ static int __init si_domain_init(int hw)
 		}
 	}
 
+	/*
+	 * Normally we use DMA domains for devices which have RMRRs. But we
+	 * loose this requirement for graphic and usb devices. Identity map
+	 * the RMRRs for graphic and USB devices so that they could use the
+	 * si_domain.
+	 */
+	for_each_rmrr_units(rmrr) {
+		for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
+					  i, dev) {
+			unsigned long long start = rmrr->base_address;
+			unsigned long long end = rmrr->end_address;
+
+			if (device_is_rmrr_locked(dev))
+				continue;
+
+			if (WARN_ON(end < start ||
+				    end >> agaw_to_width(si_domain->agaw)))
+				continue;
+
+			ret = iommu_domain_identity_map(si_domain, start, end);
+			if (ret)
+				return ret;
+		}
+	}
+
 	return 0;
 }
 
@@ -2870,9 +2896,6 @@ static int identity_mapping(struct device *dev)
 {
 	struct device_domain_info *info;
 
-	if (likely(!iommu_identity_mapping))
-		return 0;
-
 	info = dev->archdata.iommu;
 	if (info && info != DUMMY_DEVICE_DOMAIN_INFO)
 		return (info->domain == si_domain);
@@ -3459,11 +3482,9 @@ static int __init init_dmars(void)
 
 	check_tylersburg_isoch();
 
-	if (iommu_identity_mapping) {
-		ret = si_domain_init(hw_pass_through);
-		if (ret)
-			goto free_iommu;
-	}
+	ret = si_domain_init(hw_pass_through);
+	if (ret)
+		goto free_iommu;
 
 
 	/*
@@ -3656,9 +3677,6 @@ static bool iommu_need_mapping(struct device *dev)
 	if (iommu_dummy(dev))
 		return false;
 
-	if (!iommu_identity_mapping)
-		return true;
-
 	found = identity_mapping(dev);
 	if (found) {
 		if (iommu_should_identity_map(dev, 0))
@@ -5071,32 +5089,40 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 	struct dmar_domain *dmar_domain;
 	struct iommu_domain *domain;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		dmar_domain = alloc_domain(DOMAIN_FLAG_VIRTUAL_MACHINE);
+		if (!dmar_domain) {
+			pr_err("Can't allocate dmar_domain\n");
+			return NULL;
+		}
+		if (md_domain_init(dmar_domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
+			pr_err("Domain initialization failed\n");
+			domain_exit(dmar_domain);
+			return NULL;
+		}
+		domain_update_iommu_cap(dmar_domain);
 
-	dmar_domain = alloc_domain(DOMAIN_FLAG_VIRTUAL_MACHINE);
-	if (!dmar_domain) {
-		pr_err("Can't allocate dmar_domain\n");
-		return NULL;
-	}
-	if (md_domain_init(dmar_domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
-		pr_err("Domain initialization failed\n");
-		domain_exit(dmar_domain);
+		domain = &dmar_domain->domain;
+		domain->geometry.aperture_start = 0;
+		domain->geometry.aperture_end   =
+				__DOMAIN_MAX_ADDR(dmar_domain->gaw);
+		domain->geometry.force_aperture = true;
+
+		return domain;
+	case IOMMU_DOMAIN_IDENTITY:
+		return &si_domain->domain;
+	default:
 		return NULL;
 	}
-	domain_update_iommu_cap(dmar_domain);
-
-	domain = &dmar_domain->domain;
-	domain->geometry.aperture_start = 0;
-	domain->geometry.aperture_end   = __DOMAIN_MAX_ADDR(dmar_domain->gaw);
-	domain->geometry.force_aperture = true;
 
-	return domain;
+	return NULL;
 }
 
 static void intel_iommu_domain_free(struct iommu_domain *domain)
 {
-	domain_exit(to_dmar_domain(domain));
+	if (domain != &si_domain->domain)
+		domain_exit(to_dmar_domain(domain));
 }
 
 /*
-- 
2.17.1

[PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

[Lu Baolu]

This allows the iommu generic layer to allocate a dma domain and
attach it to a device through the iommu api's. With all types of
domains being delegated to upper layer, we can remove an internal
flag which was used to distinguish domains mananged internally or
externally.

Signed-off-by: James Sewart <jamessewart@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 71 ++++++++++---------------------------
 1 file changed, 18 insertions(+), 53 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 907f9323921d..d246b4a9ac1d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -302,14 +302,8 @@ static inline void context_clear_entry(struct context_entry *context)
 static struct dmar_domain *si_domain;
 static int hw_pass_through = 1;
 
-/*
- * Domain represents a virtual machine, more than one devices
- * across iommus may be owned in one domain, e.g. kvm guest.
- */
-#define DOMAIN_FLAG_VIRTUAL_MACHINE	(1 << 0)
-
 /* si_domain contains mulitple devices */
-#define DOMAIN_FLAG_STATIC_IDENTITY	(1 << 1)
+#define DOMAIN_FLAG_STATIC_IDENTITY		BIT(0)
 
 #define for_each_domain_iommu(idx, domain)			\
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
@@ -543,22 +537,11 @@ static inline void free_devinfo_mem(void *vaddr)
 	kmem_cache_free(iommu_devinfo_cache, vaddr);
 }
 
-static inline int domain_type_is_vm(struct dmar_domain *domain)
-{
-	return domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE;
-}
-
 static inline int domain_type_is_si(struct dmar_domain *domain)
 {
 	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
 }
 
-static inline int domain_type_is_vm_or_si(struct dmar_domain *domain)
-{
-	return domain->flags & (DOMAIN_FLAG_VIRTUAL_MACHINE |
-				DOMAIN_FLAG_STATIC_IDENTITY);
-}
-
 static inline int domain_pfn_supported(struct dmar_domain *domain,
 				       unsigned long pfn)
 {
@@ -606,7 +589,9 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
 	int iommu_id;
 
 	/* si_domain and vm domain should not get here. */
-	BUG_ON(domain_type_is_vm_or_si(domain));
+	if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
+		return NULL;
+
 	for_each_domain_iommu(iommu_id, domain)
 		break;
 
@@ -1675,7 +1660,6 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 	if (!iommu->domains || !iommu->domain_ids)
 		return;
 
-again:
 	spin_lock_irqsave(&device_domain_lock, flags);
 	list_for_each_entry_safe(info, tmp, &device_domain_list, global) {
 		struct dmar_domain *domain;
@@ -1689,18 +1673,6 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 		domain = info->domain;
 
 		__dmar_remove_one_dev_info(info);
-
-		if (!domain_type_is_vm_or_si(domain)) {
-			/*
-			 * The domain_exit() function  can't be called under
-			 * device_domain_lock, as it takes this lock itself.
-			 * So release the lock here and re-run the loop
-			 * afterwards.
-			 */
-			spin_unlock_irqrestore(&device_domain_lock, flags);
-			domain_exit(domain);
-			goto again;
-		}
 	}
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
@@ -2365,7 +2337,7 @@ static int domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 			  struct scatterlist *sg, unsigned long phys_pfn,
 			  unsigned long nr_pages, int prot)
 {
-	int ret;
+	int iommu_id, ret;
 	struct intel_iommu *iommu;
 
 	/* Do the real mapping first */
@@ -2373,18 +2345,8 @@ static int domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 	if (ret)
 		return ret;
 
-	/* Notify about the new mapping */
-	if (domain_type_is_vm(domain)) {
-		/* VM typed domains can have more than one IOMMUs */
-		int iommu_id;
-
-		for_each_domain_iommu(iommu_id, domain) {
-			iommu = g_iommus[iommu_id];
-			__mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
-		}
-	} else {
-		/* General domains only have one IOMMU */
-		iommu = domain_get_iommu(domain);
+	for_each_domain_iommu(iommu_id, domain) {
+		iommu = g_iommus[iommu_id];
 		__mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
 	}
 
@@ -4619,9 +4581,6 @@ static int device_notifier(struct notifier_block *nb,
 			return 0;
 
 		dmar_remove_one_dev_info(dev);
-		if (!domain_type_is_vm_or_si(domain) &&
-		    list_empty(&domain->devices))
-			domain_exit(domain);
 	} else if (action == BUS_NOTIFY_ADD_DEVICE) {
 		if (iommu_should_identity_map(dev, 1))
 			domain_add_dev_info(si_domain, dev);
@@ -5090,8 +5049,10 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 	struct iommu_domain *domain;
 
 	switch (type) {
+	case IOMMU_DOMAIN_DMA:
+	/* fallthrough */
 	case IOMMU_DOMAIN_UNMANAGED:
-		dmar_domain = alloc_domain(DOMAIN_FLAG_VIRTUAL_MACHINE);
+		dmar_domain = alloc_domain(0);
 		if (!dmar_domain) {
 			pr_err("Can't allocate dmar_domain\n");
 			return NULL;
@@ -5101,6 +5062,14 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 			domain_exit(dmar_domain);
 			return NULL;
 		}
+
+		if (type == IOMMU_DOMAIN_DMA &&
+		    init_iova_flush_queue(&dmar_domain->iovad,
+					  iommu_flush_iova, iova_entry_free)) {
+			pr_warn("iova flush queue initialization failed\n");
+			intel_iommu_strict = 1;
+		}
+
 		domain_update_iommu_cap(dmar_domain);
 
 		domain = &dmar_domain->domain;
@@ -5315,10 +5284,6 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 			rcu_read_lock();
 			dmar_remove_one_dev_info(dev);
 			rcu_read_unlock();
-
-			if (!domain_type_is_vm_or_si(old_domain) &&
-			    list_empty(&old_domain->devices))
-				domain_exit(old_domain);
 		}
 	}
 
-- 
2.17.1

Re: [PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

[Chris Wilson]

Quoting Lu Baolu (2019-05-25 06:41:28)
> This allows the iommu generic layer to allocate a dma domain and
> attach it to a device through the iommu api's. With all types of
> domains being delegated to upper layer, we can remove an internal
> flag which was used to distinguish domains mananged internally or
> externally.

I'm seeing some really strange behaviour with this patch on a 32b
Skylake system (and still present on mainline). Before this patch
everything is peaceful and appears to work correctly. Applying this patch,
and we fail to initialise the GPU with a few DMAR errors reported, e.g.

[   20.279445] DMAR: DRHD: handling fault status reg 3
[   20.279508] DMAR: [DMA Read] Request device [00:02.0] fault addr 8900a000 [fault reason 05] PTE Write access is not set

Setting an identity map for the igfx made the DMAR errors disappear, but
the GPU still failed to initialise.

There's no difference in the DMAR configuration dmesg between working and
the upset patch. And the really strange part is that switching to a 64b
kernel with this patch, it's working.

Any suggestions on what I should look for?
-Chris

Re: [PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

[Lu Baolu]

Hi Chris,

On 2020/8/22 2:33, Chris Wilson wrote:
> Quoting Lu Baolu (2019-05-25 06:41:28)
>> This allows the iommu generic layer to allocate a dma domain and
>> attach it to a device through the iommu api's. With all types of
>> domains being delegated to upper layer, we can remove an internal
>> flag which was used to distinguish domains mananged internally or
>> externally.
> 
> I'm seeing some really strange behaviour with this patch on a 32b
> Skylake system (and still present on mainline). Before this patch
> everything is peaceful and appears to work correctly. Applying this patch,
> and we fail to initialise the GPU with a few DMAR errors reported, e.g.
> 
> [   20.279445] DMAR: DRHD: handling fault status reg 3
> [   20.279508] DMAR: [DMA Read] Request device [00:02.0] fault addr 8900a000 [fault reason 05] PTE Write access is not set
> 
> Setting an identity map for the igfx made the DMAR errors disappear, but
> the GPU still failed to initialise.
> 
> There's no difference in the DMAR configuration dmesg between working and
> the upset patch. And the really strange part is that switching to a 64b
> kernel with this patch, it's working.
> 
> Any suggestions on what I should look for?

Can the patch titled "[PATCH] iommu/intel: Handle 36b addressing for
x86-32" solve this problem?

Best regards,
baolu

Re: [PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

[Chris Wilson]

Quoting Lu Baolu (2020-08-24 07:31:23)
> Hi Chris,
> 
> On 2020/8/22 2:33, Chris Wilson wrote:
> > Quoting Lu Baolu (2019-05-25 06:41:28)
> >> This allows the iommu generic layer to allocate a dma domain and
> >> attach it to a device through the iommu api's. With all types of
> >> domains being delegated to upper layer, we can remove an internal
> >> flag which was used to distinguish domains mananged internally or
> >> externally.
> > 
> > I'm seeing some really strange behaviour with this patch on a 32b
> > Skylake system (and still present on mainline). Before this patch
> > everything is peaceful and appears to work correctly. Applying this patch,
> > and we fail to initialise the GPU with a few DMAR errors reported, e.g.
> > 
> > [   20.279445] DMAR: DRHD: handling fault status reg 3
> > [   20.279508] DMAR: [DMA Read] Request device [00:02.0] fault addr 8900a000 [fault reason 05] PTE Write access is not set
> > 
> > Setting an identity map for the igfx made the DMAR errors disappear, but
> > the GPU still failed to initialise.
> > 
> > There's no difference in the DMAR configuration dmesg between working and
> > the upset patch. And the really strange part is that switching to a 64b
> > kernel with this patch, it's working.
> > 
> > Any suggestions on what I should look for?
> 
> Can the patch titled "[PATCH] iommu/intel: Handle 36b addressing for
> x86-32" solve this problem?

It does. Not sure why, but that mystery I can leave for others.
-Chris

Re: [PATCH v4 07/15] iommu/vt-d: Delegate the dma domain to upper layer

[Lu Baolu]

Hi Chris,

On 8/24/20 4:35 PM, Chris Wilson wrote:
> Quoting Lu Baolu (2020-08-24 07:31:23)
>> Hi Chris,
>>
>> On 2020/8/22 2:33, Chris Wilson wrote:
>>> Quoting Lu Baolu (2019-05-25 06:41:28)
>>>> This allows the iommu generic layer to allocate a dma domain and
>>>> attach it to a device through the iommu api's. With all types of
>>>> domains being delegated to upper layer, we can remove an internal
>>>> flag which was used to distinguish domains mananged internally or
>>>> externally.
>>>
>>> I'm seeing some really strange behaviour with this patch on a 32b
>>> Skylake system (and still present on mainline). Before this patch
>>> everything is peaceful and appears to work correctly. Applying this patch,
>>> and we fail to initialise the GPU with a few DMAR errors reported, e.g.
>>>
>>> [   20.279445] DMAR: DRHD: handling fault status reg 3
>>> [   20.279508] DMAR: [DMA Read] Request device [00:02.0] fault addr 8900a000 [fault reason 05] PTE Write access is not set
>>>
>>> Setting an identity map for the igfx made the DMAR errors disappear, but
>>> the GPU still failed to initialise.
>>>
>>> There's no difference in the DMAR configuration dmesg between working and
>>> the upset patch. And the really strange part is that switching to a 64b
>>> kernel with this patch, it's working.
>>>
>>> Any suggestions on what I should look for?
>>
>> Can the patch titled "[PATCH] iommu/intel: Handle 36b addressing for
>> x86-32" solve this problem?
> 
> It does. Not sure why, but that mystery I can leave for others.

It's caused by left switching 36 bits operation against a 32-bit
integer. Your patch fixes this by converting the integer from unsigned
long to u64. It looks good to me. Thanks!

> -Chris
> 

Best regards,
baolu

[PATCH v4 08/15] iommu/vt-d: Identify default domains replaced with private

[Lu Baolu]

When we put a device into an iommu group, the group's default
domain will be attached to the device. There are some corner
cases where the type (identity or dma) of the default domain
doesn't work for the device and the request of a new default
domain results in failure (e.x. multiple devices have already
existed in the group). In order to be compatible with the past,
we used a private domain. Mark the private domains and disallow
some iommu apis (map/unmap/iova_to_phys) on them.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 64 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d246b4a9ac1d..08da484e01d6 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -305,6 +305,14 @@ static int hw_pass_through = 1;
 /* si_domain contains mulitple devices */
 #define DOMAIN_FLAG_STATIC_IDENTITY		BIT(0)
 
+/*
+ * This is a DMA domain allocated through the iommu domain allocation
+ * interface. But one or more devices belonging to this domain have
+ * been chosen to use a private domain. We should avoid to use the
+ * map/unmap/iova_to_phys APIs on it.
+ */
+#define DOMAIN_FLAG_LOSE_CHILDREN		BIT(1)
+
 #define for_each_domain_iommu(idx, domain)			\
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])
@@ -4978,6 +4986,7 @@ static void domain_context_clear(struct intel_iommu *iommu, struct device *dev)
 
 static void __dmar_remove_one_dev_info(struct device_domain_info *info)
 {
+	struct dmar_domain *domain;
 	struct intel_iommu *iommu;
 	unsigned long flags;
 
@@ -4987,6 +4996,7 @@ static void __dmar_remove_one_dev_info(struct device_domain_info *info)
 		return;
 
 	iommu = info->iommu;
+	domain = info->domain;
 
 	if (info->dev) {
 		if (dev_is_pci(info->dev) && sm_supported(iommu))
@@ -5001,9 +5011,14 @@ static void __dmar_remove_one_dev_info(struct device_domain_info *info)
 	unlink_domain_info(info);
 
 	spin_lock_irqsave(&iommu->lock, flags);
-	domain_detach_iommu(info->domain, iommu);
+	domain_detach_iommu(domain, iommu);
 	spin_unlock_irqrestore(&iommu->lock, flags);
 
+	/* free the private domain */
+	if (domain->flags & DOMAIN_FLAG_LOSE_CHILDREN &&
+	    !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY))
+		domain_exit(info->domain);
+
 	free_devinfo_mem(info);
 }
 
@@ -5330,6 +5345,9 @@ static int intel_iommu_map(struct iommu_domain *domain,
 	int prot = 0;
 	int ret;
 
+	if (dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN)
+		return -EINVAL;
+
 	if (iommu_prot & IOMMU_READ)
 		prot |= DMA_PTE_READ;
 	if (iommu_prot & IOMMU_WRITE)
@@ -5371,6 +5389,8 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain,
 	/* Cope with horrid API which requires us to unmap more than the
 	   size argument if it happens to be a large-page mapping. */
 	BUG_ON(!pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, &level));
+	if (dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN)
+		return 0;
 
 	if (size < VTD_PAGE_SIZE << level_to_offset_bits(level))
 		size = VTD_PAGE_SIZE << level_to_offset_bits(level);
@@ -5402,6 +5422,9 @@ static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
 	int level = 0;
 	u64 phys = 0;
 
+	if (dmar_domain->flags & DOMAIN_FLAG_LOSE_CHILDREN)
+		return 0;
+
 	pte = pfn_to_dma_pte(dmar_domain, iova >> VTD_PAGE_SHIFT, &level);
 	if (pte)
 		phys = dma_pte_addr(pte);
@@ -5457,9 +5480,12 @@ static bool intel_iommu_capable(enum iommu_cap cap)
 
 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;
 	u8 bus, devfn;
+	int ret;
 
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
@@ -5473,6 +5499,42 @@ static int intel_iommu_add_device(struct device *dev)
 		return PTR_ERR(group);
 
 	iommu_group_put(group);
+
+	domain = iommu_get_domain_for_dev(dev);
+	dmar_domain = to_dmar_domain(domain);
+	if (domain->type == IOMMU_DOMAIN_DMA) {
+		if (device_def_domain_type(dev, 1) == IOMMU_DOMAIN_IDENTITY) {
+			ret = iommu_request_dm_for_dev(dev);
+			if (ret) {
+				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
+				domain_add_dev_info(si_domain, dev);
+				dev_info(dev,
+					 "Device uses a private identity domain.\n");
+				return 0;
+			}
+
+			return -ENODEV;
+		}
+	} else {
+		if (device_def_domain_type(dev, 1) == IOMMU_DOMAIN_DMA) {
+			ret = iommu_request_dma_domain_for_dev(dev);
+			if (ret) {
+				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
+				if (!get_valid_domain_for_dev(dev)) {
+					dev_warn(dev,
+						 "Failed to get a private domain.\n");
+					return -ENOMEM;
+				}
+
+				dev_info(dev,
+					 "Device uses a private dma domain.\n");
+				return 0;
+			}
+
+			return -ENODEV;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH v4 09/15] iommu/vt-d: Handle 32bit device with identity default domain

[Lu Baolu]

The iommu driver doesn't know whether the bit width of a PCI
device is sufficient for access to the whole system memory.
Hence, the driver checks this when the driver calls into the
dma APIs. If a device is using an identity domain, but the
bit width is less than the system requirement, we need to use
a dma domain instead. This also applies after we delegated
the domain life cycle management to the upper layer.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 55 +++++++++++++++----------------------
 1 file changed, 22 insertions(+), 33 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 08da484e01d6..b7f5a6390be6 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3012,25 +3012,6 @@ static int device_def_domain_type(struct device *dev, int startup)
 			return IOMMU_DOMAIN_DMA;
 	}
 
-	/*
-	 * At boot time, we don't yet know if devices will be 64-bit capable.
-	 * Assume that they will — if they turn out not to be, then we can
-	 * take them out of the 1:1 domain later.
-	 */
-	if (!startup) {
-		/*
-		 * If the device's dma_mask is less than the system's memory
-		 * size then this is not a candidate for identity mapping.
-		 */
-		u64 dma_mask = *dev->dma_mask;
-
-		if (dev->coherent_dma_mask &&
-		    dev->coherent_dma_mask < dma_mask)
-			dma_mask = dev->coherent_dma_mask;
-
-		return dma_mask >= dma_get_required_mask(dev);
-	}
-
 	return (iommu_identity_mapping & IDENTMAP_ALL) ?
 			IOMMU_DOMAIN_IDENTITY : 0;
 }
@@ -3642,14 +3623,19 @@ struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
 /* Check if the dev needs to go through non-identity map and unmap process.*/
 static bool iommu_need_mapping(struct device *dev)
 {
-	int found;
+	int ret;
 
 	if (iommu_dummy(dev))
 		return false;
 
-	found = identity_mapping(dev);
-	if (found) {
-		if (iommu_should_identity_map(dev, 0))
+	ret = identity_mapping(dev);
+	if (ret) {
+		u64 dma_mask = *dev->dma_mask;
+
+		if (dev->coherent_dma_mask && dev->coherent_dma_mask < dma_mask)
+			dma_mask = dev->coherent_dma_mask;
+
+		if (dma_mask >= dma_get_required_mask(dev))
 			return false;
 
 		/*
@@ -3657,17 +3643,20 @@ static bool iommu_need_mapping(struct device *dev)
 		 * non-identity mapping.
 		 */
 		dmar_remove_one_dev_info(dev);
-		dev_info(dev, "32bit DMA uses non-identity mapping\n");
-	} else {
-		/*
-		 * In case of a detached 64 bit DMA device from vm, the device
-		 * is put into si_domain for identity mapping.
-		 */
-		if (iommu_should_identity_map(dev, 0) &&
-		    !domain_add_dev_info(si_domain, dev)) {
-			dev_info(dev, "64bit DMA uses identity mapping\n");
-			return false;
+		ret = iommu_request_dma_domain_for_dev(dev);
+		if (ret) {
+			struct iommu_domain *domain;
+			struct dmar_domain *dmar_domain;
+
+			domain = iommu_get_domain_for_dev(dev);
+			if (domain) {
+				dmar_domain = to_dmar_domain(domain);
+				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
+			}
+			get_valid_domain_for_dev(dev);
 		}
+
+		dev_info(dev, "32bit DMA uses non-identity mapping\n");
 	}
 
 	return true;
-- 
2.17.1

[PATCH v4 10/15] iommu/vt-d: Probe DMA-capable ACPI name space devices

[Lu Baolu]

Some platforms may support ACPI name-space enumerated devices
that are capable of generating DMA requests. Platforms which
support DMA remapping explicitly declares any such DMA-capable
ACPI name-space devices in the platform through ACPI Name-space
Device Declaration (ANDD) structure and enumerate them through
the Device Scope of the appropriate remapping hardware unit.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 45 +++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b7f5a6390be6..7f9cf9188f5f 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4816,6 +4816,48 @@ static int __init platform_optin_force_iommu(void)
 	return 1;
 }
 
+static int __init probe_acpi_namespace_devices(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	struct device *dev;
+	int i, ret = 0;
+
+	for_each_active_iommu(iommu, drhd) {
+		for_each_active_dev_scope(drhd->devices,
+					  drhd->devices_cnt, i, dev) {
+			struct acpi_device_physical_node *pn;
+			struct iommu_group *group;
+			struct acpi_device *adev;
+
+			if (dev->bus != &acpi_bus_type)
+				continue;
+
+			adev = to_acpi_device(dev);
+			mutex_lock(&adev->physical_node_lock);
+			list_for_each_entry(pn,
+					    &adev->physical_node_list, node) {
+				group = iommu_group_get(pn->dev);
+				if (group) {
+					iommu_group_put(group);
+					continue;
+				}
+
+				pn->dev->bus->iommu_ops = &intel_iommu_ops;
+				ret = iommu_probe_device(pn->dev);
+				if (ret)
+					break;
+			}
+			mutex_unlock(&adev->physical_node_lock);
+
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
 int __init intel_iommu_init(void)
 {
 	int ret = -ENODEV;
@@ -4928,6 +4970,9 @@ int __init intel_iommu_init(void)
 	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
 			  intel_iommu_cpu_dead);
 
+	if (probe_acpi_namespace_devices())
+		pr_warn("ACPI name space devices didn't probe correctly\n");
+
 	/* Finally, we enable the DMA remapping hardware. */
 	for_each_iommu(iommu, drhd) {
 		if (!translation_pre_enabled(iommu))
-- 
2.17.1

Re: [PATCH v4 10/15] iommu/vt-d: Probe DMA-capable ACPI name space devices

[Christoph Hellwig]

On Sat, May 25, 2019 at 01:41:31PM +0800, Lu Baolu wrote:
> Some platforms may support ACPI name-space enumerated devices
> that are capable of generating DMA requests. Platforms which
> support DMA remapping explicitly declares any such DMA-capable
> ACPI name-space devices in the platform through ACPI Name-space
> Device Declaration (ANDD) structure and enumerate them through
> the Device Scope of the appropriate remapping hardware unit.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Isn't this something that should be handled through the IOMMU API so
that it covers other IOMMU types as well?

How does this scheme compare to the one implemented in
drivers/acpi/arm64/iort.c?

Re: [PATCH v4 10/15] iommu/vt-d: Probe DMA-capable ACPI name space devices

[Lu Baolu]

Hi,

On 5/29/19 2:16 PM, Christoph Hellwig wrote:
> On Sat, May 25, 2019 at 01:41:31PM +0800, Lu Baolu wrote:
>> Some platforms may support ACPI name-space enumerated devices
>> that are capable of generating DMA requests. Platforms which
>> support DMA remapping explicitly declares any such DMA-capable
>> ACPI name-space devices in the platform through ACPI Name-space
>> Device Declaration (ANDD) structure and enumerate them through
>> the Device Scope of the appropriate remapping hardware unit.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> 
> Isn't this something that should be handled through the IOMMU API so
> that it covers other IOMMU types as well?
> 
> How does this scheme compare to the one implemented in
> drivers/acpi/arm64/iort.c?
> 

This part of code was added to be compatible with the past.

Yes. I've ever thought about this. But these devices sit on the acpi bus
together with other devices which are not DMA-capable. And on some
platforms these devices exist on the pci bus as well.

Best regards,
Baolu

[PATCH v4 11/15] iommu/vt-d: Implement is_attach_deferred iommu ops entry

[Lu Baolu]

As a domain is now attached to a device earlier, we should
implement the is_attach_deferred call-back and use it to
defer the domain attach from iommu driver init to device
driver init when iommu is pre-enabled in kdump kernel.

Suggested-by: Tom Murphy <tmurphy@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 7f9cf9188f5f..b93d328fcceb 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -352,6 +352,8 @@ static void domain_context_clear(struct intel_iommu *iommu,
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu);
 static bool device_is_rmrr_locked(struct device *dev);
+static int intel_iommu_attach_device(struct iommu_domain *domain,
+				     struct device *dev);
 
 #ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
 int dmar_disabled = 0;
@@ -381,6 +383,7 @@ int intel_iommu_gfx_mapped;
 EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
+#define DEFER_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-2))
 static DEFINE_SPINLOCK(device_domain_lock);
 static LIST_HEAD(device_domain_list);
 
@@ -2434,8 +2437,18 @@ static struct dmar_domain *find_domain(struct device *dev)
 {
 	struct device_domain_info *info;
 
+	if (unlikely(dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO)) {
+		struct iommu_domain *domain;
+
+		dev->archdata.iommu = NULL;
+		domain = iommu_get_domain_for_dev(dev);
+		if (domain)
+			intel_iommu_attach_device(domain, dev);
+	}
+
 	/* No lock here, assumes no domain exit in normal case */
 	info = dev->archdata.iommu;
+
 	if (likely(info))
 		return info->domain;
 	return NULL;
@@ -5527,6 +5540,9 @@ static int intel_iommu_add_device(struct device *dev)
 
 	iommu_device_link(&iommu->iommu, dev);
 
+	if (translation_pre_enabled(iommu))
+		dev->archdata.iommu = DEFER_DEVICE_DOMAIN_INFO;
+
 	group = iommu_group_get_for_dev(dev);
 
 	if (IS_ERR(group))
@@ -5869,6 +5885,12 @@ intel_iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
 			dmar_domain->default_pasid : -EINVAL;
 }
 
+static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain,
+					   struct device *dev)
+{
+	return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -5891,6 +5913,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.dev_feat_enabled	= intel_iommu_dev_feat_enabled,
 	.dev_enable_feat	= intel_iommu_dev_enable_feat,
 	.dev_disable_feat	= intel_iommu_dev_disable_feat,
+	.is_attach_deferred	= intel_iommu_is_attach_deferred,
 	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
-- 
2.17.1

[PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Lu Baolu]

Previously, get_valid_domain_for_dev() is used to retrieve the
DMA domain which has been attached to the device or allocate one
if no domain has been attached yet. As we have delegated the DMA
domain management to upper layer, this function is used purely to
allocate a private DMA domain if the default domain doesn't work
for ths device. Cleanup the code for readability.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 18 ++++++++----------
 include/linux/intel-iommu.h |  1 -
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b93d328fcceb..59cd8b7e793f 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2636,7 +2636,6 @@ static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw)
 	}
 
 out:
-
 	return domain;
 }
 
@@ -3586,16 +3585,17 @@ static unsigned long intel_alloc_iova(struct device *dev,
 	return iova_pfn;
 }
 
-struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
+static struct dmar_domain *get_private_domain_for_dev(struct device *dev)
 {
 	struct dmar_domain *domain, *tmp;
 	struct dmar_rmrr_unit *rmrr;
 	struct device *i_dev;
 	int i, ret;
 
+	/* Device shouldn't be attached by any domains. */
 	domain = find_domain(dev);
 	if (domain)
-		goto out;
+		return NULL;
 
 	domain = find_or_alloc_domain(dev, DEFAULT_DOMAIN_ADDRESS_WIDTH);
 	if (!domain)
@@ -3625,11 +3625,9 @@ struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
 	}
 
 out:
-
 	if (!domain)
 		dev_err(dev, "Allocating domain failed\n");
 
-
 	return domain;
 }
 
@@ -3666,7 +3664,7 @@ static bool iommu_need_mapping(struct device *dev)
 				dmar_domain = to_dmar_domain(domain);
 				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
 			}
-			get_valid_domain_for_dev(dev);
+			get_private_domain_for_dev(dev);
 		}
 
 		dev_info(dev, "32bit DMA uses non-identity mapping\n");
@@ -3688,7 +3686,7 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 
 	BUG_ON(dir == DMA_NONE);
 
-	domain = get_valid_domain_for_dev(dev);
+	domain = find_domain(dev);
 	if (!domain)
 		return DMA_MAPPING_ERROR;
 
@@ -3903,7 +3901,7 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	if (!iommu_need_mapping(dev))
 		return dma_direct_map_sg(dev, sglist, nelems, dir, attrs);
 
-	domain = get_valid_domain_for_dev(dev);
+	domain = find_domain(dev);
 	if (!domain)
 		return 0;
 
@@ -5570,7 +5568,7 @@ static int intel_iommu_add_device(struct device *dev)
 			ret = iommu_request_dma_domain_for_dev(dev);
 			if (ret) {
 				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
-				if (!get_valid_domain_for_dev(dev)) {
+				if (!get_private_domain_for_dev(dev)) {
 					dev_warn(dev,
 						 "Failed to get a private domain.\n");
 					return -ENOMEM;
@@ -5681,7 +5679,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev)
 	u64 ctx_lo;
 	int ret;
 
-	domain = get_valid_domain_for_dev(dev);
+	domain = find_domain(dev);
 	if (!domain)
 		return -EINVAL;
 
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 6925a18a5ca3..ef2d0e2df1f3 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -654,7 +654,6 @@ extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
 
 extern int dmar_ir_support(void);
 
-struct dmar_domain *get_valid_domain_for_dev(struct device *dev);
 void *alloc_pgtable_page(int node);
 void free_pgtable_page(void *vaddr);
 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
-- 
2.17.1

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Alex Williamson]

On Sat, 25 May 2019 13:41:33 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> Previously, get_valid_domain_for_dev() is used to retrieve the
> DMA domain which has been attached to the device or allocate one
> if no domain has been attached yet. As we have delegated the DMA
> domain management to upper layer, this function is used purely to
> allocate a private DMA domain if the default domain doesn't work
> for ths device. Cleanup the code for readability.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel-iommu.c | 18 ++++++++----------
>  include/linux/intel-iommu.h |  1 -
>  2 files changed, 8 insertions(+), 11 deletions(-)

System fails to boot bisected to this commit:

ommit 4ec066c7b1476e0ca66a7acdb575627a5d1a1ee6 (HEAD, refs/bisect/bad)
Author: Lu Baolu <baolu.lu@linux.intel.com>
Date:   Sat May 25 13:41:33 2019 +0800

    iommu/vt-d: Cleanup get_valid_domain_for_dev()

[    6.857697] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[    6.870723] ehci-pci: EHCI PCI platform driver
[    6.879666] ehci-pci 0000:00:1a.0: EHCI Host Controller
[    6.890122] ehci-pci 0000:00:1a.0: new USB bus registered, assigned bus number 1
[    6.904893] ehci-pci 0000:00:1a.0: debug port 2
[    6.913961] ehci-pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
[    6.929953] ehci-pci 0000:00:1a.0: DMAR: 32bit DMA uses non-identity mapping
[    6.944033] WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:600 domain_get_iommu+0x55/0x60
[    6.945017] Modules linked in:
[    6.945017] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc2+ #37
[    6.945017] Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 04/27/2013
[    6.945017] RIP: 0010:domain_get_iommu+0x55/0x60
[    6.945017] Code: c2 01 eb 0b 48 83 c0 01 8b 34 87 85 f6 75 0b 48 63 c8 48 39 c2 75 ed 31 c0 c3 48 c1 e1 03 48 8b 05 e0 f5 44 01 48 8b 04 08 c3 <0f> 0b 31 c0 c3 31 c9 eb eb 66 90 0f 1f 44 00 00 41 55 40 0f b6 f6
[    6.945017] RSP: 0018:ffffbd02418bba98 EFLAGS: 00010293
[    6.945017] RAX: ffffffffffffffff RBX: 0000000000000000 RCX: 0000000000000000
[    6.945017] RDX: 0000000000001000 RSI: 00000000313ea000 RDI: ffff9ce9f3026e00
[    6.945017] RBP: ffff9cea144f80b0 R08: 00000000ffffffff R09: ffff9ce9f13ea000
[    6.945017] R10: 0000000000100000 R11: 0000000000000100 R12: 00000000313ea000
[    6.945017] R13: ffff9ce9f3026e00 R14: 0000000000001000 R15: 00000000313ea000
[    6.945017] FS:  0000000000000000(0000) GS:ffff9ceddf400000(0000) knlGS:0000000000000000
[    6.945017] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    6.945017] CR2: 00007fa72ed421f8 CR3: 000000011b20a001 CR4: 00000000001606f0
[    6.945017] Call Trace:
[    6.945017]  __intel_map_single+0x4e/0x120
[    6.945017]  intel_alloc_coherent+0xa7/0x110
[    6.945017]  dma_pool_alloc+0xd9/0x1e0
[    6.945017]  ehci_qh_alloc+0x55/0x130
[    6.945017]  ehci_setup+0x284/0x7b0
[    6.945017]  ehci_pci_setup+0x8b/0x41d
[    6.945017]  usb_add_hcd.cold.40+0x1c5/0x7b7
[    6.945017]  usb_hcd_pci_probe+0x2a4/0x420
[    6.945017]  local_pci_probe+0x42/0x80
[    6.945017]  pci_device_probe+0x115/0x190
[    6.945017]  really_probe+0xef/0x390
[    6.945017]  driver_probe_device+0xb4/0x100
[    6.945017]  device_driver_attach+0x4f/0x60
[    6.945017]  __driver_attach+0x86/0x140
[    6.945017]  ? device_driver_attach+0x60/0x60
[    6.945017]  bus_for_each_dev+0x77/0xc0
[    6.945017]  ? klist_add_tail+0x3b/0x60
[    6.945017]  bus_add_driver+0x14a/0x1e0
[    6.945017]  ? ehci_hcd_init+0xaa/0xaa
[    6.945017]  ? do_early_param+0x8e/0x8e
[    6.945017]  driver_register+0x6b/0xb0
[    6.945017]  ? ehci_hcd_init+0xaa/0xaa
[    6.945017]  do_one_initcall+0x46/0x1f4
[    6.945017]  ? do_early_param+0x8e/0x8e
[    6.945017]  kernel_init_freeable+0x1ac/0x255
[    6.945017]  ? rest_init+0x9f/0x9f
[    6.945017]  kernel_init+0xa/0x101
[    6.945017]  ret_from_fork+0x35/0x40
[    6.945017] ---[ end trace 1aa3219cfb92242b ]---

Kernel command line includes intel_iommu=on iommu=pt

On the merge commit of Joerg's pull request for 5.3 (6b04014f3f15),
including:

commit c57b260a7d7d60dfbcf794dd9836c1d9fdbf5434
Author: Lu Baolu <baolu.lu@linux.intel.com>
Date:   Wed Jun 12 08:28:46 2019 +0800

    iommu/vt-d: Set domain type for a private domain
    
    Otherwise, domain_get_iommu() will be broken.
    
    Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")


The fail to boot error shows as:

[    7.042371] general protection fault: 0000 [#1] SMP PTI
[    7.042968] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0+ #39
[    7.042968] Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 04/27/2013
[    7.042968] RIP: 0010:domain_context_mapping_one+0x212/0x4c0
[    7.042968] Code: 39 c2 0f 8d fc 01 00 00 48 8b 15 49 4d b5 00 eb 0c 83 e8 01 41 39 c2 0f 84 ea 01 00 00 4d 8b 00 49 81 e0 00 f0 ff ff 49 01 d0 <41> f6 00 03 75 e1 b8 f4 ff ff ff e9 be fe ff ff 44 89 e1 45 89 e0
[    7.042968] RSP: 0018:ffffb021818bb928 EFLAGS: 00010087
[    7.042968] RAX: 0000000000000002 RBX: ffff9d665f057f00 RCX: 0000000000000001
[    7.042968] RDX: ffff9d6240000000 RSI: 0000000000000030 RDI: ffff9d665f057f00
[    7.042968] RBP: 0000000000000003 R08: f0008c563000e000 R09: ffff9d627155a000
[    7.042968] R10: 0000000000000001 R11: 0000000000000804 R12: 0000000000000000
[    7.042968] R13: 0000000000000002 R14: 0000000000000000 R15: ffff9d6285934200
[    7.042968] FS:  0000000000000000(0000) GS:ffff9d665f400000(0000) knlGS:0000000000000000
[    7.042968] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    7.042968] CR2: 00007f99e08a81f8 CR3: 000000040c20a001 CR4: 00000000001606f0
[    7.042968] Call Trace:
[    7.042968]  ? domain_context_mapping_one+0x4c0/0x4c0
[    7.042968]  pci_for_each_dma_alias+0x30/0x140
[    7.042968]  dmar_insert_one_dev_info+0x33f/0x4b0
[    7.042968]  get_private_domain_for_dev.part.75+0x136/0x2f0
[    7.042968]  ? __dmar_remove_one_dev_info+0x106/0x250
[    7.042968]  iommu_need_mapping+0xac/0xc0
[    7.042968]  intel_alloc_coherent+0x20/0x110
[    7.042968]  xhci_mem_init+0x1ba/0xee1
[    7.042968]  ? xhci_dbg_trace+0x6b/0xb0
[    7.042968]  ? recalibrate_cpu_khz+0x10/0x10
[    7.042968]  ? ktime_get+0x36/0xa0
[    7.042968]  ? xhci_pci_suspend+0xd0/0xd0
[    7.042968]  xhci_init+0x81/0x170
[    7.042968]  xhci_gen_setup+0x1fe/0x360
[    7.042968]  xhci_pci_setup+0x4d/0x120
[    7.042968]  usb_add_hcd.cold.40+0x1c5/0x7b7
[    7.042968]  usb_hcd_pci_probe+0x214/0x420
[    7.042968]  xhci_pci_probe+0x29/0x1f0
[    7.042968]  local_pci_probe+0x42/0x80
[    7.042968]  pci_device_probe+0x115/0x190
[    7.042968]  really_probe+0xef/0x390
[    7.042968]  driver_probe_device+0xb4/0x100
[    7.042968]  device_driver_attach+0x4f/0x60
[    7.042968]  __driver_attach+0x86/0x140
[    7.042968]  ? device_driver_attach+0x60/0x60
[    7.042968]  bus_for_each_dev+0x77/0xc0
[    7.042968]  ? klist_add_tail+0x3b/0x60
[    7.042968]  bus_add_driver+0x14a/0x1e0
[    7.042968]  ? xhci_debugfs_create_root+0x20/0x20
[    7.042968]  ? do_early_param+0x8e/0x8e
[    7.042968]  driver_register+0x6b/0xb0
[    7.042968]  ? xhci_debugfs_create_root+0x20/0x20
[    7.042968]  do_one_initcall+0x46/0x1f4
[    7.042968]  ? do_early_param+0x8e/0x8e
[    7.042968]  kernel_init_freeable+0x1ac/0x255
[    7.042968]  ? rest_init+0x9f/0x9f
[    7.042968]  kernel_init+0xa/0x101
[    7.042968]  ret_from_fork+0x35/0x40
[    7.042968] Modules linked in:
[    7.042968] ---[ end trace 6d6cf443442a773d ]---

Thanks,
Alex

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Lu Baolu]

Hi Alex,

On 7/18/19 11:12 AM, Alex Williamson wrote:
> On Sat, 25 May 2019 13:41:33 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
>> Previously, get_valid_domain_for_dev() is used to retrieve the
>> DMA domain which has been attached to the device or allocate one
>> if no domain has been attached yet. As we have delegated the DMA
>> domain management to upper layer, this function is used purely to
>> allocate a private DMA domain if the default domain doesn't work
>> for ths device. Cleanup the code for readability.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/intel-iommu.c | 18 ++++++++----------
>>   include/linux/intel-iommu.h |  1 -
>>   2 files changed, 8 insertions(+), 11 deletions(-)
> 
> System fails to boot bisected to this commit:

Is this the same issue as this https://lkml.org/lkml/2019/7/18/840?

Best regards,
Baolu

> 
> ommit 4ec066c7b1476e0ca66a7acdb575627a5d1a1ee6 (HEAD, refs/bisect/bad)
> Author: Lu Baolu <baolu.lu@linux.intel.com>
> Date:   Sat May 25 13:41:33 2019 +0800
> 
>      iommu/vt-d: Cleanup get_valid_domain_for_dev()
> 
> [    6.857697] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> [    6.870723] ehci-pci: EHCI PCI platform driver
> [    6.879666] ehci-pci 0000:00:1a.0: EHCI Host Controller
> [    6.890122] ehci-pci 0000:00:1a.0: new USB bus registered, assigned bus number 1
> [    6.904893] ehci-pci 0000:00:1a.0: debug port 2
> [    6.913961] ehci-pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
> [    6.929953] ehci-pci 0000:00:1a.0: DMAR: 32bit DMA uses non-identity mapping
> [    6.944033] WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:600 domain_get_iommu+0x55/0x60
> [    6.945017] Modules linked in:
> [    6.945017] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc2+ #37
> [    6.945017] Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 04/27/2013
> [    6.945017] RIP: 0010:domain_get_iommu+0x55/0x60
> [    6.945017] Code: c2 01 eb 0b 48 83 c0 01 8b 34 87 85 f6 75 0b 48 63 c8 48 39 c2 75 ed 31 c0 c3 48 c1 e1 03 48 8b 05 e0 f5 44 01 48 8b 04 08 c3 <0f> 0b 31 c0 c3 31 c9 eb eb 66 90 0f 1f 44 00 00 41 55 40 0f b6 f6
> [    6.945017] RSP: 0018:ffffbd02418bba98 EFLAGS: 00010293
> [    6.945017] RAX: ffffffffffffffff RBX: 0000000000000000 RCX: 0000000000000000
> [    6.945017] RDX: 0000000000001000 RSI: 00000000313ea000 RDI: ffff9ce9f3026e00
> [    6.945017] RBP: ffff9cea144f80b0 R08: 00000000ffffffff R09: ffff9ce9f13ea000
> [    6.945017] R10: 0000000000100000 R11: 0000000000000100 R12: 00000000313ea000
> [    6.945017] R13: ffff9ce9f3026e00 R14: 0000000000001000 R15: 00000000313ea000
> [    6.945017] FS:  0000000000000000(0000) GS:ffff9ceddf400000(0000) knlGS:0000000000000000
> [    6.945017] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    6.945017] CR2: 00007fa72ed421f8 CR3: 000000011b20a001 CR4: 00000000001606f0
> [    6.945017] Call Trace:
> [    6.945017]  __intel_map_single+0x4e/0x120
> [    6.945017]  intel_alloc_coherent+0xa7/0x110
> [    6.945017]  dma_pool_alloc+0xd9/0x1e0
> [    6.945017]  ehci_qh_alloc+0x55/0x130
> [    6.945017]  ehci_setup+0x284/0x7b0
> [    6.945017]  ehci_pci_setup+0x8b/0x41d
> [    6.945017]  usb_add_hcd.cold.40+0x1c5/0x7b7
> [    6.945017]  usb_hcd_pci_probe+0x2a4/0x420
> [    6.945017]  local_pci_probe+0x42/0x80
> [    6.945017]  pci_device_probe+0x115/0x190
> [    6.945017]  really_probe+0xef/0x390
> [    6.945017]  driver_probe_device+0xb4/0x100
> [    6.945017]  device_driver_attach+0x4f/0x60
> [    6.945017]  __driver_attach+0x86/0x140
> [    6.945017]  ? device_driver_attach+0x60/0x60
> [    6.945017]  bus_for_each_dev+0x77/0xc0
> [    6.945017]  ? klist_add_tail+0x3b/0x60
> [    6.945017]  bus_add_driver+0x14a/0x1e0
> [    6.945017]  ? ehci_hcd_init+0xaa/0xaa
> [    6.945017]  ? do_early_param+0x8e/0x8e
> [    6.945017]  driver_register+0x6b/0xb0
> [    6.945017]  ? ehci_hcd_init+0xaa/0xaa
> [    6.945017]  do_one_initcall+0x46/0x1f4
> [    6.945017]  ? do_early_param+0x8e/0x8e
> [    6.945017]  kernel_init_freeable+0x1ac/0x255
> [    6.945017]  ? rest_init+0x9f/0x9f
> [    6.945017]  kernel_init+0xa/0x101
> [    6.945017]  ret_from_fork+0x35/0x40
> [    6.945017] ---[ end trace 1aa3219cfb92242b ]---
> 
> Kernel command line includes intel_iommu=on iommu=pt
> 
> On the merge commit of Joerg's pull request for 5.3 (6b04014f3f15),
> including:
> 
> commit c57b260a7d7d60dfbcf794dd9836c1d9fdbf5434
> Author: Lu Baolu <baolu.lu@linux.intel.com>
> Date:   Wed Jun 12 08:28:46 2019 +0800
> 
>      iommu/vt-d: Set domain type for a private domain
>      
>      Otherwise, domain_get_iommu() will be broken.
>      
>      Fixes: 942067f1b6b97 ("iommu/vt-d: Identify default domains replaced with private")
> 
> 
> The fail to boot error shows as:
> 
> [    7.042371] general protection fault: 0000 [#1] SMP PTI
> [    7.042968] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0+ #39
> [    7.042968] Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 3904 04/27/2013
> [    7.042968] RIP: 0010:domain_context_mapping_one+0x212/0x4c0
> [    7.042968] Code: 39 c2 0f 8d fc 01 00 00 48 8b 15 49 4d b5 00 eb 0c 83 e8 01 41 39 c2 0f 84 ea 01 00 00 4d 8b 00 49 81 e0 00 f0 ff ff 49 01 d0 <41> f6 00 03 75 e1 b8 f4 ff ff ff e9 be fe ff ff 44 89 e1 45 89 e0
> [    7.042968] RSP: 0018:ffffb021818bb928 EFLAGS: 00010087
> [    7.042968] RAX: 0000000000000002 RBX: ffff9d665f057f00 RCX: 0000000000000001
> [    7.042968] RDX: ffff9d6240000000 RSI: 0000000000000030 RDI: ffff9d665f057f00
> [    7.042968] RBP: 0000000000000003 R08: f0008c563000e000 R09: ffff9d627155a000
> [    7.042968] R10: 0000000000000001 R11: 0000000000000804 R12: 0000000000000000
> [    7.042968] R13: 0000000000000002 R14: 0000000000000000 R15: ffff9d6285934200
> [    7.042968] FS:  0000000000000000(0000) GS:ffff9d665f400000(0000) knlGS:0000000000000000
> [    7.042968] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    7.042968] CR2: 00007f99e08a81f8 CR3: 000000040c20a001 CR4: 00000000001606f0
> [    7.042968] Call Trace:
> [    7.042968]  ? domain_context_mapping_one+0x4c0/0x4c0
> [    7.042968]  pci_for_each_dma_alias+0x30/0x140
> [    7.042968]  dmar_insert_one_dev_info+0x33f/0x4b0
> [    7.042968]  get_private_domain_for_dev.part.75+0x136/0x2f0
> [    7.042968]  ? __dmar_remove_one_dev_info+0x106/0x250
> [    7.042968]  iommu_need_mapping+0xac/0xc0
> [    7.042968]  intel_alloc_coherent+0x20/0x110
> [    7.042968]  xhci_mem_init+0x1ba/0xee1
> [    7.042968]  ? xhci_dbg_trace+0x6b/0xb0
> [    7.042968]  ? recalibrate_cpu_khz+0x10/0x10
> [    7.042968]  ? ktime_get+0x36/0xa0
> [    7.042968]  ? xhci_pci_suspend+0xd0/0xd0
> [    7.042968]  xhci_init+0x81/0x170
> [    7.042968]  xhci_gen_setup+0x1fe/0x360
> [    7.042968]  xhci_pci_setup+0x4d/0x120
> [    7.042968]  usb_add_hcd.cold.40+0x1c5/0x7b7
> [    7.042968]  usb_hcd_pci_probe+0x214/0x420
> [    7.042968]  xhci_pci_probe+0x29/0x1f0
> [    7.042968]  local_pci_probe+0x42/0x80
> [    7.042968]  pci_device_probe+0x115/0x190
> [    7.042968]  really_probe+0xef/0x390
> [    7.042968]  driver_probe_device+0xb4/0x100
> [    7.042968]  device_driver_attach+0x4f/0x60
> [    7.042968]  __driver_attach+0x86/0x140
> [    7.042968]  ? device_driver_attach+0x60/0x60
> [    7.042968]  bus_for_each_dev+0x77/0xc0
> [    7.042968]  ? klist_add_tail+0x3b/0x60
> [    7.042968]  bus_add_driver+0x14a/0x1e0
> [    7.042968]  ? xhci_debugfs_create_root+0x20/0x20
> [    7.042968]  ? do_early_param+0x8e/0x8e
> [    7.042968]  driver_register+0x6b/0xb0
> [    7.042968]  ? xhci_debugfs_create_root+0x20/0x20
> [    7.042968]  do_one_initcall+0x46/0x1f4
> [    7.042968]  ? do_early_param+0x8e/0x8e
> [    7.042968]  kernel_init_freeable+0x1ac/0x255
> [    7.042968]  ? rest_init+0x9f/0x9f
> [    7.042968]  kernel_init+0xa/0x101
> [    7.042968]  ret_from_fork+0x35/0x40
> [    7.042968] Modules linked in:
> [    7.042968] ---[ end trace 6d6cf443442a773d ]---
> 
> Thanks,
> Alex
> 

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Alex Williamson]

On Fri, 19 Jul 2019 17:04:26 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> Hi Alex,
> 
> On 7/18/19 11:12 AM, Alex Williamson wrote:
> > On Sat, 25 May 2019 13:41:33 +0800
> > Lu Baolu <baolu.lu@linux.intel.com> wrote:
> >   
> >> Previously, get_valid_domain_for_dev() is used to retrieve the
> >> DMA domain which has been attached to the device or allocate one
> >> if no domain has been attached yet. As we have delegated the DMA
> >> domain management to upper layer, this function is used purely to
> >> allocate a private DMA domain if the default domain doesn't work
> >> for ths device. Cleanup the code for readability.
> >>
> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >> ---
> >>   drivers/iommu/intel-iommu.c | 18 ++++++++----------
> >>   include/linux/intel-iommu.h |  1 -
> >>   2 files changed, 8 insertions(+), 11 deletions(-)  
> > 
> > System fails to boot bisected to this commit:  
> 
> Is this the same issue as this https://lkml.org/lkml/2019/7/18/840?

Yes, the above link is after bisecting with all the bugs and fixes
squashed together to avoid landing in local bugs.  Thanks,

Alex

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Alex Williamson]

On Fri, 19 Jul 2019 09:23:03 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Fri, 19 Jul 2019 17:04:26 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
> > Hi Alex,
> > 
> > On 7/18/19 11:12 AM, Alex Williamson wrote:  
> > > On Sat, 25 May 2019 13:41:33 +0800
> > > Lu Baolu <baolu.lu@linux.intel.com> wrote:
> > >     
> > >> Previously, get_valid_domain_for_dev() is used to retrieve the
> > >> DMA domain which has been attached to the device or allocate one
> > >> if no domain has been attached yet. As we have delegated the DMA
> > >> domain management to upper layer, this function is used purely to
> > >> allocate a private DMA domain if the default domain doesn't work
> > >> for ths device. Cleanup the code for readability.
> > >>
> > >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > >> ---
> > >>   drivers/iommu/intel-iommu.c | 18 ++++++++----------
> > >>   include/linux/intel-iommu.h |  1 -
> > >>   2 files changed, 8 insertions(+), 11 deletions(-)    
> > > 
> > > System fails to boot bisected to this commit:    
> > 
> > Is this the same issue as this https://lkml.org/lkml/2019/7/18/840?  
> 
> Yes, the above link is after bisecting with all the bugs and fixes
> squashed together to avoid landing in local bugs.  Thanks,

Well, it turns out this patch is still broken too.  I was excited that
the system booted again with reverting the commit in the link above and
didn't notice that VT-d failed and de-initialized itself:

DMAR: No ATSR found
DMAR: dmar0: Using Queued invalidation
DMAR: dmar1: Using Queued invalidation
pci 0000:00:00.0: DMAR: Software identity mapping
pci 0000:00:01.0: DMAR: Software identity mapping
pci 0000:00:02.0: DMAR: Software identity mapping
pci 0000:00:16.0: DMAR: Software identity mapping
pci 0000:00:1a.0: DMAR: Software identity mapping
pci 0000:00:1b.0: DMAR: Software identity mapping
pci 0000:00:1c.0: DMAR: Software identity mapping
pci 0000:00:1c.5: DMAR: Software identity mapping
pci 0000:00:1c.6: DMAR: Software identity mapping
pci 0000:00:1c.7: DMAR: Software identity mapping
pci 0000:00:1d.0: DMAR: Software identity mapping
pci 0000:00:1f.0: DMAR: Software identity mapping
pci 0000:00:1f.2: DMAR: Software identity mapping
pci 0000:00:1f.3: DMAR: Software identity mapping
pci 0000:01:00.0: DMAR: Software identity mapping
pci 0000:01:00.1: DMAR: Software identity mapping
pci 0000:03:00.0: DMAR: Software identity mapping
pci 0000:04:00.0: DMAR: Software identity mapping
DMAR: Setting RMRR:
pci 0000:00:02.0: DMAR: Setting identity map [0xbf800000 - 0xcf9fffff]
pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
pci 0000:00:1d.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
DMAR: Prepare 0-16MiB unity mapping for LPC
pci 0000:00:1f.0: DMAR: Setting identity map [0x0 - 0xffffff]
pci 0000:00:00.0: Adding to iommu group 0
pci 0000:00:00.0: Using iommu direct mapping
pci 0000:00:01.0: Adding to iommu group 1
pci 0000:00:01.0: Using iommu direct mapping
pci 0000:00:02.0: Adding to iommu group 2
pci 0000:00:02.0: Using iommu direct mapping
pci 0000:00:16.0: Adding to iommu group 3
pci 0000:00:16.0: Using iommu direct mapping
pci 0000:00:1a.0: Adding to iommu group 4
pci 0000:00:1a.0: Using iommu direct mapping
pci 0000:00:1b.0: Adding to iommu group 5
pci 0000:00:1b.0: Using iommu direct mapping
pci 0000:00:1c.0: Adding to iommu group 6
pci 0000:00:1c.0: Using iommu direct mapping
pci 0000:00:1c.5: Adding to iommu group 7
pci 0000:00:1c.5: Using iommu direct mapping
pci 0000:00:1c.6: Adding to iommu group 8
pci 0000:00:1c.6: Using iommu direct mapping
pci 0000:00:1c.7: Adding to iommu group 9
pci 0000:00:1c.7: Using iommu direct mapping
pci 0000:00:1d.0: Adding to iommu group 10
pci 0000:00:1d.0: Using iommu direct mapping
pci 0000:00:1f.0: Adding to iommu group 11
pci 0000:00:1f.0: Using iommu direct mapping
pci 0000:00:1f.2: Adding to iommu group 11
pci 0000:00:1f.3: Adding to iommu group 11
pci 0000:01:00.0: Adding to iommu group 1
pci 0000:01:00.1: Adding to iommu group 1
pci 0000:03:00.0: Adding to iommu group 12
pci 0000:03:00.0: Using iommu direct mapping
pci 0000:04:00.0: Adding to iommu group 13
pci 0000:04:00.0: Using iommu direct mapping
pci 0000:05:00.0: Adding to iommu group 9
pci 0000:05:00.0: DMAR: Failed to get a private domain.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pci 0000:00:00.0: Removing from iommu group 0
pci 0000:00:01.0: Removing from iommu group 1
pci 0000:00:02.0: Removing from iommu group 2
pci 0000:00:16.0: Removing from iommu group 3
pci 0000:00:1a.0: Removing from iommu group 4
pci 0000:00:1b.0: Removing from iommu group 5
pci 0000:00:1c.0: Removing from iommu group 6
pci 0000:00:1c.5: Removing from iommu group 7
pci 0000:00:1c.6: Removing from iommu group 8
pci 0000:00:1c.7: Removing from iommu group 9
pci 0000:00:1d.0: Removing from iommu group 10
pci 0000:00:1f.0: Removing from iommu group 11
pci 0000:00:1f.2: Removing from iommu group 11
pci 0000:00:1f.3: Removing from iommu group 11
pci 0000:01:00.0: Removing from iommu group 1
pci 0000:01:00.1: Removing from iommu group 1
pci 0000:03:00.0: Removing from iommu group 12
pci 0000:04:00.0: Removing from iommu group 13
pci 0000:05:00.0: Removing from iommu group 9
DMAR: Intel(R) Virtualization Technology for Directed I/O

-[0000:00]-+-00.0  Intel Corporation Xeon E3-1200 v2/Ivy Bridge DRAM Controller
           +-01.0-[01]--+-00.0  NVIDIA Corporation GK208 [GeForce GT 635]
           |            \-00.1  NVIDIA Corporation GK208 HDMI/DP Audio Controller
           +-02.0  Intel Corporation Xeon E3-1200 v2/3rd Gen Core processor Graphics Controller
           +-16.0  Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #1
           +-1a.0  Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2
           +-1b.0  Intel Corporation 6 Series/C200 Series Chipset Family High Definition Audio Controller
           +-1c.0-[02]--
           +-1c.5-[03]----00.0  ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
           +-1c.6-[04]----00.0  Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
           +-1c.7-[05-06]----00.0-[06]--
           +-1d.0  Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #1
           +-1f.0  Intel Corporation H67 Express Chipset LPC Controller
           +-1f.2  Intel Corporation 6 Series/C200 Series Chipset Family 6 port Desktop SATA AHCI Controller
           \-1f.3  Intel Corporation 6 Series/C200 Series Chipset Family SMBus Controller

05:00.0 PCI bridge: ASMedia Technology Inc. ASM1083/1085 PCIe to PCI Bridge (rev 01) (prog-if 01 [Subtractive decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 32 bytes
	Interrupt: pin A routed to IRQ 5
	Bus: primary=05, secondary=06, subordinate=06, sec-latency=64
	I/O behind bridge: None
	Memory behind bridge: None
	Prefetchable memory behind bridge: None
	Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
	BridgeCtl: Parity- SERR+ NoISA- VGA- VGA16+ MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: [c0] Subsystem: ASUSTeK Computer Inc. Device 8489


With commit 4ec066c7b1476e0ca66a7acdb575627a5d1a1ee6 reverted on
v5.3-rc2:

DMAR: No ATSR found
DMAR: dmar0: Using Queued invalidation
DMAR: dmar1: Using Queued invalidation
pci 0000:00:00.0: Adding to iommu group 0
pci 0000:00:00.0: Using iommu direct mapping
pci 0000:00:01.0: Adding to iommu group 1
pci 0000:00:01.0: Using iommu direct mapping
pci 0000:00:02.0: Adding to iommu group 2
pci 0000:00:02.0: Using iommu direct mapping
pci 0000:00:16.0: Adding to iommu group 3
pci 0000:00:16.0: Using iommu direct mapping
pci 0000:00:1a.0: Adding to iommu group 4
pci 0000:00:1a.0: Using iommu direct mapping
pci 0000:00:1b.0: Adding to iommu group 5
pci 0000:00:1b.0: Using iommu direct mapping
pci 0000:00:1c.0: Adding to iommu group 6
pci 0000:00:1c.0: Using iommu direct mapping
pci 0000:00:1c.5: Adding to iommu group 7
pci 0000:00:1c.5: Using iommu direct mapping
pci 0000:00:1c.6: Adding to iommu group 8
pci 0000:00:1c.6: Using iommu direct mapping
pci 0000:00:1c.7: Adding to iommu group 9
pci 0000:00:1c.7: Using iommu direct mapping
pci 0000:00:1d.0: Adding to iommu group 10
pci 0000:00:1d.0: Using iommu direct mapping
pci 0000:00:1f.0: Adding to iommu group 11
pci 0000:00:1f.0: Using iommu direct mapping
pci 0000:00:1f.2: Adding to iommu group 11
pci 0000:00:1f.3: Adding to iommu group 11
pci 0000:01:00.0: Adding to iommu group 1
pci 0000:01:00.1: Adding to iommu group 1
pci 0000:03:00.0: Adding to iommu group 12
pci 0000:03:00.0: Using iommu direct mapping
pci 0000:04:00.0: Adding to iommu group 13
pci 0000:04:00.0: Using iommu direct mapping
pci 0000:05:00.0: Adding to iommu group 9
pci 0000:05:00.0: DMAR: Device uses a private dma domain.
DMAR: Intel(R) Virtualization Technology for Directed I/O

I'm guessing this series was maybe never tested on and doesn't account
for PCIe-to-PCI bridges.  Please fix.  Thanks,

Alex

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Lu Baolu]

Hi Alex,

Thanks for reporting this. I will try to find a machine with a
pcie-to-pci bridge and get this issue fixed. I will update you
later.

Best regards,
Baolu

On 8/2/19 9:30 AM, Alex Williamson wrote:
> On Fri, 19 Jul 2019 09:23:03 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
>> On Fri, 19 Jul 2019 17:04:26 +0800
>> Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>>> Hi Alex,
>>>
>>> On 7/18/19 11:12 AM, Alex Williamson wrote:
>>>> On Sat, 25 May 2019 13:41:33 +0800
>>>> Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>>>      
>>>>> Previously, get_valid_domain_for_dev() is used to retrieve the
>>>>> DMA domain which has been attached to the device or allocate one
>>>>> if no domain has been attached yet. As we have delegated the DMA
>>>>> domain management to upper layer, this function is used purely to
>>>>> allocate a private DMA domain if the default domain doesn't work
>>>>> for ths device. Cleanup the code for readability.
>>>>>
>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>>>> ---
>>>>>    drivers/iommu/intel-iommu.c | 18 ++++++++----------
>>>>>    include/linux/intel-iommu.h |  1 -
>>>>>    2 files changed, 8 insertions(+), 11 deletions(-)
>>>>
>>>> System fails to boot bisected to this commit:
>>>
>>> Is this the same issue as this https://lkml.org/lkml/2019/7/18/840?
>>
>> Yes, the above link is after bisecting with all the bugs and fixes
>> squashed together to avoid landing in local bugs.  Thanks,
> 
> Well, it turns out this patch is still broken too.  I was excited that
> the system booted again with reverting the commit in the link above and
> didn't notice that VT-d failed and de-initialized itself:
> 
> DMAR: No ATSR found
> DMAR: dmar0: Using Queued invalidation
> DMAR: dmar1: Using Queued invalidation
> pci 0000:00:00.0: DMAR: Software identity mapping
> pci 0000:00:01.0: DMAR: Software identity mapping
> pci 0000:00:02.0: DMAR: Software identity mapping
> pci 0000:00:16.0: DMAR: Software identity mapping
> pci 0000:00:1a.0: DMAR: Software identity mapping
> pci 0000:00:1b.0: DMAR: Software identity mapping
> pci 0000:00:1c.0: DMAR: Software identity mapping
> pci 0000:00:1c.5: DMAR: Software identity mapping
> pci 0000:00:1c.6: DMAR: Software identity mapping
> pci 0000:00:1c.7: DMAR: Software identity mapping
> pci 0000:00:1d.0: DMAR: Software identity mapping
> pci 0000:00:1f.0: DMAR: Software identity mapping
> pci 0000:00:1f.2: DMAR: Software identity mapping
> pci 0000:00:1f.3: DMAR: Software identity mapping
> pci 0000:01:00.0: DMAR: Software identity mapping
> pci 0000:01:00.1: DMAR: Software identity mapping
> pci 0000:03:00.0: DMAR: Software identity mapping
> pci 0000:04:00.0: DMAR: Software identity mapping
> DMAR: Setting RMRR:
> pci 0000:00:02.0: DMAR: Setting identity map [0xbf800000 - 0xcf9fffff]
> pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
> pci 0000:00:1d.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
> DMAR: Prepare 0-16MiB unity mapping for LPC
> pci 0000:00:1f.0: DMAR: Setting identity map [0x0 - 0xffffff]
> pci 0000:00:00.0: Adding to iommu group 0
> pci 0000:00:00.0: Using iommu direct mapping
> pci 0000:00:01.0: Adding to iommu group 1
> pci 0000:00:01.0: Using iommu direct mapping
> pci 0000:00:02.0: Adding to iommu group 2
> pci 0000:00:02.0: Using iommu direct mapping
> pci 0000:00:16.0: Adding to iommu group 3
> pci 0000:00:16.0: Using iommu direct mapping
> pci 0000:00:1a.0: Adding to iommu group 4
> pci 0000:00:1a.0: Using iommu direct mapping
> pci 0000:00:1b.0: Adding to iommu group 5
> pci 0000:00:1b.0: Using iommu direct mapping
> pci 0000:00:1c.0: Adding to iommu group 6
> pci 0000:00:1c.0: Using iommu direct mapping
> pci 0000:00:1c.5: Adding to iommu group 7
> pci 0000:00:1c.5: Using iommu direct mapping
> pci 0000:00:1c.6: Adding to iommu group 8
> pci 0000:00:1c.6: Using iommu direct mapping
> pci 0000:00:1c.7: Adding to iommu group 9
> pci 0000:00:1c.7: Using iommu direct mapping
> pci 0000:00:1d.0: Adding to iommu group 10
> pci 0000:00:1d.0: Using iommu direct mapping
> pci 0000:00:1f.0: Adding to iommu group 11
> pci 0000:00:1f.0: Using iommu direct mapping
> pci 0000:00:1f.2: Adding to iommu group 11
> pci 0000:00:1f.3: Adding to iommu group 11
> pci 0000:01:00.0: Adding to iommu group 1
> pci 0000:01:00.1: Adding to iommu group 1
> pci 0000:03:00.0: Adding to iommu group 12
> pci 0000:03:00.0: Using iommu direct mapping
> pci 0000:04:00.0: Adding to iommu group 13
> pci 0000:04:00.0: Using iommu direct mapping
> pci 0000:05:00.0: Adding to iommu group 9
> pci 0000:05:00.0: DMAR: Failed to get a private domain.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> pci 0000:00:00.0: Removing from iommu group 0
> pci 0000:00:01.0: Removing from iommu group 1
> pci 0000:00:02.0: Removing from iommu group 2
> pci 0000:00:16.0: Removing from iommu group 3
> pci 0000:00:1a.0: Removing from iommu group 4
> pci 0000:00:1b.0: Removing from iommu group 5
> pci 0000:00:1c.0: Removing from iommu group 6
> pci 0000:00:1c.5: Removing from iommu group 7
> pci 0000:00:1c.6: Removing from iommu group 8
> pci 0000:00:1c.7: Removing from iommu group 9
> pci 0000:00:1d.0: Removing from iommu group 10
> pci 0000:00:1f.0: Removing from iommu group 11
> pci 0000:00:1f.2: Removing from iommu group 11
> pci 0000:00:1f.3: Removing from iommu group 11
> pci 0000:01:00.0: Removing from iommu group 1
> pci 0000:01:00.1: Removing from iommu group 1
> pci 0000:03:00.0: Removing from iommu group 12
> pci 0000:04:00.0: Removing from iommu group 13
> pci 0000:05:00.0: Removing from iommu group 9
> DMAR: Intel(R) Virtualization Technology for Directed I/O
> 
> -[0000:00]-+-00.0  Intel Corporation Xeon E3-1200 v2/Ivy Bridge DRAM Controller
>             +-01.0-[01]--+-00.0  NVIDIA Corporation GK208 [GeForce GT 635]
>             |            \-00.1  NVIDIA Corporation GK208 HDMI/DP Audio Controller
>             +-02.0  Intel Corporation Xeon E3-1200 v2/3rd Gen Core processor Graphics Controller
>             +-16.0  Intel Corporation 6 Series/C200 Series Chipset Family MEI Controller #1
>             +-1a.0  Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #2
>             +-1b.0  Intel Corporation 6 Series/C200 Series Chipset Family High Definition Audio Controller
>             +-1c.0-[02]--
>             +-1c.5-[03]----00.0  ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
>             +-1c.6-[04]----00.0  Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
>             +-1c.7-[05-06]----00.0-[06]--
>             +-1d.0  Intel Corporation 6 Series/C200 Series Chipset Family USB Enhanced Host Controller #1
>             +-1f.0  Intel Corporation H67 Express Chipset LPC Controller
>             +-1f.2  Intel Corporation 6 Series/C200 Series Chipset Family 6 port Desktop SATA AHCI Controller
>             \-1f.3  Intel Corporation 6 Series/C200 Series Chipset Family SMBus Controller
> 
> 05:00.0 PCI bridge: ASMedia Technology Inc. ASM1083/1085 PCIe to PCI Bridge (rev 01) (prog-if 01 [Subtractive decode])
> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
> 	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0, Cache Line Size: 32 bytes
> 	Interrupt: pin A routed to IRQ 5
> 	Bus: primary=05, secondary=06, subordinate=06, sec-latency=64
> 	I/O behind bridge: None
> 	Memory behind bridge: None
> 	Prefetchable memory behind bridge: None
> 	Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
> 	BridgeCtl: Parity- SERR+ NoISA- VGA- VGA16+ MAbort- >Reset- FastB2B-
> 		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
> 	Capabilities: [c0] Subsystem: ASUSTeK Computer Inc. Device 8489
> 
> 
> With commit 4ec066c7b1476e0ca66a7acdb575627a5d1a1ee6 reverted on
> v5.3-rc2:
> 
> DMAR: No ATSR found
> DMAR: dmar0: Using Queued invalidation
> DMAR: dmar1: Using Queued invalidation
> pci 0000:00:00.0: Adding to iommu group 0
> pci 0000:00:00.0: Using iommu direct mapping
> pci 0000:00:01.0: Adding to iommu group 1
> pci 0000:00:01.0: Using iommu direct mapping
> pci 0000:00:02.0: Adding to iommu group 2
> pci 0000:00:02.0: Using iommu direct mapping
> pci 0000:00:16.0: Adding to iommu group 3
> pci 0000:00:16.0: Using iommu direct mapping
> pci 0000:00:1a.0: Adding to iommu group 4
> pci 0000:00:1a.0: Using iommu direct mapping
> pci 0000:00:1b.0: Adding to iommu group 5
> pci 0000:00:1b.0: Using iommu direct mapping
> pci 0000:00:1c.0: Adding to iommu group 6
> pci 0000:00:1c.0: Using iommu direct mapping
> pci 0000:00:1c.5: Adding to iommu group 7
> pci 0000:00:1c.5: Using iommu direct mapping
> pci 0000:00:1c.6: Adding to iommu group 8
> pci 0000:00:1c.6: Using iommu direct mapping
> pci 0000:00:1c.7: Adding to iommu group 9
> pci 0000:00:1c.7: Using iommu direct mapping
> pci 0000:00:1d.0: Adding to iommu group 10
> pci 0000:00:1d.0: Using iommu direct mapping
> pci 0000:00:1f.0: Adding to iommu group 11
> pci 0000:00:1f.0: Using iommu direct mapping
> pci 0000:00:1f.2: Adding to iommu group 11
> pci 0000:00:1f.3: Adding to iommu group 11
> pci 0000:01:00.0: Adding to iommu group 1
> pci 0000:01:00.1: Adding to iommu group 1
> pci 0000:03:00.0: Adding to iommu group 12
> pci 0000:03:00.0: Using iommu direct mapping
> pci 0000:04:00.0: Adding to iommu group 13
> pci 0000:04:00.0: Using iommu direct mapping
> pci 0000:05:00.0: Adding to iommu group 9
> pci 0000:05:00.0: DMAR: Device uses a private dma domain.
> DMAR: Intel(R) Virtualization Technology for Directed I/O
> 
> I'm guessing this series was maybe never tested on and doesn't account
> for PCIe-to-PCI bridges.  Please fix.  Thanks,
> 
> Alex
> 

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Alex Williamson]

On Fri, 2 Aug 2019 15:17:45 +0800
Lu Baolu <baolu.lu@linux.intel.com> wrote:

> Hi Alex,
> 
> Thanks for reporting this. I will try to find a machine with a
> pcie-to-pci bridge and get this issue fixed. I will update you
> later.

Further debug below...

> On 8/2/19 9:30 AM, Alex Williamson wrote:
> > DMAR: No ATSR found
> > DMAR: dmar0: Using Queued invalidation
> > DMAR: dmar1: Using Queued invalidation
> > pci 0000:00:00.0: DMAR: Software identity mapping
> > pci 0000:00:01.0: DMAR: Software identity mapping
> > pci 0000:00:02.0: DMAR: Software identity mapping
> > pci 0000:00:16.0: DMAR: Software identity mapping
> > pci 0000:00:1a.0: DMAR: Software identity mapping
> > pci 0000:00:1b.0: DMAR: Software identity mapping
> > pci 0000:00:1c.0: DMAR: Software identity mapping
> > pci 0000:00:1c.5: DMAR: Software identity mapping
> > pci 0000:00:1c.6: DMAR: Software identity mapping
> > pci 0000:00:1c.7: DMAR: Software identity mapping
> > pci 0000:00:1d.0: DMAR: Software identity mapping
> > pci 0000:00:1f.0: DMAR: Software identity mapping
> > pci 0000:00:1f.2: DMAR: Software identity mapping
> > pci 0000:00:1f.3: DMAR: Software identity mapping
> > pci 0000:01:00.0: DMAR: Software identity mapping
> > pci 0000:01:00.1: DMAR: Software identity mapping
> > pci 0000:03:00.0: DMAR: Software identity mapping
> > pci 0000:04:00.0: DMAR: Software identity mapping
> > DMAR: Setting RMRR:
> > pci 0000:00:02.0: DMAR: Setting identity map [0xbf800000 - 0xcf9fffff]
> > pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
> > pci 0000:00:1d.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
> > DMAR: Prepare 0-16MiB unity mapping for LPC
> > pci 0000:00:1f.0: DMAR: Setting identity map [0x0 - 0xffffff]
> > pci 0000:00:00.0: Adding to iommu group 0
> > pci 0000:00:00.0: Using iommu direct mapping
> > pci 0000:00:01.0: Adding to iommu group 1
> > pci 0000:00:01.0: Using iommu direct mapping
> > pci 0000:00:02.0: Adding to iommu group 2
> > pci 0000:00:02.0: Using iommu direct mapping
> > pci 0000:00:16.0: Adding to iommu group 3
> > pci 0000:00:16.0: Using iommu direct mapping
> > pci 0000:00:1a.0: Adding to iommu group 4
> > pci 0000:00:1a.0: Using iommu direct mapping
> > pci 0000:00:1b.0: Adding to iommu group 5
> > pci 0000:00:1b.0: Using iommu direct mapping
> > pci 0000:00:1c.0: Adding to iommu group 6
> > pci 0000:00:1c.0: Using iommu direct mapping
> > pci 0000:00:1c.5: Adding to iommu group 7
> > pci 0000:00:1c.5: Using iommu direct mapping
> > pci 0000:00:1c.6: Adding to iommu group 8
> > pci 0000:00:1c.6: Using iommu direct mapping
> > pci 0000:00:1c.7: Adding to iommu group 9

Note that group 9 contains device 00:1c.7

> > pci 0000:00:1c.7: Using iommu direct mapping

I'm booted with iommu=pt, so the domain type is IOMMU_DOMAIN_PT

> > pci 0000:00:1d.0: Adding to iommu group 10
> > pci 0000:00:1d.0: Using iommu direct mapping
> > pci 0000:00:1f.0: Adding to iommu group 11
> > pci 0000:00:1f.0: Using iommu direct mapping
> > pci 0000:00:1f.2: Adding to iommu group 11
> > pci 0000:00:1f.3: Adding to iommu group 11
> > pci 0000:01:00.0: Adding to iommu group 1
> > pci 0000:01:00.1: Adding to iommu group 1
> > pci 0000:03:00.0: Adding to iommu group 12
> > pci 0000:03:00.0: Using iommu direct mapping
> > pci 0000:04:00.0: Adding to iommu group 13
> > pci 0000:04:00.0: Using iommu direct mapping
> > pci 0000:05:00.0: Adding to iommu group 9

05:00.0 is downstream of 00:1c.7 and in the same group.  As above, the
domain is type IOMMU_DOMAIN_IDENTITY, so we take the following branch:

        } else {
                if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {

Default domain type is IOMMU_DOMAIN_DMA because of the code block in
device_def_domain_type() handling bridges to conventional PCI and
conventional PCI endpoints.

                        ret = iommu_request_dma_domain_for_dev(dev);

This fails in request_default_domain_for_dev() because there's more
than one device in the group.

                        if (ret) {
                                dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
                                if (!get_private_domain_for_dev(dev)) {

With this commit, this now returns NULL because find_domain() does find
a domain, the same one we found before this code block.

                                        dev_warn(dev,
                                                 "Failed to get a private domain.\n");
                                        return -ENOMEM;
                                }

So the key factors are that I'm booting with iommu=pt and I have a
PCIe-to-PCI bridge grouped with its parent root port.  The bridge
wants an IOMMU_DOMAIN_DMA, but the group domain is already of type
IOMMU_DOMAIN_IDENTITY.  A temporary workaround is to not use
passthrough mode, but this is a regression versus previous kernels.
Thanks,

Alex

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Lu Baolu]

Hi,

On 8/3/19 12:54 AM, Alex Williamson wrote:
> On Fri, 2 Aug 2019 15:17:45 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
>> Hi Alex,
>>
>> Thanks for reporting this. I will try to find a machine with a
>> pcie-to-pci bridge and get this issue fixed. I will update you
>> later.
> 
> Further debug below...

Thanks for the debugging.

> 
>> On 8/2/19 9:30 AM, Alex Williamson wrote:
>>> DMAR: No ATSR found
>>> DMAR: dmar0: Using Queued invalidation
>>> DMAR: dmar1: Using Queued invalidation
>>> pci 0000:00:00.0: DMAR: Software identity mapping
>>> pci 0000:00:01.0: DMAR: Software identity mapping
>>> pci 0000:00:02.0: DMAR: Software identity mapping
>>> pci 0000:00:16.0: DMAR: Software identity mapping
>>> pci 0000:00:1a.0: DMAR: Software identity mapping
>>> pci 0000:00:1b.0: DMAR: Software identity mapping
>>> pci 0000:00:1c.0: DMAR: Software identity mapping
>>> pci 0000:00:1c.5: DMAR: Software identity mapping
>>> pci 0000:00:1c.6: DMAR: Software identity mapping
>>> pci 0000:00:1c.7: DMAR: Software identity mapping
>>> pci 0000:00:1d.0: DMAR: Software identity mapping
>>> pci 0000:00:1f.0: DMAR: Software identity mapping
>>> pci 0000:00:1f.2: DMAR: Software identity mapping
>>> pci 0000:00:1f.3: DMAR: Software identity mapping
>>> pci 0000:01:00.0: DMAR: Software identity mapping
>>> pci 0000:01:00.1: DMAR: Software identity mapping
>>> pci 0000:03:00.0: DMAR: Software identity mapping
>>> pci 0000:04:00.0: DMAR: Software identity mapping
>>> DMAR: Setting RMRR:
>>> pci 0000:00:02.0: DMAR: Setting identity map [0xbf800000 - 0xcf9fffff]
>>> pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
>>> pci 0000:00:1d.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
>>> DMAR: Prepare 0-16MiB unity mapping for LPC
>>> pci 0000:00:1f.0: DMAR: Setting identity map [0x0 - 0xffffff]
>>> pci 0000:00:00.0: Adding to iommu group 0
>>> pci 0000:00:00.0: Using iommu direct mapping
>>> pci 0000:00:01.0: Adding to iommu group 1
>>> pci 0000:00:01.0: Using iommu direct mapping
>>> pci 0000:00:02.0: Adding to iommu group 2
>>> pci 0000:00:02.0: Using iommu direct mapping
>>> pci 0000:00:16.0: Adding to iommu group 3
>>> pci 0000:00:16.0: Using iommu direct mapping
>>> pci 0000:00:1a.0: Adding to iommu group 4
>>> pci 0000:00:1a.0: Using iommu direct mapping
>>> pci 0000:00:1b.0: Adding to iommu group 5
>>> pci 0000:00:1b.0: Using iommu direct mapping
>>> pci 0000:00:1c.0: Adding to iommu group 6
>>> pci 0000:00:1c.0: Using iommu direct mapping
>>> pci 0000:00:1c.5: Adding to iommu group 7
>>> pci 0000:00:1c.5: Using iommu direct mapping
>>> pci 0000:00:1c.6: Adding to iommu group 8
>>> pci 0000:00:1c.6: Using iommu direct mapping
>>> pci 0000:00:1c.7: Adding to iommu group 9
> 
> Note that group 9 contains device 00:1c.7

Yes.

> 
>>> pci 0000:00:1c.7: Using iommu direct mapping
> 
> I'm booted with iommu=pt, so the domain type is IOMMU_DOMAIN_PT

Yes.

> 
>>> pci 0000:00:1d.0: Adding to iommu group 10
>>> pci 0000:00:1d.0: Using iommu direct mapping
>>> pci 0000:00:1f.0: Adding to iommu group 11
>>> pci 0000:00:1f.0: Using iommu direct mapping
>>> pci 0000:00:1f.2: Adding to iommu group 11
>>> pci 0000:00:1f.3: Adding to iommu group 11
>>> pci 0000:01:00.0: Adding to iommu group 1
>>> pci 0000:01:00.1: Adding to iommu group 1
>>> pci 0000:03:00.0: Adding to iommu group 12
>>> pci 0000:03:00.0: Using iommu direct mapping
>>> pci 0000:04:00.0: Adding to iommu group 13
>>> pci 0000:04:00.0: Using iommu direct mapping
>>> pci 0000:05:00.0: Adding to iommu group 9
> 
> 05:00.0 is downstream of 00:1c.7 and in the same group.  As above, the
> domain is type IOMMU_DOMAIN_IDENTITY, so we take the following branch:
> 
>          } else {
>                  if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
> 
> Default domain type is IOMMU_DOMAIN_DMA because of the code block in
> device_def_domain_type() handling bridges to conventional PCI and
> conventional PCI endpoints.
> 
>                          ret = iommu_request_dma_domain_for_dev(dev);
> 
> This fails in request_default_domain_for_dev() because there's more
> than one device in the group.
> 
>                          if (ret) {
>                                  dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
>                                  if (!get_private_domain_for_dev(dev)) {
> 
> With this commit, this now returns NULL because find_domain() does find
> a domain, the same one we found before this code block.

This seems to be problematic.

Since it failed to request the right default domain type of group, the
driver decided to assign a private domain to it.

However, the device has already been attached by a domain, as "pci
0000:05:00.0: Adding to iommu group 9". So get_private_domain_for_dev()
returned error. It is same for identity private domain case.

We need to detach the domain first before requesting a private domain.
Can you please check whether below change works for you?

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index bdaed2da8a55..3ee9b0d20c28 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4803,7 +4803,8 @@ static void dmar_remove_one_dev_info(struct device 
*dev)

         spin_lock_irqsave(&device_domain_lock, flags);
         info = dev->archdata.iommu;
-       __dmar_remove_one_dev_info(info);
+       if (info)
+               __dmar_remove_one_dev_info(info);
         spin_unlock_irqrestore(&device_domain_lock, flags);
  }

@@ -5281,6 +5282,7 @@ static int intel_iommu_add_device(struct device *dev)
                 if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
                         ret = iommu_request_dm_for_dev(dev);
                         if (ret) {
+                               dmar_remove_one_dev_info(dev);
                                 dmar_domain->flags |= 
DOMAIN_FLAG_LOSE_CHILDREN;
                                 domain_add_dev_info(si_domain, dev);
                                 dev_info(dev,
@@ -5291,6 +5293,7 @@ static int intel_iommu_add_device(struct device *dev)
                 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,

> 
>                                          dev_warn(dev,
>                                                   "Failed to get a private domain.\n");
>                                          return -ENOMEM;
>                                  }
> 
> So the key factors are that I'm booting with iommu=pt and I have a
> PCIe-to-PCI bridge grouped with its parent root port.  The bridge
> wants an IOMMU_DOMAIN_DMA, but the group domain is already of type
> IOMMU_DOMAIN_IDENTITY.  A temporary workaround is to not use
> passthrough mode, but this is a regression versus previous kernels.

Yes, agreed.

> Thanks,
> 
> Alex
> 

Best regards,
Baolu

Re: [PATCH v4 12/15] iommu/vt-d: Cleanup get_valid_domain_for_dev()

[Lu Baolu]

Hi Alex,

On 8/3/19 12:54 AM, Alex Williamson wrote:
> On Fri, 2 Aug 2019 15:17:45 +0800
> Lu Baolu <baolu.lu@linux.intel.com> wrote:
> 
>> Hi Alex,
>>
>> Thanks for reporting this. I will try to find a machine with a
>> pcie-to-pci bridge and get this issue fixed. I will update you
>> later.
> 
> Further debug below...
> 
>> On 8/2/19 9:30 AM, Alex Williamson wrote:
>>> DMAR: No ATSR found
>>> DMAR: dmar0: Using Queued invalidation
>>> DMAR: dmar1: Using Queued invalidation
>>> pci 0000:00:00.0: DMAR: Software identity mapping
>>> pci 0000:00:01.0: DMAR: Software identity mapping
>>> pci 0000:00:02.0: DMAR: Software identity mapping
>>> pci 0000:00:16.0: DMAR: Software identity mapping
>>> pci 0000:00:1a.0: DMAR: Software identity mapping
>>> pci 0000:00:1b.0: DMAR: Software identity mapping
>>> pci 0000:00:1c.0: DMAR: Software identity mapping
>>> pci 0000:00:1c.5: DMAR: Software identity mapping
>>> pci 0000:00:1c.6: DMAR: Software identity mapping
>>> pci 0000:00:1c.7: DMAR: Software identity mapping
>>> pci 0000:00:1d.0: DMAR: Software identity mapping
>>> pci 0000:00:1f.0: DMAR: Software identity mapping
>>> pci 0000:00:1f.2: DMAR: Software identity mapping
>>> pci 0000:00:1f.3: DMAR: Software identity mapping
>>> pci 0000:01:00.0: DMAR: Software identity mapping
>>> pci 0000:01:00.1: DMAR: Software identity mapping
>>> pci 0000:03:00.0: DMAR: Software identity mapping
>>> pci 0000:04:00.0: DMAR: Software identity mapping
>>> DMAR: Setting RMRR:
>>> pci 0000:00:02.0: DMAR: Setting identity map [0xbf800000 - 0xcf9fffff]
>>> pci 0000:00:1a.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
>>> pci 0000:00:1d.0: DMAR: Setting identity map [0xbe8d1000 - 0xbe8dffff]
>>> DMAR: Prepare 0-16MiB unity mapping for LPC
>>> pci 0000:00:1f.0: DMAR: Setting identity map [0x0 - 0xffffff]
>>> pci 0000:00:00.0: Adding to iommu group 0
>>> pci 0000:00:00.0: Using iommu direct mapping
>>> pci 0000:00:01.0: Adding to iommu group 1
>>> pci 0000:00:01.0: Using iommu direct mapping
>>> pci 0000:00:02.0: Adding to iommu group 2
>>> pci 0000:00:02.0: Using iommu direct mapping
>>> pci 0000:00:16.0: Adding to iommu group 3
>>> pci 0000:00:16.0: Using iommu direct mapping
>>> pci 0000:00:1a.0: Adding to iommu group 4
>>> pci 0000:00:1a.0: Using iommu direct mapping
>>> pci 0000:00:1b.0: Adding to iommu group 5
>>> pci 0000:00:1b.0: Using iommu direct mapping
>>> pci 0000:00:1c.0: Adding to iommu group 6
>>> pci 0000:00:1c.0: Using iommu direct mapping
>>> pci 0000:00:1c.5: Adding to iommu group 7
>>> pci 0000:00:1c.5: Using iommu direct mapping
>>> pci 0000:00:1c.6: Adding to iommu group 8
>>> pci 0000:00:1c.6: Using iommu direct mapping
>>> pci 0000:00:1c.7: Adding to iommu group 9
> 
> Note that group 9 contains device 00:1c.7
> 
>>> pci 0000:00:1c.7: Using iommu direct mapping
> 
> I'm booted with iommu=pt, so the domain type is IOMMU_DOMAIN_PT
> 
>>> pci 0000:00:1d.0: Adding to iommu group 10
>>> pci 0000:00:1d.0: Using iommu direct mapping
>>> pci 0000:00:1f.0: Adding to iommu group 11
>>> pci 0000:00:1f.0: Using iommu direct mapping
>>> pci 0000:00:1f.2: Adding to iommu group 11
>>> pci 0000:00:1f.3: Adding to iommu group 11
>>> pci 0000:01:00.0: Adding to iommu group 1
>>> pci 0000:01:00.1: Adding to iommu group 1
>>> pci 0000:03:00.0: Adding to iommu group 12
>>> pci 0000:03:00.0: Using iommu direct mapping
>>> pci 0000:04:00.0: Adding to iommu group 13
>>> pci 0000:04:00.0: Using iommu direct mapping
>>> pci 0000:05:00.0: Adding to iommu group 9
> 
> 05:00.0 is downstream of 00:1c.7 and in the same group.  As above, the
> domain is type IOMMU_DOMAIN_IDENTITY, so we take the following branch:
> 
>          } else {
>                  if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
> 
> Default domain type is IOMMU_DOMAIN_DMA because of the code block in
> device_def_domain_type() handling bridges to conventional PCI and
> conventional PCI endpoints.
> 
>                          ret = iommu_request_dma_domain_for_dev(dev);
> 
> This fails in request_default_domain_for_dev() because there's more
> than one device in the group.
> 
>                          if (ret) {
>                                  dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
>                                  if (!get_private_domain_for_dev(dev)) {
> 
> With this commit, this now returns NULL because find_domain() does find
> a domain, the same one we found before this code block.
> 
>                                          dev_warn(dev,
>                                                   "Failed to get a private domain.\n");
>                                          return -ENOMEM;
>                                  }
> 
> So the key factors are that I'm booting with iommu=pt and I have a
> PCIe-to-PCI bridge grouped with its parent root port.  The bridge
> wants an IOMMU_DOMAIN_DMA, but the group domain is already of type
> IOMMU_DOMAIN_IDENTITY.  A temporary workaround is to not use
> passthrough mode, but this is a regression versus previous kernels.
> Thanks,
> 

I can reproduce this issue with a local setup. I will submit the fix and
cc it to you. Please let me know if that fix doesn't solve this problem.

Best regards,
Baolu

[PATCH v4 13/15] iommu/vt-d: Remove startup parameter from device_def_domain_type()

[Lu Baolu]

It isn't used anywhere. Remove it to make code concise.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 59cd8b7e793f..ba425db8cfc6 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2974,7 +2974,7 @@ static bool device_is_rmrr_locked(struct device *dev)
  *  - IOMMU_DOMAIN_IDENTITY: device requires an identical mapping domain
  *  - 0: both identity and dynamic domains work for this device
  */
-static int device_def_domain_type(struct device *dev, int startup)
+static int device_def_domain_type(struct device *dev)
 {
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
@@ -3028,16 +3028,16 @@ static int device_def_domain_type(struct device *dev, int startup)
 			IOMMU_DOMAIN_IDENTITY : 0;
 }
 
-static inline int iommu_should_identity_map(struct device *dev, int startup)
+static inline int iommu_should_identity_map(struct device *dev)
 {
-	return device_def_domain_type(dev, startup) == IOMMU_DOMAIN_IDENTITY;
+	return device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY;
 }
 
 static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw)
 {
 	int ret;
 
-	if (!iommu_should_identity_map(dev, 1))
+	if (!iommu_should_identity_map(dev))
 		return 0;
 
 	ret = domain_add_dev_info(si_domain, dev);
@@ -4590,7 +4590,7 @@ static int device_notifier(struct notifier_block *nb,
 
 		dmar_remove_one_dev_info(dev);
 	} else if (action == BUS_NOTIFY_ADD_DEVICE) {
-		if (iommu_should_identity_map(dev, 1))
+		if (iommu_should_identity_map(dev))
 			domain_add_dev_info(si_domain, dev);
 	}
 
@@ -5551,7 +5551,7 @@ static int intel_iommu_add_device(struct device *dev)
 	domain = iommu_get_domain_for_dev(dev);
 	dmar_domain = to_dmar_domain(domain);
 	if (domain->type == IOMMU_DOMAIN_DMA) {
-		if (device_def_domain_type(dev, 1) == IOMMU_DOMAIN_IDENTITY) {
+		if (device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
 			ret = iommu_request_dm_for_dev(dev);
 			if (ret) {
 				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
@@ -5564,7 +5564,7 @@ static int intel_iommu_add_device(struct device *dev)
 			return -ENODEV;
 		}
 	} else {
-		if (device_def_domain_type(dev, 1) == IOMMU_DOMAIN_DMA) {
+		if (device_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
 			ret = iommu_request_dma_domain_for_dev(dev);
 			if (ret) {
 				dmar_domain->flags |= DOMAIN_FLAG_LOSE_CHILDREN;
-- 
2.17.1

[PATCH v4 14/15] iommu/vt-d: Remove duplicated code for device hotplug

[Lu Baolu]

The iommu generic code has handled the device hotplug cases.
Remove the duplicated code.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ba425db8cfc6..7c7230ae6967 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4568,39 +4568,6 @@ int dmar_iommu_notify_scope_dev(struct dmar_pci_notify_info *info)
 	return 0;
 }
 
-/*
- * Here we only respond to action of unbound device from driver.
- *
- * Added device is not attached to its DMAR domain here yet. That will happen
- * when mapping the device to iova.
- */
-static int device_notifier(struct notifier_block *nb,
-				  unsigned long action, void *data)
-{
-	struct device *dev = data;
-	struct dmar_domain *domain;
-
-	if (iommu_dummy(dev))
-		return 0;
-
-	if (action == BUS_NOTIFY_REMOVED_DEVICE) {
-		domain = find_domain(dev);
-		if (!domain)
-			return 0;
-
-		dmar_remove_one_dev_info(dev);
-	} else if (action == BUS_NOTIFY_ADD_DEVICE) {
-		if (iommu_should_identity_map(dev))
-			domain_add_dev_info(si_domain, dev);
-	}
-
-	return 0;
-}
-
-static struct notifier_block device_nb = {
-	.notifier_call = device_notifier,
-};
-
 static int intel_iommu_memory_notifier(struct notifier_block *nb,
 				       unsigned long val, void *v)
 {
@@ -4975,7 +4942,6 @@ int __init intel_iommu_init(void)
 	}
 
 	bus_set_iommu(&pci_bus_type, &intel_iommu_ops);
-	bus_register_notifier(&pci_bus_type, &device_nb);
 	if (si_domain && !hw_pass_through)
 		register_memory_notifier(&intel_iommu_memory_nb);
 	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
-- 
2.17.1

[PATCH v4 15/15] iommu/vt-d: Remove static identity map code

[Lu Baolu]

The code to prepare the static identity map for various reserved
memory ranges in intel_iommu_init() is duplicated with the default
domain mechanism now. Remove it to avoid duplication.

Signed-off-by: James Sewart <jamessewart@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 144 +-----------------------------------
 1 file changed, 1 insertion(+), 143 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 7c7230ae6967..ce38c6b3763a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2789,31 +2789,6 @@ static inline int iommu_prepare_rmrr_dev(struct dmar_rmrr_unit *rmrr,
 					  rmrr->end_address);
 }
 
-#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
-static inline void iommu_prepare_isa(void)
-{
-	struct pci_dev *pdev;
-	int ret;
-
-	pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
-	if (!pdev)
-		return;
-
-	pr_info("Prepare 0-16MiB unity mapping for LPC\n");
-	ret = iommu_prepare_identity_map(&pdev->dev, 0, 16*1024*1024 - 1);
-
-	if (ret)
-		pr_err("Failed to create 0-16MiB identity map - floppy might not work\n");
-
-	pci_dev_put(pdev);
-}
-#else
-static inline void iommu_prepare_isa(void)
-{
-	return;
-}
-#endif /* !CONFIG_INTEL_IOMMU_FLPY_WA */
-
 static int md_domain_init(struct dmar_domain *domain, int guest_width);
 
 static int __init si_domain_init(int hw)
@@ -3028,68 +3003,6 @@ static int device_def_domain_type(struct device *dev)
 			IOMMU_DOMAIN_IDENTITY : 0;
 }
 
-static inline int iommu_should_identity_map(struct device *dev)
-{
-	return device_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY;
-}
-
-static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw)
-{
-	int ret;
-
-	if (!iommu_should_identity_map(dev))
-		return 0;
-
-	ret = domain_add_dev_info(si_domain, dev);
-	if (!ret)
-		dev_info(dev, "%s identity mapping\n",
-			 hw ? "Hardware" : "Software");
-	else if (ret == -ENODEV)
-		/* device not associated with an iommu */
-		ret = 0;
-
-	return ret;
-}
-
-
-static int __init iommu_prepare_static_identity_mapping(int hw)
-{
-	struct pci_dev *pdev = NULL;
-	struct dmar_drhd_unit *drhd;
-	struct intel_iommu *iommu;
-	struct device *dev;
-	int i;
-	int ret = 0;
-
-	for_each_pci_dev(pdev) {
-		ret = dev_prepare_static_identity_mapping(&pdev->dev, hw);
-		if (ret)
-			return ret;
-	}
-
-	for_each_active_iommu(iommu, drhd)
-		for_each_active_dev_scope(drhd->devices, drhd->devices_cnt, i, dev) {
-			struct acpi_device_physical_node *pn;
-			struct acpi_device *adev;
-
-			if (dev->bus != &acpi_bus_type)
-				continue;
-
-			adev= to_acpi_device(dev);
-			mutex_lock(&adev->physical_node_lock);
-			list_for_each_entry(pn, &adev->physical_node_list, node) {
-				ret = dev_prepare_static_identity_mapping(pn->dev, hw);
-				if (ret)
-					break;
-			}
-			mutex_unlock(&adev->physical_node_lock);
-			if (ret)
-				return ret;
-		}
-
-	return 0;
-}
-
 static void intel_iommu_init_qi(struct intel_iommu *iommu)
 {
 	/*
@@ -3312,11 +3225,8 @@ static int copy_translation_tables(struct intel_iommu *iommu)
 static int __init init_dmars(void)
 {
 	struct dmar_drhd_unit *drhd;
-	struct dmar_rmrr_unit *rmrr;
-	bool copied_tables = false;
-	struct device *dev;
 	struct intel_iommu *iommu;
-	int i, ret;
+	int ret;
 
 	/*
 	 * for each drhd
@@ -3409,7 +3319,6 @@ static int __init init_dmars(void)
 			} else {
 				pr_info("Copied translation tables from previous kernel for %s\n",
 					iommu->name);
-				copied_tables = true;
 			}
 		}
 
@@ -3449,57 +3358,6 @@ static int __init init_dmars(void)
 	if (ret)
 		goto free_iommu;
 
-
-	/*
-	 * If we copied translations from a previous kernel in the kdump
-	 * case, we can not assign the devices to domains now, as that
-	 * would eliminate the old mappings. So skip this part and defer
-	 * the assignment to device driver initialization time.
-	 */
-	if (copied_tables)
-		goto domains_done;
-
-	/*
-	 * If pass through is not set or not enabled, setup context entries for
-	 * identity mappings for rmrr, gfx, and isa and may fall back to static
-	 * identity mapping if iommu_identity_mapping is set.
-	 */
-	if (iommu_identity_mapping) {
-		ret = iommu_prepare_static_identity_mapping(hw_pass_through);
-		if (ret) {
-			pr_crit("Failed to setup IOMMU pass-through\n");
-			goto free_iommu;
-		}
-	}
-	/*
-	 * For each rmrr
-	 *   for each dev attached to rmrr
-	 *   do
-	 *     locate drhd for dev, alloc domain for dev
-	 *     allocate free domain
-	 *     allocate page table entries for rmrr
-	 *     if context not allocated for bus
-	 *           allocate and init context
-	 *           set present in root table for this bus
-	 *     init context with domain, translation etc
-	 *    endfor
-	 * endfor
-	 */
-	pr_info("Setting RMRR:\n");
-	for_each_rmrr_units(rmrr) {
-		/* some BIOS lists non-exist devices in DMAR table. */
-		for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
-					  i, dev) {
-			ret = iommu_prepare_rmrr_dev(rmrr, dev);
-			if (ret)
-				pr_err("Mapping reserved region failed\n");
-		}
-	}
-
-	iommu_prepare_isa();
-
-domains_done:
-
 	/*
 	 * for each drhd
 	 *   enable fault log
-- 
2.17.1

Re: [PATCH v4 00/15] iommu/vt-d: Delegate DMA domain to generic iommu

[Joerg Roedel]

Hey James, Lu Baolu,

On Sat, May 25, 2019 at 01:41:21PM +0800, Lu Baolu wrote:
> James Sewart (1):
>   iommu/vt-d: Implement apply_resv_region iommu ops entry
> 
> Lu Baolu (14):
>   iommu: Add API to request DMA domain for device
>   iommu/vt-d: Expose ISA direct mapping region via
>     iommu_get_resv_regions
>   iommu/vt-d: Enable DMA remapping after rmrr mapped
>   iommu/vt-d: Add device_def_domain_type() helper
>   iommu/vt-d: Delegate the identity domain to upper layer
>   iommu/vt-d: Delegate the dma domain to upper layer
>   iommu/vt-d: Identify default domains replaced with private
>   iommu/vt-d: Handle 32bit device with identity default domain
>   iommu/vt-d: Probe DMA-capable ACPI name space devices
>   iommu/vt-d: Implement is_attach_deferred iommu ops entry
>   iommu/vt-d: Cleanup get_valid_domain_for_dev()
>   iommu/vt-d: Remove startup parameter from device_def_domain_type()
>   iommu/vt-d: Remove duplicated code for device hotplug
>   iommu/vt-d: Remove static identity map code

Thanks for working on this. I think it is time to give it some testing
in linux-next, so I applied it to my tree. Fingers crossed this can make
it into v5.3 :)


Regards,

	Joerg