svn commit: r257199 - in head/sys/arm: allwinner arm broadcom/bcm2835 freescale/imx include lpc mv mv/orion rockchip sa11x0 samsung/exynos tegra ti ti/omap4/pandaboard versatile xilinx xscale/i8032...

[Bruce Evans]

On Sun, 27 Oct 2013, Ian Lepore wrote:

> While doing that, I noticed a whole lot of other includes in arm source
> code that just aren't needed (things like cpu.h and intr.h in virtually
> every file).  So much of our arm code gets created by wholesale pasting
> some other code and then hacking.  That's not completely invalid, I do
> it myself sometimes, but I generally start by commenting out most of the
> includes, then uncomment until it compiles.

Including cpu.h is an especially large style bug.  It is for MD
implementations of MI interfaces (mostly function-line ones; simple
macros are mostly in *param.h).  Thus it should be very small and
rarely used, especially in MD code.  Since the interfaces are MI,
they can be extern and declared in MI headers unless they are macros
or inlines.  Since MD code can get at the details directly, it shouldn't
use these MI interfaces much, and the only includes of cpu.h in MD code
should be to help implement it.

cpu.h was almost completely cleaned up on x86.  It has regressed a bit
with get_cyclecount() and xen.  The complexity and divergence of the
implementations of the relatively trivial and unimportant interface
get_cyclecount() as an inline or macro shows why you shouldn't
micro-optimize many MI interfaces with inline functions or macros.

arm cpu.h is a bit smaller than i386 cpu.h so it should be included a
bit less.  It has the following easy-to-fix (?) obvious bugs:

duplicated across all arches for at least some interfaces:
- declarations of cpu_halt, swi_vm (?!) and fork_trampoline().  These
   are extern, and should always be extern.  Since they are MI, they
   should be declared in an MI header.  They are also unsorted here.
- bad MI implementation of get_cyclecount().  (Not all implementations
   are bad or duplicated.)  There should be a way to default to an MI
   interfaces for some things, and use a less bad one here and for other
   places that use bad ones.

arm-specific:
- declarations of arm_boot_params, arm_vector_init(), identify_arm_cpu(),
   initarm().  Obviously not MI interfaces, and also not used to implement
   MI interfaces, so they don't belong here.
- badaddr_read().  Not obviously not an MI interface, but not in any other
   cpu.h, so cannot be called from MI code, so it doesn't belong here.

arm cpu.h is missing a few interaces that i386 has, e.g., cpu_exec(). 
Therefore, these must be impossible to call from MI code, so they are
misplaced on i386.  Actually, cpu_exec() is never used.  It is only
defined on amd64 and i386.  sparc64 has an XXX comment in exec_setregs()
saying that it doesn't exist.

Most interfaces named cpu_foo() are MI, and IIRC the declarations of
many are misplaced in MD headers, but most have never been misplaced
in cpu.h because they never were inlines or macros.  The cpu_ prefix
for functions should probably be reserved for MI interfaces, and it
might be worth using a variation of it for ones that should be extern.
It isn't used very much in i386/include/.  Older interfaces like
fork_trampoline() use even more inconsistent names.  cpu_exec() should
probably exist instead of exec_setregs() (unless multiple MD calls
are needed for multiple stages of exec).  exec_setregs() should have
a cpu_ prefix.

After removing the garbage, there are just 7 interfaces implemented
in arm cpu.h (btext and etext could also use an MI default.  This
reduces to 5).  <machine/cpu.h> can be about as simple as
<machine/stdarg.h> used to be before it was complicated by compiler
builtins.  It is possible to document this many interfaces! :-)
However, it is hard to document all the symbols needed to implement
these 5 interfaces.  armreg.h is included for just PSR_MODE and
PSR32_MODE and brings an undocumented number of other symbols.
But armreg.h doesn't include enough pollution to actually work --
it uses frame pointers but doesn't include frame.h.  It is unclear
how the MI code that includes it manages to include frame.h.  This
is one case where nested includes are almost justified -- cpu.h is
supposed to be self-sufficient so that MI code that uses it doesn't
need to know about the MD headers that it depends on.  i386 cpu.h
includes psl.h, frame.h and also segments.h.  segments.h seems to
be unused pollution.

TRAPF_USERMODE() is missing parenthesization of 'frame'.

Further cleaning for i386 cpu.h:

- cpu_swapin(): like cpu_exec() except sparc64 doesn't mention it.
- cpu_ops: highly MD; shouldn't have a cpu_ prefix or be declared here.
- cpu_reset(): like cpu_halt(); correct prefix, but should never be
   optimized to a macro or inline here.  Unlike cpu_halt(), it is not
   used for normal shutdowns and is needed mainly by ddb.  arm declares
   it in cpufunc.h.  This is wrong for many reasons.  It is only declared
   there and not implemented.  cpufunc.h is for access to special CPU
   instructions, preferably only 1 instruction per function.  I shouldn't
   have named it cpu*.h.

Bruce