Page 1 of 1

PCI page error?

Posted: Tue Dec 11, 2012 12:53 pm
by 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);
         }
     }
 }

Re: PCI page error?

Posted: Tue Dec 11, 2012 9:48 pm
by 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.

Re: PCI page error?

Posted: Tue Dec 11, 2012 9:49 pm
by 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);
     }
}

Re: PCI page error?

Posted: Wed Dec 12, 2012 3:40 pm
by 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.

Re: PCI page error?

Posted: Wed Dec 12, 2012 6:00 pm
by Griwes
Wouldn't changing the break into continue be smarter than just dropping the condition in this case?

Re: PCI page error?

Posted: Thu Dec 13, 2012 12:01 am
by Brendan
Hi,
metallevel wrote: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

Re: PCI page error?

Posted: Thu Dec 13, 2012 6:23 pm
by metallevel
Ok, thanks. I just don't want to be the guy who 'corrects' people who are already right :D .

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