On Nov 10, 2011, at 11:54, Scott Wood wrote: > On Thu, Nov 10, 2011 at 10:30:41AM -0600, Kumar Gala wrote: >> On Nov 10, 2011, at 10:17 AM, Moffett, Kyle D wrote: >>> Furthermore, it looks like there are a couple issues here I missed >>> before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7, >>> except when you turn on the P5020DS board option which magically >>> changes it to "6" and breaks lord-knows-what. I think my patch >>> series actually "breaks" that and makes e5500 use 7 as well. >> >> a value of '6' on E5500 / P5020DS is correct and doesn't break anything. >> Setting it to 7 is wrong and thus the code is correct today. >> >>> Are you sure that a kernel built to support E5500 can also run on >>> other 64-bit PowerPC/POWER systems? >> >> No it will not. There is not expectation of that as E5500 is an >> embedded / Book-E class part and uses that ISA version. Book-S >> (server) 64-bit machines are not OS compatible and we are not trying to >> make them as such (but we do re-use a lot of code). > > What about other 64-bit book3e chips? What cache block size does A2 have?
Ok, so I've been poking around this code a bunch and as far as I can tell, the cacheline stuff has basically always been subtly wrong in twelve different ways and it's only largely coincidence that it works today. So PowerPC64 systems have their own "ppc64_caches" structure set up before start_kernel() is called by parsing the OpenFirmware "cpu" nodes. That structure is then checked in every piece of 64-bit kernel code (except xmon) that uses the "dcbXX" and "icbXX" opcodes. There is an entirely separate mechanism built into the "cputable" that is used on all PowerPC systems to compute cacheline sizes to pass in via ELF headers for userspace to use in memset()/memcpy(), etc. Furthermore, the VDSO gets cacheline sizes stored into it, but on 64-bit they come from the ppc64_caches structure and on 32-bit they come from dcache_bsize/icache_bsize copied from the cputable. Then there's the value in arch/powerpc/include/asm/cache.h which is used throughout the kernel to figure out how far apart to space CPU-specific datastructures (EG: __cacheline_aligned_on_smp). Despite the fact that all PPC64 have an "L1_CACHE_SIZE" value of 128, the PowerPC A2 and e5500 have {d,i}cache_bsize values of 64 in cputable and presumably also get correct values from OpenFirmware, so the bogus constant in asm/cache.h does nothing more than waste a bit of memory for unnecessary padding. Unfortunately, lots of PPC32 assembly pretends that the value found in asm/cache.h is a hard truth and uses it for "dcbz", etc, which is why there are all of those ugly #ifdefs in asm/cache.h Based on all of that, my proposal is going to be a patch which does the following: (1) Conditionally set L1_CACHE_SHIFT to the maximum value used by any platform being compiled in for alignment purposes. (2) Make the ppc64_caches struct apply to ppc32 as well, and preinitialize it with a minimum value used by any platform being compiled in (for "dcbXX"/"icbXX" purposes). This is safe because the pagesize is always a multiple of the cache block size and the kernel only uses dcbXX/icbXX on whole pages. The only impact is a temporary small performance hit from flushing or zeroing the same block 8 times if too small. (3) Try to initialize the ppc_caches struct on ppc32 from the OpenFirmware device-tree. If that fails, then use the values we find in the cputable. After this is initialized any performance hit in copy_page()/zero_page() will obviously disappear. (4) Fix all of the PPC32 assembly code that is misusing L1_CACHE_SHIFT to use the ppc_caches struct instead. Does that sound like a reasonable approach? Cheers, Kyle Moffett -- Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/ _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev