[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: cpukit/bspkit split.



On Fri, 2005-02-18 at 13:18 -0600, Joel Sherrill  wrote:
> Sergei Organov wrote:
> > "Joel Sherrill <joel@OARcorp.com>" <joel.sherrill@OARcorp.com> writes:
> > 
> >>Sergei Organov wrote:
> >>
> >>>"Joel Sherrill <joel@OARcorp.com>" <joel.sherrill@OARcorp.com> writes:
> >>>[...]
> >>>
> >>
> >>>>There is a PPC_INTERRUPTS_MAX which still remains to be addressed.
> >>>>Ralf.. I think the the MIPS defines that in libcpu as a variable used by the
> >>>>cpukit.
> >>>
> >>>IMHO, it's time to define generic interface between cpukit and the rest
> >>>of RTEMS BSP that will ensure compile-time independence of cpukit, or at
> >>>least to document some guidelines. It seems that pre-multilib BSP
> >>>interface definitions don't address the issue as in fact every BSP is
> >>>now split between cpukit and the rest of libs and there is no definition
> >>>of the interface to be used on this split line. Does it sound
> >>>reasonable?
> >>
> >>That largely exists.  When we started splitting the CPU kit from the BSP
> >>and libcpu parts of the tree, most of the ports were clean.
> > 
> > 
> > Guess those that didn't have any CPU variants or where CPU variations were
> > in fact insignificant? ;)
> > 
> > We can do the same for PowerPC, -- let every CPU variant be a separate
> > RTEMS port, i.e., just forget that they are all called PowerPCs, --
> > every BSP then will be pretty clean with respect to CPUkit split ;)
> > 
> > 
> >>The porting guide is in general pretty close.
> > 
> > 
> > Well, I'm looking at
> > 
> > <http://www.rtems.com/onlinedocs/doc-current/share/rtems/html/porting/index.html>
> > 
> > Do you mean this one? If so, I'm afraid it's far from being close. It
> > even doesn't mention anything like cpukit or bspkit and immediately
> > jumps to the reference to non-existing directory c/src/exec/score/cpu:
> > 
> > <http://www.rtems.com/onlinedocs/doc-current/share/rtems/html/porting/porting00004.html>
> > 
> > Then, "some_other_cpu_dependent_info" in the "rtems_cpu_table" seems
> > to be problematic as well.
Definitely - I actually was referring to rtems_cpu_table when talking
about "data abstraction"/"classes".

> > Is there something more recent that I've missed?

I think you're on the same track as I am.

I think: cpukit needs a much more restrictive API. Such kind of
restrictive that it prevents cpukit from getting infected from "design
warts" as the powerpc port apparently has been infected from.

IMO, a point to getting started with this would be a fundamental
redesign of rtems_cpu_table. What I currently have in mind is
implementing a class hierarchy similar to this:

RTEMSScoreCPUTable
|
+- RTEMSPowerpcCPUTable
|  |
|  +- RTEMSPowerpc<1>CPUTable
|  +- RTEMSPowerpc<2>CPUTable
|  .
|  +- RTEMSPowerpc<M>CPUTable
|
+- RTEMSCPU<N>CPUTable
.
.

RTEMSScoreCPUTable would be "abstract base classes" implemented in
cpukit/score, RTEMSCPU<N>CPUTable would be specializations to be
implemented in cpukit/score/cpu/<CPU>

Finally, RTEMSCPU<N><M>CPUTable classes would be implemented in
libcpu/cpu/<cpu>.

A traditional way to implement such kind of approach in C would be to
apply "widget"-technology, as it is common in computer graphic.

However, ATM, I don't see how this can be implemented in short term.


> > If we require CPUkit to have only code that is based on ISA variant,
> > then this problem goes away, right? That once again raises a question
> > how in fact the code is divided, why it is divided this way, and isn't
> > there a better way to divide it. All these questions could and should be
> > answered by designing the interface I'm speaking about.
> 
> The base point for why is easy.  It is at a minimum divided based upon 
> the level of multilib distinctions gcc and newlib makes. We want cpukit/ 
> to be multilib'ed using the same variants the *-rtems gcc/newlib 
> binaries do.
> 
> There are n CPUs with a 603e/G2 core and gcc doesn't know ore care about 
> the model names.  Ideally RTEMS cpukit/ wouldn't know either.
"Wouldn't know"?

It "_must_ not know" nor care about it!


> >>Notice how the PowerPC port is evolving now. Ralf proposed some
> >>conditionals to remove/consolidate, then reduced the multilibs. Then
> >>everyone jumped in and said "you can remove that" or "why is that
> >>there?". Once the mess is straightened out, it will really be quite
> >>simple.
> > 
> > 
> > I'm afraid it won't be that simple anyway. Probably the number of
> > multilib variants will be reduced, but the question of figuring out the
> > exact interface between CPUkit and the rest of the system will remain.
> > Just try to put yourself on a place of a new programmer trying to
> > implement yet another CPU variant. Usually she will select a way of
> > least resistance and the situation will tend to return back where it is
> > now.
> 
> Ralf, I and others have cleaned up the other ports to have an interface 
> between that CPUkit and libcpu/bsp for that port.  Those other ports are 
> "multiib clean".  We generally did not bother anyone with the changes we 
> made over the past few years.
> 
> Now we are looking at the biggest and ugliest port and people are 
> noticing.
Right - The powerpc port in RTEMS is a "big fat ugly monster" :(

> What you are suggesting is valid.  Once we know what MAY need to be 
> known between the CPUKit and libbsp/libcpu, we can unify that and define 
> the interface more cleanly.  The reality is that it will be a set of
> data items and calls that are NOT needed on all ports.
> 
> But that doesn't always work.  One example is hooks at the VERY bottom 
> that may architecture specific.  But that can be covered by saying 
> "PowerPC CPU models must provide X."
> 
> > What I'm talking about is a [set of] '.h' file(s) that contains entire
> > CPUkit to the rest of BSPs interface and only that interface. There
> > should be defined not only what should go into the file(s), but with the
> > same significance what *must not* appear in this file(s). E.g., it
> > should be a documented requirement that there should be nothing but what
> > is actually required by the interface, -- especially no any #ifs that
> > can't be resolved by defines provided by the multilib variants.
> 
> I won't argue with that at all.  What we are doing now is cleaning up
> the existing powerpc port so that is even possible.
> 
> > Let's take a look at the current situation. Consider the authoritative
> > example of how things should be implemented, the "no_cpu" port.
ACK, I consider "no_cpu" to be the "API" and consider anything beyond
that to be potential "API-violation".

> > The
> > "cpu.h" seem to define the interface in question. It in turn includes
> > two more files, "no_cpu.h" and "types.h". I've carefully read all the
> > three and found no requirements in them that are violated by the current
> > PowerPC port's "cpu.h" and "powerpc.h". However, "powerpc.h" (as well as
> > "cpu.h") obviously violates CPUkit independence requirement as it uses
> > #ifdefs not supported by any of multilib variants. The bad thing is that
> > nothing in RTEMS documentation or no_cpu port suggests that it's broken.
> 
> That's a valid point.  There should be some guidelines in the porting 
> guide and it must also be expanding to include interactions with libcpu.
> 
> > Well, for me it seems that "no_cpu.h" and therefore "powerpc.h" should
> > be just eliminated altogether. Their purpose in cpukit is unclear and
> > comments in the no_cpu.h are misleading, IMHO.
> 
> I will argue this.  no_cpu.h is intended to be a starting point for new 
> ports of the code under cpukit.  It is compileable and a good starting 
> point.
> 
> Whether it is worth maintaining with the low number of new ports is up 
> for debate.
> 
> I can't follow how you jumped from nocpu to powerpc.h though.
AFAIU, he considers "rtems/score/no_cpu.h" as API specification/template
for "rtems/score/<cpu>.h" (Not to be mixed up with "rtems/score/cpu.h",
which is something completely different).

> > Now comes a surprise: PowerPC port has in fact 2 "powerpc.h" files,
> > 
> > c/src/lib/libcpu/powerpc/rtems/powerpc/powerpc.h
> > cpukit/score/cpu/powerpc/rtems/score/powerpc.h
> > 
> > both claiming to be
> > 
> > /**
> >  * @file rtems/score/powerpc.h
> >  */
> > 
> > what's that? Did the former has been forgotten to be removed from the
> > CVS as it seems that it's the latter that ends up being installed?
> 
> I think that's a file that was copied last week as part of the cleanup 
> and the Doxygen comment wasn't updated.
Right.

>   Don't read too much into that 
> one.  It is just left over fomr the recent moving party.
The "moving party" is me, and "the move" isn't finished yet.

What you see there is a disparate attempt to move powerpc defines out of
cpukit into libcpu without breaking the BSPs.

Therefore I copied the original
cpukit/score/cpu/powerpc/rtems/score/powerpc.h
to 
c/src/lib/libcpu/cpu/powerpc/rtems/powerpc/powerpc.h

and changed rtems/powerpc/powerpc.h to 
#include <rtems/score/powerpc.h>

Now you see duplicated defines in both files. If they conflict, the
compiler will complain. If not the compiler remains silent.

Then I started to eliminate all defines which are not used below cpukit
from rtems/score/powerpc.h.

This causes BSPs and libcpu variants which using these defines to break,
i.e. I could clearly see which BSPs and libcpu variants require special
stuff (defines, structs etc.).

To get these sources compilable again, I changed them to 
#include <rtems/powerpc/powerpc.h>

Now you can just "grep for rtems/powerpc/powerpc.h" and you'll see which
sources depend on more than the cpukit.


In a nutshell: I am using the axe to iron out the powerpc cpukit.

> > This in turn once again raises a problem of absence of separate
> > raw-exception/c-exception/interrupt handlers/masks support in the
> > RTEMS-to-user and RTEMS-to-CPU APIs. Was there any progress in this area
> > recently?
> 
> Not really.  It is a logical next step after the PPC code is cleaned up 
> and PCI API is unified.
> 
> FWIW I am going to make the x86 PCI API match that of the PPC.  This 
> looks to only require a few files to be modified.

IMNSHO it's much too early to propagate this approach to other CPUs. 

At the moment, we have different and conflicting implementations, but
NONE of them has yet proven to be sufficient and NONE of them has proven
to be "good" and general enough.

So I'd consider your step to be a fault. IMO, you are fighting symptoms
and are ignoring the actual cause: Lack of an cpukit API.

> >>The next step will be getting help comparing .S files in various BSPs and
> >>seeing if we can consolidate them under fewer files. No one can test every
> >>variant but with care, we should be able to reduce them mechanically with
> >>little risk.
> > 
> > 
> > Yeah, at least that seems to be doable and will provide better starting
> > point for subsequent cleanups and enhancements.
> 
> That's where we are now.  Cleanup and moving things around.  Once they 
> are in the right places, we can unify and change them.  I personally do 
> not want to change 27 powerpc .S files. :)
Why? Could it be you just facing the symptoms of a lack of proper API? ;-)

Ralf