Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Mon, Nov 19, 2018 at 5:50 PM Robin Murphy wrote: > > On 19/11/2018 14:18, Ramon Fried wrote: > > On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt > > wrote: > >> > >> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote: > -* Because 32-bit DMA masks are so common we expect every > architecture > -* to be able to satisfy them - either by not supporting more > physical > -* memory, or by providing a ZONE_DMA32. If neither is the > case, the > -* architecture needs to use an IOMMU instead of the direct > mapping. > -*/ > - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) > + u64 min_mask; > + > + if (IS_ENABLED(CONFIG_ZONE_DMA)) > + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > + else > + min_mask = DMA_BIT_MASK(32); > + > + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); > + > + if (mask >= phys_to_dma(dev, min_mask)) > return 0; > -#endif > return 1; > } > >>> > >>> So I believe I have run into the same issue that Guenter reported. On > >>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and > >>> all probe attempts for various devices were failing with -EIO errors. > >>> > >>> I believe the last mask check should be "if (mask < phys_to_dma(dev, > >>> min_mask))" not a ">=" check. > >> > >> Right, that test is backwards. I needed to change it here too (powermac > >> with the rest of the powerpc series). > >> > >> Cheers, > >> Ben. > >> > >> > > > > Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it > > appears that this series of patches are causing all PCI drivers that > > request 64bit mask to fail with -5. > > It's broken in 4.19. However, I just checked, it working on master. > > We may need to backport a couple of patches to 4.19. I'm not sure > > though which patches should be backported as there were at least 10 > > patches resolving this dma_direct area recently. > > Christoph, Robin. > > Can we ask Greg to backport all these changes ? What do you think ? > > As far as I'm aware, the only real issue in 4.19 was my subtle breakage > around setting bus_dma_mask - that's fixed by 6778be4e5209, which > according to my inbox got picked up by autosel for 4.19 stable last week. > > Robin. Yep, 6778be4e5209 fixes the issue. Thanks a lot ! Ramon.
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt wrote: > > On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote: > > > -* Because 32-bit DMA masks are so common we expect every > > > architecture > > > -* to be able to satisfy them - either by not supporting more > > > physical > > > -* memory, or by providing a ZONE_DMA32. If neither is the case, > > > the > > > -* architecture needs to use an IOMMU instead of the direct > > > mapping. > > > -*/ > > > - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) > > > + u64 min_mask; > > > + > > > + if (IS_ENABLED(CONFIG_ZONE_DMA)) > > > + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > > > + else > > > + min_mask = DMA_BIT_MASK(32); > > > + > > > + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); > > > + > > > + if (mask >= phys_to_dma(dev, min_mask)) > > > return 0; > > > -#endif > > > return 1; > > > } > > > > So I believe I have run into the same issue that Guenter reported. On > > an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and > > all probe attempts for various devices were failing with -EIO errors. > > > > I believe the last mask check should be "if (mask < phys_to_dma(dev, > > min_mask))" not a ">=" check. > > Right, that test is backwards. I needed to change it here too (powermac > with the rest of the powerpc series). > > Cheers, > Ben. > > Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it appears that this series of patches are causing all PCI drivers that request 64bit mask to fail with -5. It's broken in 4.19. However, I just checked, it working on master. We may need to backport a couple of patches to 4.19. I'm not sure though which patches should be backported as there were at least 10 patches resolving this dma_direct area recently. Christoph, Robin. Can we ask Greg to backport all these changes ? What do you think ? Thanks, Ramon.
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On 19/11/2018 14:18, Ramon Fried wrote: On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt wrote: On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote: -* Because 32-bit DMA masks are so common we expect every architecture -* to be able to satisfy them - either by not supporting more physical -* memory, or by providing a ZONE_DMA32. If neither is the case, the -* architecture needs to use an IOMMU instead of the direct mapping. -*/ - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) + u64 min_mask; + + if (IS_ENABLED(CONFIG_ZONE_DMA)) + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); + else + min_mask = DMA_BIT_MASK(32); + + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); + + if (mask >= phys_to_dma(dev, min_mask)) return 0; -#endif return 1; } So I believe I have run into the same issue that Guenter reported. On an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and all probe attempts for various devices were failing with -EIO errors. I believe the last mask check should be "if (mask < phys_to_dma(dev, min_mask))" not a ">=" check. Right, that test is backwards. I needed to change it here too (powermac with the rest of the powerpc series). Cheers, Ben. Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it appears that this series of patches are causing all PCI drivers that request 64bit mask to fail with -5. It's broken in 4.19. However, I just checked, it working on master. We may need to backport a couple of patches to 4.19. I'm not sure though which patches should be backported as there were at least 10 patches resolving this dma_direct area recently. Christoph, Robin. Can we ask Greg to backport all these changes ? What do you think ? As far as I'm aware, the only real issue in 4.19 was my subtle breakage around setting bus_dma_mask - that's fixed by 6778be4e5209, which according to my inbox got picked up by autosel for 4.19 stable last week. Robin.
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote: > > -* Because 32-bit DMA masks are so common we expect every > > architecture > > -* to be able to satisfy them - either by not supporting more > > physical > > -* memory, or by providing a ZONE_DMA32. If neither is the case, > > the > > -* architecture needs to use an IOMMU instead of the direct mapping. > > -*/ > > - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) > > + u64 min_mask; > > + > > + if (IS_ENABLED(CONFIG_ZONE_DMA)) > > + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > > + else > > + min_mask = DMA_BIT_MASK(32); > > + > > + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); > > + > > + if (mask >= phys_to_dma(dev, min_mask)) > > return 0; > > -#endif > > return 1; > > } > > So I believe I have run into the same issue that Guenter reported. On > an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and > all probe attempts for various devices were failing with -EIO errors. > > I believe the last mask check should be "if (mask < phys_to_dma(dev, > min_mask))" not a ">=" check. Right, that test is backwards. I needed to change it here too (powermac with the rest of the powerpc series). Cheers, Ben.
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Thu, Sep 27, 2018 at 3:38 PM Christoph Hellwig wrote:
>
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA. Apparently that is a common
> case on powerpc.
>
> Signed-off-by: Christoph Hellwig
> Reviewed-by: Robin Murphy
> ---
> kernel/dma/direct.c | 28
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 60c433b880e0..170bd322a94a 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct
> scatterlist *sgl, int nents,
> return nents;
> }
>
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or
> by
> + * providing a ZONE_DMA32. If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
> int dma_direct_supported(struct device *dev, u64 mask)
> {
> -#ifdef CONFIG_ZONE_DMA
> - if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> - return 0;
> -#else
> - /*
> -* Because 32-bit DMA masks are so common we expect every architecture
> -* to be able to satisfy them - either by not supporting more physical
> -* memory, or by providing a ZONE_DMA32. If neither is the case, the
> -* architecture needs to use an IOMMU instead of the direct mapping.
> -*/
> - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> + u64 min_mask;
> +
> + if (IS_ENABLED(CONFIG_ZONE_DMA))
> + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> + else
> + min_mask = DMA_BIT_MASK(32);
> +
> + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
> +
> + if (mask >= phys_to_dma(dev, min_mask))
> return 0;
> -#endif
> return 1;
> }
So I believe I have run into the same issue that Guenter reported. On
an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and
all probe attempts for various devices were failing with -EIO errors.
I believe the last mask check should be "if (mask < phys_to_dma(dev,
min_mask))" not a ">=" check.
- Alex
[PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
This way an architecture with less than 4G of RAM can support dma_mask
smaller than 32-bit without a ZONE_DMA. Apparently that is a common
case on powerpc.
Signed-off-by: Christoph Hellwig
Reviewed-by: Robin Murphy
---
kernel/dma/direct.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 60c433b880e0..170bd322a94a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -284,21 +284,25 @@ int dma_direct_map_sg(struct device *dev, struct
scatterlist *sgl, int nents,
return nents;
}
+/*
+ * Because 32-bit DMA masks are so common we expect every architecture to be
+ * able to satisfy them - either by not supporting more physical memory, or by
+ * providing a ZONE_DMA32. If neither is the case, the architecture needs to
+ * use an IOMMU instead of the direct mapping.
+ */
int dma_direct_supported(struct device *dev, u64 mask)
{
-#ifdef CONFIG_ZONE_DMA
- if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
- return 0;
-#else
- /*
-* Because 32-bit DMA masks are so common we expect every architecture
-* to be able to satisfy them - either by not supporting more physical
-* memory, or by providing a ZONE_DMA32. If neither is the case, the
-* architecture needs to use an IOMMU instead of the direct mapping.
-*/
- if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
+ u64 min_mask;
+
+ if (IS_ENABLED(CONFIG_ZONE_DMA))
+ min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+ else
+ min_mask = DMA_BIT_MASK(32);
+
+ min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
+
+ if (mask >= phys_to_dma(dev, min_mask))
return 0;
-#endif
return 1;
}
--
2.19.0
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
[ oops, I should have looked at the replies first, now I see Ben already had the same thing to say about #3... ] On 27/09/18 14:49, Christoph Hellwig wrote: On Thu, Sep 27, 2018 at 11:50:14AM +1000, Benjamin Herrenschmidt wrote: -* to be able to satisfy them - either by not supporting more physical -* memory, or by providing a ZONE_DMA32. If neither is the case, the -* architecture needs to use an IOMMU instead of the direct mapping. -*/ - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) + u64 min_mask; + + if (IS_ENABLED(CONFIG_ZONE_DMA)) + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); + else + min_mask = min_t(u64, DMA_BIT_MASK(32), +(max_pfn - 1) << PAGE_SHIFT); + + if (mask >= phys_to_dma(dev, min_mask)) return 0; nitpick ... to be completely "correct", I would have written if (IS_ENABLED(CONFIG_ZONE_DMA)) min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); else min_mask = DMA_BIT_MASK(32); min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's big enough to fit all memory :-) Yeah, we could do that. FWIW I like it even if just for looking slightly more readable. With that fixup, Reviewed-by: Robin Murphy
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Thu, Sep 27, 2018 at 11:50:14AM +1000, Benjamin Herrenschmidt wrote: > > -* to be able to satisfy them - either by not supporting more physical > > -* memory, or by providing a ZONE_DMA32. If neither is the case, the > > -* architecture needs to use an IOMMU instead of the direct mapping. > > -*/ > > - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) > > + u64 min_mask; > > + > > + if (IS_ENABLED(CONFIG_ZONE_DMA)) > > + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > > + else > > + min_mask = min_t(u64, DMA_BIT_MASK(32), > > +(max_pfn - 1) << PAGE_SHIFT); > > + > > + if (mask >= phys_to_dma(dev, min_mask)) > > return 0; > > nitpick ... to be completely "correct", I would have written > > if (IS_ENABLED(CONFIG_ZONE_DMA)) > min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > else > min_mask = DMA_BIT_MASK(32); > > min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); > > In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's > big enough to fit all memory :-) Yeah, we could do that.
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Thu, 2018-09-20 at 20:52 +0200, Christoph Hellwig wrote:
> This way an architecture with less than 4G of RAM can support dma_mask
> smaller than 32-bit without a ZONE_DMA. Apparently that is a common
> case on powerpc.
Anything that uses a b43 wifi adapter which has a 31-bit limitation
actually :-)
>
> Signed-off-by: Christoph Hellwig
> ---
> kernel/dma/direct.c | 28
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 64466b7ef67b..d1e103c6b107 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -4,6 +4,7 @@
> *
> * DMA operations that map physical memory directly without using an IOMMU.
> */
> +#include /* for max_pfn */
> #include
> #include
> #include
> @@ -283,21 +284,24 @@ int dma_direct_map_sg(struct device *dev, struct
> scatterlist *sgl, int nents,
> return nents;
> }
>
> +/*
> + * Because 32-bit DMA masks are so common we expect every architecture to be
> + * able to satisfy them - either by not supporting more physical memory, or
> by
> + * providing a ZONE_DMA32. If neither is the case, the architecture needs to
> + * use an IOMMU instead of the direct mapping.
> + */
> int dma_direct_supported(struct device *dev, u64 mask)
> {
> -#ifdef CONFIG_ZONE_DMA
> - if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
> - return 0;
> -#else
> - /*
> - * Because 32-bit DMA masks are so common we expect every architecture
> - * to be able to satisfy them - either by not supporting more physical
> - * memory, or by providing a ZONE_DMA32. If neither is the case, the
> - * architecture needs to use an IOMMU instead of the direct mapping.
> - */
> - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
> + u64 min_mask;
> +
> + if (IS_ENABLED(CONFIG_ZONE_DMA))
> + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
> + else
> + min_mask = min_t(u64, DMA_BIT_MASK(32),
> + (max_pfn - 1) << PAGE_SHIFT);
> +
> + if (mask >= phys_to_dma(dev, min_mask))
> return 0;
nitpick ... to be completely "correct", I would have written
if (IS_ENABLED(CONFIG_ZONE_DMA))
min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
else
min_mask = DMA_BIT_MASK(32);
min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT);
In "theory" it's also ok to have a mask < ZONE_DMA_BITS as long as it's
big enough to fit all memory :-)
> -#endif
> return 1;
> }
>
[PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
This way an architecture with less than 4G of RAM can support dma_mask
smaller than 32-bit without a ZONE_DMA. Apparently that is a common
case on powerpc.
Signed-off-by: Christoph Hellwig
---
kernel/dma/direct.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 64466b7ef67b..d1e103c6b107 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -4,6 +4,7 @@
*
* DMA operations that map physical memory directly without using an IOMMU.
*/
+#include /* for max_pfn */
#include
#include
#include
@@ -283,21 +284,24 @@ int dma_direct_map_sg(struct device *dev, struct
scatterlist *sgl, int nents,
return nents;
}
+/*
+ * Because 32-bit DMA masks are so common we expect every architecture to be
+ * able to satisfy them - either by not supporting more physical memory, or by
+ * providing a ZONE_DMA32. If neither is the case, the architecture needs to
+ * use an IOMMU instead of the direct mapping.
+ */
int dma_direct_supported(struct device *dev, u64 mask)
{
-#ifdef CONFIG_ZONE_DMA
- if (mask < phys_to_dma(dev, DMA_BIT_MASK(ARCH_ZONE_DMA_BITS)))
- return 0;
-#else
- /*
-* Because 32-bit DMA masks are so common we expect every architecture
-* to be able to satisfy them - either by not supporting more physical
-* memory, or by providing a ZONE_DMA32. If neither is the case, the
-* architecture needs to use an IOMMU instead of the direct mapping.
-*/
- if (mask < phys_to_dma(dev, DMA_BIT_MASK(32)))
+ u64 min_mask;
+
+ if (IS_ENABLED(CONFIG_ZONE_DMA))
+ min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+ else
+ min_mask = min_t(u64, DMA_BIT_MASK(32),
+(max_pfn - 1) << PAGE_SHIFT);
+
+ if (mask >= phys_to_dma(dev, min_mask))
return 0;
-#endif
return 1;
}
--
2.18.0
