| Message ID | 20190430110032.25301-6-hch@lst.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/7] MIPS: remove the _dma_cache_wback_inv export | expand |
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.
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,
Stop providing our arch alloc/free hooks and just expose the segment offset instead. Signed-off-by: Christoph Hellwig <hch@lst.de> --- 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(-)