[PATCH v3 0/5] Clean up VMD DMA Map Ops

[PATCH v3 0/5] Clean up VMD DMA Map Ops

[Jon Derrick]

v2 Set: https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derrick@intel.com/T/#t
v1 Set: https://lore.kernel.org/linux-iommu/20200107134125.GD30750@8bytes.org/T/#t

VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at the
VMD endpoint. The problem with this approach is that the VMD endpoint's
device-specific attributes, such as the DMA Mask Bits, are used instead.

This set cleans up VMD by removing the override that redirects DMA map
operations to the VMD endpoint. Instead it introduces a new DMA alias mechanism
into the existing DMA alias infrastructure.

v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
pci_dev and did the necessary DMA alias and IOMMU modifications.

v2 introduced a new weak function to reference the 'Direct DMA Alias', and
removed the need to add a pointer in struct device or pci_dev. Weak functions
are generally frowned upon when it's a single architecture implementation, so I
am open to alternatives.

v3 references the pci_dev rather than the struct device for the PCI
'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
device, though I don't expect the VMD endpoint to need intra-bus DMA aliases.

Changes from v2:
Uses struct pci_dev for PCI Device 'Direct DMA aliasing' (pci_direct_dma_alias)
Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct DMA alias'

Changes from v1:
Removed 1/5 & 2/5 misc fix patches that were merged
Uses Christoph's staging/cleanup patches
Introduce weak function rather than including pointer in struct device or pci_dev.

Based on Joerg's next:
https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/

Jon Derrick (5):
  x86/pci: Add a to_pci_sysdata helper
  x86/PCI: Expose VMD's PCI Device in pci_sysdata
  PCI: Introduce pci_direct_dma_alias()
  PCI: vmd: Stop overriding dma_map_ops
  x86/pci: Remove X86_DEV_DMA_OPS

 arch/x86/Kconfig               |   3 -
 arch/x86/include/asm/device.h  |  10 ---
 arch/x86/include/asm/pci.h     |  31 ++++-----
 arch/x86/pci/common.c          |  45 ++----------
 drivers/iommu/intel-iommu.c    |  18 +++--
 drivers/pci/controller/Kconfig |   1 -
 drivers/pci/controller/vmd.c   | 152 +----------------------------------------
 drivers/pci/pci.c              |  19 +++++-
 drivers/pci/search.c           |   7 ++
 include/linux/pci.h            |   1 +
 10 files changed, 61 insertions(+), 226 deletions(-)

-- 
1.8.3.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 1/5] x86/pci: Add a to_pci_sysdata helper

[Jon Derrick]

From: Christoph Hellwig <hch@lst.de>

Various helpers need the pci_sysdata just to dereference a single field
in it.  Add a little helper that returns the properly typed sysdata
pointer to require a little less boilerplate code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
[jonathan.derrick: added un-const cast]
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 arch/x86/include/asm/pci.h | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 90d0731..cf680c5 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -35,12 +35,15 @@ struct pci_sysdata {
 
 #ifdef CONFIG_PCI
 
+static inline struct pci_sysdata *to_pci_sysdata(struct pci_bus *bus)
+{
+	return bus->sysdata;
+}
+
 #ifdef CONFIG_PCI_DOMAINS
 static inline int pci_domain_nr(struct pci_bus *bus)
 {
-	struct pci_sysdata *sd = bus->sysdata;
-
-	return sd->domain;
+	return to_pci_sysdata(bus)->domain;
 }
 
 static inline int pci_proc_domain(struct pci_bus *bus)
@@ -52,23 +55,20 @@ static inline int pci_proc_domain(struct pci_bus *bus)
 #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
 static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
 {
-	struct pci_sysdata *sd = bus->sysdata;
-
-	return sd->fwnode;
+	return to_pci_sysdata(bus)->fwnode;
 }
 
 #define pci_root_bus_fwnode	_pci_root_bus_fwnode
 #endif
 
+#if IS_ENABLED(CONFIG_VMD)
 static inline bool is_vmd(struct pci_bus *bus)
 {
-#if IS_ENABLED(CONFIG_VMD)
-	struct pci_sysdata *sd = bus->sysdata;
-
-	return sd->vmd_domain;
+	return to_pci_sysdata(bus)->vmd_domain;
+}
 #else
-	return false;
-#endif
+#define is_vmd(bus)		false
+#endif /* CONFIG_VMD */
 }
 
 /* Can be used to override the logic in pci_scan_bus for skipping
@@ -124,9 +124,7 @@ static inline void early_quirks(void) { }
 /* Returns the node based on pci bus */
 static inline int __pcibus_to_node(const struct pci_bus *bus)
 {
-	const struct pci_sysdata *sd = bus->sysdata;
-
-	return sd->node;
+	return to_pci_sysdata((struct pci_bus *) bus)->node;
 }
 
 static inline const struct cpumask *
-- 
1.8.3.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 2/5] x86/PCI: Expose VMD's PCI Device in pci_sysdata

[Jon Derrick]

To be used by Intel-IOMMU code to find the correct domain.

CC: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 arch/x86/include/asm/pci.h   | 5 ++---
 drivers/pci/controller/vmd.c | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index cf680c5..b982d9c 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -25,7 +25,7 @@ struct pci_sysdata {
 	void		*fwnode;	/* IRQ domain for MSI assignment */
 #endif
 #if IS_ENABLED(CONFIG_VMD)
-	bool vmd_domain;		/* True if in Intel VMD domain */
+	struct pci_dev	*vmd_dev;	/* VMD Device if in Intel VMD domain */
 #endif
 };
 
@@ -64,12 +64,11 @@ static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
 #if IS_ENABLED(CONFIG_VMD)
 static inline bool is_vmd(struct pci_bus *bus)
 {
-	return to_pci_sysdata(bus)->vmd_domain;
+	return to_pci_sysdata(bus)->vmd_dev != NULL;
 }
 #else
 #define is_vmd(bus)		false
 #endif /* CONFIG_VMD */
-}
 
 /* Can be used to override the logic in pci_scan_bus for skipping
    already-configured bus numbers - to be used for buggy BIOSes
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 2128422..d67ad56 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -679,7 +679,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 		.parent = res,
 	};
 
-	sd->vmd_domain = true;
+	sd->vmd_dev = vmd->dev;
 	sd->domain = vmd_find_free_domain();
 	if (sd->domain < 0)
 		return sd->domain;
-- 
1.8.3.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 3/5] PCI: Introduce pci_direct_dma_alias()

[Jon Derrick]

The current DMA alias implementation requires the aliased device be on
the same PCI bus as the requester ID. This introduces an arch-specific
mechanism to point to another PCI device when doing mapping and
PCI DMA alias search.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 arch/x86/pci/common.c |  7 +++++++
 drivers/pci/pci.c     | 19 ++++++++++++++++++-
 drivers/pci/search.c  |  7 +++++++
 include/linux/pci.h   |  1 +
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 1e59df0..83334a5 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -736,3 +736,10 @@ int pci_ext_cfg_avail(void)
 	else
 		return 0;
 }
+
+#if IS_ENABLED(CONFIG_VMD)
+struct pci_dev *pci_direct_dma_alias(struct pci_dev *dev)
+{
+	return to_pci_sysdata(dev->bus)->vmd_dev;
+}
+#endif
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ad746d9..1362694 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6034,7 +6034,9 @@ bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2)
 	return (dev1->dma_alias_mask &&
 		test_bit(dev2->devfn, dev1->dma_alias_mask)) ||
 	       (dev2->dma_alias_mask &&
-		test_bit(dev1->devfn, dev2->dma_alias_mask));
+		test_bit(dev1->devfn, dev2->dma_alias_mask)) ||
+	       (pci_direct_dma_alias(dev1) == dev2) ||
+	       (pci_direct_dma_alias(dev2) == dev1);
 }
 
 bool pci_device_is_present(struct pci_dev *pdev)
@@ -6058,6 +6060,21 @@ void pci_ignore_hotplug(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_ignore_hotplug);
 
+/**
+ * pci_direct_dma_alias - Get PCI DMA alias for PCI device
+ * @dev: the PCI device that may have a PCI DMA alias
+ *
+ * Permits the platform to provide architecture-specific functionality to
+ * devices needing to alias DMA to another PCI device on another PCI bus. If
+ * the PCI device is on the same bus, it is recommended to use
+ * pci_add_dma_alias(). This is the default implementation. Architecture
+ * implementations can override this.
+ */
+struct pci_dev __weak *pci_direct_dma_alias(struct pci_dev *dev)
+{
+	return NULL;
+}
+
 resource_size_t __weak pcibios_default_alignment(void)
 {
 	return 0;
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index bade140..12811b3 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -32,6 +32,13 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	struct pci_bus *bus;
 	int ret;
 
+	/*
+	 * The device may have an explicit alias requester ID for DMA where the
+	 * requester is on another PCI bus.
+	 */
+	if (unlikely(pci_direct_dma_alias(pdev)))
+		pdev = pci_direct_dma_alias(pdev);
+
 	ret = fn(pdev, pci_dev_id(pdev), data);
 	if (ret)
 		return ret;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c393dff..cb6677b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1202,6 +1202,7 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
 int pci_select_bars(struct pci_dev *dev, unsigned long flags);
 bool pci_device_is_present(struct pci_dev *pdev);
 void pci_ignore_hotplug(struct pci_dev *dev);
+struct pci_dev *pci_direct_dma_alias(struct pci_dev *dev);
 
 int __printf(6, 7) pci_request_irq(struct pci_dev *dev, unsigned int nr,
 		irq_handler_t handler, irq_handler_t thread_fn, void *dev_id,
-- 
1.8.3.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 4/5] PCI: vmd: Stop overriding dma_map_ops

[Jon Derrick]

Devices on the VMD domain use the VMD endpoint's requester ID and have
been relying on the VMD endpoint's DMA operations. The problem with this
was that VMD domain devices would use the VMD endpoint's attributes when
doing DMA and IOMMU mapping. We can be smarter about this by only using
the VMD endpoint when mapping and providing the correct child device's
attributes during DMA operations.

This patch modifies Intel-IOMMU to check for a 'Direct DMA Alias' and
refer to it for mapping.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/iommu/intel-iommu.c    |  18 +++--
 drivers/pci/controller/Kconfig |   1 -
 drivers/pci/controller/vmd.c   | 150 -----------------------------------------
 3 files changed, 13 insertions(+), 156 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 716347e2..7ca807a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -804,14 +804,14 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
 
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pf_pdev;
+		struct pci_dev *dma_alias;
 
 		pdev = to_pci_dev(dev);
 
-#ifdef CONFIG_X86
-		/* VMD child devices currently cannot be handled individually */
-		if (is_vmd(pdev->bus))
-			return NULL;
-#endif
+		/* DMA aliased devices use the DMA alias's IOMMU */
+		dma_alias = pci_direct_dma_alias(pdev);
+		if (dma_alias)
+			pdev = dma_alias;
 
 		/* VFs aren't listed in scope tables; we need to look up
 		 * the PF instead to find the IOMMU. */
@@ -2521,6 +2521,14 @@ struct dmar_domain *find_domain(struct device *dev)
 		     dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO))
 		return NULL;
 
+	if (dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		struct pci_dev *dma_alias = pci_direct_dma_alias(pdev);
+
+		if (dma_alias)
+			dev = &dma_alias->dev;
+	}
+
 	/* No lock here, assumes no domain exit in normal case */
 	info = dev->archdata.iommu;
 	if (likely(info))
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index c77069c..55671429 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -239,7 +239,6 @@ config PCIE_TANGO_SMP8759
 
 config VMD
 	depends on PCI_MSI && X86_64 && SRCU
-	select X86_DEV_DMA_OPS
 	tristate "Intel Volume Management Device Driver"
 	---help---
 	  Adds support for the Intel Volume Management Device (VMD). VMD is a
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d67ad56..fe1acb0 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -98,9 +98,6 @@ struct vmd_dev {
 	struct irq_domain	*irq_domain;
 	struct pci_bus		*bus;
 	u8			busn_start;
-
-	struct dma_map_ops	dma_ops;
-	struct dma_domain	dma_domain;
 };
 
 static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
@@ -295,151 +292,6 @@ static void vmd_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
 	.chip		= &vmd_msi_controller,
 };
 
-/*
- * VMD replaces the requester ID with its own.  DMA mappings for devices in a
- * VMD domain need to be mapped for the VMD, not the device requiring
- * the mapping.
- */
-static struct device *to_vmd_dev(struct device *dev)
-{
-	struct pci_dev *pdev = to_pci_dev(dev);
-	struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
-
-	return &vmd->dev->dev;
-}
-
-static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
-		       gfp_t flag, unsigned long attrs)
-{
-	return dma_alloc_attrs(to_vmd_dev(dev), size, addr, flag, attrs);
-}
-
-static void vmd_free(struct device *dev, size_t size, void *vaddr,
-		     dma_addr_t addr, unsigned long attrs)
-{
-	return dma_free_attrs(to_vmd_dev(dev), size, vaddr, addr, attrs);
-}
-
-static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
-		    void *cpu_addr, dma_addr_t addr, size_t size,
-		    unsigned long attrs)
-{
-	return dma_mmap_attrs(to_vmd_dev(dev), vma, cpu_addr, addr, size,
-			attrs);
-}
-
-static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
-			   void *cpu_addr, dma_addr_t addr, size_t size,
-			   unsigned long attrs)
-{
-	return dma_get_sgtable_attrs(to_vmd_dev(dev), sgt, cpu_addr, addr, size,
-			attrs);
-}
-
-static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
-			       unsigned long offset, size_t size,
-			       enum dma_data_direction dir,
-			       unsigned long attrs)
-{
-	return dma_map_page_attrs(to_vmd_dev(dev), page, offset, size, dir,
-			attrs);
-}
-
-static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t size,
-			   enum dma_data_direction dir, unsigned long attrs)
-{
-	dma_unmap_page_attrs(to_vmd_dev(dev), addr, size, dir, attrs);
-}
-
-static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int nents,
-		      enum dma_data_direction dir, unsigned long attrs)
-{
-	return dma_map_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
-}
-
-static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
-			 enum dma_data_direction dir, unsigned long attrs)
-{
-	dma_unmap_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
-}
-
-static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
-				    size_t size, enum dma_data_direction dir)
-{
-	dma_sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
-}
-
-static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
-				       size_t size, enum dma_data_direction dir)
-{
-	dma_sync_single_for_device(to_vmd_dev(dev), addr, size, dir);
-}
-
-static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
-				int nents, enum dma_data_direction dir)
-{
-	dma_sync_sg_for_cpu(to_vmd_dev(dev), sg, nents, dir);
-}
-
-static void vmd_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
-				   int nents, enum dma_data_direction dir)
-{
-	dma_sync_sg_for_device(to_vmd_dev(dev), sg, nents, dir);
-}
-
-static int vmd_dma_supported(struct device *dev, u64 mask)
-{
-	return dma_supported(to_vmd_dev(dev), mask);
-}
-
-static u64 vmd_get_required_mask(struct device *dev)
-{
-	return dma_get_required_mask(to_vmd_dev(dev));
-}
-
-static void vmd_teardown_dma_ops(struct vmd_dev *vmd)
-{
-	struct dma_domain *domain = &vmd->dma_domain;
-
-	if (get_dma_ops(&vmd->dev->dev))
-		del_dma_domain(domain);
-}
-
-#define ASSIGN_VMD_DMA_OPS(source, dest, fn)	\
-	do {					\
-		if (source->fn)			\
-			dest->fn = vmd_##fn;	\
-	} while (0)
-
-static void vmd_setup_dma_ops(struct vmd_dev *vmd)
-{
-	const struct dma_map_ops *source = get_dma_ops(&vmd->dev->dev);
-	struct dma_map_ops *dest = &vmd->dma_ops;
-	struct dma_domain *domain = &vmd->dma_domain;
-
-	domain->domain_nr = vmd->sysdata.domain;
-	domain->dma_ops = dest;
-
-	if (!source)
-		return;
-	ASSIGN_VMD_DMA_OPS(source, dest, alloc);
-	ASSIGN_VMD_DMA_OPS(source, dest, free);
-	ASSIGN_VMD_DMA_OPS(source, dest, mmap);
-	ASSIGN_VMD_DMA_OPS(source, dest, get_sgtable);
-	ASSIGN_VMD_DMA_OPS(source, dest, map_page);
-	ASSIGN_VMD_DMA_OPS(source, dest, unmap_page);
-	ASSIGN_VMD_DMA_OPS(source, dest, map_sg);
-	ASSIGN_VMD_DMA_OPS(source, dest, unmap_sg);
-	ASSIGN_VMD_DMA_OPS(source, dest, sync_single_for_cpu);
-	ASSIGN_VMD_DMA_OPS(source, dest, sync_single_for_device);
-	ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_cpu);
-	ASSIGN_VMD_DMA_OPS(source, dest, sync_sg_for_device);
-	ASSIGN_VMD_DMA_OPS(source, dest, dma_supported);
-	ASSIGN_VMD_DMA_OPS(source, dest, get_required_mask);
-	add_dma_domain(domain);
-}
-#undef ASSIGN_VMD_DMA_OPS
-
 static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
 				  unsigned int devfn, int reg, int len)
 {
@@ -709,7 +561,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	}
 
 	vmd_attach_resources(vmd);
-	vmd_setup_dma_ops(vmd);
 	dev_set_msi_domain(&vmd->bus->dev, vmd->irq_domain);
 
 	pci_scan_child_bus(vmd->bus);
@@ -824,7 +675,6 @@ static void vmd_remove(struct pci_dev *dev)
 	pci_stop_root_bus(vmd->bus);
 	pci_remove_root_bus(vmd->bus);
 	vmd_cleanup_srcu(vmd);
-	vmd_teardown_dma_ops(vmd);
 	vmd_detach_resources(vmd);
 	irq_domain_remove(vmd->irq_domain);
 }
-- 
1.8.3.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v3 5/5] x86/pci: Remove X86_DEV_DMA_OPS

[Jon Derrick]

From: Christoph Hellwig <hch@lst.de>

There are no users of X86_DEV_DMA_OPS left, so remove the code.

Reviewed-by: Jon Derrick <jonathan.derrick@intel.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/Kconfig              |  3 ---
 arch/x86/include/asm/device.h | 10 ----------
 arch/x86/pci/common.c         | 38 --------------------------------------
 3 files changed, 51 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5e89499..77f9426 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2955,9 +2955,6 @@ config HAVE_ATOMIC_IOMAP
 	def_bool y
 	depends on X86_32
 
-config X86_DEV_DMA_OPS
-	bool
-
 source "drivers/firmware/Kconfig"
 
 source "arch/x86/kvm/Kconfig"
diff --git a/arch/x86/include/asm/device.h b/arch/x86/include/asm/device.h
index 5e12c63..7e31f7f 100644
--- a/arch/x86/include/asm/device.h
+++ b/arch/x86/include/asm/device.h
@@ -8,16 +8,6 @@ struct dev_archdata {
 #endif
 };
 
-#if defined(CONFIG_X86_DEV_DMA_OPS) && defined(CONFIG_PCI_DOMAINS)
-struct dma_domain {
-	struct list_head node;
-	const struct dma_map_ops *dma_ops;
-	int domain_nr;
-};
-void add_dma_domain(struct dma_domain *domain);
-void del_dma_domain(struct dma_domain *domain);
-#endif
-
 struct pdev_archdata {
 };
 
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 83334a5..8f60d52 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -625,43 +625,6 @@ unsigned int pcibios_assign_all_busses(void)
 	return (pci_probe & PCI_ASSIGN_ALL_BUSSES) ? 1 : 0;
 }
 
-#if defined(CONFIG_X86_DEV_DMA_OPS) && defined(CONFIG_PCI_DOMAINS)
-static LIST_HEAD(dma_domain_list);
-static DEFINE_SPINLOCK(dma_domain_list_lock);
-
-void add_dma_domain(struct dma_domain *domain)
-{
-	spin_lock(&dma_domain_list_lock);
-	list_add(&domain->node, &dma_domain_list);
-	spin_unlock(&dma_domain_list_lock);
-}
-EXPORT_SYMBOL_GPL(add_dma_domain);
-
-void del_dma_domain(struct dma_domain *domain)
-{
-	spin_lock(&dma_domain_list_lock);
-	list_del(&domain->node);
-	spin_unlock(&dma_domain_list_lock);
-}
-EXPORT_SYMBOL_GPL(del_dma_domain);
-
-static void set_dma_domain_ops(struct pci_dev *pdev)
-{
-	struct dma_domain *domain;
-
-	spin_lock(&dma_domain_list_lock);
-	list_for_each_entry(domain, &dma_domain_list, node) {
-		if (pci_domain_nr(pdev->bus) == domain->domain_nr) {
-			pdev->dev.dma_ops = domain->dma_ops;
-			break;
-		}
-	}
-	spin_unlock(&dma_domain_list_lock);
-}
-#else
-static void set_dma_domain_ops(struct pci_dev *pdev) {}
-#endif
-
 static void set_dev_domain_options(struct pci_dev *pdev)
 {
 	if (is_vmd(pdev->bus))
@@ -697,7 +660,6 @@ int pcibios_add_device(struct pci_dev *dev)
 		pa_data = data->next;
 		memunmap(data);
 	}
-	set_dma_domain_ops(dev);
 	set_dev_domain_options(dev);
 	return 0;
 }
-- 
1.8.3.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

[Lorenzo Pieralisi]

On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> v2 Set: https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derrick@intel.com/T/#t
> v1 Set: https://lore.kernel.org/linux-iommu/20200107134125.GD30750@8bytes.org/T/#t
> 
> VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at the
> VMD endpoint. The problem with this approach is that the VMD endpoint's
> device-specific attributes, such as the DMA Mask Bits, are used instead.
> 
> This set cleans up VMD by removing the override that redirects DMA map
> operations to the VMD endpoint. Instead it introduces a new DMA alias mechanism
> into the existing DMA alias infrastructure.
> 
> v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> pci_dev and did the necessary DMA alias and IOMMU modifications.
> 
> v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> removed the need to add a pointer in struct device or pci_dev. Weak functions
> are generally frowned upon when it's a single architecture implementation, so I
> am open to alternatives.
> 
> v3 references the pci_dev rather than the struct device for the PCI
> 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> device, though I don't expect the VMD endpoint to need intra-bus DMA aliases.
> 
> Changes from v2:
> Uses struct pci_dev for PCI Device 'Direct DMA aliasing' (pci_direct_dma_alias)
> Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct DMA alias'
> 
> Changes from v1:
> Removed 1/5 & 2/5 misc fix patches that were merged
> Uses Christoph's staging/cleanup patches
> Introduce weak function rather than including pointer in struct device or pci_dev.
> 
> Based on Joerg's next:
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> 
> Jon Derrick (5):
>   x86/pci: Add a to_pci_sysdata helper
>   x86/PCI: Expose VMD's PCI Device in pci_sysdata
>   PCI: Introduce pci_direct_dma_alias()
>   PCI: vmd: Stop overriding dma_map_ops
>   x86/pci: Remove X86_DEV_DMA_OPS
> 
>  arch/x86/Kconfig               |   3 -
>  arch/x86/include/asm/device.h  |  10 ---
>  arch/x86/include/asm/pci.h     |  31 ++++-----
>  arch/x86/pci/common.c          |  45 ++----------
>  drivers/iommu/intel-iommu.c    |  18 +++--
>  drivers/pci/controller/Kconfig |   1 -
>  drivers/pci/controller/vmd.c   | 152 +----------------------------------------
>  drivers/pci/pci.c              |  19 +++++-
>  drivers/pci/search.c           |   7 ++
>  include/linux/pci.h            |   1 +
>  10 files changed, 61 insertions(+), 226 deletions(-)

Jon, Christoph,

AFAICS this series supersedes/overrides:

https://patchwork.kernel.org/patch/11114831/

Please let me know if that's correct, actually I was waiting to
see consensus on the patch above but if this series supersedes
it I would drop it from the PCI review queue.

Thanks,
Lorenzo
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 1/5] x86/pci: Add a to_pci_sysdata helper

[Bjorn Helgaas]

On Fri, Jan 10, 2020 at 10:21:09AM -0700, Jon Derrick wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> Various helpers need the pci_sysdata just to dereference a single field
> in it.  Add a little helper that returns the properly typed sysdata
> pointer to require a little less boilerplate code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> [jonathan.derrick: added un-const cast]
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  arch/x86/include/asm/pci.h | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> index 90d0731..cf680c5 100644
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -35,12 +35,15 @@ struct pci_sysdata {
>  
>  #ifdef CONFIG_PCI
>  
> +static inline struct pci_sysdata *to_pci_sysdata(struct pci_bus *bus)
> +{
> +	return bus->sysdata;
> +}
> +
>  #ifdef CONFIG_PCI_DOMAINS
>  static inline int pci_domain_nr(struct pci_bus *bus)
>  {
> -	struct pci_sysdata *sd = bus->sysdata;
> -
> -	return sd->domain;
> +	return to_pci_sysdata(bus)->domain;
>  }
>  
>  static inline int pci_proc_domain(struct pci_bus *bus)
> @@ -52,23 +55,20 @@ static inline int pci_proc_domain(struct pci_bus *bus)
>  #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
>  static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
>  {
> -	struct pci_sysdata *sd = bus->sysdata;
> -
> -	return sd->fwnode;
> +	return to_pci_sysdata(bus)->fwnode;
>  }
>  
>  #define pci_root_bus_fwnode	_pci_root_bus_fwnode
>  #endif
>  
> +#if IS_ENABLED(CONFIG_VMD)
>  static inline bool is_vmd(struct pci_bus *bus)
>  {
> -#if IS_ENABLED(CONFIG_VMD)
> -	struct pci_sysdata *sd = bus->sysdata;
> -
> -	return sd->vmd_domain;
> +	return to_pci_sysdata(bus)->vmd_domain;
> +}
>  #else
> -	return false;
> -#endif
> +#define is_vmd(bus)		false
> +#endif /* CONFIG_VMD */
>  }

I think this patch leaves this stray close brace here (it's cleaned up
in the next patch, but looks like it will break bisection).

Also, when you fix this, can you update the subject lines?  There's a
mix of "x86/PCI" and "x86/pci" (the convention per "git log --oneline"
is "x86/PCI").

>  /* Can be used to override the logic in pci_scan_bus for skipping
> @@ -124,9 +124,7 @@ static inline void early_quirks(void) { }
>  /* Returns the node based on pci bus */
>  static inline int __pcibus_to_node(const struct pci_bus *bus)
>  {
> -	const struct pci_sysdata *sd = bus->sysdata;
> -
> -	return sd->node;
> +	return to_pci_sysdata((struct pci_bus *) bus)->node;
>  }
>  
>  static inline const struct cpumask *
> -- 
> 1.8.3.1
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

[Derrick, Jonathan]

Hi Lorenzo,

On Mon, 2020-01-13 at 12:08 +0000, Lorenzo Pieralisi wrote:
> On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> > v2 Set: https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derrick@intel.com/T/#t
> > v1 Set: https://lore.kernel.org/linux-iommu/20200107134125.GD30750@8bytes.org/T/#t
> > 
> > VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at the
> > VMD endpoint. The problem with this approach is that the VMD endpoint's
> > device-specific attributes, such as the DMA Mask Bits, are used instead.
> > 
> > This set cleans up VMD by removing the override that redirects DMA map
> > operations to the VMD endpoint. Instead it introduces a new DMA alias mechanism
> > into the existing DMA alias infrastructure.
> > 
> > v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> > pci_dev and did the necessary DMA alias and IOMMU modifications.
> > 
> > v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> > removed the need to add a pointer in struct device or pci_dev. Weak functions
> > are generally frowned upon when it's a single architecture implementation, so I
> > am open to alternatives.
> > 
> > v3 references the pci_dev rather than the struct device for the PCI
> > 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> > pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> > device, though I don't expect the VMD endpoint to need intra-bus DMA aliases.
> > 
> > Changes from v2:
> > Uses struct pci_dev for PCI Device 'Direct DMA aliasing' (pci_direct_dma_alias)
> > Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct DMA alias'
> > 
> > Changes from v1:
> > Removed 1/5 & 2/5 misc fix patches that were merged
> > Uses Christoph's staging/cleanup patches
> > Introduce weak function rather than including pointer in struct device or pci_dev.
> > 
> > Based on Joerg's next:
> > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> > 
> > Jon Derrick (5):
> >   x86/pci: Add a to_pci_sysdata helper
> >   x86/PCI: Expose VMD's PCI Device in pci_sysdata
> >   PCI: Introduce pci_direct_dma_alias()
> >   PCI: vmd: Stop overriding dma_map_ops
> >   x86/pci: Remove X86_DEV_DMA_OPS
> > 
> >  arch/x86/Kconfig               |   3 -
> >  arch/x86/include/asm/device.h  |  10 ---
> >  arch/x86/include/asm/pci.h     |  31 ++++-----
> >  arch/x86/pci/common.c          |  45 ++----------
> >  drivers/iommu/intel-iommu.c    |  18 +++--
> >  drivers/pci/controller/Kconfig |   1 -
> >  drivers/pci/controller/vmd.c   | 152 +----------------------------------------
> >  drivers/pci/pci.c              |  19 +++++-
> >  drivers/pci/search.c           |   7 ++
> >  include/linux/pci.h            |   1 +
> >  10 files changed, 61 insertions(+), 226 deletions(-)
> 
> Jon, Christoph,
> 
> AFAICS this series supersedes/overrides:
> 
> https://patchwork.kernel.org/patch/11114831/
> 
> Please let me know if that's correct, actually I was waiting to
> see consensus on the patch above but if this series supersedes
> it I would drop it from the PCI review queue.
> 
> Thanks,
> Lorenzo

It does supercede it (with Christoph's blessing). By the way, I have
been basing on Joerg's repo  due to the v1/RFC IOMMU modifcations. As
there's more pci work at this point, should I base it on Bjorn's repo
instead?
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

[Bjorn Helgaas]

On Mon, Jan 13, 2020 at 05:13:38PM +0000, Derrick, Jonathan wrote:
> On Mon, 2020-01-13 at 12:08 +0000, Lorenzo Pieralisi wrote:
> > On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> > > v2 Set: https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derrick@intel.com/T/#t
> > > v1 Set: https://lore.kernel.org/linux-iommu/20200107134125.GD30750@8bytes.org/T/#t
> > > 
> > > VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at the
> > > VMD endpoint. The problem with this approach is that the VMD endpoint's
> > > device-specific attributes, such as the DMA Mask Bits, are used instead.
> > > 
> > > This set cleans up VMD by removing the override that redirects DMA map
> > > operations to the VMD endpoint. Instead it introduces a new DMA alias mechanism
> > > into the existing DMA alias infrastructure.
> > > 
> > > v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> > > pci_dev and did the necessary DMA alias and IOMMU modifications.
> > > 
> > > v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> > > removed the need to add a pointer in struct device or pci_dev. Weak functions
> > > are generally frowned upon when it's a single architecture implementation, so I
> > > am open to alternatives.
> > > 
> > > v3 references the pci_dev rather than the struct device for the PCI
> > > 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> > > pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> > > device, though I don't expect the VMD endpoint to need intra-bus DMA aliases.
> > > 
> > > Changes from v2:
> > > Uses struct pci_dev for PCI Device 'Direct DMA aliasing' (pci_direct_dma_alias)
> > > Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct DMA alias'
> > > 
> > > Changes from v1:
> > > Removed 1/5 & 2/5 misc fix patches that were merged
> > > Uses Christoph's staging/cleanup patches
> > > Introduce weak function rather than including pointer in struct device or pci_dev.
> > > 
> > > Based on Joerg's next:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> > > 
> > > Jon Derrick (5):
> > >   x86/pci: Add a to_pci_sysdata helper
> > >   x86/PCI: Expose VMD's PCI Device in pci_sysdata
> > >   PCI: Introduce pci_direct_dma_alias()
> > >   PCI: vmd: Stop overriding dma_map_ops
> > >   x86/pci: Remove X86_DEV_DMA_OPS
> > > 
> > >  arch/x86/Kconfig               |   3 -
> > >  arch/x86/include/asm/device.h  |  10 ---
> > >  arch/x86/include/asm/pci.h     |  31 ++++-----
> > >  arch/x86/pci/common.c          |  45 ++----------
> > >  drivers/iommu/intel-iommu.c    |  18 +++--
> > >  drivers/pci/controller/Kconfig |   1 -
> > >  drivers/pci/controller/vmd.c   | 152 +----------------------------------------
> > >  drivers/pci/pci.c              |  19 +++++-
> > >  drivers/pci/search.c           |   7 ++
> > >  include/linux/pci.h            |   1 +
> > >  10 files changed, 61 insertions(+), 226 deletions(-)
> > 
> > Jon, Christoph,
> > 
> > AFAICS this series supersedes/overrides:
> > 
> > https://patchwork.kernel.org/patch/11114831/
> > 
> > Please let me know if that's correct, actually I was waiting to
> > see consensus on the patch above but if this series supersedes
> > it I would drop it from the PCI review queue.
> > 
> > Thanks,
> > Lorenzo
> 
> It does supercede it (with Christoph's blessing). By the way, I have
> been basing on Joerg's repo  due to the v1/RFC IOMMU modifcations. As
> there's more pci work at this point, should I base it on Bjorn's repo
> instead?

In general if I'm going to apply something, I prefer it based on my
"master" branch unless there's a reason to the contrary.  I think
Lorenzo works pretty much the same way.

Lorenzo will probably handle this series, but I applied it
experimentally to check out the brace thing, and it applied fine to my
"master" branch.  So I think everything's fine as-is.

Bjorn
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

[Lorenzo Pieralisi]

On Mon, Jan 13, 2020 at 12:01:13PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 13, 2020 at 05:13:38PM +0000, Derrick, Jonathan wrote:
> > On Mon, 2020-01-13 at 12:08 +0000, Lorenzo Pieralisi wrote:
> > > On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> > > > v2 Set: https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derrick@intel.com/T/#t
> > > > v1 Set: https://lore.kernel.org/linux-iommu/20200107134125.GD30750@8bytes.org/T/#t
> > > > 
> > > > VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at the
> > > > VMD endpoint. The problem with this approach is that the VMD endpoint's
> > > > device-specific attributes, such as the DMA Mask Bits, are used instead.
> > > > 
> > > > This set cleans up VMD by removing the override that redirects DMA map
> > > > operations to the VMD endpoint. Instead it introduces a new DMA alias mechanism
> > > > into the existing DMA alias infrastructure.
> > > > 
> > > > v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> > > > pci_dev and did the necessary DMA alias and IOMMU modifications.
> > > > 
> > > > v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> > > > removed the need to add a pointer in struct device or pci_dev. Weak functions
> > > > are generally frowned upon when it's a single architecture implementation, so I
> > > > am open to alternatives.
> > > > 
> > > > v3 references the pci_dev rather than the struct device for the PCI
> > > > 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> > > > pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> > > > device, though I don't expect the VMD endpoint to need intra-bus DMA aliases.
> > > > 
> > > > Changes from v2:
> > > > Uses struct pci_dev for PCI Device 'Direct DMA aliasing' (pci_direct_dma_alias)
> > > > Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct DMA alias'
> > > > 
> > > > Changes from v1:
> > > > Removed 1/5 & 2/5 misc fix patches that were merged
> > > > Uses Christoph's staging/cleanup patches
> > > > Introduce weak function rather than including pointer in struct device or pci_dev.
> > > > 
> > > > Based on Joerg's next:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> > > > 
> > > > Jon Derrick (5):
> > > >   x86/pci: Add a to_pci_sysdata helper
> > > >   x86/PCI: Expose VMD's PCI Device in pci_sysdata
> > > >   PCI: Introduce pci_direct_dma_alias()
> > > >   PCI: vmd: Stop overriding dma_map_ops
> > > >   x86/pci: Remove X86_DEV_DMA_OPS
> > > > 
> > > >  arch/x86/Kconfig               |   3 -
> > > >  arch/x86/include/asm/device.h  |  10 ---
> > > >  arch/x86/include/asm/pci.h     |  31 ++++-----
> > > >  arch/x86/pci/common.c          |  45 ++----------
> > > >  drivers/iommu/intel-iommu.c    |  18 +++--
> > > >  drivers/pci/controller/Kconfig |   1 -
> > > >  drivers/pci/controller/vmd.c   | 152 +----------------------------------------
> > > >  drivers/pci/pci.c              |  19 +++++-
> > > >  drivers/pci/search.c           |   7 ++
> > > >  include/linux/pci.h            |   1 +
> > > >  10 files changed, 61 insertions(+), 226 deletions(-)
> > > 
> > > Jon, Christoph,
> > > 
> > > AFAICS this series supersedes/overrides:
> > > 
> > > https://patchwork.kernel.org/patch/11114831/
> > > 
> > > Please let me know if that's correct, actually I was waiting to
> > > see consensus on the patch above but if this series supersedes
> > > it I would drop it from the PCI review queue.
> > > 
> > > Thanks,
> > > Lorenzo
> > 
> > It does supercede it (with Christoph's blessing). By the way, I have
> > been basing on Joerg's repo  due to the v1/RFC IOMMU modifcations. As
> > there's more pci work at this point, should I base it on Bjorn's repo
> > instead?
> 
> In general if I'm going to apply something, I prefer it based on my
> "master" branch unless there's a reason to the contrary.  I think
> Lorenzo works pretty much the same way.
> 
> Lorenzo will probably handle this series, but I applied it
> experimentally to check out the brace thing, and it applied fine to my
> "master" branch.  So I think everything's fine as-is.

I think it is mostly x86+core PCI code, VMD code is mostly deletions
so I think you can take it, let me know, I can ACK the VMD changes
straight away.

Thanks,
Lorenzo
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 1/5] x86/pci: Add a to_pci_sysdata helper

[Christoph Hellwig]

On Fri, Jan 10, 2020 at 10:21:09AM -0700, Jon Derrick wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> Various helpers need the pci_sysdata just to dereference a single field
> in it.  Add a little helper that returns the properly typed sysdata
> pointer to require a little less boilerplate code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> [jonathan.derrick: added un-const cast]

> +	return to_pci_sysdata((struct pci_bus *) bus)->node;

Instead of this case I think we'd be better off marking the bus
argument to to_pci_sysdata const.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 2/5] x86/PCI: Expose VMD's PCI Device in pci_sysdata

[Christoph Hellwig]

On Fri, Jan 10, 2020 at 10:21:10AM -0700, Jon Derrick wrote:
> To be used by Intel-IOMMU code to find the correct domain.
> 
> CC: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>

Looks good, modulo the brace bisection issue already pointed out:

Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 3/5] PCI: Introduce pci_direct_dma_alias()

[Christoph Hellwig]

On Fri, Jan 10, 2020 at 10:21:11AM -0700, Jon Derrick wrote:
> The current DMA alias implementation requires the aliased device be on
> the same PCI bus as the requester ID. This introduces an arch-specific
> mechanism to point to another PCI device when doing mapping and
> PCI DMA alias search.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  arch/x86/pci/common.c |  7 +++++++
>  drivers/pci/pci.c     | 19 ++++++++++++++++++-
>  drivers/pci/search.c  |  7 +++++++
>  include/linux/pci.h   |  1 +
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 1e59df0..83334a5 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -736,3 +736,10 @@ int pci_ext_cfg_avail(void)
>  	else
>  		return 0;
>  }
> +
> +#if IS_ENABLED(CONFIG_VMD)
> +struct pci_dev *pci_direct_dma_alias(struct pci_dev *dev)
> +{
> +	return to_pci_sysdata(dev->bus)->vmd_dev;
> +}
> +#endif
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index ad746d9..1362694 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6034,7 +6034,9 @@ bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2)
>  	return (dev1->dma_alias_mask &&
>  		test_bit(dev2->devfn, dev1->dma_alias_mask)) ||
>  	       (dev2->dma_alias_mask &&
> -		test_bit(dev1->devfn, dev2->dma_alias_mask));
> +		test_bit(dev1->devfn, dev2->dma_alias_mask)) ||
> +	       (pci_direct_dma_alias(dev1) == dev2) ||
> +	       (pci_direct_dma_alias(dev2) == dev1);

No need for the inner braces here.

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 4/5] PCI: vmd: Stop overriding dma_map_ops

[Christoph Hellwig]

On Fri, Jan 10, 2020 at 10:21:12AM -0700, Jon Derrick wrote:
> Devices on the VMD domain use the VMD endpoint's requester ID and have
> been relying on the VMD endpoint's DMA operations. The problem with this
> was that VMD domain devices would use the VMD endpoint's attributes when
> doing DMA and IOMMU mapping. We can be smarter about this by only using
> the VMD endpoint when mapping and providing the correct child device's
> attributes during DMA operations.
> 
> This patch modifies Intel-IOMMU to check for a 'Direct DMA Alias' and
> refer to it for mapping.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> ---
>  drivers/iommu/intel-iommu.c    |  18 +++--
>  drivers/pci/controller/Kconfig |   1 -
>  drivers/pci/controller/vmd.c   | 150 -----------------------------------------
>  3 files changed, 13 insertions(+), 156 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 716347e2..7ca807a 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -804,14 +804,14 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
>  
>  	if (dev_is_pci(dev)) {
>  		struct pci_dev *pf_pdev;
> +		struct pci_dev *dma_alias;
>  
>  		pdev = to_pci_dev(dev);
>  
> -#ifdef CONFIG_X86
> -		/* VMD child devices currently cannot be handled individually */
> -		if (is_vmd(pdev->bus))
> -			return NULL;
> -#endif

Don't we need this sanity check to prevent assingning vmd subdevices?

> +		/* DMA aliased devices use the DMA alias's IOMMU */
> +		dma_alias = pci_direct_dma_alias(pdev);
> +		if (dma_alias)
> +			pdev = dma_alias;
>  
>  		/* VFs aren't listed in scope tables; we need to look up
>  		 * the PF instead to find the IOMMU. */
> @@ -2521,6 +2521,14 @@ struct dmar_domain *find_domain(struct device *dev)
>  		     dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO))
>  		return NULL;
>  
> +	if (dev_is_pci(dev)) {
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +		struct pci_dev *dma_alias = pci_direct_dma_alias(pdev);
> +
> +		if (dma_alias)
> +			dev = &dma_alias->dev;

Instead of all these duplicate calls, shouldn't pci_direct_dma_alias be
replaced with a pci_real_dma_dev helper that either returns the
dma parent if it exiѕts, or the actual device?

Also I think this patch should be split - one for intel-iommu that
just adds the real device checks, and then one that wires up vmd to
the new mechanism and then removes all the cruft.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 4/5] PCI: vmd: Stop overriding dma_map_ops

[Derrick, Jonathan]

On Tue, 2020-01-14 at 09:54 +0100, Christoph Hellwig wrote:
> On Fri, Jan 10, 2020 at 10:21:12AM -0700, Jon Derrick wrote:
> > Devices on the VMD domain use the VMD endpoint's requester ID and have
> > been relying on the VMD endpoint's DMA operations. The problem with this
> > was that VMD domain devices would use the VMD endpoint's attributes when
> > doing DMA and IOMMU mapping. We can be smarter about this by only using
> > the VMD endpoint when mapping and providing the correct child device's
> > attributes during DMA operations.
> > 
> > This patch modifies Intel-IOMMU to check for a 'Direct DMA Alias' and
> > refer to it for mapping.
> > 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> > ---
> >  drivers/iommu/intel-iommu.c    |  18 +++--
> >  drivers/pci/controller/Kconfig |   1 -
> >  drivers/pci/controller/vmd.c   | 150 -----------------------------------------
> >  3 files changed, 13 insertions(+), 156 deletions(-)
> > 
> > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > index 716347e2..7ca807a 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -804,14 +804,14 @@ static struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devf
> >  
> >  	if (dev_is_pci(dev)) {
> >  		struct pci_dev *pf_pdev;
> > +		struct pci_dev *dma_alias;
> >  
> >  		pdev = to_pci_dev(dev);
> >  
> > -#ifdef CONFIG_X86
> > -		/* VMD child devices currently cannot be handled individually */
> > -		if (is_vmd(pdev->bus))
> > -			return NULL;
> > -#endif
> 
> Don't we need this sanity check to prevent assingning vmd subdevices?
I don't think it's necessary now. The new code results in the child
devices being assigned the same IOMMU group as the VMD endpoint.
(AFAIK) You have to assign all devices in a group when doing
assignment, so by unbinding the VMD endpoint in order to assign it, you
would lose the child devices in the host.


> 
> > +		/* DMA aliased devices use the DMA alias's IOMMU */
> > +		dma_alias = pci_direct_dma_alias(pdev);
> > +		if (dma_alias)
> > +			pdev = dma_alias;
> >  
> >  		/* VFs aren't listed in scope tables; we need to look up
> >  		 * the PF instead to find the IOMMU. */
> > @@ -2521,6 +2521,14 @@ struct dmar_domain *find_domain(struct device *dev)
> >  		     dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO))
> >  		return NULL;
> >  
> > +	if (dev_is_pci(dev)) {
> > +		struct pci_dev *pdev = to_pci_dev(dev);
> > +		struct pci_dev *dma_alias = pci_direct_dma_alias(pdev);
> > +
> > +		if (dma_alias)
> > +			dev = &dma_alias->dev;
> 
> Instead of all these duplicate calls, shouldn't pci_direct_dma_alias be
> replaced with a pci_real_dma_dev helper that either returns the
> dma parent if it exiѕts, or the actual device?
> 
> Also I think this patch should be split - one for intel-iommu that
> just adds the real device checks, and then one that wires up vmd to
> the new mechanism and then removes all the cruft.


Thanks for the review. I'll work on the suggestion
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu