Re: [PATCH 1/4] powerpc: consolidate feature fixup code

[Olof Johansson <olof@lixom.net>]

On Fri, 13 Oct 2006 00:47:29 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> > > +void do_feature_fixups(unsigned long offset, unsigned long value,
> > > +		       void *fixup_start, void *fixup_end)
> > > +{
> > > +	struct fixup_entry {
> > > +		unsigned long	mask;
> > > +		unsigned long	value;
> > > +		unsigned int	*start;
> > > +		unsigned int	*end;
> > > +	} *fcur, *fend;
> > 
> > Shouldn't there be a better place to keep this struct definition than
> > in the function it's used? Some header file?
> 
> It's not used anywhere else.... which header would you put it in ?

Near where the asm-corresponding ones are located (cputable.h).


-Olof

> > Ok, now it's in C, no reason to do a dcbst/icbi for every word. We did
> > it in asm for simplicity's sake. Split it in two loops, one to nop,
> > second to step per line and do the flushes. :-)
> 
> Do we care ? :) But yah, I suppose I can do that.

Close call. :-) It's just easier to do now, might as well do it. But
yeah, no big deal.

It will also be easier to graft in a branch forward instead of a large
amount of nops for large feature sections. I guess it will take some
benchmarking to see what the breakeven point is.


-Olof