From: David Miller <[email protected]>
>From: <[email protected]>
>Date: Fri, 20 Oct 2006 11:57:06 +0900
>
>> I am sorry. I assumed ROM base address will be 0xc0000 if VGA Enable
>> bit in Bridge Control Register set. This assuming is incorrect.
>
>Please also notice another point we are trying to explain in this
>thread, in fact several times.
>
>This "bridge control register" is only valid for "PCI to PCI" bridges.
>
>I repeat:
>
> This "bridge control register" is only valid for "PCI to PCI"
> bridges.
>
>It is not valid for "host to PCI" bridges, yet the pci_fixup_video()
>code is testing the bridge control register, blindly, in every bus
>device it sees as it walks up the device tree to the root.
>
>The bridge control register is valid for PCI header type BRIDGE, or
>CARDBUS. Host to PCI controllers use PCI header type NORMAL. For
>example, here is the lspci dump for my host bridge:
>
>0000:00:00.0 Host bridge: Sun Microsystems Computer Corp. Tomatillo PCI Bus Module
>00: 8e 10 01 a8 46 01 a0 22 00 00 00 06 00 40 00 00
>
>and here is a dump for a PCI->PCI bridge in the same machine:
>
>0000:00:02.0 PCI bridge: Texas Instruments PCI2250 PCI-to-PCI Bridge (rev 02)
>00: 4c 10 23 ac 07 00 10 02 02 00 04 06 10 40 01 00
>
>Clearly, the host bridge has PCI header type 0x00 (NORMAL) at offset
>0x0e, and the PCI-PCI bridge has header type 0x01 (BRIDGE).
>
>Back to the code, look at the loop:
>
> bus = pdev->bus;
> while (bus) {
> bridge = bus->self;
> if (bridge) {
> pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
> &config);
> if (!(config & PCI_BRIDGE_CTL_VGA))
> return;
> }
> bus = bus->parent;
> }
>
>Where is it making sure that this is a PCI to PCI bus and not a
>host to PCI bus? It's not, and that's a serious bug. There should
>be a PCI header type check here, or similar. The PCI device probing
>layer correctly checks the PCI header type before trying to access
>the bridge control register of any PCI device.
>
>And also, it is also true that the ioremap() calls done by
>pci_map_rom() for the "0xc0000" case are totally invalid. For several
>reasons:
>
>1) That is going to be RAM, not I/O memory space, therefore accessing
> it with ioremap() and asm/io.h accessors such as readl() and
> memcpy_fromio() will result in bus errors and other faults.
>
>2) It is illegal to pass raw physical addresses to ioremap() as the
> first argument. The first argument to ioremap() is an architecture
> defined opaque "address cookie" which by definition must be setup
> by platform specific code.
>
>Just copying the x86 code over to IA64 to 'fix the problem' doesn't
>fix any of these bugs (illegal access to bridge control register on
>devices with PCI header type NORMAL) or portability problems (invalid
>first argument to ioremap()).
>
I am sorry again. I understood we need to fix pci_fixup_video and ...
Thank you for your good comments.