In message: <mtuptj6lnmb.fsf@contents-vnder-pressvre.mit.edu>
            "Nathan J. Williams" <nathanw@wasabisystems.com> writes:
: Not at all. It's the Vendor ID that is the place to start.

There are buggy devices that break this rule.  Otherwise you'd be
correct.  However, one could debate the desirability of dealing with
these buggy devices.

: Section 6.1: "System software may need to scan the PCI bus to
: determine what devices are actually present. To do this, the
: configuration software must read the Vendor ID in each possible PCI
: "slot". The host bus to PCI bridge must unambigously report attempts
: to read the Vendor ID of non-existent devices. Since 0FFFFh is an
: invalid Vendor ID, it is adequate for the host bus to PCI bridge to
: return a value of all 1's on read accesses to Configuration Space
: registers of non-existent devices. (Note that these accesses will be
: terminated with a Master-Abort).
: ...
: Read accesses to reserved or unimplemented registers must be
: completed normally and a data value of 0 returned".
: 
: So it should be adequate to check for a valid Vendor ID, and then read
: all of configuration space.

This will also work correctly.  However, prior to the chagnes I made
in FreeBSD, the code bogusly would check all the functions when the
multifunction bit is set.  For devices that aren't there, this bit is
not valid.

Back to this section, you'll notice it doesn't unambiguously say that
'if vendorid == 0xffff for function 0, then there are no other
functions'  It just says that vendorid == 0xffff is invalid and that
reads from devices that aren't there must return 0xffff.  A subtle
difference, but one that has been observed in the wild at least once.

: "This byte identifies the layout of the second part of the predefined
: header (beginning at byte 10h in Configuration Space) and also whether
: or not the device contains multiple functions. Bit 7 in this register
: is used to identify a multi-function device. If the bit is 0, then the
: device is single-function. If the bit is 0, then the device has
: multiple functions. Bits 6 through 0 identify the layout of the second
: part of the predefined header. The encoding 00h specifies the layout
: shown in figure 6-1. The encoding 01h is defined for PCI-to-PCI
: bridges and is defined in the document _PCI to PCI Bridge Architecture
: Specification_. The encoding 02h is defined for a CardBus bridge and
: is documented in the _PC Card Standard_. All other encodings are
: reserved."

This is true for those devices that are present.  When the device is
not present, the above section requires this to read back as 0xff.

: I'm willing to believe that FreeBSD's PCI code doesn't happen to
: generate the sequence that breaks the Geode's PCI hardware, but it
: really sounds like the hardware is buggy and FreeBSD is lucky.

Actually, FreeBSD's code used to generate the sequence that breaks
Geode's hardware.  But that's because we were imporperly scanning all
Now we use the header type, but could just as easily use vendor id
(absent the questionable devices seen in the field).  The reason that
we're using header type is because there are some sun chips that have
a vendorid of 0xffff in function 0, but a header type of 0x80 and
actual devices at higher function numbers.  The sparc64 port folks can
provide the details.

My copy of the standard says, at the end of 6.1, that 'If a device
supports *the function* that the register is concerned with, the
device must implement the defined location.' (emphasis mine).  It
isn't 100% clear that
	if function 0's vendor id is 0xfffff, then none of functions
	are valid.

This is only implied by the cited paragraph in section 6.1 that says
that the "host bridge must ... return a value of all 1's on read
accesses to Configuration Space registers of non-existent devices).
So, checking for 0xffff as a vendor id AND checking the header types
are both valid ways to filter devices absentany other statements in
the standard to the contrary.  There are two schools of though for the
headertype check: 'only accept what you know to be valid' (what I
think is itojun's change) or 'only reject what you know to be invalid'
(eg, reading it and checking against 0xff).  Given the other verbage
in the standard, it likely is sufficient to filter on a simple 0xff
for the header type.

Before I made my change, FreeBSD would read all functions' Vendor ID.
After my change, it will only read things from function 0 unless we
know the Multifunction bit is legitimately set.  The act of reading >
fucntion 0 for non-existant devices is what triggers the GEODE bug.

: It sounds like checking the header type register *before* validating
: the existence of a device with the Vendor ID register is problematic,
: though, and leads to seeing the FFh value for the header type that you
: describe.

Yes.  That's the problem.  There are two ways to get around it.  One
is to validate the header type, as recommended actually not in the
standard, but by the PCI book I have.  

Looking at NetBSD's old code, I don't see the sequence that we found
in FreeBSD caused the problems in either the MI or MD code.  At worst
NetBSD would reject devices that might really be there, but have a
Vendor ID of 0xffff for some reason.  While one could argue over the
value of accepting such devices, it should have been entirely
sufficient to protect against the sequence that triggers the GEODE
hang.  I'm very surprised changes were necessary.  OpenBSD didn't need
changes to their code, and it looks similar to NetBSD's.

Before itojun's changes, the NetBSD code was:

    foreach device
	read vendor id of function 0
	if == 0 or == 0xffff
		continue
	if mfc bit
		scan 8 functions
	else
		scan 1 function
	foreach function scanned    	
		if vendor id == 0xffff next function
		use this function

before FreeBSD's change the sequence was (for scanning for bridges on
386):
    foreach device
	if mfc bit
		scan 8 functions
	else
		scan 1 function
	foreach function scanned    	
		if vendor id == 0xffff
		   continue
		if not a bridge
		   continue
		do bridge things.

and after the change freebsd code was basically the following:
    foreach device
        if hdrtype & 0x7f > 2 (another way could be == 0xff)
		continue
	if mfc bit
		scan 8 functions
	else
		scan 1 function
	foreach function scanned    	
		if vendor id == 0xffff
		   continue
		if not a bridge
		   continue
		do bridge things.

It is also know than filtering on vendorid avoids the hang.

Warner