RFC: Cleanup/detection patch

RFC: Cleanup/detection patch

[Keith M Wesolowski]

I have posted an initial copy of my patch for machine detection,
namespace cleanup, and promlib abstraction at
http://foobazco.org/~wesolows/mips64-machine.diff.  This is against
2.4.2 CURRENT oss.  It currently passes my regression testsuite which
unfortunately does not include an ip27 boot test.

There are several goals to this patch: to be able to support multiple
machines with a single kernel binary; to reduce code duplication among
the various machines, and to provide a well-defined architecture for a
future increase in the number and diversity of systems we support.

This patch affects mips64 only at this time.  However, it will be
ported to the 32-bit mips tree following comment.  It is also
incomplete - I know for a fact that certain pci functions need this
kind of abstraction as well, and the promlib support is fairly
minimal.  The accompanying documentation, while useful especially for
new porters, is slightly out of date already.

For those (embedded systems people) who think the ability to support
multiple machines in a single kernel is useless bloat, consider
instead that you will have significantly less code to maintain and to
write, since more functionality will be provided by generic functions.
No more copying the generic files, changing three lines, and having to
maintain the copies forever.  This patch as written adds less than 300
net lines of code to the kernel (the entire difference is contained in
__init functions), and when finished will probably result in
single-machine kernels which are approximately the same size as
current versions.

Please offer your comments.  Barring a shockingly brutal rejection of
the fundamental principles, I fully expect this to be integrated by
2.5.x for mips and mips64.  Speak now or forever hold your peace.

If you are interested, this code is also being maintained in cvs at
cvs.foobazco.org, username cvs, password cvs, repository linux.  This
is not a fork; the patches will either be put into oss or dropped.
This tree also has early 64-bit SGI O2 support based on work by Harald
Koerfgen, nick@snowman.net, and me.

-- 
Keith M Wesolowski <wesolows@foobazco.org> http://foobazco.org/~wesolows
------(( Project Foobazco Coordinator and Network Administrator ))------
"I should have crushed his marketing-addled skull with a fucking bat."

Re: RFC: Cleanup/detection patch

[Jun Sun]

Keith M Wesolowski wrote:
> 
> I have posted an initial copy of my patch for machine detection,
> namespace cleanup, and promlib abstraction at
> http://foobazco.org/~wesolows/mips64-machine.diff.  This is against
> 2.4.2 CURRENT oss.  It currently passes my regression testsuite which
> unfortunately does not include an ip27 boot test.
> 

Keith,  I am interested in this stuff, but I have not got time around to look
into it.  

Here are some of my thoughts on this issues.  Maybe you can clarify them here.

1. Right now, our tree (at least 32-bit) does not even support multiple CPUs
(with the same machine/board).  Take a look of
arch/mips/mm/loadmmu.c:loadmmu(), and you will see what I mean.  The CPU
specific ld_mmu_xxx is #ifdef'ed.  So if you enable multiple CPU, the last
ld_mmu_xxx will win!

So a modest step forward would be fixing that first.

2. Currently all CPU specific ld_mmu_xxx stuff lump cache and TLB together. 
That is not very good.  I have seen CPUs that can share cache but not TLB. 
Vice versa.  Personally I like to see their separation first before a more
dramatic scheme is in place.

3. Unfortunally not all CPUs can be fully probed at the run-time, specifically
the external cache size and geometry.  I was thinking perhaps a board
detection routine should be placed at the beginning which will supply external
cache info.  In addition it will probably set prom_init() pointer - yes, we do
have conflicting prom_init() from every board-specific implementation - and
board_setup() pointer.  What do you think?

Sorry for not giving you patch specific comments, but I figure if I don't spit
it out now it will be probably never. :-)

Jun

Re: RFC: Cleanup/detection patch

[Keith M Wesolowski]

On Tue, Apr 03, 2001 at 07:43:07PM -0700, Jun Sun wrote:

> 1. Right now, our tree (at least 32-bit) does not even support multiple CPUs
> (with the same machine/board).  Take a look of
> arch/mips/mm/loadmmu.c:loadmmu(), and you will see what I mean.  The CPU
> specific ld_mmu_xxx is #ifdef'ed.  So if you enable multiple CPU, the last
> ld_mmu_xxx will win!
> 
> So a modest step forward would be fixing that first.

In the patch, the mips_cpu structure has a load_cache and load_tlb
function associated with it which are assigned during cpu_probe.  It
is now possible - I believe - to compile both andes.c and r4k* into
the kernel and have the right routines run at boot time.  No reason
this can't work for other CPUs also.

> 2. Currently all CPU specific ld_mmu_xxx stuff lump cache and TLB together. 
> That is not very good.  I have seen CPUs that can share cache but not TLB. 
> Vice versa.  Personally I like to see their separation first before a more
> dramatic scheme is in place.

The patch addresses this; look at the removal of r4xx0.c and its
replacements.  It's been split into four pieces - this may become
three later.  One for cache, one for tlb, one for copy/clear page and
miscellaneous outlined assembly (yes, in real assembly), and one for
initialization functions.  There's no reason it couldn't be further
split; for example, to do r4600/r5k style caches in a separate module.

> 3. Unfortunally not all CPUs can be fully probed at the run-time,
> specifically the external cache size and geometry.  I was thinking
> perhaps a board detection routine should be placed at the beginning
> which will supply external

The CPU-specific load_cache is responsible for this.  I'm open to the
idea of having separate cache detection for cache problems that are
*not* cpu-specific.  For example, if r10k indy with boardcache existed
that might be applicable.  But I think the load_cache should be able
to handle this.

> cache info.  In addition it will probably set prom_init() pointer -
> yes, we do have conflicting prom_init() from every board-specific
> implementation - and board_setup() pointer.  What do you think?

Namespace collisions like that must die.  For now the mips_init->init
is called at about the same place that prom_init used to be; in many
cases the equivalent will be needed to successfully probe a machine
and thus will be done by the probe function.  In either case, there's
no need for each machine to have a separate function with that name.

> Sorry for not giving you patch specific comments, but I figure if I
> don't spit it out now it will be probably never. :-)

Yep.

-- 
Keith M Wesolowski <wesolows@foobazco.org> http://foobazco.org/~wesolows
------(( Project Foobazco Coordinator and Network Administrator ))------
"I should have crushed his marketing-addled skull with a fucking bat."

Re: RFC: Cleanup/detection patch

[Jun Sun]

Keith M Wesolowski wrote:
> 
> On Tue, Apr 03, 2001 at 07:43:07PM -0700, Jun Sun wrote:
> 
> > 1. Right now, our tree (at least 32-bit) does not even support multiple CPUs
> > (with the same machine/board).  Take a look of
> > arch/mips/mm/loadmmu.c:loadmmu(), and you will see what I mean.  The CPU
> > specific ld_mmu_xxx is #ifdef'ed.  So if you enable multiple CPU, the last
> > ld_mmu_xxx will win!
> >
> > So a modest step forward would be fixing that first.
> 
> In the patch, the mips_cpu structure has a load_cache and load_tlb
> function associated with it which are assigned during cpu_probe.  It
> is now possible - I believe - to compile both andes.c and r4k* into
> the kernel and have the right routines run at boot time.  No reason
> this can't work for other CPUs also.
> 
> > 2. Currently all CPU specific ld_mmu_xxx stuff lump cache and TLB together.
> > That is not very good.  I have seen CPUs that can share cache but not TLB.
> > Vice versa.  Personally I like to see their separation first before a more
> > dramatic scheme is in place.
> 
> The patch addresses this; look at the removal of r4xx0.c and its
> replacements.  It's been split into four pieces - this may become
> three later.  One for cache, one for tlb, one for copy/clear page and
> miscellaneous outlined assembly (yes, in real assembly), and one for
> initialization functions.  There's no reason it couldn't be further
> split; for example, to do r4600/r5k style caches in a separate module.
> 

Cool - this seems to be in the line I was thinking of.

On a minor note, pretty much all existing r4k and above CPUs (I believe) can
share the same cache code if 1) number of ways and 2) way selection offset are
introduced.  All the diverging in cache code comes from the difference in how
the way in a cache set is specified in indexed operations.

I think current mips_cpu struct already logs number of ways.  We just need to
introduce the constant offset for way selection.


> > 3. Unfortunally not all CPUs can be fully probed at the run-time,
> > specifically the external cache size and geometry.  I was thinking
> > perhaps a board detection routine should be placed at the beginning
> > which will supply external
> 
> The CPU-specific load_cache is responsible for this.  I'm open to the
> idea of having separate cache detection for cache problems that are
> *not* cpu-specific.  For example, if r10k indy with boardcache existed
> that might be applicable.  But I think the load_cache should be able
> to handle this.
>

I am not sure if load_cache() can handle that by itself.

Load_cache() is affiniated with cpu.  The external cache info is associated
with machine.

For example, on the ocelot board I am working on you need to read a
board-specific register to get 3rd level cache config.  Presummably if the the
same Rm7k cpu is used another board, the load_cache_rm7k() has to know how to
figure out that on the other board.  Obviously you will need some sort of
#ifdef to do that.  Then you need to consider what if both machines are
configured in.

Does not look very clean to me.

For that and some other purposes, I am convinced we need to *know* exactly
what machine we are on.  I understand some machine cannot detect itself.  Too
bad!  That just means that machine CANNOT be configured into a multi-machine
kernel.  It should be trivial to detect that (possibly through the return
value of <board>_detect() routine).

Anyhow, I am thinking about something like below:

/* invoked at the very beginning of init_arch() */
void mips_machine_detection()
{
#if defined(CONFIG_DDB5476)
   retval = ddb5476_detect();
   /* check retval - 0: negative; 1: possitive; 2: cannot detect, possitive by
config only */
#endif
#if defined(CONFIG_SNI)
   retval = sni_detect();
   /* check for ret val conflict */
#endif
....

   /* check if we at least detect one valid machine */
   return;
}

If we do it right, I think this pretty much the *ONLY* source file one needs
to modify when one adds support for a new machine/board.  (hmm, am I dreaming
again?)

Jun

Re: RFC: Cleanup/detection patch

[Keith M Wesolowski]

On Wed, Apr 04, 2001 at 10:54:32AM -0700, Jun Sun wrote:

> On a minor note, pretty much all existing r4k and above CPUs (I
> believe) can share the same cache code if 1) number of ways and 2)
> way selection offset are introduced.  All the diverging in cache
> code comes from the difference in how the way in a cache set is
> specified in indexed operations.

So we introduce the waybit member into struct cache (where it belongs
anyway).  That was easy.

> I am not sure if load_cache() can handle that by itself.
> 
> Load_cache() is affiniated with cpu.  The external cache info is associated
> with machine.

So we basically have an Indy-style boardcache then.  I think we must
make a distinction very cleanly between caches which are under CPU
control and those which are not.  The ones under CPU control use the
stuff in load_cache and the cache descriptors in mips_cpu.  The
boardcaches use something like bc_ops.  The generic cacheflushing
operations can then simply have hooks to call the appropriate bcop.
This wastes time on sane hardware, so any machine that has this must
set CONFIG_STUPID_CACHE.  Without that the call is ifdef'd out.  Yes,
I hate ifdefs but there's no reason that everyone should suffer just
because some hardware is braindead.

There is another way.  I hate to suggest it because it's pure
evil...look at the sparc btfixup code some time.  We could actually
use something similar for all our caches if we wanted to.

> figure out that on the other board.  Obviously you will need some sort of
> #ifdef to do that.  Then you need to consider what if both machines are
> configured in.

If both machines are configured in, what gets done depends on which is
detected at boot time.

> For that and some other purposes, I am convinced we need to *know* exactly
> what machine we are on.  I understand some machine cannot detect itself.  Too
> bad!  That just means that machine CANNOT be configured into a multi-machine
> kernel.  It should be trivial to detect that (possibly through the return
> value of <board>_detect() routine).

This is exactly what the documentation says.  I am pretty sure that
any machine can be detected uniquely.  The important thing is that it
can be detected without the detection routine crashing other systems.
If for some reason it's not possible for a given machine (*cough* ip27
*cough*) to live with the others - ifdefs in head.S let's say - then
that config option is exclusive.  Simple.

All of this is details anyway.  The big problems we have aren't in the
code - structs filled with function pointers are child's play to
implement and use.  Our problem is that it seems like about 80% of
linux is implemented as inline functions in header files.  Some of
these have lots of nasty ifdefs around them.  Then there are lovely
things like the address space macros which are cpu dependent.  Or
SERIAL_PORT_DFNS, which seems specifically intended to thwart efforts
to support multiple machines.  And, finally, we have things like pref
which must not be used if *any* cpu configured in does not support it
(or, it must be overwritten by bootup code with nop if it's not
supported *shudder*).

There's the plain and simple fact that more dynamic detection is going
to slow the kernel down.  That's not going to sit well with the origin
folks for example.  So care needs to be taken to make sure that the
performance penalties are small and more than offset by ease of
maintenance.  And from what I've heard about the irix code, people
working on it probably aren't likely to be impressed by things like
maintainability.

For people building new machines targeting linux - this is important -
you can arrange for the prom to pass arguments; these will be passed
to the detection routine (in fact, they will get whatever is in a0,
a1, and a2).  Someone who wanted to make his or her life easy would
arrange to pass "mips_machine=fuxinator2000" in argv somewhere and
just test that.  Or, read from some address in the PROM (ARC[S] does
this) to find the name of your machine where the firmware designers
thoughtfully placed it.  Since the whole point of this exercise is
easier support and maintenance for new hardware, this seems like the
most obvious possible way to solve the problem.  If some oddball
system made 8 years ago can't be detected cleanly, so what.

> /* invoked at the very beginning of init_arch() */
> void mips_machine_detection()
...

That's pretty much what the code does.  There is an array of probing
functions.  They are tried in succession in known but undefined order.
Any which succeeds (indicated by returning 1) is expected to have set
up the machine-specific functions.  These probes must return 0 if
support for the given system is not compiled in.

> If we do it right, I think this pretty much the *ONLY* source file one needs
> to modify when one adds support for a new machine/board.  (hmm, am I dreaming
> again?)

Pretty much.  In my implementation there are actually two places - the
probe function itself and adding it to the list.  See the
documentation.  I am also making an effort to document exactly what
other functions are mandatory and what are optional.  In the end a
poorly-trained but literate monkey with a mild psychiatric disorder
should be able to handle a port of mips/linux to a new piece of
hardware.

Now if only the linux requirements for implementing pci on a new
machine were documented somewhere...

-- 
Keith M Wesolowski <wesolows@foobazco.org> http://foobazco.org/~wesolows
------(( Project Foobazco Coordinator and Network Administrator ))------
"I should have crushed his marketing-addled skull with a fucking bat."

Re: RFC: Cleanup/detection patch

[Jun Sun]

Keith M Wesolowski wrote:
> 
> So we basically have an Indy-style boardcache then.  I think we must
> make a distinction very cleanly between caches which are under CPU
> control and those which are not.  The ones under CPU control use the
> stuff in load_cache and the cache descriptors in mips_cpu.  The
> boardcaches use something like bc_ops. 

I don't like bc_ops idea.  Usually the external cache capability is still
integral part of the CPU.

I favor the idea where the cache takes care of external cache dynamically,
based on some parameters set up by board detection routine.

> 
> Now if only the linux requirements for implementing pci on a new
> machine were documented somewhere...
> 

That should come soon ...

Jun

Re: RFC: Cleanup/detection patch

[Keith M Wesolowski]

On Thu, Apr 05, 2001 at 01:29:13PM -0700, Jun Sun wrote:

> I don't like bc_ops idea.  Usually the external cache capability is still
> integral part of the CPU.

How can it be both an integral part of the CPU and board-specific?
Either it's under the direct control of the cpu or it's not.  If it
is, that's cpu-specific and handled by the regular cacheops.  If it's
not, that's board-specific and is called from a hook into something
which the machine detection has set up.

> I favor the idea where the cache takes care of external cache dynamically,
> based on some parameters set up by board detection routine.

So we end up filling the cache routines with tests for board-specific
stuff?  No way.  The cache routines should be dependent ONLY on the
CPU - two completely different boards with radically different designs
should be able to use the exact same foo-cache.c if they have the same
CPU.  If it's board-specific we can put generic hooks in but testing
for various boards in the cacheops is too expensive and too ugly.

-- 
Keith M Wesolowski <wesolows@foobazco.org> http://foobazco.org/~wesolows
------(( Project Foobazco Coordinator and Network Administrator ))------
"I should have crushed his marketing-addled skull with a fucking bat."

Re: RFC: Cleanup/detection patch

[Jun Sun]

Keith M Wesolowski wrote:
> 
> On Thu, Apr 05, 2001 at 01:29:13PM -0700, Jun Sun wrote:
> 
> > I don't like bc_ops idea.  Usually the external cache capability is still
> > integral part of the CPU.
> 
> How can it be both an integral part of the CPU and board-specific?

Hmm, I am thinking about something like Rm7k, where the tertiary cache is
controlled by the CPU (through cache instruction and TagLo/TagHi regsiters,
etc) but the size is only known to the board.

I think some CPUs can have secondary, external cache but cannot figure the
size and, especially, the layout (# of ways, ...) automatically.  (Any
confirmation?)

Those are what I was referring to.

The key point is that machine_detection() should happen BEFORE
cpu_probe()/cache_probe() so that we have an elegant way to address the above
situation.

Jun

Re: RFC: Cleanup/detection patch

[Geert Uytterhoeven]

On Thu, 5 Apr 2001, Keith M Wesolowski wrote:
> (or, it must be overwritten by bootup code with nop if it's not
> supported *shudder*).

FYI, this approach is currently being implemented on PPC.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven ------------- Sony Software Development Center Europe (SDCE)
Geert.Uytterhoeven@sonycom.com ------------------- Sint-Stevens-Woluwestraat 55
Voice +32-2-7248626 Fax +32-2-7262686 ---------------- B-1130 Brussels, Belgium