* [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device
@ 2021-01-26 11:53 Lianbo Jiang
2021-01-26 11:53 ` [PATCH v3 1/2] dma-iommu: use static-key to minimize the impact in the fast-path Lianbo Jiang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Lianbo Jiang @ 2021-01-26 11:53 UTC (permalink / raw)
To: linux-kernel
Cc: joro, will, iommu, jsnitsel, thomas.lendacky, robin.murphy, bhe,
hch, baolu.lu
This patchset is to fix the failure of deferred attach for iommu attach
device, it includes the following two patches:
[1] [PATCH 1/2] dma-iommu: use static-key to minimize the impact in the fast-path
This is a prepared patch for the second one, move out the is_kdump_kernel()
check from iommu_dma_deferred_attach() to iommu_dma_init(), and use the
static-key in the fast-path to minimize the impact in the normal case.
[2] [PATCH 2/2] iommu: use the __iommu_attach_device() directly for deferred attach
Move the handling currently in iommu_dma_deferred_attach() into the
iommu core code so that it can call the __iommu_attach_device()
directly instead of the iommu_attach_device(). The external interface
iommu_attach_device() is not suitable for handling this situation.
Changes since v1:
[1] use the __iommu_attach_device() directly for deferred attach
[2] use static-key to minimize the impact in the fast-path
Changes since v2:
[1] remove the underscores for the variable "__deferred_attach", and change
its name to iommu_deferred_attach_enabled [Suggested by Christoph Hellwig]
[2] remove the "do_" from the iommu_do_deferred_attach(), and change its
name to iommu_deferred_attach()
[3] remove the "extern" from the definition of iommu_deferred_attach() in
include/linux/iommu.h
Lianbo Jiang (2):
dma-iommu: use static-key to minimize the impact in the fast-path
iommu: use the __iommu_attach_device() directly for deferred attach
drivers/iommu/dma-iommu.c | 29 +++++++++++------------------
drivers/iommu/iommu.c | 10 ++++++++++
include/linux/iommu.h | 1 +
3 files changed, 22 insertions(+), 18 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 1/2] dma-iommu: use static-key to minimize the impact in the fast-path 2021-01-26 11:53 [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Lianbo Jiang @ 2021-01-26 11:53 ` Lianbo Jiang 2021-01-26 11:53 ` [PATCH v3 2/2] iommu: use the __iommu_attach_device() directly for deferred attach Lianbo Jiang 2021-01-28 12:28 ` [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Joerg Roedel 2 siblings, 0 replies; 5+ messages in thread From: Lianbo Jiang @ 2021-01-26 11:53 UTC (permalink / raw) To: linux-kernel Cc: joro, will, iommu, jsnitsel, thomas.lendacky, robin.murphy, bhe, hch, baolu.lu Let's move out the is_kdump_kernel() check from iommu_dma_deferred_attach() to iommu_dma_init(), and use the static-key in the fast-path to minimize the impact in the normal case. Signed-off-by: Lianbo Jiang <lijiang@redhat.com> Co-developed-by: Robin Murphy <robin.murphy@arm.com> Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- drivers/iommu/dma-iommu.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 4078358ed66e..c80056f6c9f9 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -51,6 +51,8 @@ struct iommu_dma_cookie { struct iommu_domain *fq_domain; }; +static DEFINE_STATIC_KEY_FALSE(iommu_deferred_attach_enabled); + void iommu_dma_free_cpu_cached_iovas(unsigned int cpu, struct iommu_domain *domain) { @@ -383,9 +385,6 @@ static int iommu_dma_deferred_attach(struct device *dev, { const struct iommu_ops *ops = domain->ops; - if (!is_kdump_kernel()) - return 0; - if (unlikely(ops->is_attach_deferred && ops->is_attach_deferred(domain, dev))) return iommu_attach_device(domain, dev); @@ -535,7 +534,8 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys, size_t iova_off = iova_offset(iovad, phys); dma_addr_t iova; - if (unlikely(iommu_dma_deferred_attach(dev, domain))) + if (static_branch_unlikely(&iommu_deferred_attach_enabled) && + iommu_dma_deferred_attach(dev, domain)) return DMA_MAPPING_ERROR; size = iova_align(iovad, size + iova_off); @@ -693,7 +693,8 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size, *dma_handle = DMA_MAPPING_ERROR; - if (unlikely(iommu_dma_deferred_attach(dev, domain))) + if (static_branch_unlikely(&iommu_deferred_attach_enabled) && + iommu_dma_deferred_attach(dev, domain)) return NULL; min_size = alloc_sizes & -alloc_sizes; @@ -976,7 +977,8 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, unsigned long mask = dma_get_seg_boundary(dev); int i; - if (unlikely(iommu_dma_deferred_attach(dev, domain))) + if (static_branch_unlikely(&iommu_deferred_attach_enabled) && + iommu_dma_deferred_attach(dev, domain)) return 0; if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) @@ -1424,6 +1426,9 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc, static int iommu_dma_init(void) { + if (is_kdump_kernel()) + static_branch_enable(&iommu_deferred_attach_enabled); + return iova_cache_get(); } arch_initcall(iommu_dma_init); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] iommu: use the __iommu_attach_device() directly for deferred attach 2021-01-26 11:53 [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Lianbo Jiang 2021-01-26 11:53 ` [PATCH v3 1/2] dma-iommu: use static-key to minimize the impact in the fast-path Lianbo Jiang @ 2021-01-26 11:53 ` Lianbo Jiang 2021-01-26 12:12 ` Robin Murphy 2021-01-28 12:28 ` [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Joerg Roedel 2 siblings, 1 reply; 5+ messages in thread From: Lianbo Jiang @ 2021-01-26 11:53 UTC (permalink / raw) To: linux-kernel Cc: joro, will, iommu, jsnitsel, thomas.lendacky, robin.murphy, bhe, hch, baolu.lu Currently, because domain attach allows to be deferred from iommu driver to device driver, and when iommu initializes, the devices on the bus will be scanned and the default groups will be allocated. Due to the above changes, some devices could be added to the same group as below: [ 3.859417] pci 0000:01:00.0: Adding to iommu group 16 [ 3.864572] pci 0000:01:00.1: Adding to iommu group 16 [ 3.869738] pci 0000:02:00.0: Adding to iommu group 17 [ 3.874892] pci 0000:02:00.1: Adding to iommu group 17 But when attaching these devices, it doesn't allow that a group has more than one device, otherwise it will return an error. This conflicts with the deferred attaching. Unfortunately, it has two devices in the same group for my side, for example: [ 9.627014] iommu_group_device_count(): device name[0]:0000:01:00.0 [ 9.633545] iommu_group_device_count(): device name[1]:0000:01:00.1 ... [ 10.255609] iommu_group_device_count(): device name[0]:0000:02:00.0 [ 10.262144] iommu_group_device_count(): device name[1]:0000:02:00.1 Finally, which caused the failure of tg3 driver when tg3 driver calls the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma(). [ 9.660310] tg3 0000:01:00.0: DMA engine test failed, aborting [ 9.754085] tg3: probe of 0000:01:00.0 failed with error -12 [ 9.997512] tg3 0000:01:00.1: DMA engine test failed, aborting [ 10.043053] tg3: probe of 0000:01:00.1 failed with error -12 [ 10.288905] tg3 0000:02:00.0: DMA engine test failed, aborting [ 10.334070] tg3: probe of 0000:02:00.0 failed with error -12 [ 10.578303] tg3 0000:02:00.1: DMA engine test failed, aborting [ 10.622629] tg3: probe of 0000:02:00.1 failed with error -12 In addition, the similar situations also occur in other drivers such as the bnxt_en driver. That can be reproduced easily in kdump kernel when SME is active. Let's move the handling currently in iommu_dma_deferred_attach() into the iommu core code so that it can call the __iommu_attach_device() directly instead of the iommu_attach_device(). The external interface iommu_attach_device() is not suitable for handling this situation. Signed-off-by: Lianbo Jiang <lijiang@redhat.com> --- drivers/iommu/dma-iommu.c | 18 +++--------------- drivers/iommu/iommu.c | 10 ++++++++++ include/linux/iommu.h | 1 + 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index c80056f6c9f9..f659395e7959 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -380,18 +380,6 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, return iova_reserve_iommu_regions(dev, domain); } -static int iommu_dma_deferred_attach(struct device *dev, - struct iommu_domain *domain) -{ - const struct iommu_ops *ops = domain->ops; - - if (unlikely(ops->is_attach_deferred && - ops->is_attach_deferred(domain, dev))) - return iommu_attach_device(domain, dev); - - return 0; -} - /** * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API * page flags. @@ -535,7 +523,7 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys, dma_addr_t iova; if (static_branch_unlikely(&iommu_deferred_attach_enabled) && - iommu_dma_deferred_attach(dev, domain)) + iommu_deferred_attach(dev, domain)) return DMA_MAPPING_ERROR; size = iova_align(iovad, size + iova_off); @@ -694,7 +682,7 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size, *dma_handle = DMA_MAPPING_ERROR; if (static_branch_unlikely(&iommu_deferred_attach_enabled) && - iommu_dma_deferred_attach(dev, domain)) + iommu_deferred_attach(dev, domain)) return NULL; min_size = alloc_sizes & -alloc_sizes; @@ -978,7 +966,7 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int i; if (static_branch_unlikely(&iommu_deferred_attach_enabled) && - iommu_dma_deferred_attach(dev, domain)) + iommu_deferred_attach(dev, domain)) return 0; if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index ffeebda8d6de..15b5fd6bd554 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1980,6 +1980,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) } EXPORT_SYMBOL_GPL(iommu_attach_device); +int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) +{ + const struct iommu_ops *ops = domain->ops; + + if (ops->is_attach_deferred && ops->is_attach_deferred(domain, dev)) + return __iommu_attach_device(domain, dev); + + return 0; +} + /* * Check flags and other user provided data for valid combinations. We also * make sure no reserved fields or unused flags are set. This is to ensure diff --git a/include/linux/iommu.h b/include/linux/iommu.h index b3f0e2018c62..92ee9b56f118 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -376,6 +376,7 @@ int iommu_device_sysfs_add(struct iommu_device *iommu, void iommu_device_sysfs_remove(struct iommu_device *iommu); int iommu_device_link(struct iommu_device *iommu, struct device *link); void iommu_device_unlink(struct iommu_device *iommu, struct device *link); +int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain); static inline void __iommu_device_set_ops(struct iommu_device *iommu, const struct iommu_ops *ops) -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] iommu: use the __iommu_attach_device() directly for deferred attach 2021-01-26 11:53 ` [PATCH v3 2/2] iommu: use the __iommu_attach_device() directly for deferred attach Lianbo Jiang @ 2021-01-26 12:12 ` Robin Murphy 0 siblings, 0 replies; 5+ messages in thread From: Robin Murphy @ 2021-01-26 12:12 UTC (permalink / raw) To: Lianbo Jiang, linux-kernel; +Cc: thomas.lendacky, will, hch, iommu On 2021-01-26 11:53, Lianbo Jiang wrote: > Currently, because domain attach allows to be deferred from iommu > driver to device driver, and when iommu initializes, the devices > on the bus will be scanned and the default groups will be allocated. > > Due to the above changes, some devices could be added to the same > group as below: > > [ 3.859417] pci 0000:01:00.0: Adding to iommu group 16 > [ 3.864572] pci 0000:01:00.1: Adding to iommu group 16 > [ 3.869738] pci 0000:02:00.0: Adding to iommu group 17 > [ 3.874892] pci 0000:02:00.1: Adding to iommu group 17 > > But when attaching these devices, it doesn't allow that a group has > more than one device, otherwise it will return an error. This conflicts > with the deferred attaching. Unfortunately, it has two devices in the > same group for my side, for example: > > [ 9.627014] iommu_group_device_count(): device name[0]:0000:01:00.0 > [ 9.633545] iommu_group_device_count(): device name[1]:0000:01:00.1 > ... > [ 10.255609] iommu_group_device_count(): device name[0]:0000:02:00.0 > [ 10.262144] iommu_group_device_count(): device name[1]:0000:02:00.1 > > Finally, which caused the failure of tg3 driver when tg3 driver calls > the dma_alloc_coherent() to allocate coherent memory in the tg3_test_dma(). > > [ 9.660310] tg3 0000:01:00.0: DMA engine test failed, aborting > [ 9.754085] tg3: probe of 0000:01:00.0 failed with error -12 > [ 9.997512] tg3 0000:01:00.1: DMA engine test failed, aborting > [ 10.043053] tg3: probe of 0000:01:00.1 failed with error -12 > [ 10.288905] tg3 0000:02:00.0: DMA engine test failed, aborting > [ 10.334070] tg3: probe of 0000:02:00.0 failed with error -12 > [ 10.578303] tg3 0000:02:00.1: DMA engine test failed, aborting > [ 10.622629] tg3: probe of 0000:02:00.1 failed with error -12 > > In addition, the similar situations also occur in other drivers such > as the bnxt_en driver. That can be reproduced easily in kdump kernel > when SME is active. > > Let's move the handling currently in iommu_dma_deferred_attach() into > the iommu core code so that it can call the __iommu_attach_device() > directly instead of the iommu_attach_device(). The external interface > iommu_attach_device() is not suitable for handling this situation. No objection to the cosmetic change - arguably parsing "attach" as a verb rather than a noun is more normal anyway :) Thus my previous review still stands: Reviewed-by: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Lianbo Jiang <lijiang@redhat.com> > --- > drivers/iommu/dma-iommu.c | 18 +++--------------- > drivers/iommu/iommu.c | 10 ++++++++++ > include/linux/iommu.h | 1 + > 3 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index c80056f6c9f9..f659395e7959 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -380,18 +380,6 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, > return iova_reserve_iommu_regions(dev, domain); > } > > -static int iommu_dma_deferred_attach(struct device *dev, > - struct iommu_domain *domain) > -{ > - const struct iommu_ops *ops = domain->ops; > - > - if (unlikely(ops->is_attach_deferred && > - ops->is_attach_deferred(domain, dev))) > - return iommu_attach_device(domain, dev); > - > - return 0; > -} > - > /** > * dma_info_to_prot - Translate DMA API directions and attributes to IOMMU API > * page flags. > @@ -535,7 +523,7 @@ static dma_addr_t __iommu_dma_map(struct device *dev, phys_addr_t phys, > dma_addr_t iova; > > if (static_branch_unlikely(&iommu_deferred_attach_enabled) && > - iommu_dma_deferred_attach(dev, domain)) > + iommu_deferred_attach(dev, domain)) > return DMA_MAPPING_ERROR; > > size = iova_align(iovad, size + iova_off); > @@ -694,7 +682,7 @@ static void *iommu_dma_alloc_remap(struct device *dev, size_t size, > *dma_handle = DMA_MAPPING_ERROR; > > if (static_branch_unlikely(&iommu_deferred_attach_enabled) && > - iommu_dma_deferred_attach(dev, domain)) > + iommu_deferred_attach(dev, domain)) > return NULL; > > min_size = alloc_sizes & -alloc_sizes; > @@ -978,7 +966,7 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, > int i; > > if (static_branch_unlikely(&iommu_deferred_attach_enabled) && > - iommu_dma_deferred_attach(dev, domain)) > + iommu_deferred_attach(dev, domain)) > return 0; > > if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC)) > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index ffeebda8d6de..15b5fd6bd554 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -1980,6 +1980,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) > } > EXPORT_SYMBOL_GPL(iommu_attach_device); > > +int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) > +{ > + const struct iommu_ops *ops = domain->ops; > + > + if (ops->is_attach_deferred && ops->is_attach_deferred(domain, dev)) > + return __iommu_attach_device(domain, dev); > + > + return 0; > +} > + > /* > * Check flags and other user provided data for valid combinations. We also > * make sure no reserved fields or unused flags are set. This is to ensure > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index b3f0e2018c62..92ee9b56f118 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -376,6 +376,7 @@ int iommu_device_sysfs_add(struct iommu_device *iommu, > void iommu_device_sysfs_remove(struct iommu_device *iommu); > int iommu_device_link(struct iommu_device *iommu, struct device *link); > void iommu_device_unlink(struct iommu_device *iommu, struct device *link); > +int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain); > > static inline void __iommu_device_set_ops(struct iommu_device *iommu, > const struct iommu_ops *ops) > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device 2021-01-26 11:53 [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Lianbo Jiang 2021-01-26 11:53 ` [PATCH v3 1/2] dma-iommu: use static-key to minimize the impact in the fast-path Lianbo Jiang 2021-01-26 11:53 ` [PATCH v3 2/2] iommu: use the __iommu_attach_device() directly for deferred attach Lianbo Jiang @ 2021-01-28 12:28 ` Joerg Roedel 2 siblings, 0 replies; 5+ messages in thread From: Joerg Roedel @ 2021-01-28 12:28 UTC (permalink / raw) To: Lianbo Jiang Cc: linux-kernel, will, iommu, jsnitsel, thomas.lendacky, robin.murphy, bhe, hch, baolu.lu On Tue, Jan 26, 2021 at 07:53:35PM +0800, Lianbo Jiang wrote: > Lianbo Jiang (2): > dma-iommu: use static-key to minimize the impact in the fast-path > iommu: use the __iommu_attach_device() directly for deferred attach > > drivers/iommu/dma-iommu.c | 29 +++++++++++------------------ > drivers/iommu/iommu.c | 10 ++++++++++ > include/linux/iommu.h | 1 + > 3 files changed, 22 insertions(+), 18 deletions(-) Sorry, missed that there was a newer version. Applied this instead of v2. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-28 12:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-01-26 11:53 [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Lianbo Jiang 2021-01-26 11:53 ` [PATCH v3 1/2] dma-iommu: use static-key to minimize the impact in the fast-path Lianbo Jiang 2021-01-26 11:53 ` [PATCH v3 2/2] iommu: use the __iommu_attach_device() directly for deferred attach Lianbo Jiang 2021-01-26 12:12 ` Robin Murphy 2021-01-28 12:28 ` [PATCH v3 0/2] iommu: fix the failure of deferred attach for iommu attach device Joerg Roedel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox