MIPS: sync after cacheflush
PATCH RESEND 1/6 MIPS: sync after cacheflush
On processors with deep write buffers, it is likely that many cycles will pass between a CACHE instruction and the time the data actually gets written out to DRAM. Add a SYNC instruction to ensure that the buffers get emptied before the flush functions return.
Actual problem seen in the wild:
1) dma_alloc_coherent() allocates cached memory
2) memset() is called to clear the new pages
3) dma_cache_wback_inv() is called to flush the zero data out to memory
4) dma_alloc_coherent() returns an uncached (kseg1) pointer to the freshly allocated pages
5) Caller writes data through the kseg1 pointer
6) Buffered writeback data finally gets flushed out to DRAM
7) Part of caller's data is inexplicably zeroed out
This patch adds SYNC between steps 3 and 4, which fixed the problem.
--- arch/mips/mm/c-r4k.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
arch/mips/mm/c-r4k.c +4 -0
@@ -604,6 +604,7 @@ static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size)r4k_blast_scache (); else blast_scache_range(addr, addr + size); +__sync ();return ; }
@@ -620,6 +621,7 @@ static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size) }bc_wback_inv (addr, size); +__sync (); }static void r4k_dma_cache_inv (unsigned long addr,unsigned long size)
@@ -647,6 +649,7 @@ static void r4k_dma_cache_inv(unsigned long addr, unsigned long size) (addr + size - 1) & almask);blast_inv_scache_range (addr, addr + size); } +__sync ();return ; }
@@ -663,6 +666,7 @@ static void r4k_dma_cache_inv(unsigned long addr, unsigned long size) }bc_inv (addr, size); +__sync (); }#endif /* CONFIG_DMA_NONCOHERENT */
Signature · 1 line
1.7.0.4
PATCH RESEND 2/6 MIPS: pfn_valid() is broken on low memory HIGHMEM systems
pfn_valid() compares the PFN to max_mapnr:
__pfn >= min_low_pfn && __pfn < max_mapnr;
On HIGHMEM kernels, highend_pfn is used to set the value of max_mapnr. Unfortunately, highend_pfn is left at zero if the system does not actually have enough RAM to reach into the HIGHMEM range. This causes pfn_valid() to always return false, and when debug checks are enabled the kernel will fail catastrophically:
Memory: 22432k/32768k available (2249k kernel code, 10336k reserved, 653k data, 1352k init, 0k highmem) NR_IRQS:128kfree_debugcheck: out of range ptr 81c02900h.
Kernel bug detected[#1]:
Cpu 0
$ 0 : 00000000 10008400 00000034 00000000 $ 4 : 8003e160 802a0000 8003e160 00000000 $ 8 : 00000000 0000003e 00000747 00000747 ...
On such a configuration, max_low_pfn should be used to set max_mapnr.
This was seen on 2.6.34.
--- arch/mips/mm/init.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/mips/mm/init.c +1 -1
@@ -370,7 +370,7 @@ void __init mem_init(void)#ifdef CONFIG_DISCONTIGMEM #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"#endif - max_mapnr = highend_pfn; + max_mapnr = highend_pfn ? : max_low_pfn;#else max_mapnr = max_low_pfn;#endif
Signature · 1 line
1.7.0.4
Re: [PATCH RESEND 2/6] MIPS: pfn_valid() is broken on low memory HIGHMEM systems
On 01/05/2011 11:31 PM, Kevin Cernekee wrote:
[...]
arch/mips/mm/init.c void __init mem_init(void) · line 373
Quoted 10 lines
arch/mips/mm/init.c +1
-1@@ -370,7 +370,7 @@ void __init mem_init(void)#ifdef CONFIG_DISCONTIGMEM #error "CONFIG_HIGHMEM and CONFIG_DISCONTIGMEM dont work together yet"#endif - max_mapnr = highend_pfn; + max_mapnr = highend_pfn ? : max_low_pfn;
That is not standard C.
How about: max_mapnr = highend_pfn ? highend_pfn : max_low_pfn;
David Daney
PATCH RESEND v2 3/6 MIPS: Move FIXADDR_TOP into spaces.h
In reply to Kevin CernekeeMemory maps and addressing quirks are normally defined in <spaces.h>. There are already three targets that need to override FIXADDR_TOP, and others exist. This will be a cleaner approach than adding lots of ifdefs in fixmap.h .
--- arch/mips/include/asm/fixmap.h | 10 +--------- arch/mips/include/asm/mach-bcm63xx/spaces.h | 17 +++++++++++++++++ arch/mips/include/asm/mach-generic/spaces.h | 4 ++++ arch/mips/include/asm/mach-tx39xx/spaces.h | 17 +++++++++++++++++ arch/mips/include/asm/mach-tx49xx/spaces.h | 17 +++++++++++++++++ 5 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 arch/mips/include/asm/mach-bcm63xx/spaces.h create mode 100644 arch/mips/include/asm/mach-tx39xx/spaces.h create mode 100644 arch/mips/include/asm/mach-tx49xx/spaces.h
arch/mips/include/asm/fixmap.h +1 -9
@@ -14,6 +14,7 @@#define _ASM_FIXMAP_H#include <asm/page.h> +#include <spaces.h> #ifdef CONFIG_HIGHMEM #include <linux/threads.h> #include <asm/kmap_types.h>
@@ -67,15 +68,6 @@ enum fixed_addresses {* the start of the fixmap, and leave one page empty * at the top of mem.. */ -#ifdef CONFIG_BCM63XX -#define FIXADDR_TOP ((unsigned long)(long)(int)0xff000000) -#else -#if defined(CONFIG_CPU_TX39XX ) || defined(CONFIG_CPU_TX49XX ) -#define FIXADDR_TOP ((unsigned long)(long)(int)(0xff000000 - 0x20000)) -#else -#define FIXADDR_TOP ((unsigned long)(long)(int)0xfffe0000) -#endif -#endif #define FIXADDR_SIZE (__end_of_fixed_addresses << PAGE_SHIFT)#define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE)
arch/mips/include/asm/mach-bcm63xx/spaces.h new file +17 -0
@@ -0,0 +1,17 @@ +/* +* This file is subject to the terms and conditions of the GNU General Public +* License. See the file "COPYING" in the main directory of this archive +* for more details. +* +* Copyright (C) 1994 - 1999, 2000, 03, 04 Ralf Baechle +* Copyright (C) 2000, 2002 Maciej W. Rozycki +* Copyright (C) 1990, 1999, 2000 Silicon Graphics, Inc. +*/ +#ifndef _ASM_BCM63XX_SPACES_H +#define _ASM_BCM63XX_SPACES_H + +#define FIXADDR_TOP ((unsigned long)(long)(int)0xff000000) + +#include <asm/mach-generic/spaces.h> + +#endif /* __ASM_BCM63XX_SPACES_H */
arch/mips/include/asm/mach-generic/spaces.h +4 -0
@@ -82,4 +82,8 @@#define PAGE_OFFSET (CAC_BASE + PHYS_OFFSET)#endif +#ifndef FIXADDR_TOP +#define FIXADDR_TOP ((unsigned long)(long)(int)0xfffe0000) +#endif +#endif /* __ASM_MACH_GENERIC_SPACES_H */
arch/mips/include/asm/mach-tx39xx/spaces.h new file +17 -0
@@ -0,0 +1,17 @@ +/* +* This file is subject to the terms and conditions of the GNU General Public +* License. See the file "COPYING" in the main directory of this archive +* for more details. +* +* Copyright (C) 1994 - 1999, 2000, 03, 04 Ralf Baechle +* Copyright (C) 2000, 2002 Maciej W. Rozycki +* Copyright (C) 1990, 1999, 2000 Silicon Graphics, Inc. +*/ +#ifndef _ASM_TX39XX_SPACES_H +#define _ASM_TX39XX_SPACES_H + +#define FIXADDR_TOP ((unsigned long)(long)(int)0xfefe0000) + +#include <asm/mach-generic/spaces.h> + +#endif /* __ASM_TX39XX_SPACES_H */
arch/mips/include/asm/mach-tx49xx/spaces.h new file +17 -0
@@ -0,0 +1,17 @@ +/* +* This file is subject to the terms and conditions of the GNU General Public +* License. See the file "COPYING" in the main directory of this archive +* for more details. +* +* Copyright (C) 1994 - 1999, 2000, 03, 04 Ralf Baechle +* Copyright (C) 2000, 2002 Maciej W. Rozycki +* Copyright (C) 1990, 1999, 2000 Silicon Graphics, Inc. +*/ +#ifndef _ASM_TX49XX_SPACES_H +#define _ASM_TX49XX_SPACES_H + +#define FIXADDR_TOP ((unsigned long)(long)(int)0xfefe0000) + +#include <asm/mach-generic/spaces.h> + +#endif /* __ASM_TX49XX_SPACES_H */
Signature · 1 line
1.7.0.4
PATCH RESEND v4 4/6 MIPS: HIGHMEM DMA on noncoherent MIPS32 processors
In reply to Kevin CernekeeThe MIPS DMA coherency functions do not work properly (i.e. kernel oops) when HIGHMEM pages are passed in as arguments. Use kmap_atomic() to temporarily map high pages for cache maintenance operations.
Tested on a 2.6.37-rc7 1GB HIGHMEM SMP no-alias system.
--- arch/mips/mm/dma-default.c | 114 ++++++++++++++++++++++++++------------------ 1 files changed, 68 insertions(+), 46 deletions(-)
arch/mips/mm/dma-default.c +68 -46
@@ -15,18 +15,18 @@#include <linux/scatterlist.h> #include <linux/string.h> #include <linux/gfp.h> +#include <linux/highmem.h> #include <asm/cache.h> #include <asm/io.h> #include <dma-coherence.h> -static inline unsigned long dma_addr_to_virt (struct device *dev, +static inline struct page *dma_addr_to_page (struct device *dev,dma_addr_t dma_addr) { -unsigned long addr =plat_dma_addr_to_phys (dev, dma_addr); - -return (unsigned long )phys_to_virt (addr); +return pfn_to_page ( +plat_dma_addr_to_phys (dev, dma_addr) >>PAGE_SHIFT ); }/*
@@ -148,20 +148,20 @@ static void mips_dma_free_coherent(struct device *dev, size_t size, void *vaddr,free_pages (addr,get_order (size)); } -static inline void __dma_sync (unsigned long addr,size_t size, +static inline void __dma_sync_virtual (void *addr,size_t size,enum dma_data_direction direction) {switch (direction) {case DMA_TO_DEVICE : -dma_cache_wback (addr, size); +dma_cache_wback ((unsigned long )addr, size);break ;case DMA_FROM_DEVICE : -dma_cache_inv (addr, size); +dma_cache_inv ((unsigned long )addr, size);break ;case DMA_BIDIRECTIONAL : -dma_cache_wback_inv (addr, size); +dma_cache_wback_inv ((unsigned long )addr, size);break ;default :
@@ -169,12 +169,49 @@ static inline void __dma_sync(unsigned long addr, size_t size, } } +/* +* A single sg entry may refer to multiple physically contiguous +* pages. But we still need to process highmem pages individually. +* If highmem is not configured then the bulk of this loop gets +* optimized out. +*/ +static inline void __dma_sync (struct page *page, +unsigned long offset,size_t size,enum dma_data_direction direction) +{ +size_t left = size; + +do { +size_t len = left; + +if (PageHighMem (page)) { +void *addr; + +if (offset + len >PAGE_SIZE ) { +if (offset >=PAGE_SIZE ) { + page += offset >>PAGE_SHIFT ; + offset &= ~PAGE_MASK ; + } + len =PAGE_SIZE - offset; + } + + addr =kmap_atomic (page); +__dma_sync_virtual (addr + offset, len, direction); +kunmap_atomic (addr); + }else +__dma_sync_virtual (page_address (page) + offset, + size, direction); + offset =0 ; + page++; + left -= len; + }while (left); +} +static void mips_dma_unmap_page (struct device *dev,dma_addr_t dma_addr,size_t size,enum dma_data_direction direction,struct dma_attrs *attrs) {if (cpu_is_noncoherent_r10000 (dev)) -__dma_sync (dma_addr_to_virt (dev, dma_addr), size, - direction); +__dma_sync (dma_addr_to_page (dev, dma_addr), + dma_addr & ~PAGE_MASK , size, direction);plat_unmap_dma_mem (dev, dma_addr, size, direction); }
@@ -185,13 +222,11 @@ static int mips_dma_map_sg(struct device *dev, struct scatterlist *sg,int i;for (i =0 ; i < nents; i++, sg++) { -unsigned long addr; - - addr = (unsigned long )sg_virt (sg); -if (!plat_device_is_coherent (dev) && addr) -__dma_sync (addr, sg->length, direction); - sg->dma_address =plat_map_dma_mem (dev, - (void *)addr, sg->length); +if (!plat_device_is_coherent (dev)) +__dma_sync (sg_page (sg), sg->offset, sg->length, + direction); + sg->dma_address =plat_map_dma_mem_page (dev,sg_page (sg)) + + sg->offset; }return nents;
@@ -201,30 +236,23 @@ static dma_addr_t mips_dma_map_page(struct device *dev, struct page *page, unsigned long offset, size_t size, enum dma_data_direction direction, struct dma_attrs *attrs) { - unsigned long addr; - - addr = (unsigned long )page_address (page) + offset; - if (!plat_device_is_coherent(dev)) -__dma_sync (addr, size, direction); + __dma_sync(page, offset, size, direction); -return plat_map_dma_mem (dev, (void *)addr, size); + return plat_map_dma_mem_page(dev, page) + offset; }static void mips_dma_unmap_sg (struct device *dev,struct scatterlist *sg,int nhwentries,enum dma_data_direction direction,struct dma_attrs *attrs) { -unsigned long addr;int i;for (i =0 ; i < nhwentries; i++, sg++) {if (!plat_device_is_coherent (dev) && - direction !=DMA_TO_DEVICE ) { - addr = (unsigned long )sg_virt (sg); -if (addr) -__dma_sync (addr, sg->length, direction); - } + direction !=DMA_TO_DEVICE ) +__dma_sync (sg_page (sg), sg->offset, sg->length, + direction);plat_unmap_dma_mem (dev, sg->dma_address, sg->length, direction); } }
@@ -232,24 +260,18 @@ static void mips_dma_unmap_sg(struct device *dev, struct scatterlist *sg,static void mips_dma_sync_single_for_cpu (struct device *dev,dma_addr_t dma_handle,size_t size,enum dma_data_direction direction) { -if (cpu_is_noncoherent_r10000 (dev)) { -unsigned long addr; - - addr =dma_addr_to_virt (dev, dma_handle); -__dma_sync (addr, size, direction); - } +if (cpu_is_noncoherent_r10000 (dev)) +__dma_sync (dma_addr_to_page (dev, dma_handle), + dma_handle & ~PAGE_MASK , size, direction); }static void mips_dma_sync_single_for_device (struct device *dev,dma_addr_t dma_handle,size_t size,enum dma_data_direction direction) {plat_extra_sync_for_device (dev); -if (!plat_device_is_coherent (dev)) { -unsigned long addr; - - addr =dma_addr_to_virt (dev, dma_handle); -__dma_sync (addr, size, direction); - } +if (!plat_device_is_coherent (dev)) +__dma_sync (dma_addr_to_page (dev, dma_handle), + dma_handle & ~PAGE_MASK , size, direction); } static void mips_dma_sync_sg_for_cpu(struct device *dev,
@@ -260,8 +282,8 @@ static void mips_dma_sync_sg_for_cpu(struct device *dev,/* Make sure that gcc doesn't leave the empty loop body. */ for (i =0 ; i < nelems; i++, sg++) {if (cpu_is_noncoherent_r10000 (dev)) -__dma_sync ((unsigned long )page_address (sg_page (sg)), - sg->length, direction); +__dma_sync (sg_page (sg), sg->offset, sg->length, + direction); } }
@@ -273,8 +295,8 @@ static void mips_dma_sync_sg_for_device(struct device *dev,/* Make sure that gcc doesn't leave the empty loop body. */ for (i =0 ; i < nelems; i++, sg++) {if (!plat_device_is_coherent (dev)) -__dma_sync ((unsigned long )page_address (sg_page (sg)), - sg->length, direction); +__dma_sync (sg_page (sg), sg->offset, sg->length, + direction); } }
@@ -295,7 +317,7 @@ void mips_dma_cache_sync(struct device *dev, void *vaddr, size_t size,plat_extra_sync_for_device (dev);if (!plat_device_is_coherent (dev)) -__dma_sync ((unsigned long )vaddr, size, direction); +__dma_sync_virtual (vaddr, size, direction); }static struct dma_map_ops mips_default_dma_map_ops = {
Signature · 1 line
1.7.0.4
PATCH RESEND 5/6 MIPS: Install handlers for BMIPS software IRQs
In reply to Kevin CernekeeBMIPS4350/4380/5000 CMT/SMT all use SW INT0/INT1 for inter-thread signaling.
--- arch/mips/kernel/irq_cpu.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-)
arch/mips/kernel/irq_cpu.c +6 -8
@@ -107,14 +107,12 @@ void __init mips_cpu_irq_init(void)clear_c0_status (ST0_IM );clear_c0_cause (CAUSEF_IP ); -/* -* Only MT is using the software interrupts currently, so we just -* leave them uninitialized for other processors. -*/ -if (cpu_has_mipsmt) -for (i = irq_base; i < irq_base +2 ; i++) -set_irq_chip_and_handler (i, &mips_mt_cpu_irq_controller, - handle_percpu_irq); +/* Software interrupts are used for MT/CMT IPI */ +for (i = irq_base; i < irq_base +2 ; i++) +set_irq_chip_and_handler (i, cpu_has_mipsmt ? + &mips_mt_cpu_irq_controller : + &mips_cpu_irq_controller, + handle_percpu_irq);for (i = irq_base +2 ; i < irq_base +8 ; i++) set_irq_chip_and_handler(i, &mips_cpu_irq_controller,
Signature · 1 line
1.7.0.4
PATCH 6/6 MIPS: Limit fixrange_init() to the FIXMAP region
In reply to Kevin Cernekeefixrange_init() allocates page tables for all addresses higher than FIXADDR_TOP. On processors that override the default FIXADDR_TOP address of 0xfffe_0000, this can consume up to 4 pages (1 page per 4MB) for pgd's that are never used.
--- arch/mips/mm/init.c | 6 +++--- arch/mips/mm/pgtable-32.c | 2 +- arch/mips/mm/pgtable-64.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
arch/mips/mm/init.c +3 -3
@@ -279,11 +279,11 @@ void __init fixrange_init(unsigned long start, unsigned long end, k =__pmd_offset (vaddr); pgd = pgd_base + i; -for ( ; (i <PTRS_PER_PGD ) && (vaddr != end); pgd++, i++) { +for ( ; (i <PTRS_PER_PGD ) && (vaddr < end); pgd++, i++) { pud = (pud_t *)pgd; -for ( ; (j <PTRS_PER_PUD ) && (vaddr != end); pud++, j++) { +for ( ; (j <PTRS_PER_PUD ) && (vaddr < end); pud++, j++) { pmd = (pmd_t *)pud; -for (; (k <PTRS_PER_PMD ) && (vaddr != end); pmd++, k++) { +for (; (k <PTRS_PER_PMD ) && (vaddr < end); pmd++, k++) {if (pmd_none (*pmd)) { pte = (pte_t *)alloc_bootmem_low_pages (PAGE_SIZE );set_pmd (pmd,__pmd ((unsigned long )pte));
arch/mips/mm/pgtable-32.c +1 -1
@@ -52,7 +52,7 @@ void __init pagetable_init(void)* Fixed mappings: */ vaddr =__fix_to_virt (__end_of_fixed_addresses -1 ) &PMD_MASK ; -fixrange_init (vaddr,0 , pgd_base); +fixrange_init (vaddr, vaddr +FIXADDR_SIZE , pgd_base);#ifdef CONFIG_HIGHMEM /*
arch/mips/mm/pgtable-64.c +1 -1
@@ -76,5 +76,5 @@ void __init pagetable_init(void)* Fixed mappings: */ vaddr =__fix_to_virt (__end_of_fixed_addresses -1 ) &PMD_MASK ; -fixrange_init (vaddr,0 , pgd_base); +fixrange_init (vaddr, vaddr +FIXADDR_SIZE , pgd_base); }
Signature · 1 line
1.7.0.4
Re: [PATCH 6/6] MIPS: Limit fixrange_init() to the FIXMAP region
Thanks, queued for 2.6.41.
Ralf
Hi,
sorry for my silence for several resending posts. I talked with Ralf on this briefly, when the patch was posted for the first time last year. He seems busy these days, so I'm trying to move things forward.
Quoted 4 lines · Kevin Cernekee
On processors with deep write buffers, it is likely that many cycles will pass between a CACHE instruction and the time the data actually gets written out to DRAM. Add a SYNC instruction to ensure that the buffers get emptied before the flush functions return.
Having a talk with appropriate people (probably including MIPS guys), such CACHE behavior would be eventually considered to be _legitimate_, Ralf said. Thus we agree with this change required not only for BMIPS cores, but also for the rest of processors, as one of cache features.
The problem is how to implement it.
Most of SYNC-equipped processors work fine without this treatment, so unless it's required, we don't want to have __sync() after every cacheflush operations. Having SYNC there is somewhat waste of time.
arch/mips/mm/c-r4k.c static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size) · line 610
Quoted 12 lines
arch/mips/mm/c-r4k.c +1
-0@@ -604,6 +604,7 @@ static void r4k_dma_cache_wback_inv(unsigned long addr, unsigned long size)r4k_blast_scache (); else blast_scache_range(addr, addr + size); +__sync ();return ; }
Just random thought, how it's going to be when implementing this as,
- sort of barrier
- sort of hazard
- one of cache features (e.g., cpu_has_xxx or similar way)
- and so on...
Another concern is that, except for four __sync() insertions, are we missing other places?
Ralf also thinks that run-time probe & detection would be better than build-time configuration. Optimization is easy and won't be problem here, so we'd like to keep things synthesized as much as possible.
I couldn't imagine the final shape how this patch should be as of now, sorry!
Shinya