[Bug 1607] __RTEMS_SIZEOF_VOID_P__ flawed design

[bugzilla-daemon at rtems.org]

https://www.rtems.org/bugzilla/show_bug.cgi?id=1607

--- Comment #3 from Joel Sherrill <joel.sherrill at oarcorp.com> 2010-07-07 10:20:28 CDT ---
(In reply to comment #2)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > This change is a massive design flaw:
> > > 
> > > > 2010-06-29 Joel Sherrill <joel.sherrill at oarcorp.com>
> > > > 
> > > > * configure.ac, score/include/rtems/score/percpu.h: Add
> > > > __RTEMS_SIZEOF_VOID_P__ to cpuopts.h so percpu.h has this information
> > > > available during build and after installation.
> > > 
> > > Sizes are compile time constants and therefore may depend upon compiler 
> > > flags (e.g. multilibs) flags. Defining them in a globally exported 
> > > header being shared by different architecture variants qualifies is absurd.
> > > 
> > > Proposal: Joel to revert his whole __RTEMS_SIZEOF_VOID_P__ patch-set.

It was the least intrusive solution at hand. We have to have a per cpu
data structure to support SMP and it has to have a few fields accessed from
assembly language.  Please help find an acceptable way to support this.  


> > Maybe we should add this define to a no-installation header file?
> Right. #1 candidate would be config.h.

I would be OK with that EXCEPT that some (or all to follow your proposal
below) of the interrupt processing code is outside the CPU Kit and needs to
know this.  If we don't publish this information from the CPU Kit somehow, 
then each subsystem providing IRQ processing will end up having to do
something itself.

In fact it was once I started propagating the sizeof void * autoconf
macro into various BSPs configure.ac.  That seemed worse than 
adding it once to the CPUKit somewhere.

> Unfortunately Joel has added further defines (in percpu.h) using
> __RTEMS_SIZEOF_VOID_P__ which make tracking use case of this define very hard.

x86 and SPARC (and other ports using offsets) use PER_CPU_XXX in .S files.
Other ports will use ISR_NEST_LEVEL, DISPATCH_NEEDED, and possibly
INTERRUPT_STACK_LOW or INTERRUPT_STACK_HIGH.

> I.e. an alternative to reverting Joel's stuff would be to extract all defines
> which are using __RTEMS_SIZEOF_VOID_P__ to a private per-BSP header and to
> remove all use case from cpukit (Unfortunately, this doesn't seem to easily 
> achievable).

That will require moving all IRQ processing code from the CPU Kit and moving it
to libbsp somewhere.  Plus adding some check in each BSP configure.ac.

Offhand, I don't think moving the IRQ code is THAT bad.  I have come to like
having the context switch and IRQ code separate since it shrinks the files.
But to split it means that the IRQ code has to be moved somewhere and 
we have to decide on where.  That means modifying a few (if in libcpu)
or a lot (if in libbsp) of Makefile.am's.

Moving it to libbsp is OK with me since the BSP provides the SMP
specific "my CPU number" code and knows if it supports SMP.  This
is part of the per cpu table lookup for IRQ processing on SMP systems.
I already have split the SPARC and moved it in my tree to libbsp/sparc/irq.

> > Btw. is SIZEOF_CPU_CONTEXT also something suspicious in cpuopts.h?
> Suspicious, yet. Whether it actually is depends. 
> ATM, without having looked into the source-tree, I am a bit surprised it is
> still around (I thought it was gone).

I don't think it is used but my grep only found it in an old ChangeLog:

$ grep -r SIZEOF_CPU_CONTEXT .
./cpukit/ChangeLog-pre2007:    Add SIZEOF_CPU_CONTEXT to cpuopts.h.

Where do you see it?  Did you already do something to remove it?

-- 
Configure bugmail: https://www.rtems.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.