[coreboot] fmaptool: fmap_config.h: Base offset missing

[Rolf Evers-Fischer]

On Mon, 20 Jun 2016, Julius Werner wrote:

> Yes, this definitely looks like a bug... thanks a lot for reporting
> it. The fix is simple and I've uploaded it here:
> https://review.coreboot.org/15273
> 

Thank you for this commit. I've just tested it - and now it works 
correctly.

> I'm curious why this hasn't been noticed anywhere before, though...
> (on some boards this just happens to work out because the parent
> section of COREBOOT starts at offset 0 anyway, but on others as you
> found out it doesn't).
> 

I don't know either. Let's hope that this commit doesn't break any of the 
other boards. But in my opinion the behaviour is now correct.

> On Thu, Jun 16, 2016 at 6:55 AM, Rolf Evers-Fischer
> <embedded24 at evers-fischer.de> wrote:
> > Hello,
> > my ApolloLake board was not able to find the "romstage" in the SPI memory,
> > because it searched in the wrong part of the memory.
> >
> > ___FMAP__COREBOOT_BASE was set to the relative offset within the parent
> > section, but rdev_readat needs the *absolute* address: There was a
> > mismatch.
> > However, I was able to fix this problem locally by adding an additional
> > fixed offset to fmap_top, like
> >
> > diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
> > index aa652c2..f36a499 100644
> > --- a/src/lib/cbfs.c
> > +++ b/src/lib/cbfs.c
> > @@ -230,7 +230,7 @@ static int cbfs_master_header_props(struct cbfs_props *props
> >         if (bdev == NULL)
> >                 return -1;
> >
> > -       size_t fmap_top = ___FMAP__COREBOOT_BASE + ___FMAP__COREBOOT_SIZE;
> > +       size_t fmap_top = ___FMAP__COREBOOT_BASE + ___FMAP__COREBOOT_SIZE + 0x300000;
> >
> >         /* Find location of header using signed 32-bit offset from
> >          * end of CBFS region. */
> >
> >
> > But imho this shouldn't be a permanent solution!
> >
> > Therefore I suggest to modify the 'fmaptool' in such way that it outputs
> > the absolute address for (at least) ___FMAP__COREBOOT_BASE, because the
> > 'rdev_readat()' function  of the 'boot_device_ro()' needs it in this form.
> >
> > What is your opinion?
> >
> > Kind regards,
> >  Rolf
> >
> > --
> > coreboot mailing list: coreboot at coreboot.org
> > https://www.coreboot.org/mailman/listinfo/coreboot
>