[coreboot] pci_check_sanity is confusing; do we need VSA to read LX CS5536 config registers? ; how to handle VSA?

Stefan Reinauer stepan at coresystems.de
Sat Jan 19 18:53:48 CET 2008

ron minnich wrote:
> pci_sanity_check is confusing. It looks like this:
> static int pci_sanity_check(const struct pci_bus_operations *o)
> {
>         u16 class, vendor;
>         unsigned bus;
>         int devfn;
>         struct bus pbus; /* Dummy device */
> #define PCI_CLASS_BRIDGE_HOST           0x0600
> #define PCI_CLASS_DISPLAY_VGA           0x0300
> #define PCI_VENDOR_ID_COMPAQ            0x0e11
> #define PCI_VENDOR_ID_INTEL             0x8086
> #define PCI_VENDOR_ID_MOTOROLA          0x1057
>         for (bus = 0, devfn = 0; devfn < 0x100; devfn++) {
>                 class = o->read16(&pbus, bus, devfn, PCI_CLASS_DEVICE);
>                 vendor = o->read16(&pbus, bus, devfn, PCI_VENDOR_ID);
>                 printk(BIOS_SPEW, "%s: read @ devfn %04x class %04x
> vendor %04x\n", __FUNCTION__, devfn, class, vendor);
>                 if (((class == PCI_CLASS_BRIDGE_HOST) || (class ==
>                         ((vendor == PCI_VENDOR_ID_INTEL) || (vendor ==
>                                 (vendor == PCI_VENDOR_ID_MOTOROLA))) {
>                         return 1;
>                 }
>         }
>         printk(BIOS_ERR, "PCI: Sanity check failed\n");
>         return 0;
> }
> So, why does finding an intel or moto or whatever mean it is sane?
> What if you have no intel or moto? Or Bridge or VGA?
Intel, Motorola and Compaq probably were the only ones that built 
machines without either a host bridge or a VGA card when this code was 

To me it looks like this check is bogus. What kind of sanity should be 
checked is unclear, I bet there is no comment explaining what the author 
planned. Maybe he attempted to find out whether the firmware finds any 
reasonable PCI devices at all with the used method (ie. pci access 
method 1 vs 2, in aaaancient times?)
> In the lspci from the alix1c, under standard bios, however, there is a
> host bridge at 0:1.0. But under current v3, which does not run VSA, a
> config read of 0:1.0 returns an 0xffff for all config registers.
Hm. So are you saying if we dont run VSA before the PCI scan, the 
virtual devices will not be set up correctly and end up without 
resources? Then VSA needs to run earlier imho.
> Anyway, I added this:
>                if (((class == PCI_CLASS_BRIDGE_HOST) || (class ==
>                         ((vendor == PCI_VENDOR_ID_INTEL) || (vendor ==
>                         (vendor == PCI_VENDOR_ID_AMD) ||
> <<<<<<----------------------------------------------------------
>                                 (vendor == PCI_VENDOR_ID_MOTOROLA))) {
>                         return 1;
>                 }
> I.e. check AMD too. Now it returns, but still, this is really weird:
> devfn 00 (the north) returns 0xffff for the vendor and class. In fact
> the first devfn that gets any AMD vendor id is devfn 0x78!
Looks reasonable. I think devfn does not mean much anymore with the 
virtual hw these days.

> I am wondering  for VSA if we should not have a post-ram step where we
> find all LAR entries with names like
> prestage2/*
I am against this from the perspective of doing all kinds of implicit 
failing rom lar scans that might or might not happen.

No matter whether we speed up lar scans or not, I consider any lar file 
open to a file that does not exist a broken design. We don't want to do 
things per default if we don't need them.

I think the geode is special enough that we can put a vsa.rom or several 
in a special directory in the lar (geode? vsa?) and look there. Do we 
have one big VSA module, or one per functionality?

> and run them. Still not sure of the right approach for VSA. Is VSA so
> special that we shouldn't worry about handling it in the general case?
In my opinion, yes. Other opinions?


coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080119/d6e71449/attachment.sig>

More information about the coreboot mailing list