[v3] r1129 makes bootblock too big

[Ronald Hoogenboom]

Hi,

After updating to r1129, adds call to allow_all_aps_stop to initram.c,
bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)

Ronald.

[Myles Watson]

On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> Hi,
>
> After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)

Did we ever decide on how big was too big for the bootblock?  I
thought it was clear that we needed it to be larger for k8.  Should we
make it a parameter?  I realize it's hardcoded in lar right now.

Thanks,
Myles

[ron minnich]

On Thu, Feb 12, 2009 at 1:28 PM, Myles Watson  wrote:
> On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> > Hi,
> >
> > After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> > bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)
>
> Did we ever decide on how big was too big for the bootblock?  I
> thought it was clear that we needed it to be larger for k8.  Should we
> make it a parameter?  I realize it's hardcoded in lar right now.

it's really hardcoded in a few places.

ron

[Myles Watson]

On Thu, Feb 12, 2009 at 2:29 PM, ron minnich  wrote:
> On Thu, Feb 12, 2009 at 1:28 PM, Myles Watson  wrote:
> > On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> > > Hi,
> > >
> > > After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> > > bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)
> >
> > Did we ever decide on how big was too big for the bootblock?  I
> > thought it was clear that we needed it to be larger for k8.  Should we
> > make it a parameter?  I realize it's hardcoded in lar right now.
>
> it's really hardcoded in a few places.

So he should just take out a few printk statements to make it fit again?

Thanks,
Myles

[Ronald Hoogenboom]

On Thu, 2009-02-12 at 14:44 -0700, Myles Watson wrote:
> On Thu, Feb 12, 2009 at 2:29 PM, ron minnich  wrote:
> > On Thu, Feb 12, 2009 at 1:28 PM, Myles Watson  wrote:
> > > On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> > > > Hi,
> > > >
> > > > After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> > > > bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)
> > >
> > > Did we ever decide on how big was too big for the bootblock?  I
> > > thought it was clear that we needed it to be larger for k8.  Should we
> > > make it a parameter?  I realize it's hardcoded in lar right now.
> >
> > it's really hardcoded in a few places.
>
> So he should just take out a few printk statements to make it fit again?
>
> Thanks,
> Myles

Sorry, its not the addition to initram.c, but the extra stuff in
arch/x86/amd/stage0.S that makes it too big. No printk in there...

Ronald.

[Ronald Hoogenboom]

On Thu, 2009-02-12 at 23:16 +0100, Ronald Hoogenboom wrote:
> On Thu, 2009-02-12 at 14:44 -0700, Myles Watson wrote:
> > On Thu, Feb 12, 2009 at 2:29 PM, ron minnich  wrote:
> > > On Thu, Feb 12, 2009 at 1:28 PM, Myles Watson  wrote:
> > > > On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> > > > > Hi,
> > > > >
> > > > > After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> > > > > bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)
> > > >
> > > > Did we ever decide on how big was too big for the bootblock?  I
> > > > thought it was clear that we needed it to be larger for k8.  Should we
> > > > make it a parameter?  I realize it's hardcoded in lar right now.
> > >
> > > it's really hardcoded in a few places.
> >
> > So he should just take out a few printk statements to make it fit again?
> >
> > Thanks,
> > Myles
>
> Sorry, its not the addition to initram.c, but the extra stuff in
> arch/x86/amd/stage0.S that makes it too big. No printk in there...
>
> Ronald.

r1128:
build/arch/x86/amd/stage0.o:     file format elf32-i386

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         000001e8  00000000  00000000  00000034  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE

r1129:
build/arch/x86/amd/stage0.o:     file format elf32-i386

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         000001fc  00000000  00000000  00000034  2**2
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE

---- There was not much room left ----

Ronald.

[Myles Watson]

On Thu, Feb 12, 2009 at 3:16 PM, Ronald Hoogenboom  wrote:
> On Thu, 2009-02-12 at 14:44 -0700, Myles Watson wrote:
> > On Thu, Feb 12, 2009 at 2:29 PM, ron minnich  wrote:
> > > On Thu, Feb 12, 2009 at 1:28 PM, Myles Watson  wrote:
> > > > On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> > > > > Hi,
> > > > >
> > > > > After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> > > > > bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)
> > > >
> > > > Did we ever decide on how big was too big for the bootblock?  I
> > > > thought it was clear that we needed it to be larger for k8.  Should we
> > > > make it a parameter?  I realize it's hardcoded in lar right now.
> > >
> > > it's really hardcoded in a few places.
> >
> > So he should just take out a few printk statements to make it fit again?
> >
> > Thanks,
> > Myles
>
> Sorry, its not the addition to initram.c, but the extra stuff in
> arch/x86/amd/stage0.S that makes it too big. No printk in there...

You're right :)  I spoke too quickly.

Thanks,
Myles

[Ronald Hoogenboom]

On Thu, 2009-02-12 at 15:29 -0700, Myles Watson wrote:
> On Thu, Feb 12, 2009 at 3:16 PM, Ronald Hoogenboom  wrote:
> > On Thu, 2009-02-12 at 14:44 -0700, Myles Watson wrote:
> > > On Thu, Feb 12, 2009 at 2:29 PM, ron minnich  wrote:
> > > > On Thu, Feb 12, 2009 at 1:28 PM, Myles Watson  wrote:
> > > > > On Thu, Feb 12, 2009 at 2:19 PM, Ronald Hoogenboom  wrote:
> > > > > > Hi,
> > > > > >
> > > > > > After updating to r1129, adds call to allow_all_aps_stop to initram.c,
> > > > > > bootblock is too big (16 bytes) and build fails. (gigabyte m57sli)
> > > > >
> > > > > Did we ever decide on how big was too big for the bootblock?  I
> > > > > thought it was clear that we needed it to be larger for k8.  Should we
> > > > > make it a parameter?  I realize it's hardcoded in lar right now.
> > > >
> > > > it's really hardcoded in a few places.
> > >
> > > So he should just take out a few printk statements to make it fit again?
> > >
> > > Thanks,
> > > Myles
> >
> > Sorry, its not the addition to initram.c, but the extra stuff in
> > arch/x86/amd/stage0.S that makes it too big. No printk in there...
>
> You're right :)  I spoke too quickly.
>
> Thanks,
> Myles

[coreboot-v3]$ objdump -t build/stage0-prefixed.o | awk '/.c$/{print
$6}' | sort -u
stage0_clog2.c
stage0_console.c
stage0_delay.c
stage0_dualcore_id.c
stage0_get_nodes.c
stage0_incoherent_ht_chain.c
stage0_lar.c
stage0_libstage1.c
stage0_lzma.c
stage0_mc146818rtc.c
stage0_mem.c
stage0_option_table.c
stage0_pci_ops_conf1.c
stage0_pnp_raw.c
stage0_post_code.c
stage0_resourcemap.c
stage0_serial.c
stage0_stage1.c
stage0_stage1_enable_rom.c
stage0_stage1_mtrr.c
stage0_stage1_smbus.c
stage0_string.c
stage0_uart8250.c
stage0_udelay_io.c
stage0_vsprintf.c
stage0_vtxprintf.c

Is ALL this stuff actually needed in stage0???

[ron minnich]

On Thu, Feb 12, 2009 at 2:49 PM, Ronald Hoogenboom  wrote:
> [coreboot-v3]$ objdump -t build/stage0-prefixed.o | awk '/.c$/{print
> $6}' | sort -u
> stage0_clog2.c
> stage0_console.c
> stage0_delay.c

almost certainly.
> stage0_dualcore_id.c
> stage0_get_nodes.c
> stage0_incoherent_ht_chain.c

probably.
> stage0_lar.c
> stage0_libstage1.c
> stage0_lzma.c
> stage0_mc146818rtc.c
> stage0_mem.c
> stage0_option_table.c
> stage0_pci_ops_conf1.c

yes
> stage0_pnp_raw.c
> stage0_post_code.c
> stage0_resourcemap.c
> stage0_serial.c

I think so.
> stage0_stage1.c
> stage0_stage1_enable_rom.c
> stage0_stage1_mtrr.c
> stage0_stage1_smbus.c
> stage0_string.c
> stage0_uart8250.c
> stage0_udelay_io.c
> stage0_vsprintf.c
> stage0_vtxprintf.c
>
> Is ALL this stuff actually needed in stage0???

You can look some more, but this stuff may be hard to remove. I tried.

ron

[Carl-Daniel Hailfinger]

On 13.02.2009 01:27, ron minnich wrote:
> On Thu, Feb 12, 2009 at 2:49 PM, Ronald Hoogenboom  wrote:
> > [coreboot-v3]$ objdump -t build/stage0-prefixed.o | awk '/.c$/{print
> > $6}' | sort -u
> > stage0_clog2.c
> > stage0_console.c
> > stage0_delay.c
>
> almost certainly.
> > stage0_dualcore_id.c
> > stage0_get_nodes.c
> > stage0_incoherent_ht_chain.c
>
> probably.
> > stage0_lar.c
> > stage0_libstage1.c
> > stage0_lzma.c
> > stage0_mc146818rtc.c
> > stage0_mem.c
> > stage0_option_table.c
> > stage0_pci_ops_conf1.c
>
> yes
> > stage0_pnp_raw.c
> > stage0_post_code.c
> > stage0_resourcemap.c
> > stage0_serial.c
>
> I think so.
> > stage0_stage1.c
> > stage0_stage1_enable_rom.c
> > stage0_stage1_mtrr.c
> > stage0_stage1_smbus.c
> > stage0_string.c
> > stage0_uart8250.c
> > stage0_udelay_io.c
> > stage0_vsprintf.c
> > stage0_vtxprintf.c
> >
> > Is ALL this stuff actually needed in stage0???
>
> You can look some more, but this stuff may be hard to remove. I tried.

If you don't need backtraces or similar stuff, you can try to enable
combined compilation. Depending on your compiler and the selected
coreboot features, saving up to 20% executable size may be possible.

(I'm not sure whether all of my combined compilation patches were
merged, so your results may vary.)

Regards,
Carl-Daniel