Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel

[Josh Poimboeuf <jpoimboe@linux.vnet.ibm.com>]

On Wed, 2011-11-09 at 12:03 +0530, Suzuki Poulose wrote:
> On Tue, 08 Nov 2011 10:19:05 -0600
> Josh Poimboeuf <jpoimboe@linux.vnet.ibm.com> wrote:
> 
> > On Tue, 2011-11-08 at 12:41 +0530, Suzuki Poulose wrote:
> > > What I was suggesting is, instead of flushing the cache in
> > > relocate(), lets do it like:
> > > 
> > > for e.g, on 440x, (in head_44x.S :)
> > > 
> > > #ifdef CONFIG_RELOCATABLE
> > > 	...
> > > 	bl relocate
> > > 
> > > 	#Flush the d-cache and invalidate the i-cache here
> > > #endif
> > > 
> > > 
> > > This would let the different platforms do the the cache
> > > invalidation in their own way.
> > > 
> > > Btw, I didn't find an instruction to flush the entire d-cache in
> > > PPC440 manual. We have instructions to flush only a block
> > > corresponding to an address.
> > > 
> > > However, we have 'iccci' which would invalidate the entire i-cache
> > > which, which I think is better than 80,000 i-cache invalidates.
> > 
> > In misc_32.S there are already some platform-independent cache
> > management functions.  If we use those, then relocate() could simply
> > call them.  Then the different platforms calling relocate() wouldn't
> > have to worry about flushing/invalidating caches.
> > 
> > For example, there's a clean_dcache_range() function.  Given any range
> > twice the size of the d-cache, it should flush the entire d-cache.
> > But the only drawback is that it would require the caller to know the
> > size of the d-cache.
> > 
> > Instead, I think it would be preferable to create a new clean_dcache()
> > (or clean_dcache_all()?) function in misc_32.S, which could call
> > clean_dcache_range() with the appropriate args for flushing the entire
> > d-cache.  relocate() could then call the platform-independent
> > clean_dcache().
> > 
> 
> 
> How about using clean_dcache_range() to flush the range runtime
> address range [ _stext, _end ] ? That would flush the entire
> instructions.

Wouldn't that result in more cache flushing than the original solution? 

For example, my kernel is 3.5MB.  Assuming a 32 byte cache line size,
clean_dcache_range(_stext, _end) would result in about 115,000 dcbst's
(3.5MB / 32).


> 
> 
> > For i-cache invalidation there's already the (incorrectly named?)
> > flush_instruction_cache().  It uses the appropriate platform-specific
> > methods (e.g. iccci for 44x) to invalidate the entire i-cache.
> 
> Agreed. The only thing that worries me is the use of KERNELBASE in the 
> flush_instruction_cache() for CONFIG_4xx. Can we safely assume all 4xx
> implementations ignore the arguments passed to iccci ? 

Good question.  I don't know the answer. :-)

That also may suggest a bigger can of worms.  A grep of the powerpc code
shows many uses of KERNELBASE.  For a relocatable kernel, nobody should
be relying on KERNELBASE except for the early relocation code.  Are we
sure that all the other usages of KERNELBASE are "safe"?

For example -- the DEBUG_CRIT_EXCEPTION macro, which head_44x.S uses,
relies on KERNELBASE.  That doesn't seem right to me.


Josh