Myles Watson wrote:
On Fri, Nov 6, 2009 at 10:18 AM, Peter Stuge <peter@stuge.se mailto:peter@stuge.se> wrote:
Myles Watson wrote: > I don't think we should depend on the value from the card. The > point of that check is to make sure we don't run the wrong types of > ROMs. One way to work around it would be to correct the VGA BIOS > from your card and put it into CBFS. Stefan, what about Myles' comment above? I think it's a good point.We started discussing it off list. This should catch the list up:
On Thu, Nov 5, 2009 at 11:03 AM, Stefan Reinauer <stepan@coresystems.de mailto:stepan@coresystems.de> wrote:
Myles Watson wrote: > Did you see my concern with checking the device instead of the ROM? > > Thanks, > Myles oops sorry.. I missed that one..No problem.
Hm.. not sure.. that particular check does not seem to be a consistency check but rather a check to determine from which location to run the VGA OPROM from... Generally, VGA oproms should run from 0xc0000.Yes. His card is broken. I just wanted to make sure the change doesn't break other things to enable his 10-year-old card.
Maybe we should add another check whether class of the card and class of the device are the same?If we did, that would break his card again.
I guess someone will complain if/when it matters. I'm not sure what the "right" thing to do is.
I did think about this before I submitted my original patch, but didn't write my thoughts down anywhere, so here goes:
In pci_rom.c::pci_rom_brobe there is a check to see if the VID/PID of the device, as read from the PCI config space, matches the ROM image. This check was in the original code, and should be there. I like it - it protects against user error.
There is also a check to see if the "class of device" as read from the PCI config space matches that contained in the ROM image. This seems like a reasonable check to make, and to print a message if it fails, but I think the code should push on, even if this fails.
The change I made was a little later on, once we have decided to load the ROM image. We have to do something different if the device we are loading the ROM for is a VGA (it has to be at 0xC000, and should be done early). My change was to make this check based on the PCI config space - not the ROM image. My thinking was along the lines that if someone made a PCI card that didn't declare itself to be a VGA in the PCI config space then most software wouldn't treat it as a VGA device. (in other words the PCI config space is more likely to be correct than the ROM).
This also more closely matches my understanding of how this code might be called. There would be one call early on where the first VGA device has it's ROM loaded, and then there would be a second pass where the non VGA devices have there ROMs loaded.
The final argument for having the code like I had it was that my PCI VGA card was quite popular, back in the day[*], and I assume that it must have worked with almost every motherboard out there, so legacy BIOSs must have been equally lax.
MM