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

[Stefan Reinauer]

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 ==
PCI_CLASS_DISPLAY_VGA)) ||
                        ((vendor == PCI_VENDOR_ID_INTEL) || (vendor ==
PCI_VENDOR_ID_COMPAQ) ||
                                (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 written.

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 ==
PCI_CLASS_DISPLAY_VGA)) ||
                        ((vendor == PCI_VENDOR_ID_INTEL) || (vendor ==
PCI_VENDOR_ID_COMPAQ) ||
                        (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?

Stefan

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

signature.asc
Description: OpenPGP digital signature

-- 
coreboot mailing list
coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot