rm elfboot and ELF from v3 (not lar)

[Myles Watson]

This is another attempt at removing ELF support from v3.  I'll try to
do a better job of explaining and justifying this time.  The patch
should be followed by:
svn rm include/elf_boot.h
svn rm include/elf.h
svn rm include/arch/x86/arch/elf.h
svn rm lib/elfboot.c
svn rm arch/x86/archelfboot.c

Another follow-on would be to fix lar so it never puts an ELF into the
lar without parsing it.

Here are my reasons:
1. It wasn't being used anyway.
           Unless you used lar to load the elf in, the Makefile always
parsed the elf with -e
2. It duplicated code.
           There is no reason for v3 to understand ELF when lar parses it.
3. It saves space.

Here's the last little bit of stage0.init.map

Without the patch:

ffffcc8c T rawpnp_write_config
ffffcca8 T rawpnp_set_logical_device
ffffcccb T rawpnp_set_iobase
ffffccf4 T rawpnp_set_enable
ffffcd19 T rawpnp_enter_ext_func_mode
ffffcd20 T rawpnp_exit_ext_func_mode
ffffd51f t __FUNCTION__.1410
ffffd540 t console_test.1503
ffffd57b T _estage0_1
fffffff0 A _ROMTOP
fffffff0 T _resetjump

With the patch:

ffffc885 T rawpnp_enter_ext_func_mode
ffffc88c T rawpnp_exit_ext_func_mode
ffffce20 t console_test.1503
ffffce5b T _estage0_1
fffffff0 A _ROMTOP
fffffff0 T _resetjump

I'm calculating the difference by subtracting the locations of _estage0_1
fffd57b - fffce5b = 0x720 = 1824 bytes

Not earth-shattering, but significant.

Myles

Signed-off-by: Myles Watson

[Carl-Daniel Hailfinger]

On 15.02.2008 22:33, Myles Watson wrote:
> This is another attempt at removing ELF support from v3.  I'll try to
> do a better job of explaining and justifying this time.  The patch
> should be followed by:
> svn rm include/elf_boot.h
> svn rm include/elf.h
> svn rm include/arch/x86/arch/elf.h
> svn rm lib/elfboot.c
> svn rm arch/x86/archelfboot.c
>
> Another follow-on would be to fix lar so it never puts an ELF into the
> lar without parsing it.
>
> Here are my reasons:
> It wasn't being used anyway.
>         Unless you used lar to load the elf in, the Makefile always
>
> parsed the elf with -e
> 2. It duplicated code.
>            There is no reason for v3 to understand ELF when lar parses it.
> 3. It saves space.
>
> Here's the last little bit of stage0.init.map
>
> Without the patch:
>
> ffffcc8c T rawpnp_write_config
> ffffcca8 T rawpnp_set_logical_device
> ffffcccb T rawpnp_set_iobase
> ffffccf4 T rawpnp_set_enable
> ffffcd19 T rawpnp_enter_ext_func_mode
> ffffcd20 T rawpnp_exit_ext_func_mode
> ffffd51f t __FUNCTION__.1410
> ffffd540 t console_test.1503
> ffffd57b T _estage0_1
> fffffff0 A _ROMTOP
> fffffff0 T _resetjump
>
> With the patch:
>
> ffffc885 T rawpnp_enter_ext_func_mode
> ffffc88c T rawpnp_exit_ext_func_mode
> ffffce20 t console_test.1503
> ffffce5b T _estage0_1
> fffffff0 A _ROMTOP
> fffffff0 T _resetjump
>
> I'm calculating the difference by subtracting the locations of _estage0_1
> fffd57b - fffce5b = 0x720 = 1824 bytes
>
> Not earth-shattering, but significant.
>
> Myles
>
> Signed-off-by: Myles Watson

Acked-by: Carl-Daniel Hailfinger

Please wait for Stefan's comments before committing, though. Making the
code dependent on a Kconfig variable instead of removing it may be a
better first step.

Regards,
Carl-Daniel

[Stefan Reinauer]

* Myles Watson  [080215 22:33]:
> Here are my reasons:
> It wasn't being used anyway.
>         Unless you used lar to load the elf in, the Makefile always
>
> parsed the elf with -e

Yes, using ELF was broken by this badly reviewed patch.

This piece of code is just broken:

# We now parse initram as ELF, so we need PARSEELF enabled
# unconditionally.
ifeq ($(CONFIG_PAYLOAD_PREPARSE_ELF), y)
        PARSEELF = -e
else
        PARSEELF = -e
endif

This is kind of bad decay. It's unclean and hackish.
> It duplicated code.
>         There is no reason for v3 to understand ELF when lar parses it.

There are many reasons. See the list for reasons.

Don't get me wrong. Feel free to fix the config option to disable ELF
loading and feel free to use it in all your builds.

But don't just drop this stuff.
> Signed-off-by: Myles Watson

NACK.

[Carl-Daniel Hailfinger]

On 16.02.2008 15:56, Stefan Reinauer wrote:
> Myles Watson  [080215 22:33]:
> > Here are my reasons:
> > It wasn't being used anyway.
> >         Unless you used lar to load the elf in, the Makefile always
> >
> > parsed the elf with -e
>
> Yes, using ELF was broken by this badly reviewed patch.
>
> This piece of code is just broken:
>
> # We now parse initram as ELF, so we need PARSEELF enabled
> # unconditionally.
> ifeq ($(CONFIG_PAYLOAD_PREPARSE_ELF), y)
>         PARSEELF = -e
> else
>         PARSEELF = -e
> endif
>
> This is kind of bad decay. It's unclean and hackish.

To really fix this, we have to separate payload parsing from
initram+stage2 parsing and use the PARSEELF parameter only for payload
parsing.
> > It duplicated code.
> >         There is no reason for v3 to understand ELF when lar parses it.
>
> There are many reasons. See the list for reasons.
>
> Don't get me wrong. Feel free to fix the config option to disable ELF
> loading and feel free to use it in all your builds.
>
> But don't just drop this stuff.
> > Signed-off-by: Myles Watson
>
> NACK.

Myles, I think going with a config option for now is the best thing you
can do.

Regards,
Carl-Daniel