Re: [SeaBIOS] [PATCH 1/3] Add multiboot header to bios.bin.raw

[Kevin O'Connor]

On Mon, May 18, 2015 at 08:05:17PM +0000, Vladimir 'phcoder' Serbinenko wrote:
>
> diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> index dd770fe..04e7efa 100755
> --- a/scripts/layoutrom.py
> +++ b/scripts/layoutrom.py
> @@ -646,9 +646,10 @@ def main():
>          entrysym = symbols['16'].get('entry_csm')
>      else:
>          entrysym = symbols['16'].get('reset_vector')
> mb_section = symbols['32flat'].get('mb_head').section
> anchorsections = [entrysym.section] + [
>     section for section in allsections
>    if section.name.startswith('.fixedaddr.')]
>    if section.name.startswith('.fixedaddr.')] + [mb_section]
> keepsections = findReachable(anchorsections, checkKeep, symbols)
>    sections = [section for section in allsections if section in keepsections]
>
> diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c
> index 8fd8449..6610b3a 100644
> --- a/src/fw/coreboot.c
> +++ b/src/fw/coreboot.c
> @@ -551,3 +551,19 @@ cbfs_payload_setup(void)
>          boot_add_cbfs(cfile->fhdr, desc, bootprio_find_named_rom(filename, 0));
>      }
>  }
>
> +void VARLOW
> +mb_head(u32 mbptr)
> +{
> asm volatile(
> ".align 4\n"
> "mb_header:\n"
> ".long 0x1BADB002\n"
> ".long 0x00010000\n"
> ".long -(0x1BADB002+0x00010000)\n"
> ".long mb_header\n"
> ".long _reloc_abs_start\n"
> ".long 0\n"
> ".long 0\n"
> ".long entry_elf\n");
>
> +}

Can this header really exist anywhere in the rom file?  Is 0x1BADB002
a required signature?

If this is just a header, it should not be placed into a function, and
it shouldn't be marked with VARLOW.  Instead it should be placed in
in a top-level assembler (see ASM macro) or in romlayout.S.

-Kevin

[PATCH 1/3] Add multiboot header to bios.bin.raw

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 19.05.2015 04:10, Kevin O'Connor wrote:
> On Mon, May 18, 2015 at 08:05:17PM +0000, Vladimir 'phcoder' Serbinenko wrote:
> >
> > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > index dd770fe..04e7efa 100755
> > --- a/scripts/layoutrom.py
> > +++ b/scripts/layoutrom.py
> > @@ -646,9 +646,10 @@ def main():
> >          entrysym = symbols['16'].get('entry_csm')
> >      else:
> >          entrysym = symbols['16'].get('reset_vector')
> > mb_section = symbols['32flat'].get('mb_head').section
> > anchorsections = [entrysym.section] + [
> >     section for section in allsections
> >    if section.name.startswith('.fixedaddr.')]
> >    if section.name.startswith('.fixedaddr.')] + [mb_section]
> > keepsections = findReachable(anchorsections, checkKeep, symbols)
> >    sections = [section for section in allsections if section in keepsections]
> >
> > diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c
> > index 8fd8449..6610b3a 100644
> > --- a/src/fw/coreboot.c
> > +++ b/src/fw/coreboot.c
> > @@ -551,3 +551,19 @@ cbfs_payload_setup(void)
> >          boot_add_cbfs(cfile->fhdr, desc, bootprio_find_named_rom(filename, 0));
> >      }
> >  }
> >
> > +void VARLOW
> > +mb_head(u32 mbptr)
> > +{
> > asm volatile(
> > ".align 4\n"
> > "mb_header:\n"
> > ".long 0x1BADB002\n"
> > ".long 0x00010000\n"
> > ".long -(0x1BADB002+0x00010000)\n"
> > ".long mb_header\n"
> > ".long _reloc_abs_start\n"
> > ".long 0\n"
> > ".long 0\n"
> > ".long entry_elf\n");
> >
> > +}
>
> Can this header really exist anywhere in the rom file?

It needs to be in the first 8K, aligned to 4 bytes.
> Is 0x1BADB002
> a required signature?

Yes
> If this is just a header, it should not be placed into a function, and
> it shouldn't be marked with VARLOW.

VARLOW is to get it in first 8K. 32-bit function is for easy access to
_reloc_abs_start which is address of the first byte that needs to be loaded.
> Instead it should be placed in
> in a top-level assembler (see ASM macro) or in romlayout.S.
>
> -Kevin

[PATCH 1/3] Add multiboot header to bios.bin.raw

[Kevin O'Connor]

On Tue, May 19, 2015 at 08:14:56AM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 19.05.2015 04:10, Kevin O'Connor wrote:
> > On Mon, May 18, 2015 at 08:05:17PM +0000, Vladimir 'phcoder' Serbinenko wrote:
> > >
> > > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > > index dd770fe..04e7efa 100755
> > > --- a/scripts/layoutrom.py
> > > +++ b/scripts/layoutrom.py
> > > @@ -646,9 +646,10 @@ def main():
> > >          entrysym = symbols['16'].get('entry_csm')
> > >      else:
> > >          entrysym = symbols['16'].get('reset_vector')
> > > mb_section = symbols['32flat'].get('mb_head').section
> > > anchorsections = [entrysym.section] + [
> > >     section for section in allsections
> > >    if section.name.startswith('.fixedaddr.')]
> > >    if section.name.startswith('.fixedaddr.')] + [mb_section]
> > > keepsections = findReachable(anchorsections, checkKeep, symbols)
> > >    sections = [section for section in allsections if section in keepsections]
> > >
> > > diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c
> > > index 8fd8449..6610b3a 100644
> > > --- a/src/fw/coreboot.c
> > > +++ b/src/fw/coreboot.c
> > > @@ -551,3 +551,19 @@ cbfs_payload_setup(void)
> > >          boot_add_cbfs(cfile->fhdr, desc, bootprio_find_named_rom(filename, 0));
> > >      }
> > >  }
> > >
> > > +void VARLOW
> > > +mb_head(u32 mbptr)
> > > +{
> > > asm volatile(
> > > ".align 4\n"
> > > "mb_header:\n"
> > > ".long 0x1BADB002\n"
> > > ".long 0x00010000\n"
> > > ".long -(0x1BADB002+0x00010000)\n"
> > > ".long mb_header\n"
> > > ".long _reloc_abs_start\n"
> > > ".long 0\n"
> > > ".long 0\n"
> > > ".long entry_elf\n");
> > >
> > > +}
> >
> > Can this header really exist anywhere in the rom file?
>
> It needs to be in the first 8K, aligned to 4 bytes.
> > Is 0x1BADB002
> > a required signature?
>
> Yes
> > If this is just a header, it should not be placed into a function, and
> > it shouldn't be marked with VARLOW.
>
> VARLOW is to get it in first 8K. 32-bit function is for easy access to
> _reloc_abs_start which is address of the first byte that needs to be loaded.

The VARLOW macro does other things in addition to location in the
final binary - namely it treats the space as a variable and reserves
space in low memory.  If there is a specific location the table needs
to be, it really should be placed in that location by
layoutrom.py:doLayout().

The _reloc_abs_start definition isn't available if
CONFIG_RELOCATE_INIT is disabled - I think the symbol you want is
code32flat_start.  Both should be available in a top level ASM32FLAT()
and in romlayout.S.

Also, I just noticed in your comments that the bios.bin.raw file was
used in your example.  That's not an exported file today (the build
could fail while that file still exists).  Does multi-boot require a
raw image to deploy (that is, a single linear area of memory that is
copied to a final location)?

-Kevin

[PATCH 1/3] Add multiboot header to bios.bin.raw

[Vladimir 'phcoder' Serbinenko]

Le 19 mai 2015 15:51, "Kevin O'Connor"  a écrit :
> On Tue, May 19, 2015 at 08:14:56AM +0200, Vladimir 'φ-coder/phcoder'

Serbinenko wrote:
> > On 19.05.2015 04:10, Kevin O'Connor wrote:
> > > On Mon, May 18, 2015 at 08:05:17PM +0000, Vladimir 'phcoder'

Serbinenko wrote:
> > > >
> > > > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > > > index dd770fe..04e7efa 100755
> > > > --- a/scripts/layoutrom.py
> > > > +++ b/scripts/layoutrom.py
> > > > @@ -646,9 +646,10 @@ def main():
> > > >          entrysym = symbols['16'].get('entry_csm')
> > > >      else:
> > > >          entrysym = symbols['16'].get('reset_vector')
> > > > mb_section = symbols['32flat'].get('mb_head').section
> > > > anchorsections = [entrysym.section] + [
> > > >     section for section in allsections
> > > >    if section.name.startswith('.fixedaddr.')]
> > > >    if section.name.startswith('.fixedaddr.')] + [mb_section]
> > > > keepsections = findReachable(anchorsections, checkKeep, symbols)
> > > >    sections = [section for section in allsections if section in

keepsections]
> > > > diff --git a/src/fw/coreboot.c b/src/fw/coreboot.c
> > > > index 8fd8449..6610b3a 100644
> > > > --- a/src/fw/coreboot.c
> > > > +++ b/src/fw/coreboot.c
> > > > @@ -551,3 +551,19 @@ cbfs_payload_setup(void)
> > > >          boot_add_cbfs(cfile->fhdr, desc,

bootprio_find_named_rom(filename, 0));
> > > >  }
> > > >
> > > > }
> > > >
> > > > +void VARLOW
> > > > +mb_head(u32 mbptr)
> > > > +{
> > > > asm volatile(
> > > > ".align 4\n"
> > > > "mb_header:\n"
> > > > ".long 0x1BADB002\n"
> > > > ".long 0x00010000\n"
> > > > ".long -(0x1BADB002+0x00010000)\n"
> > > > ".long mb_header\n"
> > > > ".long _reloc_abs_start\n"
> > > > ".long 0\n"
> > > > ".long 0\n"
> > > > ".long entry_elf\n");
> > > >
> > > > +}
> > >
> > > Can this header really exist anywhere in the rom file?
> >
> > It needs to be in the first 8K, aligned to 4 bytes.
> > > Is 0x1BADB002
> > > a required signature?
> >
> > Yes
> > > If this is just a header, it should not be placed into a function, and
> > > it shouldn't be marked with VARLOW.
> >
> > VARLOW is to get it in first 8K. 32-bit function is for easy access to
> > _reloc_abs_start which is address of the first byte that needs to be

loaded.
> The VARLOW macro does other things in addition to location in the
> final binary - namely it treats the space as a variable and reserves
> space in low memory.  If there is a specific location the table needs
> to be, it really should be placed in that location by
> layoutrom.py:doLayout().

Ok
> The _reloc_abs_start definition isn't available if
> CONFIG_RELOCATE_INIT is disabled - I think the symbol you want is
> code32flat_start.  Both should be available in a top level ASM32FLAT()
> and in romlayout.S.

code32flat-start misses all the relocation entries
> Also, I just noticed in your comments that the bios.bin.raw file was
> used in your example.  That's not an exported file today (the build
> could fail while that file still exists).  Does multi-boot require a
> raw image to deploy (that is, a single linear area of memory that is
> copied to a final location)?

Multiboot can either load ELF or a linear file. In either case the header
needs to be in first 8K of resulting file. File produced by SeaBIOS
contains few hundred kilobytes of zeros. I didn't manage to insert 12 bytes
header in first 8K because of it. I'll give it another try
> -Kevin