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 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 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 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 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 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
[ 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
[ 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, 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.
