[coreboot] Bug in your code
adurbin at chromium.org
Fri Apr 18 17:52:52 CEST 2014
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 macro.
> 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.
> On Fri, Apr 18, 2014 at 7:39 AM, WANG FEI <wangfei.jimei at gmail.com> wrote:
>> I just noticed a bug in your code, I've added the comment to coreboot review
>> 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,
> coreboot mailing list: coreboot at coreboot.org
More information about the coreboot