* stop overriding dma_ops in vmd v2
@ 2019-08-28 14:14 Christoph Hellwig
2019-08-28 14:14 ` [PATCH 1/5] x86/pci: Remove an ifdef __KERNEL__ from pci.h Christoph Hellwig
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Christoph Hellwig @ 2019-08-28 14:14 UTC (permalink / raw)
To: Keith Busch, Derrick, Jonathan
Cc: x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel
Hi all,
this is a new version of the vmd dma_map_ops removal, which does not
require vmd to be built in. Instead we slightly expand the vmd-specific
field in the x86 pci_sysdata to cover that information.
Note that I do not have a vmd-enable system, so some testing by the
maintainers would be welcome.
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/5] x86/pci: Remove an ifdef __KERNEL__ from pci.h 2019-08-28 14:14 stop overriding dma_ops in vmd v2 Christoph Hellwig @ 2019-08-28 14:14 ` Christoph Hellwig 2019-08-28 14:14 ` [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper Christoph Hellwig ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-08-28 14:14 UTC (permalink / raw) To: Keith Busch, Derrick, Jonathan Cc: x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel Non-UAPI headers can't be included by userspace, so remove the pointless ifdef. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/x86/include/asm/pci.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h index e662f987dfa2..6fa846920f5f 100644 --- a/arch/x86/include/asm/pci.h +++ b/arch/x86/include/asm/pci.h @@ -12,8 +12,6 @@ #include <asm/pat.h> #include <asm/x86_init.h> -#ifdef __KERNEL__ - struct pci_sysdata { int domain; /* PCI domain */ int node; /* NUMA node */ @@ -118,7 +116,6 @@ void native_restore_msi_irqs(struct pci_dev *dev); #define native_setup_msi_irqs NULL #define native_teardown_msi_irq NULL #endif -#endif /* __KERNEL__ */ #ifdef CONFIG_X86_64 #include <asm/pci_64.h> -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper 2019-08-28 14:14 stop overriding dma_ops in vmd v2 Christoph Hellwig 2019-08-28 14:14 ` [PATCH 1/5] x86/pci: Remove an ifdef __KERNEL__ from pci.h Christoph Hellwig @ 2019-08-28 14:14 ` Christoph Hellwig 2019-08-28 16:41 ` Derrick, Jonathan 2019-08-28 14:14 ` [PATCH 3/5] x86/pci: Replace the vmd_domain field with a vmd_dev pointer Christoph Hellwig ` (2 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-08-28 14:14 UTC (permalink / raw) To: Keith Busch, Derrick, Jonathan Cc: x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel 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> --- 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 6fa846920f5f..75fe28492290 100644 --- a/arch/x86/include/asm/pci.h +++ b/arch/x86/include/asm/pci.h @@ -35,12 +35,15 @@ extern int noioapicreroute; #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 @@ -128,9 +128,7 @@ void native_restore_msi_irqs(struct pci_dev *dev); /* 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(bus)->node; } static inline const struct cpumask * -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper 2019-08-28 14:14 ` [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper Christoph Hellwig @ 2019-08-28 16:41 ` Derrick, Jonathan 2019-08-29 14:13 ` hch 0 siblings, 1 reply; 10+ messages in thread From: Derrick, Jonathan @ 2019-08-28 16:41 UTC (permalink / raw) To: hch, kbusch; +Cc: iommu, joro, linux-pci, bhelgaas, x86, dwmw2, linux-kernel On Wed, 2019-08-28 at 16:14 +0200, Christoph Hellwig wrote: > 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> > --- > 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 6fa846920f5f..75fe28492290 100644 > --- a/arch/x86/include/asm/pci.h > +++ b/arch/x86/include/asm/pci.h > @@ -35,12 +35,15 @@ extern int noioapicreroute; > > #ifdef CONFIG_PCI > > +static inline struct pci_sysdata *to_pci_sysdata(struct pci_bus *bus) Can you make the argument const to avoid all the warnings from callers passing const struct pci_bus snip ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper 2019-08-28 16:41 ` Derrick, Jonathan @ 2019-08-29 14:13 ` hch 0 siblings, 0 replies; 10+ messages in thread From: hch @ 2019-08-29 14:13 UTC (permalink / raw) To: Derrick, Jonathan Cc: hch, kbusch, iommu, joro, linux-pci, bhelgaas, x86, dwmw2, linux-kernel On Wed, Aug 28, 2019 at 04:41:45PM +0000, Derrick, Jonathan wrote: > > diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h > > index 6fa846920f5f..75fe28492290 100644 > > --- a/arch/x86/include/asm/pci.h > > +++ b/arch/x86/include/asm/pci.h > > @@ -35,12 +35,15 @@ extern int noioapicreroute; > > > > #ifdef CONFIG_PCI > > > > +static inline struct pci_sysdata *to_pci_sysdata(struct pci_bus *bus) > Can you make the argument const to avoid all the warnings from callers > passing const struct pci_bus Yes, I already fixed this up after getting a build bot warning for a NUMA config (which seems to be the only one passing a const). ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] x86/pci: Replace the vmd_domain field with a vmd_dev pointer 2019-08-28 14:14 stop overriding dma_ops in vmd v2 Christoph Hellwig 2019-08-28 14:14 ` [PATCH 1/5] x86/pci: Remove an ifdef __KERNEL__ from pci.h Christoph Hellwig 2019-08-28 14:14 ` [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper Christoph Hellwig @ 2019-08-28 14:14 ` Christoph Hellwig 2019-08-28 14:14 ` [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops Christoph Hellwig 2019-08-28 14:14 ` [PATCH 5/5] x86/pci: Remove X86_DEV_DMA_OPS Christoph Hellwig 4 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-08-28 14:14 UTC (permalink / raw) To: Keith Busch, Derrick, Jonathan Cc: x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel Store the actual VMD device in struct pci_sysdata, so that we can later use it directly for DMA mappings. Signed-off-by: Christoph Hellwig <hch@lst.de> --- 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 75fe28492290..a9bb4cdb66d4 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 device *vmd_dev; /* Main 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 4575e0c6dc4b..785cb657c8c2 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -660,7 +660,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) .parent = res, }; - sd->vmd_domain = true; + sd->vmd_dev = &vmd->dev->dev; sd->domain = vmd_find_free_domain(); if (sd->domain < 0) return sd->domain; -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops 2019-08-28 14:14 stop overriding dma_ops in vmd v2 Christoph Hellwig ` (2 preceding siblings ...) 2019-08-28 14:14 ` [PATCH 3/5] x86/pci: Replace the vmd_domain field with a vmd_dev pointer Christoph Hellwig @ 2019-08-28 14:14 ` Christoph Hellwig 2019-08-28 15:01 ` Keith Busch 2019-08-28 14:14 ` [PATCH 5/5] x86/pci: Remove X86_DEV_DMA_OPS Christoph Hellwig 4 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-08-28 14:14 UTC (permalink / raw) To: Keith Busch, Derrick, Jonathan Cc: x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel With a little tweak to the intel-iommu code we should be able to work around the VMD mess for the requester IDs without having to create giant amounts of boilerplate DMA ops wrapping code. The other advantage of this scheme is that we can respect the real DMA masks for the actual devices, and I bet it will only be a matter of time until we'll see the first DMA challeneged NVMe devices. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/iommu/intel-iommu.c | 25 ++++++ drivers/pci/controller/Kconfig | 1 - drivers/pci/controller/vmd.c | 150 --------------------------------- 3 files changed, 25 insertions(+), 151 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 12d094d08c0a..aaa35ac73956 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -373,6 +373,23 @@ EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped); static DEFINE_SPINLOCK(device_domain_lock); static LIST_HEAD(device_domain_list); +/* + * For VMD we need to use the VMD devices for mapping requests instead of the + * actual device to get the proper PCIe requester ID. + */ +static inline struct device *vmd_real_dev(struct device *dev) +{ +#if IS_ENABLED(CONFIG_VMD) + if (dev_is_pci(dev)) { + struct pci_sysdata *sd = to_pci_dev(dev)->bus->sysdata; + + if (sd->vmd_dev) + return sd->vmd_dev; + } +#endif + return dev; +} + /* * Iterate over elements in device_domain_list and call the specified * callback @fn against each element. @@ -3520,6 +3537,7 @@ static dma_addr_t intel_map_page(struct device *dev, struct page *page, enum dma_data_direction dir, unsigned long attrs) { + dev = vmd_real_dev(dev); if (iommu_need_mapping(dev)) return __intel_map_single(dev, page_to_phys(page) + offset, size, dir, *dev->dma_mask); @@ -3530,6 +3548,7 @@ static dma_addr_t intel_map_resource(struct device *dev, phys_addr_t phys_addr, size_t size, enum dma_data_direction dir, unsigned long attrs) { + dev = vmd_real_dev(dev); if (iommu_need_mapping(dev)) return __intel_map_single(dev, phys_addr, size, dir, *dev->dma_mask); @@ -3585,6 +3604,7 @@ static void intel_unmap_page(struct device *dev, dma_addr_t dev_addr, size_t size, enum dma_data_direction dir, unsigned long attrs) { + dev = vmd_real_dev(dev); if (iommu_need_mapping(dev)) intel_unmap(dev, dev_addr, size); else @@ -3594,6 +3614,7 @@ static void intel_unmap_page(struct device *dev, dma_addr_t dev_addr, static void intel_unmap_resource(struct device *dev, dma_addr_t dev_addr, size_t size, enum dma_data_direction dir, unsigned long attrs) { + dev = vmd_real_dev(dev); if (iommu_need_mapping(dev)) intel_unmap(dev, dev_addr, size); } @@ -3605,6 +3626,7 @@ static void *intel_alloc_coherent(struct device *dev, size_t size, struct page *page = NULL; int order; + dev = vmd_real_dev(dev); if (!iommu_need_mapping(dev)) return dma_direct_alloc(dev, size, dma_handle, flags, attrs); @@ -3641,6 +3663,7 @@ static void intel_free_coherent(struct device *dev, size_t size, void *vaddr, int order; struct page *page = virt_to_page(vaddr); + dev = vmd_real_dev(dev); if (!iommu_need_mapping(dev)) return dma_direct_free(dev, size, vaddr, dma_handle, attrs); @@ -3661,6 +3684,7 @@ static void intel_unmap_sg(struct device *dev, struct scatterlist *sglist, struct scatterlist *sg; int i; + dev = vmd_real_dev(dev); if (!iommu_need_mapping(dev)) return dma_direct_unmap_sg(dev, sglist, nelems, dir, attrs); @@ -3685,6 +3709,7 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele struct intel_iommu *iommu; BUG_ON(dir == DMA_NONE); + dev = vmd_real_dev(dev); if (!iommu_need_mapping(dev)) return dma_direct_map_sg(dev, sglist, nelems, dir, attrs); diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index fe9f9f13ce11..920546cb84e2 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -267,7 +267,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 785cb657c8c2..ba017ebba6a7 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -94,9 +94,6 @@ struct vmd_dev { struct resource resources[3]; struct irq_domain *irq_domain; struct pci_bus *bus; - - struct dma_map_ops dma_ops; - struct dma_domain dma_domain; }; static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus) @@ -291,151 +288,6 @@ static struct msi_domain_info vmd_msi_domain_info = { .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) { @@ -690,7 +542,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); @@ -805,7 +656,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); } -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops 2019-08-28 14:14 ` [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops Christoph Hellwig @ 2019-08-28 15:01 ` Keith Busch 2019-08-29 14:14 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Keith Busch @ 2019-08-28 15:01 UTC (permalink / raw) To: Christoph Hellwig Cc: Derrick, Jonathan, x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel On Wed, Aug 28, 2019 at 07:14:42AM -0700, Christoph Hellwig wrote: > With a little tweak to the intel-iommu code we should be able to work > around the VMD mess for the requester IDs without having to create giant > amounts of boilerplate DMA ops wrapping code. The other advantage of > this scheme is that we can respect the real DMA masks for the actual > devices, and I bet it will only be a matter of time until we'll see the > first DMA challeneged NVMe devices. This tests out fine on VMD hardware, but it's quite different than the previous patch. In v1, the original dev was used in iommu_need_mapping(), but this time it's the vmd device. Is this still using the actual device's DMA mask then? > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/iommu/intel-iommu.c | 25 ++++++ > drivers/pci/controller/Kconfig | 1 - > drivers/pci/controller/vmd.c | 150 --------------------------------- > 3 files changed, 25 insertions(+), 151 deletions(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index 12d094d08c0a..aaa35ac73956 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -373,6 +373,23 @@ EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped); > static DEFINE_SPINLOCK(device_domain_lock); > static LIST_HEAD(device_domain_list); > > +/* > + * For VMD we need to use the VMD devices for mapping requests instead of the > + * actual device to get the proper PCIe requester ID. > + */ > +static inline struct device *vmd_real_dev(struct device *dev) > +{ > +#if IS_ENABLED(CONFIG_VMD) > + if (dev_is_pci(dev)) { > + struct pci_sysdata *sd = to_pci_dev(dev)->bus->sysdata; > + > + if (sd->vmd_dev) > + return sd->vmd_dev; > + } > +#endif > + return dev; > +} > + > /* > * Iterate over elements in device_domain_list and call the specified > * callback @fn against each element. > @@ -3520,6 +3537,7 @@ static dma_addr_t intel_map_page(struct device *dev, struct page *page, > enum dma_data_direction dir, > unsigned long attrs) > { > + dev = vmd_real_dev(dev); > if (iommu_need_mapping(dev)) > return __intel_map_single(dev, page_to_phys(page) + offset, > size, dir, *dev->dma_mask); > @@ -3530,6 +3548,7 @@ static dma_addr_t intel_map_resource(struct device *dev, phys_addr_t phys_addr, > size_t size, enum dma_data_direction dir, > unsigned long attrs) > { > + dev = vmd_real_dev(dev); > if (iommu_need_mapping(dev)) > return __intel_map_single(dev, phys_addr, size, dir, > *dev->dma_mask); > @@ -3585,6 +3604,7 @@ static void intel_unmap_page(struct device *dev, dma_addr_t dev_addr, > size_t size, enum dma_data_direction dir, > unsigned long attrs) > { > + dev = vmd_real_dev(dev); > if (iommu_need_mapping(dev)) > intel_unmap(dev, dev_addr, size); > else > @@ -3594,6 +3614,7 @@ static void intel_unmap_page(struct device *dev, dma_addr_t dev_addr, > static void intel_unmap_resource(struct device *dev, dma_addr_t dev_addr, > size_t size, enum dma_data_direction dir, unsigned long attrs) > { > + dev = vmd_real_dev(dev); > if (iommu_need_mapping(dev)) > intel_unmap(dev, dev_addr, size); > } > @@ -3605,6 +3626,7 @@ static void *intel_alloc_coherent(struct device *dev, size_t size, > struct page *page = NULL; > int order; > > + dev = vmd_real_dev(dev); > if (!iommu_need_mapping(dev)) > return dma_direct_alloc(dev, size, dma_handle, flags, attrs); > > @@ -3641,6 +3663,7 @@ static void intel_free_coherent(struct device *dev, size_t size, void *vaddr, > int order; > struct page *page = virt_to_page(vaddr); > > + dev = vmd_real_dev(dev); > if (!iommu_need_mapping(dev)) > return dma_direct_free(dev, size, vaddr, dma_handle, attrs); > > @@ -3661,6 +3684,7 @@ static void intel_unmap_sg(struct device *dev, struct scatterlist *sglist, > struct scatterlist *sg; > int i; > > + dev = vmd_real_dev(dev); > if (!iommu_need_mapping(dev)) > return dma_direct_unmap_sg(dev, sglist, nelems, dir, attrs); > > @@ -3685,6 +3709,7 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele > struct intel_iommu *iommu; > > BUG_ON(dir == DMA_NONE); > + dev = vmd_real_dev(dev); > if (!iommu_need_mapping(dev)) > return dma_direct_map_sg(dev, sglist, nelems, dir, attrs); > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > index fe9f9f13ce11..920546cb84e2 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -267,7 +267,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 785cb657c8c2..ba017ebba6a7 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -94,9 +94,6 @@ struct vmd_dev { > struct resource resources[3]; > struct irq_domain *irq_domain; > struct pci_bus *bus; > - > - struct dma_map_ops dma_ops; > - struct dma_domain dma_domain; > }; > > static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus) > @@ -291,151 +288,6 @@ static struct msi_domain_info vmd_msi_domain_info = { > .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) > { > @@ -690,7 +542,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); > @@ -805,7 +656,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); > } > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops 2019-08-28 15:01 ` Keith Busch @ 2019-08-29 14:14 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-08-29 14:14 UTC (permalink / raw) To: Keith Busch Cc: Christoph Hellwig, Derrick, Jonathan, x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel On Wed, Aug 28, 2019 at 09:01:06AM -0600, Keith Busch wrote: > On Wed, Aug 28, 2019 at 07:14:42AM -0700, Christoph Hellwig wrote: > > With a little tweak to the intel-iommu code we should be able to work > > around the VMD mess for the requester IDs without having to create giant > > amounts of boilerplate DMA ops wrapping code. The other advantage of > > this scheme is that we can respect the real DMA masks for the actual > > devices, and I bet it will only be a matter of time until we'll see the > > first DMA challeneged NVMe devices. > > This tests out fine on VMD hardware, but it's quite different than the > previous patch. In v1, the original dev was used in iommu_need_mapping(), > but this time it's the vmd device. Is this still using the actual device's > DMA mask then? True. But then again I think the old one was broken as well, as it will pass the wrong dev to identity_mapping() or iommu_request_dma_domain_for_dev. So I guess I'll need to respin it a bit to do the work in iommu_need_mapping again, and then factor that one to make it obvious what device we deal with. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] x86/pci: Remove X86_DEV_DMA_OPS 2019-08-28 14:14 stop overriding dma_ops in vmd v2 Christoph Hellwig ` (3 preceding siblings ...) 2019-08-28 14:14 ` [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops Christoph Hellwig @ 2019-08-28 14:14 ` Christoph Hellwig 4 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-08-28 14:14 UTC (permalink / raw) To: Keith Busch, Derrick, Jonathan Cc: x86, joro, linux-pci, bhelgaas, dwmw2, iommu, linux-kernel There are no users of X86_DEV_DMA_OPS left, so remove the code. 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 222855cc0158..35597dae38b7 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2905,9 +2905,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 a8f6c809d9b1..3e6c75a6d070 100644 --- a/arch/x86/include/asm/device.h +++ b/arch/x86/include/asm/device.h @@ -11,16 +11,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 9acab6ac28f5..d2ac803b6c00 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; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-08-29 14:14 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-28 14:14 stop overriding dma_ops in vmd v2 Christoph Hellwig 2019-08-28 14:14 ` [PATCH 1/5] x86/pci: Remove an ifdef __KERNEL__ from pci.h Christoph Hellwig 2019-08-28 14:14 ` [PATCH 2/5] x86/pci: Add a to_pci_sysdata helper Christoph Hellwig 2019-08-28 16:41 ` Derrick, Jonathan 2019-08-29 14:13 ` hch 2019-08-28 14:14 ` [PATCH 3/5] x86/pci: Replace the vmd_domain field with a vmd_dev pointer Christoph Hellwig 2019-08-28 14:14 ` [PATCH 4/5] PCI/vmd: Stop overriding dma_map_ops Christoph Hellwig 2019-08-28 15:01 ` Keith Busch 2019-08-29 14:14 ` Christoph Hellwig 2019-08-28 14:14 ` [PATCH 5/5] x86/pci: Remove X86_DEV_DMA_OPS Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).