| Message ID | 1-v1-5f734af130a3+34f-iommu_fwspec_jgg@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Solve iommu probe races around iommu_fwspec | expand |
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
On Fri, Nov 03, 2023 at 01:44:46PM -0300, Jason Gunthorpe wrote: > This is not being used to pass ops, it is just a way to tell if an > iommu driver was probed. These days this can be detected directly via > device_iommu_mapped(). Call device_iommu_mapped() in the two places that > need to check it and remove the iommu parameter everywhere. Yes, that's much better than exposing the iommu ops to a place that should not care about them: Acked-by: Christoph Hellwig <hch@lst.de>
On 2023/11/4 0:44, Jason Gunthorpe wrote: > This is not being used to pass ops, it is just a way to tell if an > iommu driver was probed. These days this can be detected directly via > device_iommu_mapped(). Call device_iommu_mapped() in the two places that > need to check it and remove the iommu parameter everywhere. > > Signed-off-by: Jason Gunthorpe<jgg@nvidia.com> > --- > arch/arc/mm/dma.c | 2 +- > arch/arm/mm/dma-mapping-nommu.c | 2 +- > arch/arm/mm/dma-mapping.c | 10 +++++----- > arch/arm64/mm/dma-mapping.c | 4 ++-- > arch/mips/mm/dma-noncoherent.c | 2 +- > arch/riscv/mm/dma-noncoherent.c | 2 +- > drivers/acpi/scan.c | 3 +-- > drivers/hv/hv_common.c | 2 +- > drivers/of/device.c | 2 +- > include/linux/dma-map-ops.h | 4 ++-- > 10 files changed, 16 insertions(+), 17 deletions(-) Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> Best regards, baolu
On Fri, Nov 03, 2023 at 01:44:46PM -0300, Jason Gunthorpe wrote: > This is not being used to pass ops, it is just a way to tell if an > iommu driver was probed. These days this can be detected directly via > device_iommu_mapped(). Call device_iommu_mapped() in the two places that > need to check it and remove the iommu parameter everywhere. > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > arch/arc/mm/dma.c | 2 +- > arch/arm/mm/dma-mapping-nommu.c | 2 +- > arch/arm/mm/dma-mapping.c | 10 +++++----- > arch/arm64/mm/dma-mapping.c | 4 ++-- > arch/mips/mm/dma-noncoherent.c | 2 +- > arch/riscv/mm/dma-noncoherent.c | 2 +- > drivers/acpi/scan.c | 3 +-- > drivers/hv/hv_common.c | 2 +- > drivers/of/device.c | 2 +- Acked-by: Rob Herring <robh@kernel.org> > include/linux/dma-map-ops.h | 4 ++-- > 10 files changed, 16 insertions(+), 17 deletions(-)
On Fri, Nov 03, 2023 at 01:44:46PM -0300, Jason Gunthorpe wrote: > This is not being used to pass ops, it is just a way to tell if an > iommu driver was probed. These days this can be detected directly via > device_iommu_mapped(). Call device_iommu_mapped() in the two places that > need to check it and remove the iommu parameter everywhere. > > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Moritz Fischer <mdf@kernel.org> > --- > arch/arc/mm/dma.c | 2 +- > arch/arm/mm/dma-mapping-nommu.c | 2 +- > arch/arm/mm/dma-mapping.c | 10 +++++----- > arch/arm64/mm/dma-mapping.c | 4 ++-- > arch/mips/mm/dma-noncoherent.c | 2 +- > arch/riscv/mm/dma-noncoherent.c | 2 +- > drivers/acpi/scan.c | 3 +-- > drivers/hv/hv_common.c | 2 +- > drivers/of/device.c | 2 +- > include/linux/dma-map-ops.h | 4 ++-- > 10 files changed, 16 insertions(+), 17 deletions(-) > > diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c > index 2a7fbbb83b7056..197707bc765889 100644 > --- a/arch/arc/mm/dma.c > +++ b/arch/arc/mm/dma.c > @@ -91,7 +91,7 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, > * Plug in direct dma map ops. > */ > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > /* > * IOC hardware snoops all DMA traffic keeping the caches consistent > diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c > index cfd9c933d2f09c..b94850b579952a 100644 > --- a/arch/arm/mm/dma-mapping-nommu.c > +++ b/arch/arm/mm/dma-mapping-nommu.c > @@ -34,7 +34,7 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, > } > > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > if (IS_ENABLED(CONFIG_CPU_V7M)) { > /* > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > index 5409225b4abc06..6c359a3af8d9c7 100644 > --- a/arch/arm/mm/dma-mapping.c > +++ b/arch/arm/mm/dma-mapping.c > @@ -1713,7 +1713,7 @@ void arm_iommu_detach_device(struct device *dev) > EXPORT_SYMBOL_GPL(arm_iommu_detach_device); > > static void arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > struct dma_iommu_mapping *mapping; > > @@ -1748,7 +1748,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev) > #else > > static void arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > } > > @@ -1757,7 +1757,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev) { } > #endif /* CONFIG_ARM_DMA_USE_IOMMU */ > > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > /* > * Due to legacy code that sets the ->dma_coherent flag from a bus > @@ -1776,8 +1776,8 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > if (dev->dma_ops) > return; > > - if (iommu) > - arm_setup_iommu_dma_ops(dev, dma_base, size, iommu, coherent); > + if (device_iommu_mapped(dev)) > + arm_setup_iommu_dma_ops(dev, dma_base, size, coherent); > > xen_setup_dma_ops(dev); > dev->archdata.dma_ops_setup = true; > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > index 3cb101e8cb29ba..61886e43e3a10f 100644 > --- a/arch/arm64/mm/dma-mapping.c > +++ b/arch/arm64/mm/dma-mapping.c > @@ -47,7 +47,7 @@ void arch_teardown_dma_ops(struct device *dev) > #endif > > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > int cls = cache_line_size_of_cpu(); > > @@ -58,7 +58,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > ARCH_DMA_MINALIGN, cls); > > dev->dma_coherent = coherent; > - if (iommu) > + if (device_iommu_mapped(dev)) > iommu_setup_dma_ops(dev, dma_base, dma_base + size - 1); > > xen_setup_dma_ops(dev); > diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c > index 3c4fc97b9f394b..0f3cec663a12cd 100644 > --- a/arch/mips/mm/dma-noncoherent.c > +++ b/arch/mips/mm/dma-noncoherent.c > @@ -138,7 +138,7 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, > > #ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > dev->dma_coherent = coherent; > } > diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c > index b76e7e192eb183..f91fa741c41211 100644 > --- a/arch/riscv/mm/dma-noncoherent.c > +++ b/arch/riscv/mm/dma-noncoherent.c > @@ -135,7 +135,7 @@ void arch_dma_prep_coherent(struct page *page, size_t size) > } > > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent) > + bool coherent) > { > WARN_TAINT(!coherent && riscv_cbom_block_size > ARCH_DMA_MINALIGN, > TAINT_CPU_OUT_OF_SPEC, > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index 691d4b7686ee7e..a6891ad0ceee2c 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1636,8 +1636,7 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr, > if (PTR_ERR(iommu) == -EPROBE_DEFER) > return -EPROBE_DEFER; > > - arch_setup_dma_ops(dev, 0, U64_MAX, > - iommu, attr == DEV_DMA_COHERENT); > + arch_setup_dma_ops(dev, 0, U64_MAX, attr == DEV_DMA_COHERENT); > > return 0; > } > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c > index ccad7bca3fd3da..fd938b6dfa7ed4 100644 > --- a/drivers/hv/hv_common.c > +++ b/drivers/hv/hv_common.c > @@ -489,7 +489,7 @@ void hv_setup_dma_ops(struct device *dev, bool coherent) > * Hyper-V does not offer a vIOMMU in the guest > * VM, so pass 0/NULL for the IOMMU settings > */ > - arch_setup_dma_ops(dev, 0, 0, NULL, coherent); > + arch_setup_dma_ops(dev, 0, 0, coherent); > } > EXPORT_SYMBOL_GPL(hv_setup_dma_ops); > > diff --git a/drivers/of/device.c b/drivers/of/device.c > index 1ca42ad9dd159d..65c71be71a8d45 100644 > --- a/drivers/of/device.c > +++ b/drivers/of/device.c > @@ -193,7 +193,7 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, > dev_dbg(dev, "device is%sbehind an iommu\n", > iommu ? " " : " not "); > > - arch_setup_dma_ops(dev, dma_start, size, iommu, coherent); > + arch_setup_dma_ops(dev, dma_start, size, coherent); > > if (!iommu) > of_dma_set_restricted_buffer(dev, np); > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h > index f2fc203fb8a1a2..2cb98a12c50348 100644 > --- a/include/linux/dma-map-ops.h > +++ b/include/linux/dma-map-ops.h > @@ -426,10 +426,10 @@ bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, > > #ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS > void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, > - const struct iommu_ops *iommu, bool coherent); > + bool coherent); > #else > static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, > - u64 size, const struct iommu_ops *iommu, bool coherent) > + u64 size, bool coherent) > { > } > #endif /* CONFIG_ARCH_HAS_SETUP_DMA_OPS */ > -- > 2.42.0 >
diff --git a/arch/arc/mm/dma.c b/arch/arc/mm/dma.c index 2a7fbbb83b7056..197707bc765889 100644 --- a/arch/arc/mm/dma.c +++ b/arch/arc/mm/dma.c @@ -91,7 +91,7 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, * Plug in direct dma map ops. */ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { /* * IOC hardware snoops all DMA traffic keeping the caches consistent diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c index cfd9c933d2f09c..b94850b579952a 100644 --- a/arch/arm/mm/dma-mapping-nommu.c +++ b/arch/arm/mm/dma-mapping-nommu.c @@ -34,7 +34,7 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, } void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { if (IS_ENABLED(CONFIG_CPU_V7M)) { /* diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 5409225b4abc06..6c359a3af8d9c7 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1713,7 +1713,7 @@ void arm_iommu_detach_device(struct device *dev) EXPORT_SYMBOL_GPL(arm_iommu_detach_device); static void arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { struct dma_iommu_mapping *mapping; @@ -1748,7 +1748,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev) #else static void arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { } @@ -1757,7 +1757,7 @@ static void arm_teardown_iommu_dma_ops(struct device *dev) { } #endif /* CONFIG_ARM_DMA_USE_IOMMU */ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { /* * Due to legacy code that sets the ->dma_coherent flag from a bus @@ -1776,8 +1776,8 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, if (dev->dma_ops) return; - if (iommu) - arm_setup_iommu_dma_ops(dev, dma_base, size, iommu, coherent); + if (device_iommu_mapped(dev)) + arm_setup_iommu_dma_ops(dev, dma_base, size, coherent); xen_setup_dma_ops(dev); dev->archdata.dma_ops_setup = true; diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index 3cb101e8cb29ba..61886e43e3a10f 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -47,7 +47,7 @@ void arch_teardown_dma_ops(struct device *dev) #endif void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { int cls = cache_line_size_of_cpu(); @@ -58,7 +58,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, ARCH_DMA_MINALIGN, cls); dev->dma_coherent = coherent; - if (iommu) + if (device_iommu_mapped(dev)) iommu_setup_dma_ops(dev, dma_base, dma_base + size - 1); xen_setup_dma_ops(dev); diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c index 3c4fc97b9f394b..0f3cec663a12cd 100644 --- a/arch/mips/mm/dma-noncoherent.c +++ b/arch/mips/mm/dma-noncoherent.c @@ -138,7 +138,7 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, #ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { dev->dma_coherent = coherent; } diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c index b76e7e192eb183..f91fa741c41211 100644 --- a/arch/riscv/mm/dma-noncoherent.c +++ b/arch/riscv/mm/dma-noncoherent.c @@ -135,7 +135,7 @@ void arch_dma_prep_coherent(struct page *page, size_t size) } void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent) + bool coherent) { WARN_TAINT(!coherent && riscv_cbom_block_size > ARCH_DMA_MINALIGN, TAINT_CPU_OUT_OF_SPEC, diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 691d4b7686ee7e..a6891ad0ceee2c 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1636,8 +1636,7 @@ int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr, if (PTR_ERR(iommu) == -EPROBE_DEFER) return -EPROBE_DEFER; - arch_setup_dma_ops(dev, 0, U64_MAX, - iommu, attr == DEV_DMA_COHERENT); + arch_setup_dma_ops(dev, 0, U64_MAX, attr == DEV_DMA_COHERENT); return 0; } diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index ccad7bca3fd3da..fd938b6dfa7ed4 100644 --- a/drivers/hv/hv_common.c +++ b/drivers/hv/hv_common.c @@ -489,7 +489,7 @@ void hv_setup_dma_ops(struct device *dev, bool coherent) * Hyper-V does not offer a vIOMMU in the guest * VM, so pass 0/NULL for the IOMMU settings */ - arch_setup_dma_ops(dev, 0, 0, NULL, coherent); + arch_setup_dma_ops(dev, 0, 0, coherent); } EXPORT_SYMBOL_GPL(hv_setup_dma_ops); diff --git a/drivers/of/device.c b/drivers/of/device.c index 1ca42ad9dd159d..65c71be71a8d45 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -193,7 +193,7 @@ int of_dma_configure_id(struct device *dev, struct device_node *np, dev_dbg(dev, "device is%sbehind an iommu\n", iommu ? " " : " not "); - arch_setup_dma_ops(dev, dma_start, size, iommu, coherent); + arch_setup_dma_ops(dev, dma_start, size, coherent); if (!iommu) of_dma_set_restricted_buffer(dev, np); diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h index f2fc203fb8a1a2..2cb98a12c50348 100644 --- a/include/linux/dma-map-ops.h +++ b/include/linux/dma-map-ops.h @@ -426,10 +426,10 @@ bool arch_dma_unmap_sg_direct(struct device *dev, struct scatterlist *sg, #ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size, - const struct iommu_ops *iommu, bool coherent); + bool coherent); #else static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, - u64 size, const struct iommu_ops *iommu, bool coherent) + u64 size, bool coherent) { } #endif /* CONFIG_ARCH_HAS_SETUP_DMA_OPS */
This is not being used to pass ops, it is just a way to tell if an iommu driver was probed. These days this can be detected directly via device_iommu_mapped(). Call device_iommu_mapped() in the two places that need to check it and remove the iommu parameter everywhere. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- arch/arc/mm/dma.c | 2 +- arch/arm/mm/dma-mapping-nommu.c | 2 +- arch/arm/mm/dma-mapping.c | 10 +++++----- arch/arm64/mm/dma-mapping.c | 4 ++-- arch/mips/mm/dma-noncoherent.c | 2 +- arch/riscv/mm/dma-noncoherent.c | 2 +- drivers/acpi/scan.c | 3 +-- drivers/hv/hv_common.c | 2 +- drivers/of/device.c | 2 +- include/linux/dma-map-ops.h | 4 ++-- 10 files changed, 16 insertions(+), 17 deletions(-)