Hi all,
can you take a look at this series? It lifts the support for mips-style
uncached segements to the dma-direct layer, thus removing the need
to have arch_dma_alloc/free routines for these architectures.
Stop providing our arch alloc/free hooks and just expose the segment
offset instead.
Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/mips/Kconfig | 1 +
arch/mips/include/asm/page.h | 3 ---
arch/mips/jazz/jazzdma.c | 6 ------
arch/mips/mm/dma-noncoherent.c | 27 ++++++++++-----------------
4 files changed, 11 insertions(+), 26 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 4a5f5b0ee9a9..cde4b490f3c7 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -9,6 +9,7 @@ config MIPS
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UBSAN_SANITIZE_ALL
+ select ARCH_HAS_UNCACHED_SEGMENT
select ARCH_SUPPORTS_UPROBES
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 6b31c93b5eaa..23e0f1386e04 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -258,9 +258,6 @@ extern int __virt_addr_valid(const volatile void *kaddr);
((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-#define UNCAC_ADDR(addr) (UNCAC_BASE + __pa(addr))
-#define CAC_ADDR(addr) ((unsigned long)__va((addr) - UNCAC_BASE))
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c
index bedb5047aff3..1804dc9d8136 100644
--- a/arch/mips/jazz/jazzdma.c
+++ b/arch/mips/jazz/jazzdma.c
@@ -575,10 +575,6 @@ static void *jazz_dma_alloc(struct device *dev, size_t size,
return NULL;
}
- if (!(attrs & DMA_ATTR_NON_CONSISTENT)) {
- dma_cache_wback_inv((unsigned long)ret, size);
- ret = (void *)UNCAC_ADDR(ret);
- }
return ret;
}
@@ -586,8 +582,6 @@ static void jazz_dma_free(struct device *dev, size_t size, void *vaddr,
dma_addr_t dma_handle, unsigned long attrs)
{
vdma_free(dma_handle);
- if (!(attrs & DMA_ATTR_NON_CONSISTENT))
- vaddr = (void *)CAC_ADDR((unsigned long)vaddr);
dma_direct_free_pages(dev, size, vaddr, dma_handle, attrs);
}
diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c
index f9549d2fbea3..f739f42c9d3c 100644
--- a/arch/mips/mm/dma-noncoherent.c
+++ b/arch/mips/mm/dma-noncoherent.c
@@ -44,33 +44,26 @@ static inline bool cpu_needs_post_dma_flush(struct device *dev)
}
}
-void *arch_dma_alloc(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
+void arch_dma_prep_coherent(struct page *page, size_t size)
{
- void *ret;
-
- ret = dma_direct_alloc_pages(dev, size, dma_handle, gfp, attrs);
- if (ret && !(attrs & DMA_ATTR_NON_CONSISTENT)) {
- dma_cache_wback_inv((unsigned long) ret, size);
- ret = (void *)UNCAC_ADDR(ret);
- }
+ if (!PageHighMem(page))
+ dma_cache_wback_inv((unsigned long)page_address(page), size);
+}
- return ret;
+void *uncached_kernel_address(void *addr)
+{
+ return (void *)(__pa(addr) + UNCAC_BASE);
}
-void arch_dma_free(struct device *dev, size_t size, void *cpu_addr,
- dma_addr_t dma_addr, unsigned long attrs)
+void *cached_kernel_address(void *addr)
{
- if (!(attrs & DMA_ATTR_NON_CONSISTENT))
- cpu_addr = (void *)CAC_ADDR((unsigned long)cpu_addr);
- dma_direct_free_pages(dev, size, cpu_addr, dma_addr, attrs);
+ return __va(addr) - UNCAC_BASE;
}
long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr,
dma_addr_t dma_addr)
{
- unsigned long addr = CAC_ADDR((unsigned long)cpu_addr);
- return page_to_pfn(virt_to_page((void *)addr));
+ return page_to_pfn(virt_to_page(cached_kernel_address(cpu_addr)));
}
pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
--
2.20.1
Hi Christoph,
On Tue, Apr 30, 2019 at 07:00:30AM -0400, Christoph Hellwig wrote:
> diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c
> index f9549d2fbea3..f739f42c9d3c 100644
> --- a/arch/mips/mm/dma-noncoherent.c
> +++ b/arch/mips/mm/dma-noncoherent.c
> @@ -44,33 +44,26 @@ static inline bool cpu_needs_post_dma_flush(struct device *dev)
> }
> }
>
> -void *arch_dma_alloc(struct device *dev, size_t size,
> - dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
> +void arch_dma_prep_coherent(struct page *page, size_t size)
> {
> - void *ret;
> -
> - ret = dma_direct_alloc_pages(dev, size, dma_handle, gfp, attrs);
> - if (ret && !(attrs & DMA_ATTR_NON_CONSISTENT)) {
> - dma_cache_wback_inv((unsigned long) ret, size);
> - ret = (void *)UNCAC_ADDR(ret);
> - }
> + if (!PageHighMem(page))
> + dma_cache_wback_inv((unsigned long)page_address(page), size);
> +}
This series looks like a nice cleanup to me - the one thing that puzzles
me is the !PageHighMem check above.
As far as I can see arch_dma_prep_coherent() should never be called with
a highmem page, so would it make more sense to either drop this check or
perhaps wrap it in a WARN_ON()?
Thanks,
Paul
On Tue, Apr 30, 2019 at 08:10:43PM +0000, Paul Burton wrote:
> This series looks like a nice cleanup to me - the one thing that puzzles
> me is the !PageHighMem check above.
>
> As far as I can see arch_dma_prep_coherent() should never be called with
> a highmem page, so would it make more sense to either drop this check or
> perhaps wrap it in a WARN_ON()?
dma_alloc_from_contigous can return highmem pages depending on where
the CMA area is located. But given that these pages don't have a
direct kernel mapping we also shouldn't have to flush the caches
for them.
Hi Christoph,
On Tue, Apr 30, 2019 at 10:29:47PM +0200, Christoph Hellwig wrote:
> On Tue, Apr 30, 2019 at 08:10:43PM +0000, Paul Burton wrote:
> > This series looks like a nice cleanup to me - the one thing that puzzles
> > me is the !PageHighMem check above.
> >
> > As far as I can see arch_dma_prep_coherent() should never be called with
> > a highmem page, so would it make more sense to either drop this check or
> > perhaps wrap it in a WARN_ON()?
>
> dma_alloc_from_contigous can return highmem pages depending on where
> the CMA area is located. But given that these pages don't have a
> direct kernel mapping we also shouldn't have to flush the caches
> for them.
Right but dma_direct_alloc_pages() already checks for the PageHighMem
case & returns before ever calling arch_dma_prep_coherent(), no?
Thanks,
Paul
On Tue, Apr 30, 2019 at 09:11:07PM +0000, Paul Burton wrote:
> Right but dma_direct_alloc_pages() already checks for the PageHighMem
> case & returns before ever calling arch_dma_prep_coherent(), no?
True. And of course it can't be remapped into the uncached segment
anyway. So yes, we should drop it. Eventually I want to add generic
support for DMA_ATTR_NO_KERNEL_MAPPING, but that'll involved auditing
all instances anyway.
Stop providing our arch alloc/free hooks and just expose the segment
offset instead.
Signed-off-by: Christoph Hellwig <[email protected]>
---
arch/mips/Kconfig | 1 +
arch/mips/include/asm/page.h | 3 ---
arch/mips/jazz/jazzdma.c | 6 ------
arch/mips/mm/dma-noncoherent.c | 26 +++++++++-----------------
4 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 4a5f5b0ee9a9..cde4b490f3c7 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -9,6 +9,7 @@ config MIPS
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UBSAN_SANITIZE_ALL
+ select ARCH_HAS_UNCACHED_SEGMENT
select ARCH_SUPPORTS_UPROBES
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index 6b31c93b5eaa..23e0f1386e04 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -258,9 +258,6 @@ extern int __virt_addr_valid(const volatile void *kaddr);
((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-#define UNCAC_ADDR(addr) (UNCAC_BASE + __pa(addr))
-#define CAC_ADDR(addr) ((unsigned long)__va((addr) - UNCAC_BASE))
-
#include <asm-generic/memory_model.h>
#include <asm-generic/getorder.h>
diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c
index bedb5047aff3..1804dc9d8136 100644
--- a/arch/mips/jazz/jazzdma.c
+++ b/arch/mips/jazz/jazzdma.c
@@ -575,10 +575,6 @@ static void *jazz_dma_alloc(struct device *dev, size_t size,
return NULL;
}
- if (!(attrs & DMA_ATTR_NON_CONSISTENT)) {
- dma_cache_wback_inv((unsigned long)ret, size);
- ret = (void *)UNCAC_ADDR(ret);
- }
return ret;
}
@@ -586,8 +582,6 @@ static void jazz_dma_free(struct device *dev, size_t size, void *vaddr,
dma_addr_t dma_handle, unsigned long attrs)
{
vdma_free(dma_handle);
- if (!(attrs & DMA_ATTR_NON_CONSISTENT))
- vaddr = (void *)CAC_ADDR((unsigned long)vaddr);
dma_direct_free_pages(dev, size, vaddr, dma_handle, attrs);
}
diff --git a/arch/mips/mm/dma-noncoherent.c b/arch/mips/mm/dma-noncoherent.c
index f9549d2fbea3..ed56c6fa7be2 100644
--- a/arch/mips/mm/dma-noncoherent.c
+++ b/arch/mips/mm/dma-noncoherent.c
@@ -44,33 +44,25 @@ static inline bool cpu_needs_post_dma_flush(struct device *dev)
}
}
-void *arch_dma_alloc(struct device *dev, size_t size,
- dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
+void arch_dma_prep_coherent(struct page *page, size_t size)
{
- void *ret;
-
- ret = dma_direct_alloc_pages(dev, size, dma_handle, gfp, attrs);
- if (ret && !(attrs & DMA_ATTR_NON_CONSISTENT)) {
- dma_cache_wback_inv((unsigned long) ret, size);
- ret = (void *)UNCAC_ADDR(ret);
- }
+ dma_cache_wback_inv((unsigned long)page_address(page), size);
+}
- return ret;
+void *uncached_kernel_address(void *addr)
+{
+ return (void *)(__pa(addr) + UNCAC_BASE);
}
-void arch_dma_free(struct device *dev, size_t size, void *cpu_addr,
- dma_addr_t dma_addr, unsigned long attrs)
+void *cached_kernel_address(void *addr)
{
- if (!(attrs & DMA_ATTR_NON_CONSISTENT))
- cpu_addr = (void *)CAC_ADDR((unsigned long)cpu_addr);
- dma_direct_free_pages(dev, size, cpu_addr, dma_addr, attrs);
+ return __va(addr) - UNCAC_BASE;
}
long arch_dma_coherent_to_pfn(struct device *dev, void *cpu_addr,
dma_addr_t dma_addr)
{
- unsigned long addr = CAC_ADDR((unsigned long)cpu_addr);
- return page_to_pfn(virt_to_page((void *)addr));
+ return page_to_pfn(virt_to_page(cached_kernel_address(cpu_addr)));
}
pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
--
2.20.1
Hi Christoph,
On Wed, May 01, 2019 at 03:13:39PM +0200, Christoph Hellwig wrote:
> Stop providing our arch alloc/free hooks and just expose the segment
> offset instead.
>
> Signed-off-by: Christoph Hellwig <[email protected]>
> ---
> arch/mips/Kconfig | 1 +
> arch/mips/include/asm/page.h | 3 ---
> arch/mips/jazz/jazzdma.c | 6 ------
> arch/mips/mm/dma-noncoherent.c | 26 +++++++++-----------------
> 4 files changed, 10 insertions(+), 26 deletions(-)
This one looks good to me now, for patches 1 & 5:
Acked-by: Paul Burton <[email protected]>
Thanks,
Paul
On Wed, May 01, 2019 at 05:13:57PM +0000, Paul Burton wrote:
> Hi Christoph,
>
> On Wed, May 01, 2019 at 03:13:39PM +0200, Christoph Hellwig wrote:
> > Stop providing our arch alloc/free hooks and just expose the segment
> > offset instead.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > ---
> > arch/mips/Kconfig | 1 +
> > arch/mips/include/asm/page.h | 3 ---
> > arch/mips/jazz/jazzdma.c | 6 ------
> > arch/mips/mm/dma-noncoherent.c | 26 +++++++++-----------------
> > 4 files changed, 10 insertions(+), 26 deletions(-)
>
> This one looks good to me now, for patches 1 & 5:
>
> Acked-by: Paul Burton <[email protected]>
Thanks, I've merged thos into the dma-mapping tree.
On Mon, Jun 3, 2019 at 8:50 AM Christoph Hellwig <[email protected]> wrote:
>
> On Wed, May 01, 2019 at 05:13:57PM +0000, Paul Burton wrote:
> > Hi Christoph,
> >
> > On Wed, May 01, 2019 at 03:13:39PM +0200, Christoph Hellwig wrote:
> > > Stop providing our arch alloc/free hooks and just expose the segment
> > > offset instead.
> > >
> > > Signed-off-by: Christoph Hellwig <[email protected]>
> > > ---
> > > arch/mips/Kconfig | 1 +
> > > arch/mips/include/asm/page.h | 3 ---
> > > arch/mips/jazz/jazzdma.c | 6 ------
> > > arch/mips/mm/dma-noncoherent.c | 26 +++++++++-----------------
> > > 4 files changed, 10 insertions(+), 26 deletions(-)
> >
> > This one looks good to me now, for patches 1 & 5:
> >
> > Acked-by: Paul Burton <[email protected]>
>
> Thanks, I've merged thos into the dma-mapping tree.
I think this is the cause of some kernelci failures in current
linux-next builds:
https://kernelci.org/build/next/branch/master/kernel/next-20190702/
bigsur_defconfig ‐ mips3 warnings — 1 error
cavium_octeon_defconfig ‐ mips3 warnings — 1 error
ip27_defconfig ‐ mips3 warnings — 1 error
loongson3_defconfig ‐ mips3 warnings — 1 error
mips_paravirt_defconfig ‐ mips3 warnings — 1 error
nlm_xlp_defconfig ‐ mips3 warnings — 1 error
nlm_xlr_defconfig ‐ mips1 warning — 1 error
/home/buildslave/workspace/workspace/kernel-build@8/linux/build/../kernel/dma/direct.c:144:
undefined reference to `arch_dma_prep_coherent'
2/home/buildslave/workspace/kernel-build/linux/build/../kernel/dma/direct.c:144:
undefined reference to `arch_dma_prep_coherent'
2(.text+0xafc): undefined reference to `arch_dma_prep_coherent'
1direct.c:(.text+0x934): undefined reference to `arch_dma_prep_coherent'
1(.text+0xb84): undefined reference to `arch_dma_prep_coherent'
I haven't looked into the details, but I suspect all machines
with cache-coherent DMA are broken.
Arnd
On Wed, Jul 03, 2019 at 10:54:05AM +0200, Arnd Bergmann wrote:
> I think this is the cause of some kernelci failures in current
> linux-next builds:
Yes, Guenther reported this already and I sent a fix. I've been waiting
for an ACK from the mips maintaines, but given the breakage I
might as well just pull it in without an ACK.