[coreboot] Bug in your code
wangfei.jimei at gmail.com
Fri Apr 18 18:08:50 CEST 2014
The first option to change makemagic('B', 'S', 'S', ' ') of
PAYLOAD_SEGMENT_BSS in util/cbfstool/cbfs.h would be a better solution.
On Fri, Apr 18, 2014 at 4:52 PM, Aaron Durbin <adurbin at chromium.org> wrote:
> On Fri, Apr 18, 2014 at 9:49 AM, ron minnich <rminnich at gmail.com> wrote:
> > Can somebody give me a sanity check? I can't see the error with the
> > I won't say too much here -- just take a look. I'm not convinced the
> > code is wrong.
> #define PAYLOAD_SEGMENT_CODE 0x45444F43
> #define PAYLOAD_SEGMENT_DATA 0x41544144
> #define PAYLOAD_SEGMENT_BSS 0x20535342
> #define PAYLOAD_SEGMENT_PARAMS 0x41524150
> #define PAYLOAD_SEGMENT_ENTRY 0x52544E45
> does no endianness correction. The segment->type is taken verbatim
> what is in cbfs.
> #define makemagic(b3, b2, b1, b0)\
> (((b3)<<24) | ((b2) << 16) | ((b1) << 8) | (b0))
> #define PAYLOAD_SEGMENT_CODE makemagic('C', 'O', 'D', 'E')
> #define PAYLOAD_SEGMENT_DATA makemagic('D', 'A', 'T', 'A')
> #define PAYLOAD_SEGMENT_BSS makemagic(' ', 'B', 'S', 'S')
> #define PAYLOAD_SEGMENT_PARAMS makemagic('P', 'A', 'R', 'A')
> #define PAYLOAD_SEGMENT_ENTRY makemagic('E', 'N', 'T', 'R')
> I would see the following result for all host cbfstool compilations:
> But we xdr to big endian... so one needs to make the
> src/include/cbfs_core.h be architecture endian aware which it isn't.
> For the x86 case:
> makemagic('B', 'S', 'S', ' ') would make the xdr.be_put32 yield
> 0x20535342 when a little endian machine read the 32-bit word from a
> big endian serialization.
> The real question is what are the payload magic numbers and what
> should the encoding be? If they are serialized as big endian then the
> runtime needs an equivalent makemagic and be2host() to do the proper
> comparison. Having the runtime code be compiled as a straight up
> define is not correct for every machine because the underlying
> endinanness could be different.
> I think people are getting hung up on strings when the code is dealing
> w/ 32-bit values.
> 1. change makemagic('B', 'S', 'S', ' ') for PAYLOAD_SEGMENT_BSS that
> should fix little endian target machines
> 2. fix the runtime in coreboot to match cbfsutil where it sucks out
> the data as big endian and compares against the correct value.
> > thanks
> > ron
> > On Fri, Apr 18, 2014 at 7:39 AM, WANG FEI <wangfei.jimei at gmail.com>
> >> Ronald,
> >> I just noticed a bug in your code, I've added the comment to coreboot
> >> syste, but I'm not farmilar with this system, not sure if it will send
> you a
> >> notice mail automatically, so I just send you a mail to inform you this.
> >> Here is the link of comment,
> >> http://review.coreboot.org/#/c/5098/
> >> -Fei
> > --
> > coreboot mailing list: coreboot at coreboot.org
> > http://www.coreboot.org/mailman/listinfo/coreboot
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the coreboot