build service results for r5841

[repository service]

Dear coreboot readers!

This is the automatic build system of coreboot.

The developer "oxygene" checked in revision 5841 to
the coreboot repository. This caused the following 
changes:

Change Log:
Make globals in romstage break the build, so we don't have to
wonder why variables in .data or .bss (both somewhere in ROM space)
are wrong.

Signed-off-by: Patrick Georgi 
Acked-by: Stefan Reinauer

Build Log:
Compilation of amd:serengeti_cheetah_fam10 has been broken
See the error log at http://qa.coreboot.org/log_buildbrd.php?revision=5841&device=serengeti_c...
Compilation of hp:dl165_g6_fam10 has been broken
See the error log at http://qa.coreboot.org/log_buildbrd.php?revision=5841&device=dl165_g6_fa...
Compilation of supermicro:h8dmr_fam10 has been broken
See the error log at http://qa.coreboot.org/log_buildbrd.php?revision=5841&device=h8dmr_fam10...
Compilation of supermicro:h8qme_fam10 has been broken
See the error log at http://qa.coreboot.org/log_buildbrd.php?revision=5841&device=h8qme_fam10...
Compilation of tyan:s2912_fam10 has been broken
See the error log at http://qa.coreboot.org/log_buildbrd.php?revision=5841&device=s2912_fam10...

If something broke during this checkin please be a pain 
in oxygene's neck until the issue is fixed.

If this issue is not fixed within 24h the revision should 
be backed out.

Best regards,
     coreboot automatic build system

[Patrick Georgi]

Am 25.09.2010 16:40, schrieb repository service:
> Change Log:
> Make globals in romstage break the build, so we don't have to
> wonder why variables in .data or .bss (both somewhere in ROM space)
> are wrong.

Instant proof that this test is useful :-)

Digging through these boards, the culprit is
static u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE,
CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; in
src/northbridge/amd/amdht/ht_wrapper.c's
static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, u8 **List).

This is put in .data, as it might be modified by some other user of the
pointer. As far as I can see, it isn't, so that choice is harmless.

I made it const, as well as its users, and the tree compiles, but
feedback on the approach of the patch and/or testing is _very_ welcome.

Signed-off-by: Patrick Georgi

[Stefan Reinauer]

On 9/25/10 5:19 PM, Patrick Georgi wrote:
> Am 25.09.2010 16:40, schrieb repository service:
> > Change Log:
> > Make globals in romstage break the build, so we don't have to
> > wonder why variables in .data or .bss (both somewhere in ROM space)
> > are wrong.
>
> Instant proof that this test is useful :-)

I was wondering if that would happen. :-) Very nice!
> Digging through these boards, the culprit is
> static u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE,
> CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; in
> src/northbridge/amd/amdht/ht_wrapper.c's
> static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, u8 **List).
>
> This is put in .data, as it might be modified by some other user of the
> pointer. As far as I can see, it isn't, so that choice is harmless.
>
> I made it const, as well as its users, and the tree compiles, but
> feedback on the approach of the patch and/or testing is _very_ welcome.
>
> Signed-off-by: Patrick Georgi

Acked-by: Stefan Reinauer

[Marc Jones]

On Sat, Sep 25, 2010 at 11:15 AM, Stefan Reinauer
 wrote:
> On 9/25/10 5:19 PM, Patrick Georgi wrote:
> > Am 25.09.2010 16:40, schrieb repository service:
> > > Change Log:
> > > Make globals in romstage break the build, so we don't have to
> > > wonder why variables in .data or .bss (both somewhere in ROM space)
> > > are wrong.
> >
> > Instant proof that this test is useful :-)
>
> I was wondering if that would happen. :-) Very nice!
> > Digging through these boards, the culprit is
> > static u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE,
> > CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; in
> > src/northbridge/amd/amdht/ht_wrapper.c's
> > static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, u8 **List).
> >
> > This is put in .data, as it might be modified by some other user of the
> > pointer. As far as I can see, it isn't, so that choice is harmless.
> >
> > I made it const, as well as its users, and the tree compiles, but
> > feedback on the approach of the patch and/or testing is _very_ welcome.
> >
> > Signed-off-by: Patrick Georgi
>
> Acked-by: Stefan Reinauer

That should be fine and shouldn't break anything. It is a buildtime setting.
Marc