The first option to change makemagic('B', 'S', 'S', ' ') of PAYLOAD_SEGMENT_BSS in util/cbfstool/cbfs.h would be a better solution.<br><br>
<div class="gmail_quote">On Fri, Apr 18, 2014 at 4:52 PM, Aaron Durbin <span dir="ltr"><<a href="mailto:adurbin@chromium.org" target="_blank">adurbin@chromium.org</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">
<div>On Fri, Apr 18, 2014 at 9:49 AM, ron minnich <<a href="mailto:rminnich@gmail.com">rminnich@gmail.com</a>> wrote:<br>> Can somebody give me a sanity check? I can't see the error with the macro.<br>> I won't say too much here -- just take a look. I'm not convinced the<br>
> code is wrong.<br><br></div><a href="http://review.coreboot.org/gitweb?p=coreboot.git;a=blob;f=src/include/cbfs_core.h;h=a1d8127de20d997359cb86757dc46345ac14a88c;hb=refs/heads/master" target="_blank">http://review.coreboot.org/gitweb?p=coreboot.git;a=blob;f=src/include/cbfs_core.h;h=a1d8127de20d997359cb86757dc46345ac14a88c;hb=refs/heads/master</a><br>
<br>shows<br><br>#define PAYLOAD_SEGMENT_CODE 0x45444F43<br>#define PAYLOAD_SEGMENT_DATA 0x41544144<br>#define PAYLOAD_SEGMENT_BSS 0x20535342<br>#define PAYLOAD_SEGMENT_PARAMS 0x41524150<br>#define PAYLOAD_SEGMENT_ENTRY 0x52544E45<br>
<br>and<br><a href="http://review.coreboot.org/gitweb?p=coreboot.git;a=blob;f=src/lib/selfboot.c;h=feff03e14385f85f0606e83359ae2b17ca52ea51;hb=refs/heads/master" target="_blank">http://review.coreboot.org/gitweb?p=coreboot.git;a=blob;f=src/lib/selfboot.c;h=feff03e14385f85f0606e83359ae2b17ca52ea51;hb=refs/heads/master</a><br>
<br>does no endianness correction. The segment->type is taken verbatim<br>what is in cbfs.<br><br>#define makemagic(b3, b2, b1, b0)\<br> (((b3)<<24) | ((b2) << 16) | ((b1) << 8) | (b0))<br><br>
#define PAYLOAD_SEGMENT_CODE makemagic('C', 'O', 'D', 'E')<br>#define PAYLOAD_SEGMENT_DATA makemagic('D', 'A', 'T', 'A')<br>#define PAYLOAD_SEGMENT_BSS makemagic(' ', 'B', 'S', 'S')<br>
#define PAYLOAD_SEGMENT_PARAMS makemagic('P', 'A', 'R', 'A')<br>#define PAYLOAD_SEGMENT_ENTRY makemagic('E', 'N', 'T', 'R')<br><br>I would see the following result for all host cbfstool compilations:<br>
<br>0x434F4445<br>0x44415441<br>0x20425353<br>0x50415241<br>0x454E5452<br><br>But we xdr to big endian... so one needs to make the<br>src/include/cbfs_core.h be architecture endian aware which it isn't.<br><br>For the x86 case:<br>
<br>makemagic('B', 'S', 'S', ' ') would make the xdr.be_put32 yield<br>0x20535342 when a little endian machine read the 32-bit word from a<br>big endian serialization.<br><br>The real question is what are the payload magic numbers and what<br>
should the encoding be? If they are serialized as big endian then the<br>runtime needs an equivalent makemagic and be2host() to do the proper<br>comparison. Having the runtime code be compiled as a straight up<br>define is not correct for every machine because the underlying<br>
endinanness could be different.<br><br>I think people are getting hung up on strings when the code is dealing<br>w/ 32-bit values.<br><br>1. change makemagic('B', 'S', 'S', ' ') for PAYLOAD_SEGMENT_BSS that<br>
should fix little endian target machines<br>2. fix the runtime in coreboot to match cbfsutil where it sucks out<br>the data as big endian and compares against the correct value.<br>
<div class="HOEnZb">
<div class="h5"><br><br><br>><br>> thanks<br>><br>> ron<br>><br>> On Fri, Apr 18, 2014 at 7:39 AM, WANG FEI <<a href="mailto:wangfei.jimei@gmail.com">wangfei.jimei@gmail.com</a>> wrote:<br>>> Ronald,<br>
>><br>>> I just noticed a bug in your code, I've added the comment to coreboot review<br>>> syste, but I'm not farmilar with this system, not sure if it will send you a<br>>> notice mail automatically, so I just send you a mail to inform you this.<br>
>><br>>> Here is the link of comment,<br>>> <a href="http://review.coreboot.org/#/c/5098/" target="_blank">http://review.coreboot.org/#/c/5098/</a><br>>><br>>> -Fei<br>><br></div></div><span class="HOEnZb"><font color="#888888">> --<br>
> coreboot mailing list: <a href="mailto:coreboot@coreboot.org">coreboot@coreboot.org</a><br>> <a href="http://www.coreboot.org/mailman/listinfo/coreboot" target="_blank">http://www.coreboot.org/mailman/listinfo/coreboot</a><br>
</font></span></blockquote></div><br>