PCI page error?

[metallevel]

In the "Enumerating PCI Buses" section, the code for checking all the functions of a multi-function device starts at function 0 and breaks as soon as a function isn't detected. This requires all the functions of a multi-function device to be numbered consecutively, which I don't think is always true (Bochs for one doesn't appear to).

Code: Select all

 void checkDevice(uint8_t bus, uint8_t device) {
     uint8_t function = 0;
 
     vendorID = getVendorID(bus, device, function);
     if(vendorID = 0xFFFF) return;        // Device doesn't exist
     checkFunction(bus, device, function);
     headerType = getHeaderType(bus, device, function);
     if( (headerType & 0x80) != 0) {
         /* It is a multi-function device, so check remaining functions */
         for(function = 1; function < 8; function++) {
             if(getVendorID(bus, device, function) == 0xFFFF) break;   // I don't think this is right
             checkFunction(bus, device, function);
         }
     }
 }

[trinopoty]

Removing the line

Code: Select all

if(getVendorID(bus, device, function) == 0xFFFF) break;   // I don't think this is right

may work. In-fact, that line has almost no use.

[trinopoty]

So the new code is

Code: Select all

void checkDevice(uint8_t bus, uint8_t device) {
     uint8_t function = 0;

     vendorID = getVendorID(bus, device, function);
     if(vendorID = 0xFFFF) return;        // Device doesn't exist
     checkFunction(bus, device, function);
     headerType = getHeaderType(bus, device, function);
     if( (headerType & 0x80) != 0) {
         /* It is a multi-function device, so check remaining functions */
         for(function = 1; function < 8; function++) 
             checkFunction(bus, device, function);
     }
}

[metallevel]

That's what I'm thinking. That line is there to cut down on the number of I/O accesses by exiting the function checking loop at the first function that isn't detected. Problem is I don't believe the functions are always consecutive, so bailing out at the first missing function could cause you to miss some of them.

I would discuss this on the articles talk page, but I did some searching on here and this seems to be a common algorithm. Even the venerable Brendan posted some code a while back that quit the multi-function loop in the same fashion . Hence why I'm a bit hesitant to call this an error on the PCI page when it could just as easily be Bochs or me.

[Griwes]

Wouldn't changing the break into continue be smarter than just dropping the condition in this case?

[Brendan]

Hi,

I would discuss this on the articles talk page, but I did some searching on here and this seems to be a common algorithm. Even the venerable Brendan posted some code a while back that quit the multi-function loop in the same fashion . Hence why I'm a bit hesitant to call this an error on the PCI page when it could just as easily be Bochs or me.

A while ago someone was asking questions about PCI device enumeration. I was just going to tell them "here's the relevant part of the wiki page!" but the information wasn't in the wiki, so I couldn't. It's possible that I posted the pseudo-code as a reply, then slapped myself, then added the same pseudo-code to the wiki.

In general, you should never assume anyone is correct, including me. I also think you're right (and that my pseudo-code was wrong).


Cheers,

Brendan

[metallevel]

Ok, thanks. I just don't want to be the guy who 'corrects' people who are already right .

@Griwes yeah that would be better, fewer IO accesses.