[PATCH]Hide crt0s from user

[Patrick Georgi]

Hi,

attached patch moves all crt0s from the mainboards' Makefiles to
src/arch/i386/Makefile.inc, adding conditionals where necessary. Thus,
for adding a board, you don't have to care about these anymore.

The unification leads to some changes, some (presumably) harmless,
others for the good:

enable_fpu.inc, enable_sse.inc etc. appear on lots more boards.
These I think are harmless

Some romstraps were missing (or in one case too much), and this is much
more regular now, depending only on the presence of the chipset
component that requires them. In particular, these boards were missing
romstraps, which were lost on the newconfig/kconfig transition:
- asus/a8n_e
- msi/ms7260
- msi/ms9282
- sunw/ultra40
It might be that these didn't work with Kconfig and will work again
after this patch.

via/epia-m had the northbridge/via/vx800 romstrap without using that
northbridge.

There's also a new target "printcrt0s" in the toplevel Makefile which I
used for debugging. It might be helpful for people who want to debug
crt0s for the transition, it certainly was for me.

The instructions to migration locally developed boards are simple:
- Apply the Makefile patch only
- Note the output of "make printcrt0s"
  so you have a list of crt0s for your board.
- Apply the patch
- Remove all crt0s (and crt0-y if you didn't update in a while) in your
  mainboard's Makefile.inc
- Compare the new output of "make printcrt0s" with the old one.
  It doesn't hurt if a romstrap is at a different place in the list.

Alternatively, just remove crt0s and try if your board still boots ;-)

There are three options how to move on from here (if this change is
accepted):
1. Keep the decisions in Makefile.inc
2. Move chip specific includes into the chip's Makefile.inc
3. Create a static file, instead of building it from crt0s

Option 2 would move chipset decisions to the chipsets, option 3 would
simplify the buildsystem. Both are simple, now that things are centralized.

The other future development would be to do the same to ldscripts, but I
didn't want to increase the patch size too much.

Signed-off-by: Patrick Georgi

[Uwe Hermann]

On Thu, Feb 25, 2010 at 10:40:41AM +0100, Patrick Georgi wrote:
> attached patch moves all crt0s from the mainboards' Makefiles to
> src/arch/i386/Makefile.inc, adding conditionals where necessary. Thus,
> for adding a board, you don't have to care about these anymore.

Nice!
> Some romstraps were missing (or in one case too much), and this is much
> more regular now, depending only on the presence of the chipset
> component that requires them. In particular, these boards were missing
> romstraps, which were lost on the newconfig/kconfig transition:
> asus/a8n_e
> msi/ms7260
> msi/ms9282
> sunw/ultra40
>
> It might be that these didn't work with Kconfig and will work again
> after this patch.

Confirmed, I had a patch to fix this (at least for A8N-E), which should
be obsolete now (but I will test on hardware later).
> There are three options how to move on from here (if this change is
> accepted):
> Keep the decisions in Makefile.inc
> Move chip specific includes into the chip's Makefile.inc
> Create a static file, instead of building it from crt0s
>
> Option 2 would move chipset decisions to the chipsets, option 3 would
> simplify the buildsystem. Both are simple, now that things are centralized.

Hm, not sure what the best option is, both 1 and 2 sound good.
> Signed-off-by: Patrick Georgi

Acked-by: Uwe Hermann

(if the build doesn't break, which I assume is tested)
> Index: src/cpu/via/model_c3/Kconfig
>
> --- src/cpu/via/model_c3/Kconfig	(revision 5158)
> +++ src/cpu/via/model_c3/Kconfig	(working copy)
> @@ -1,3 +1,4 @@
>  config CPU_VIA_C3
>    bool
>    select UDELAY_TSC
> select MMX
> Index: src/cpu/via/model_c7/Kconfig
>
> --- src/cpu/via/model_c7/Kconfig	(revision 5158)
> +++ src/cpu/via/model_c7/Kconfig	(working copy)
> @@ -1,3 +1,4 @@
>  config CPU_VIA_C7
>    bool
>    select UDELAY_TSC
> select MMX

Are we sure all CPUs have MMX and SSE options set correctly? Are some
missing?
> Index: src/mainboard/iwill/dk8_htx/Makefile.inc
>
> --- src/mainboard/iwill/dk8_htx/Makefile.inc	(revision 5158)
> +++ src/mainboard/iwill/dk8_htx/Makefile.inc	(working copy)
> @@ -35,12 +35,6 @@
>
> initobj-y += crt0.o
>  # FIXME in $(top)/Makefile

I guess the FIXME comment can also be removed? Or does it refer to some
other lines? If no, please fix it in all files, there are multiple ones
with that comment.
> Index: src/mainboard/intel/eagleheights/Makefile.inc
>
> --- src/mainboard/intel/eagleheights/Makefile.inc	(revision 5158)
> +++ src/mainboard/intel/eagleheights/Makefile.inc	(working copy)
> @@ -10,13 +10,7 @@
>
> initobj-y += crt0.o
>  # FIXME in $(top)/Makefile
> -crt0s := $(src)/cpu/x86/16bit/entry16.inc
> -crt0s += $(src)/cpu/x86/32bit/entry32.inc
> -crt0s += $(src)/cpu/x86/16bit/reset16.inc
> -crt0s += $(src)/arch/i386/lib/id.inc
> # Use Intel Core (not Core 2) code for CAR init, any CPU might be used.
> -crt0s += $(src)/cpu/intel/model_6ex/cache_as_ram.inc

This comment is now in the wrong place.
> +# who else could use this?
> +ifeq ($(CONFIG_BOARD_TYAN_S2735),y)
> +crt0s += $(src)/cpu/x86/car/cache_as_ram.inc
> +endif

Hm, in theory hopefully more Intel boards which use romcc now. Or is the
CAR file not generic enough (i.e. specific to the S2735 or its
CPU/chipset?)

Uwe.

[Patrick Georgi]

Am 25.02.2010 16:27, schrieb Uwe Hermann:
> > Index: src/cpu/via/model_c3/Kconfig
> >
> > --- src/cpu/via/model_c3/Kconfig	(revision 5158)
> > +++ src/cpu/via/model_c3/Kconfig	(working copy)
> > @@ -1,3 +1,4 @@
> >  config CPU_VIA_C3
> >    bool
> >    select UDELAY_TSC
> > select MMX
> > Index: src/cpu/via/model_c7/Kconfig
> >
> > --- src/cpu/via/model_c7/Kconfig	(revision 5158)
> > +++ src/cpu/via/model_c7/Kconfig	(working copy)
> > @@ -1,3 +1,4 @@
> >  config CPU_VIA_C7
> >    bool
> >    select UDELAY_TSC
> > select MMX
>
> Are we sure all CPUs have MMX and SSE options set correctly? Are some
> missing?

At least those that require it for enable_mmx.inc and the like are set
correctly.
> > initobj-y += crt0.o
> >  # FIXME in $(top)/Makefile
>
> I guess the FIXME comment can also be removed? Or does it refer to some
> other lines? If no, please fix it in all files, there are multiple ones
> with that comment.

Will do.
In principle, initobj-y += crt0.o could be moved to
src/arch/i386/Makefile.inc as well. Yes, I intend to make mainboard
Makefile.inc's optional.
> > # Use Intel Core (not Core 2) code for CAR init, any CPU might be used.
> > -crt0s += $(src)/cpu/intel/model_6ex/cache_as_ram.inc
>
> This comment is now in the wrong place.

Thanks for spotting that, will move it.
> > +# who else could use this?
> > +ifeq ($(CONFIG_BOARD_TYAN_S2735),y)
> > +crt0s += $(src)/cpu/x86/car/cache_as_ram.inc
> > +endif
>
> Hm, in theory hopefully more Intel boards which use romcc now. Or is the
> CAR file not generic enough (i.e. specific to the S2735 or its
> CPU/chipset?)

I have no idea which CPU this applies to.

There are also some Via C7 boards that could be migrated to CAR, but I
don't have the hardware.

Patrick

[ron minnich]

On Thu, Feb 25, 2010 at 8:51 AM, Patrick Georgi  wrote:
> Am 25.02.2010 16:27, schrieb Uwe Hermann:
> At least those that require it for enable_mmx.inc and the like are set
> correctly.

be careful here. I had one problem at one point where the CPU was
taking a GPF because enable_sse.inc was included. The issue was that
the code had the CPU touch an SSE register that did not exist -->
BOOM.

Are you sure that you are carefully checking the feature set and not
touching sse.inc if it is not there and/or not assembling that code if
the CPU does not support it? I'm sorry, have not looked at your code
;-( but wanted to warn you. I assume that no sse instructions are
executed unless the feature is known to exist.

But this is another huge improvement that i think we have to just push
our way through to making it work no matter what!

Thanks

ron

[Patrick Georgi]

Am 25.02.2010 17:59, schrieb ron minnich:
> Are you sure that you are carefully checking the feature set and not
> touching sse.inc if it is not there and/or not assembling that code if
> the CPU does not support it? I'm sorry, have not looked at your code
> ;-( but wanted to warn you. I assume that no sse instructions are
> executed unless the feature is known to exist.

We use CONFIG_SSE for that. no CONFIG_SSE, no enable_sse.inc. If we have
some board were non-SSE CPUs can be used but CONFIG_SSE gets its way in
somehow, we can fix that.

Good enough?

Patrick

[ron minnich]

On Thu, Feb 25, 2010 at 9:00 AM, Patrick Georgi  wrote:
> Am 25.02.2010 17:59, schrieb ron minnich:
> > Are you sure that you are carefully checking the feature set and not
> > touching sse.inc if it is not there and/or not assembling that code if
> > the CPU does not support it? I'm sorry, have not looked at your code
> > ;-( but wanted to warn you. I assume that no sse instructions are
> > executed unless the feature is known to exist.
>
> We use CONFIG_SSE for that. no CONFIG_SSE, no enable_sse.inc. If we have
> some board were non-SSE CPUs can be used but CONFIG_SSE gets its way in
> somehow, we can fix that.
>
> Good enough?

Wonderful. Just wanted to double check.

Ah, you guys do outstanding work. This tree just keeps getting better
by the day.

thanks

ron

[Joseph Smith]

On Thu, 25 Feb 2010 09:07:41 -0800, ron minnich  wrote:
> On Thu, Feb 25, 2010 at 9:00 AM, Patrick Georgi 
> wrote:
> > Am 25.02.2010 17:59, schrieb ron minnich:
> > > Are you sure that you are carefully checking the feature set and not
> > > touching sse.inc if it is not there and/or not assembling that code if
> > > the CPU does not support it? I'm sorry, have not looked at your code
> > > ;-( but wanted to warn you. I assume that no sse instructions are
> > > executed unless the feature is known to exist.
> >
> > We use CONFIG_SSE for that. no CONFIG_SSE, no enable_sse.inc. If we have
> > some board were non-SSE CPUs can be used but CONFIG_SSE gets its way in
> > somehow, we can fix that.
> >
> > Good enough?

How many CPU's do not support SSE, There can't be that many....

[ron minnich]

On Thu, Feb 25, 2010 at 10:31 AM, Joseph Smith  wrote:
> How many CPU's do not support SSE, There can't be that many....

doesn't matter if you have one of the ones that doesn't :-)

ron