[coreboot] PCI register read/mod/write code

Uwe Hermann uwe at hermann-uwe.de
Tue Oct 5 03:08:00 CEST 2010

On Mon, Oct 04, 2010 at 02:02:12PM -0600, Myles Watson wrote:
> On Sun, Oct 3, 2010 at 5:10 PM, Peter Stuge <peter at stuge.se> wrote:
> > Rudolf just found a bug in the sb700 code:
> >
> > u32 dword;
> > ..
> > dword = pci_read_config8(dev, 0x64);
> > dword |= 1 << 10;
> > pci_write_config8(dev, 0x64, dword);
> >
> >
> > And I'm ranting now, because a pci_set8() macro/function could have
> > found this bug at compile time, and because I don't like these
> > constructs. (Compiler warnings would also have indicated a problem.
> > They're currently disabled for this code.)
> How and where are compiler warnings disabled for this code?

That was a misunderstanding I think, I mentioed (on IRC) warnings being
disabled for some parts of the coreboot code (tiny bootblock and others),
but that was not related to this specific code (sb700_sm.c).

The file gets compiled with:

gcc -m32 -Wa,--divide -fno-stack-protector -Wl,--build-id=none   -MMD
-Isrc -Isrc/include -Ibuild -Isrc/arch/i386/include
-Isrc/devices/oprom/include -include /home/uwe/v4_bar/build/config.h -Os
-nostdinc -pipe -g -nostdlib -Wall -Wundef -Wstrict-prototypes
-Wmissing-prototypes -Wwrite-strings -Wredundant-decls -Wno-trigraphs
-Wstrict-aliasing -Wshadow -Werror -fno-common -ffreestanding
-fno-builtin -fomit-frame-pointer -c -o

However, that doesn't expose the bug in this case. What would work is to
add '-Wconversion', that finds the bug:

src/southbridge/amd/sb700/sb700_sm.c:88: error: conversion to ‘uint8_t’ from ‘u32’ may alter its value

However, it also spews 30 or more of these lines for every single file
we have in coreboot, so it's not really a practical option to enable
permanently (or at least not at this point in time).

http://hermann-uwe.de     | http://sigrok.org
http://randomprojects.org | http://unmaintained-free-software.org

More information about the coreboot mailing list