Andi Kleen wrote:

>>x86_64 inherits arch/x86_64/kernel/setup.c:probe_roms() from i386 and 
>>shares its problems (minus the first) described at:
>>
>>http://marc.theaimsgroup.com/?l=linux-kernel&m=107991009932451&w=2
>>
>>Like the i386 version, the attached x86_64 version also c99ifies the 
>>data and adds IORESOURCE_* type flags. It's been (cross-)compiled but 
>>not booted due to lack of hardware.
> 
> The C99ification is really ugly, because the tables end up more or less 
> unreadable unlike the previous version.

Our tastes differ. Dec99ed version attached.

> I don't think it's a good idea to use values from the ROM unchecked
> (like you did with the length for the checksum). In fact this
> checksum change looks quite dangerous on i386 because there is very
> likely a machine out somewhere with bogus length bytes which explodes
> when you do an access after the video ROM.

This is not the case though. As to the "unchecked"; the only way _to_ 
check the length byte is via the checksum. The new code accepts it if it 
checks out, and hardcodes 0xc7fff as before if not. The only other thing 
that could be done is accept the length byte unconditionally which is 
inadvisable since, as you say, it's somewhat likely that there's some 
VGA out there with a bogus length byte. But note, somewhat, not very. A 
"standard PC BIOS" checks it the same as this code does.

Futhermore, note how the original code already unconditionally touched 
every 2k boundary between c8000 and e0000. So for this to be a problem, 
you'd need a VGA ROM smaller than 32K, a bogus length byte and this 
unthinkable chipset erratum that makes the machine explode while doing 
an access after the videorom, but before c8000, and didn't do so while 
the BIOS did the same thing on boot.

> Chipsets sometimes have interesting errata in these areas.
> [that's probably more an issue on i386 than on x86-64, but it's better
> to be safe than sorry]

It's not an issue on i386 either. I'm all for caution, and yes, I do 
know all about the vast amount of broken PC hardware out there, but 
there's defensive programming and then there's madness :-)

> The other changes look ok. If you send me a patch just for these 
> I will apply it.

Hope you agree with this c99less version given the explanation. As said, 
the only other option is to accept the length byte without any check 
whatsoever or hardcoding it as 32K as the current code does which is not 
nice with almost all current-day VGAs hosting a larger ROM. That, in 
fact, was the reason I looked there; linux confused me by claiming 
(through /proc/iomem) that I had a 32K ROM, while it's actually 48K.

Rene.