r4k_dma_cache_wback_inv function fails when size=0...

Greetings.

The 'r4k_dma_cache_wback_inv' function will fail when the requested size equals 0 AND when the address is a multiple of the line size. I discovered this bug while using the National Semiconductor DP8381x series PCI ethernet driver. I have attached a test program showing the bug as well as a patch for comment. Okay to apply?

-Steve

/*
* Test program for faulty 'r4k_dma_cache_wback_inv' routine where
* calculated end address can be incorrect when the size = 0. *
* Copyright (C) 2004 TimeSys Corp.
* S. James Hill ([email protected])
* ([email protected])
*

 *  This program is free software; you can redistribute it and/or modify it
 *  under the terms of the GNU General Public License as published by the
 *  Free Software Foundation; either version 2 of the License, or (at your
 *  option) any later version.
 *
 *  THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
 *  WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
 *  MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
 *  IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
 *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
 *  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
 *  OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 *  ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
 *  TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 *  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 *
 *  You should have received a copy of the GNU General Public License along
 *  with this program; if not, write to the Free Software Foundation, Inc.,
 *  675 Mass Ave, Cambridge, MA 02139, USA.

*/
#include <stdio.h>

int main (void)

{
	unsigned long addr, a, end;
	unsigned int size, line_size;

	/*
	 * Choose an arbitrary test start address and ending address
	 * (not the calculated end address).
	 */
	for (addr = 0x803e4000; addr < 0x803e8000; addr+=4)
	{
		/*
		 * We assume cache line sizes are always a multiple
		 * of 16 bytes.
		 */
		line_size = 16;
		//line_size = 32;
		{
			/*
			 * We try all 4-byte aligned sizes up to
			 * the size of a page.
			 */
			for (size = 0; size < 4096; size+=4)
			{
				a = addr & ~(line_size - 1);
				end = (addr + size - 1) & ~(line_size - 1);
				if (end < a)
					printf("FAIL: a=0x%08lx, end=0x%08lx,"
						"line_size=%i, size=%i\n",
						a, end, line_size, size);
			}
		}
	}

	return 0;
}

Index: c-r4k.c
===================================================================
RCS file: /home/cvs/linux/arch/mips/mm/c-r4k.c,v
retrieving revision 1.3.2.67

diff -d -u -r1.3.2.67 c-r4k.c
@@ -482,7 +482,7 @@
 		}
 
 		a = addr & ~(sc_lsize - 1);
-		end = (addr + size - 1) & ~(sc_lsize - 1);
+		end = (addr + size + sc_lsize - 1) & ~(sc_lsize - 1);
 		while (1) {
 			flush_scache_line(a);	/* Hit_Writeback_Inv_SD */
 			if (a == end)

@@ -504,7 +504,7 @@
 
 		R4600_HIT_CACHEOP_WAR_IMPL;
 		a = addr & ~(dc_lsize - 1);
-		end = (addr + size - 1) & ~(dc_lsize - 1);
+		end = (addr + size + dc_lsize - 1) & ~(dc_lsize - 1);
 		while (1) {
 			flush_dcache_line(a);	/* Hit_Writeback_Inv_D */
 			if (a == end)

@@ -529,7 +529,7 @@
 		}
 
 		a = addr & ~(sc_lsize - 1);
-		end = (addr + size - 1) & ~(sc_lsize - 1);
+		end = (addr + size + sc_lsize - 1) & ~(sc_lsize - 1);
 		while (1) {
 			flush_scache_line(a);	/* Hit_Writeback_Inv_SD */
 			if (a == end)

@@ -546,7 +546,7 @@
 
 		R4600_HIT_CACHEOP_WAR_IMPL;
 		a = addr & ~(dc_lsize - 1);
-		end = (addr + size - 1) & ~(dc_lsize - 1);
+		end = (addr + size + dc_lsize - 1) & ~(dc_lsize - 1);
 		while (1) {
 			flush_dcache_line(a);	/* Hit_Writeback_Inv_D */
 			if (a == end)

sjhill> The 'r4k_dma_cache_wback_inv' function will fail when the sjhill> requested size equals 0 AND when the address is a multiple of sjhill> the line size. I discovered this bug while using the National sjhill> Semiconductor DP8381x series PCI ethernet driver. I have sjhill> attached a test program showing the bug as well as a patch for sjhill> comment. Okay to apply?

I think your patch is overkill. It flushes many one line then needed.

How about just inserting

	if (unlikely(size == 0))
		return;

in beginning of each function?

---
Atsushi Nemoto

So far I was simply considering any use with size 0 a bug. In this case a 0 argument was passed to pci_unmap_XXX, so I'll have to figure out with the other architecture maintainers if that's just odd usage of the API or an outright bug.

Ralf