pci_fixup_video change blows up on sparc64

pci_fixup_video change blows up on sparc64

[David Miller]

This change in 2.6.19-GIT:

commit b5e4efe7e061ff52ac97b9fa45acca529d8daeea
Author: eiichiro.oiwa.nm@hitachi.com <eiichiro.oiwa.nm@hitachi.com>
Date:   Thu Sep 28 13:55:47 2006 +0900

    PCI: Turn pci_fixup_video into generic for embedded VGA

breaks sparc64 with ATI Radeon and ATY128 cards.

The problem is that there is no system rom at 0xc0000 on sparc64, and
therefore nothing copies the VGA bios of the graphics card there on
bootup.  Therefore all of this code is bogus and will just result in
bus errors when the Radeon or ATY128 driver tries to pci_map_rom() and
read the graphics card ROM.  Nothing will respond to accesses at the
0xc0000 region on sparc64.

The existence of a primary video ROM at 0xc0000 is quite platform
specific.  If some non-x86 systems have this too, that's great.
However, assuming all systems do is not correct.

Re:pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

>
>This change in 2.6.19-GIT:
>
>commit b5e4efe7e061ff52ac97b9fa45acca529d8daeea
>Author: eiichiro.oiwa.nm@hitachi.com <eiichiro.oiwa.nm@hitachi.com>
>Date:   Thu Sep 28 13:55:47 2006 +0900
>
>    PCI: Turn pci_fixup_video into generic for embedded VGA
>
>breaks sparc64 with ATI Radeon and ATY128 cards.
>
>The problem is that there is no system rom at 0xc0000 on sparc64, and
>therefore nothing copies the VGA bios of the graphics card there on
>bootup.  Therefore all of this code is bogus and will just result in
>bus errors when the Radeon or ATY128 driver tries to pci_map_rom() and
>read the graphics card ROM.  Nothing will respond to accesses at the
>0xc0000 region on sparc64.
>
>The existence of a primary video ROM at 0xc0000 is quite platform
>specific.  If some non-x86 systems have this too, that's great.
>However, assuming all systems do is not correct.
>

Does ATI Radeon card have an expansion ROM (video ROM)?
Could you show me "lspci -vv" on sparc64?

If an expansion ROM exists on ATI Radeon or ATY128 card, pci_map_rom returns
the expansion ROM base address instead of 0xC0000 because fixup_video checks
the VGA Enable bit in the Bridge Control register.
The Bridge Control register describes in "PCI-to-PCI Bridge Architecture
Specification Revision 1.2".
This specification is the standard specification in PCI.

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: <eiichiro.oiwa.nm@hitachi.com>
Date: Thu, 19 Oct 2006 16:54:19 +0900

> Does ATI Radeon card have an expansion ROM (video ROM)?

The video card has a PCI expansion ROM.

> Could you show me "lspci -vv" on sparc64?

Attached below.

> If an expansion ROM exists on ATI Radeon or ATY128 card, pci_map_rom returns
> the expansion ROM base address instead of 0xC0000 because fixup_video checks
> the VGA Enable bit in the Bridge Control register.

It is not valid to expect the bridge control register to return
anything meaningful on PCI "host bridge".  The Radeon card here sits
on the root, just under the PCI Host Controller.  The code in
fixup_video appears to assume that every bus up to the root from
the VGA device is a PCI-PCI bridge, which is not a valid assumption.
There can be a PCI host bridge at the root.

Also, and more importantly, you cannot use the 0xc0000 address in a
raw way like this.  There are multiple PCI domains possible in a
given system, and the 0xc0000 address you wish to use must be relative
to that PCI domain.

Therefore, in the presence of multiple PCI domains:

	x = ioremap(0xc0000, ...);

doesn't make any sense, is extremely non-portable, and will crash
on many non-x86 systems.

ioremap()'s first argument is an IO address "cookie" and therefore
using fixed constants for it can never work properly.  It must be
created by the platform specific code, and usually this occurs via PCI
resource computation at PCI probe time.  There are portable ways
defined to do this kind of thing, for example with the
pcibios_bus_to_resource() interface, used by routines such as
drivers/pci/quirks.c:quirk_io_region().

All of this pci_fixup_video code was perfectly fine when it was only
used on x86, where assumptions like this happened to work, but it is
not possible to continue making these assumptions if this code will
now run on every single architecture.

Here is the lspci output:

0000:00:00.0 Host bridge: Sun Microsystems Computer Corp. Tomatillo PCI Bus Module
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B-
	Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR-
	Latency: 64
	Interrupt: pin ? routed to IRQ 1

0000:00:02.0 PCI bridge: Texas Instruments PCI2250 PCI-to-PCI Bridge (rev 02) (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64, Cache Line Size: 0x10 (64 bytes)
	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
	Memory behind bridge: 03000000-030fffff
	BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
	Capabilities: [dc] Power Management version 1
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:00:04.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5703X Gigabit Ethernet (rev 02)
	Subsystem: Broadcom Corporation NetXtreme BCM5703 1000Base-T
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (16000ns min), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin A routed to IRQ 20
	Region 0: Memory at 000007ff00110000 (64-bit, non-prefetchable) [size=64K]
	Expansion ROM at 0000000000120000 [disabled] [size=64K]
	Capabilities: [40] PCI-X non-bridge device.
		Command: DPERE- ERO+ RBC=0 OST=0
		Status: Bus=255 Dev=31 Func=1 64bit+ 133MHz+ SCD- USC-, DC=simple, DMMRBC=2, DMOST=0, DMCRS=1, RSCEM-
	Capabilities: [48] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot+,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=1 PME-
	Capabilities: [50] Vital Product Data
	Capabilities: [58] Message Signalled Interrupts: 64bit+ Queue=0/3 Enable-
		Address: 6ffffffffffffffc  Data: fffd

0000:00:05.0 Ethernet controller: Intel Corporation 82544EI Gigabit Ethernet Controller (Copper) (rev 02)
	Subsystem: Intel Corporation PRO/1000 XT Server Adapter
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (63750ns min), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin A routed to IRQ 21
	Region 0: Memory at 000007ff00140000 (32-bit, non-prefetchable) [size=128K]
	Region 1: Memory at 000007ff00160000 (32-bit, non-prefetchable) [size=128K]
	Region 2: I/O ports at 1000a40 [size=32]
	Expansion ROM at 0000000000180000 [disabled] [size=128K]
	Capabilities: [dc] Power Management version 2
		Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=1 PME-
	Capabilities: [e4] PCI-X non-bridge device.
		Command: DPERE- ERO+ RBC=0 OST=0
		Status: Bus=0 Dev=0 Func=0 64bit+ 133MHz+ SCD- USC-, DC=simple, DMMRBC=2, DMOST=0, DMCRS=1, RSCEM-
	Capabilities: [f0] Message Signalled Interrupts: 64bit+ Queue=0/0 Enable-
		Address: 0000000000000000  Data: 0000

0000:00:06.0 Non-VGA unclassified device: ALi Corporation M7101 Power Management Controller [PMU]
	Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-

0000:00:07.0 ISA bridge: ALi Corporation M1533 PCI to ISA Bridge [Aladdin IV]
	Control: I/O+ Mem+ BusMaster+ SpecCycle+ MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 0
	Capabilities: [a0] Power Management version 1
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:00:08.0 Multimedia audio controller: ALi Corporation M5451 PCI AC-Link Controller Audio Device (rev 02)
	Subsystem: ALi Corporation HP Compaq nc4010 (DY885AA#ABN)
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (500ns min, 6000ns max)
	Interrupt: pin A routed to IRQ 12
	Region 0: I/O ports at 1000900 [size=256]
	Region 1: Memory at 000007ff00100000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [dc] Power Management version 2
		Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:00:0a.0 USB Controller: ALi Corporation USB 1.1 Controller (rev 03) (prog-if 10 [OHCI])
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (20000ns max)
	Interrupt: pin A routed to IRQ 13
	Region 0: Memory at 000007ff01000000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:00:0b.0 USB Controller: ALi Corporation USB 1.1 Controller (rev 03) (prog-if 10 [OHCI])
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (20000ns max)
	Interrupt: pin A routed to IRQ 14
	Region 0: Memory at 000007ff02000000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:00:0d.0 IDE interface: ALi Corporation M5229 IDE (rev c4) (prog-if ff)
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (500ns min, 1000ns max)
	Interrupt: pin A routed to IRQ 15
	Region 0: I/O ports at 1000a00 [size=8]
	Region 1: I/O ports at 1000a18 [size=4]
	Region 2: I/O ports at 1000a10 [size=8]
	Region 3: I/O ports at 1000a08 [size=4]
	Region 4: I/O ports at 1000a20 [size=16]
	Capabilities: [60] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:01:08.0 USB Controller: NEC Corporation USB (rev 43) (prog-if 10 [OHCI])
	Subsystem: Risq Modular Systems, Inc.: Unknown device 0035
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (250ns min, 10500ns max), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin A routed to IRQ 16
	Region 0: Memory at 000007ff03000000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [40] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:01:08.1 USB Controller: NEC Corporation USB (rev 43) (prog-if 10 [OHCI])
	Subsystem: Risq Modular Systems, Inc.: Unknown device 0035
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (250ns min, 10500ns max), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin B routed to IRQ 17
	Region 0: Memory at 000007ff03002000 (32-bit, non-prefetchable) [size=4K]
	Capabilities: [40] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:01:08.2 USB Controller: NEC Corporation USB 2.0 (rev 04) (prog-if 20 [EHCI])
	Subsystem: Risq Modular Systems, Inc.: Unknown device 00e0
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (4000ns min, 8500ns max), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin C routed to IRQ 18
	Region 0: Memory at 000007ff03004000 (32-bit, non-prefetchable) [size=256]
	Capabilities: [40] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

0000:01:0b.0 FireWire (IEEE 1394): Texas Instruments TSB43AB23 IEEE-1394a-2000 Controller (PHY/Link) (prog-if 10 [OHCI])
	Subsystem: Risq Modular Systems, Inc.: Unknown device 8024
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (500ns min, 1000ns max), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin A routed to IRQ 19
	Region 0: Memory at 000007ff03006000 (32-bit, non-prefetchable) [size=2K]
	Region 1: Memory at 000007ff03008000 (32-bit, non-prefetchable) [size=16K]
	Capabilities: [44] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME+

0001:00:00.0 Host bridge: Sun Microsystems Computer Corp. Tomatillo PCI Bus Module
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B-
	Status: Cap- 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ >SERR- <PERR-
	Latency: 64
	Interrupt: pin ? routed to IRQ 22

0001:00:02.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5703 Gigabit Ethernet
	Subsystem: Broadcom Corporation NetXtreme BCM5703 Gigabit Ethernet
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (16000ns min), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin A routed to IRQ 27
	Region 0: Memory at 000007f700200000 (64-bit, non-prefetchable) [size=64K]
	Capabilities: [40] PCI-X non-bridge device.
		Command: DPERE- ERO- RBC=0 OST=0
		Status: Bus=255 Dev=31 Func=1 64bit+ 133MHz+ SCD- USC-, DC=simple, DMMRBC=2, DMOST=0, DMCRS=1, RSCEM-
	Capabilities: [48] Power Management version 2
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot+,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=1 PME-
	Capabilities: [50] Vital Product Data
	Capabilities: [58] Message Signalled Interrupts: 64bit+ Queue=0/3 Enable-
		Address: dfdfefefdfbffffc  Data: fffe

0001:00:03.0 VGA compatible controller: ATI Technologies Inc Radeon RV100 QY [Radeon 7000/VE] (prog-if 00 [VGA])
	Subsystem: ATI Technologies Inc: Unknown device 0908
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping+ SERR- FastB2B-
	Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR-
	Latency: 64 (2000ns min), Cache Line Size: 0x10 (64 bytes)
	Interrupt: pin A routed to IRQ 28
	Region 0: Memory at 000007f708000000 (32-bit, prefetchable) [size=128M]
	Region 1: I/O ports at 1000300 [size=256]
	Region 2: Memory at 000007f700100000 (32-bit, non-prefetchable) [size=64K]
	Expansion ROM at 0000000000120000 [disabled] [size=128K]
	Capabilities: [50] Power Management version 2
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
		Status: D0 PME-Enable- DSel=0 DScale=0 PME-

Re: pci_fixup_video change blows up on sparc64

[Alan Cox]

On Thu, Oct 19, 2006 at 01:37:32AM -0700, David Miller wrote:
> defined to do this kind of thing, for example with the
> pcibios_bus_to_resource() interface, used by routines such as
> drivers/pci/quirks.c:quirk_io_region().

pci_iomap() ?

Alan

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: Alan Cox <alan@redhat.com>
Date: Thu, 19 Oct 2006 05:22:56 -0400

> On Thu, Oct 19, 2006 at 01:37:32AM -0700, David Miller wrote:
> > defined to do this kind of thing, for example with the
> > pcibios_bus_to_resource() interface, used by routines such as
> > drivers/pci/quirks.c:quirk_io_region().
> 
> pci_iomap() ?

Yep, but that interface needs a BAR.

The "0xc0000" usage here for the VGA rom stuff, and some of the quirk
examples, want something a little bit different.

I suppose we could create something that cooked up a fake PCI
device and a BAR, but that is a bit too much for what we're
trying to do here I'm afraid.

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

David Miller wrote:
>From: Alan Cox <alan@redhat.com>
>Date: Thu, 19 Oct 2006 05:22:56 -0400
>
>> On Thu, Oct 19, 2006 at 01:37:32AM -0700, David Miller wrote:
>> > defined to do this kind of thing, for example with the
>> > pcibios_bus_to_resource() interface, used by routines such as
>> > drivers/pci/quirks.c:quirk_io_region().
>> 
>> pci_iomap() ?
>
>Yep, but that interface needs a BAR.
>
>The "0xc0000" usage here for the VGA rom stuff, and some of the quirk
>examples, want something a little bit different.
>
>I suppose we could create something that cooked up a fake PCI
>device and a BAR, but that is a bit too much for what we're
>trying to do here I'm afraid.

The "0xc0000" is a physical address. The BAR (PCI base address) is also
a physcail address. There are no difference.

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: <eiichiro.oiwa.nm@hitachi.com>
Date: Thu, 19 Oct 2006 19:49:26 +0900

> The "0xc0000" is a physical address. The BAR (PCI base address) is also
> a physcail address. There are no difference.

Your assertion that the BAR is a physical address is very platform
specific.  It may be a "physical address in PCI bus space", but
that has no relation to the first argument passed to ioremap()
which is defined in a completely different way.

On many platforms, the BAR of PCI devices are translated into an
appropriate "ioremap() cookie" in the struct pci_dev resource[] array
entries, so that they can be used properly as the first argument to
ioremap().  Only address cookies properly setup by the platform may be
legally passed into ioremap() as the first argument.  No such setups
are being made on this raw 0xc0000 address.

So, as you can see, I/O port and I/O memory space work differently on
different platforms and this abstraction of the first argument to
ioremap() is how we provide support for such differences.

If you try to access 0xc0000 via ioremap() on sparc64, it is going to
try and access that area non-cacheable which, since 0xc0000 is
physical RAM, will result in a BUS ERROR and a crash.

This physical location might be the area for the video ROM on x86,
x86_64, and perhaps even IA64, but it certainly is not used this way
on sparc64 systems.

I really would like to see this regression fixed, or at the very
least this code protected by X86, X86_64, IA64 conditionals.

Re: pci_fixup_video change blows up on sparc64

[Greg KH]

On Thu, Oct 19, 2006 at 03:32:28PM -0700, David Miller wrote:
> From: <eiichiro.oiwa.nm@hitachi.com>
> Date: Thu, 19 Oct 2006 19:49:26 +0900
> 
> > The "0xc0000" is a physical address. The BAR (PCI base address) is also
> > a physcail address. There are no difference.
> 
> Your assertion that the BAR is a physical address is very platform
> specific.  It may be a "physical address in PCI bus space", but
> that has no relation to the first argument passed to ioremap()
> which is defined in a completely different way.
> 
> On many platforms, the BAR of PCI devices are translated into an
> appropriate "ioremap() cookie" in the struct pci_dev resource[] array
> entries, so that they can be used properly as the first argument to
> ioremap().  Only address cookies properly setup by the platform may be
> legally passed into ioremap() as the first argument.  No such setups
> are being made on this raw 0xc0000 address.
> 
> So, as you can see, I/O port and I/O memory space work differently on
> different platforms and this abstraction of the first argument to
> ioremap() is how we provide support for such differences.
> 
> If you try to access 0xc0000 via ioremap() on sparc64, it is going to
> try and access that area non-cacheable which, since 0xc0000 is
> physical RAM, will result in a BUS ERROR and a crash.
> 
> This physical location might be the area for the video ROM on x86,
> x86_64, and perhaps even IA64, but it certainly is not used this way
> on sparc64 systems.
> 
> I really would like to see this regression fixed, or at the very
> least this code protected by X86, X86_64, IA64 conditionals.

I agree.  Eiichiro, care to send me an patch to fix this somehow?  Or do
you want me to just revert it?

thanks,

greg k-h

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

From: Greg KH <greg@kroah.com>
>On Thu, Oct 19, 2006 at 03:32:28PM -0700, David Miller wrote:
>> From: <eiichiro.oiwa.nm@hitachi.com>
>> Date: Thu, 19 Oct 2006 19:49:26 +0900
>> 
>> > The "0xc0000" is a physical address. The BAR (PCI base address) is also
>> > a physcail address. There are no difference.
>> 
>> Your assertion that the BAR is a physical address is very platform
>> specific.  It may be a "physical address in PCI bus space", but
>> that has no relation to the first argument passed to ioremap()
>> which is defined in a completely different way.
>> 
>> On many platforms, the BAR of PCI devices are translated into an
>> appropriate "ioremap() cookie" in the struct pci_dev resource[] array
>> entries, so that they can be used properly as the first argument to
>> ioremap().  Only address cookies properly setup by the platform may be
>> legally passed into ioremap() as the first argument.  No such setups
>> are being made on this raw 0xc0000 address.
>> 
>> So, as you can see, I/O port and I/O memory space work differently on
>> different platforms and this abstraction of the first argument to
>> ioremap() is how we provide support for such differences.
>> 
>> If you try to access 0xc0000 via ioremap() on sparc64, it is going to
>> try and access that area non-cacheable which, since 0xc0000 is
>> physical RAM, will result in a BUS ERROR and a crash.
>> 
>> This physical location might be the area for the video ROM on x86,
>> x86_64, and perhaps even IA64, but it certainly is not used this way
>> on sparc64 systems.
>> 
>> I really would like to see this regression fixed, or at the very
>> least this code protected by X86, X86_64, IA64 conditionals.
>
>I agree.  Eiichiro, care to send me an patch to fix this somehow?  Or do
>you want me to just revert it?
>
>thanks,
>
>greg k-h
>

Ok, I sent an patch to fix on only x86, x86_64 and IA64 for 2.6.18.
Do you need an patch aganist 2.6.19-git?

thanks,
Eiichiro

Re: pci_fixup_video change blows up on sparc64

[Greg KH]

On Fri, Oct 20, 2006 at 12:21:24PM +0900, eiichiro.oiwa.nm@hitachi.com wrote:
> From: Greg KH <greg@kroah.com>
> >On Thu, Oct 19, 2006 at 03:32:28PM -0700, David Miller wrote:
> >> From: <eiichiro.oiwa.nm@hitachi.com>
> >> Date: Thu, 19 Oct 2006 19:49:26 +0900
> >> 
> >> > The "0xc0000" is a physical address. The BAR (PCI base address) is also
> >> > a physcail address. There are no difference.
> >> 
> >> Your assertion that the BAR is a physical address is very platform
> >> specific.  It may be a "physical address in PCI bus space", but
> >> that has no relation to the first argument passed to ioremap()
> >> which is defined in a completely different way.
> >> 
> >> On many platforms, the BAR of PCI devices are translated into an
> >> appropriate "ioremap() cookie" in the struct pci_dev resource[] array
> >> entries, so that they can be used properly as the first argument to
> >> ioremap().  Only address cookies properly setup by the platform may be
> >> legally passed into ioremap() as the first argument.  No such setups
> >> are being made on this raw 0xc0000 address.
> >> 
> >> So, as you can see, I/O port and I/O memory space work differently on
> >> different platforms and this abstraction of the first argument to
> >> ioremap() is how we provide support for such differences.
> >> 
> >> If you try to access 0xc0000 via ioremap() on sparc64, it is going to
> >> try and access that area non-cacheable which, since 0xc0000 is
> >> physical RAM, will result in a BUS ERROR and a crash.
> >> 
> >> This physical location might be the area for the video ROM on x86,
> >> x86_64, and perhaps even IA64, but it certainly is not used this way
> >> on sparc64 systems.
> >> 
> >> I really would like to see this regression fixed, or at the very
> >> least this code protected by X86, X86_64, IA64 conditionals.
> >
> >I agree.  Eiichiro, care to send me an patch to fix this somehow?  Or do
> >you want me to just revert it?
> >
> >thanks,
> >
> >greg k-h
> >
> 
> Ok, I sent an patch to fix on only x86, x86_64 and IA64 for 2.6.18.
> Do you need an patch aganist 2.6.19-git?

I can't apply a patch against an old kernel, especially when the problem
is with the new release :)

Please make it against Linus's latest tree, which is where the problem
is.  Also, please address David's latest comments about the patch.

thanks,

greg k-h

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

From: Greg KH <greg@kroah.com>
>On Fri, Oct 20, 2006 at 12:21:24PM +0900, eiichiro.oiwa.nm@hitachi.com wrote:
>> From: Greg KH <greg@kroah.com>
>> >On Thu, Oct 19, 2006 at 03:32:28PM -0700, David Miller wrote:
>> >> From: <eiichiro.oiwa.nm@hitachi.com>
>> >> Date: Thu, 19 Oct 2006 19:49:26 +0900
>> >> 
>> >> > The "0xc0000" is a physical address. The BAR (PCI base address) is also
>> >> > a physcail address. There are no difference.
>> >> 
>> >> Your assertion that the BAR is a physical address is very platform
>> >> specific.  It may be a "physical address in PCI bus space", but
>> >> that has no relation to the first argument passed to ioremap()
>> >> which is defined in a completely different way.
>> >> 
>> >> On many platforms, the BAR of PCI devices are translated into an
>> >> appropriate "ioremap() cookie" in the struct pci_dev resource[] array
>> >> entries, so that they can be used properly as the first argument to
>> >> ioremap().  Only address cookies properly setup by the platform may be
>> >> legally passed into ioremap() as the first argument.  No such setups
>> >> are being made on this raw 0xc0000 address.
>> >> 
>> >> So, as you can see, I/O port and I/O memory space work differently on
>> >> different platforms and this abstraction of the first argument to
>> >> ioremap() is how we provide support for such differences.
>> >> 
>> >> If you try to access 0xc0000 via ioremap() on sparc64, it is going to
>> >> try and access that area non-cacheable which, since 0xc0000 is
>> >> physical RAM, will result in a BUS ERROR and a crash.
>> >> 
>> >> This physical location might be the area for the video ROM on x86,
>> >> x86_64, and perhaps even IA64, but it certainly is not used this way
>> >> on sparc64 systems.
>> >> 
>> >> I really would like to see this regression fixed, or at the very
>> >> least this code protected by X86, X86_64, IA64 conditionals.
>> >
>> >I agree.  Eiichiro, care to send me an patch to fix this somehow?  Or do
>> >you want me to just revert it?
>> >
>> >thanks,
>> >
>> >greg k-h
>> >
>> 
>> Ok, I sent an patch to fix on only x86, x86_64 and IA64 for 2.6.18.
>> Do you need an patch aganist 2.6.19-git?
>
>I can't apply a patch against an old kernel, especially when the problem
>is with the new release :)
>
>Please make it against Linus's latest tree, which is where the problem
>is.  Also, please address David's latest comments about the patch.
>
>thanks,
>
>greg k-h
Ok, I understood. Thank you a lot.

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

From: Greg KH <greg@kroah.com>
>I can't apply a patch against an old kernel, especially when the problem
>is with the new release :)
>
>Please make it against Linus's latest tree, which is where the problem
>is.  Also, please address David's latest comments about the patch.
>
>thanks,
>
>greg k-h
>

>From comments provided by "David Miller" <davem@davemloft.net>:
>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.
>
>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.

I added this PCI header type check and David's comments.

>From comments provided by "David Miller" <davem@davemloft.net>:
>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.

I'm sorry. I can't modify ioremap to portable ioremap.
I know that x86, x86_64, IA64 dig and IA64 hpzx1 are supporting legacy
memory map. But I don't know other machines. Therefore, I added machine
type check.

I attached an patch below.
I tested this patch on x86, x86_64 and our IA64 dig. I don't have
other type machines.
Could you test on other type machines?

Thank you for David's comments and bug report.
Thank you for Grep, Alan and Jesse.

From: David Miller <davem@davemloft.net>
Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>
---

diff -dupNr linux-2.6.19-rc2-git3.orig/arch/i386/pci/fixup.c linux-2.6.19-rc2-git3/arch/i386/pci/fixup.c
--- linux-2.6.19-rc2-git3.orig/arch/i386/pci/fixup.c	2006-10-20 13:48:45.000000000 +0900
+++ linux-2.6.19-rc2-git3/arch/i386/pci/fixup.c	2006-10-20 15:46:06.000000000 +0900
@@ -343,6 +343,64 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_IN
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_rootport_aspm_quirk );
 
 /*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+	u8 config_header;
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	bus = pdev->bus;
+	while (bus) {
+		bridge = bus->self;
+		if (bridge) {
+			/*
+			 * From information provided by 
+			 * "David Miller" <davem@davemloft.net>
+			 * The bridge control register is valid for PCI header
+			 * type BRIDGE, or CARDBUS. Host to PCI controllers use
+			 * PCI header type NORMAL.
+			 */
+			pci_read_config_byte(bridge, PCI_HEADER_TYPE,
+						&config_header);
+			if ((config_header == PCI_HEADER_TYPE_BRIDGE) 
+			    ||(config_header == PCI_HEADER_TYPE_CARDBUS)){
+				pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+							&config);
+				if (!(config & PCI_BRIDGE_CTL_VGA))
+					return;
+			}
+		}
+		bus = bus->parent;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+
+/*
  * Some Toshiba laptops need extra code to enable their TI TSB43AB22/A.
  *
  * We pretend to bring them out of full D3 state, and restore the proper
diff -dupNr linux-2.6.19-rc2-git3.orig/arch/ia64/pci/fixup.c linux-2.6.19-rc2-git3/arch/ia64/pci/fixup.c
--- linux-2.6.19-rc2-git3.orig/arch/ia64/pci/fixup.c	1970-01-01 09:00:00.000000000 +0900
+++ linux-2.6.19-rc2-git3/arch/ia64/pci/fixup.c	2006-10-20 15:46:35.000000000 +0900
@@ -0,0 +1,72 @@
+/*
+ * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
+ * Derived from fixup.c of i386 tree.
+ */
+
+#include <linux/pci.h>
+#include <linux/init.h>
+
+#include <asm/machvec.h>
+
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+	u8 config_header;
+
+	if ((strcmp(platform_name, "dig") != 0)
+	    && (strcmp(platform_name, "hpzx1")  != 0))
+		return;
+	/* Maybe, this machine supports legacy memory map */
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	bus = pdev->bus;
+	while (bus) {
+		bridge = bus->self;
+		if (bridge) {
+			/*
+			 * From information provided by 
+			 * "David Miller" <davem@davemloft.net>
+			 * The bridge control register is valid for PCI header
+			 * type BRIDGE, or CARDBUS. Host to PCI controllers use
+			 * PCI header type NORMAL.
+			 */
+			pci_read_config_byte(bridge, PCI_HEADER_TYPE,
+						&config_header);
+			if ((config_header == PCI_HEADER_TYPE_BRIDGE) 
+			    ||(config_header == PCI_HEADER_TYPE_CARDBUS)){
+				pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+							&config);
+				if (!(config & PCI_BRIDGE_CTL_VGA))
+					return;
+			}
+		}
+		bus = bus->parent;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
diff -dupNr linux-2.6.19-rc2-git3.orig/arch/ia64/pci/Makefile linux-2.6.19-rc2-git3/arch/ia64/pci/Makefile
--- linux-2.6.19-rc2-git3.orig/arch/ia64/pci/Makefile	2006-10-14 01:25:04.000000000 +0900
+++ linux-2.6.19-rc2-git3/arch/ia64/pci/Makefile	2006-10-20 14:47:13.000000000 +0900
@@ -1,4 +1,4 @@
 #
 # Makefile for the ia64-specific parts of the pci bus
 #
-obj-y		:= pci.o
+obj-y		:= pci.o fixup.o
diff -dupNr linux-2.6.19-rc2-git3.orig/drivers/pci/quirks.c linux-2.6.19-rc2-git3/drivers/pci/quirks.c
--- linux-2.6.19-rc2-git3.orig/drivers/pci/quirks.c	2006-10-20 13:48:46.000000000 +0900
+++ linux-2.6.19-rc2-git3/drivers/pci/quirks.c	2006-10-20 14:42:57.000000000 +0900
@@ -1619,52 +1619,6 @@ static void __devinit fixup_rev1_53c810(
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NCR, PCI_DEVICE_ID_NCR_53C810, fixup_rev1_53c810);
 
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
- * is marked here since the boot video device will be the only enabled
- * video device at this point.
- */
-
-static void __devinit fixup_video(struct pci_dev *pdev)
-{
-	struct pci_dev *bridge;
-	struct pci_bus *bus;
-	u16 config;
-
-	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-		return;
-
-	/* Is VGA routed to us? */
-	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;
-	}
-	pci_read_config_word(pdev, PCI_COMMAND, &config);
-	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
-		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
-		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
-	}
-}
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, fixup_video);
-
-
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f, struct pci_fixup *end)
 {
 	while (f < end) {
diff -dupNr linux-2.6.19-rc2-git3.orig/drivers/pci/rom.c linux-2.6.19-rc2-git3/drivers/pci/rom.c
--- linux-2.6.19-rc2-git3.orig/drivers/pci/rom.c	2006-10-20 13:48:46.000000000 +0900
+++ linux-2.6.19-rc2-git3/drivers/pci/rom.c	2006-10-20 15:45:12.000000000 +0900
@@ -72,8 +72,9 @@ void __iomem *pci_map_rom(struct pci_dev
 	int last_image;
 
 	/*
-	 * IORESOURCE_ROM_SHADOW set if the VGA enable bit of the Bridge Control
-	 * register is set for embedded VGA.
+	 * IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy
+	 * memory map if the VGA enable bit of the Bridge Control register is
+	 * set for embedded VGA.
 	 */
 	if (res->flags & IORESOURCE_ROM_SHADOW) {
 		/* primary video rom always starts here */

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: <eiichiro.oiwa.nm@hitachi.com>
Date: Fri, 20 Oct 2006 23:20:21 +0900

> +			/*
> +			 * From information provided by 
> +			 * "David Miller" <davem@davemloft.net>
> +			 * The bridge control register is valid for PCI header
> +			 * type BRIDGE, or CARDBUS. Host to PCI controllers use
> +			 * PCI header type NORMAL.
> +			 */
> +			pci_read_config_byte(bridge, PCI_HEADER_TYPE,
> +						&config_header);

Use the software copy in "bridge->hdr_type", it's already been read
for you by the PCI device probing layer.

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

From: David Miller <davem@davemloft.net>
>From: <eiichiro.oiwa.nm@hitachi.com>
>Date: Fri, 20 Oct 2006 23:20:21 +0900
>
>> +			/*
>> +			 * From information provided by 
>> +			 * "David Miller" <davem@davemloft.net>
>> +			 * The bridge control register is valid for PCI header
>> +			 * type BRIDGE, or CARDBUS. Host to PCI controllers use
>> +			 * PCI header type NORMAL.
>> +			 */
>> +			pci_read_config_byte(bridge, PCI_HEADER_TYPE,
>> +						&config_header);
>
>Use the software copy in "bridge->hdr_type", it's already been read
>for you by the PCI device probing layer.

Ok, I fixed, and tested on x86, x86_64 and IA64 dig.
Thank you.

From: David Miller <davem@davemloft.net>
Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>
---

diff -dupNr linux-2.6.19-rc2-git3.orig/arch/i386/pci/fixup.c linux-2.6.19-rc2-git3/arch/i386/pci/fixup.c
--- linux-2.6.19-rc2-git3.orig/arch/i386/pci/fixup.c	2006-10-20 13:48:45.000000000 +0900
+++ linux-2.6.19-rc2-git3/arch/i386/pci/fixup.c	2006-10-20 15:46:06.000000000 +0900
@@ -343,6 +343,61 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_IN
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_rootport_aspm_quirk );
 
 /*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	bus = pdev->bus;
+	while (bus) {
+		bridge = bus->self;
+
+		/*
+		 * From information provided by
+		 * "David Miller" <davem@davemloft.net>
+		 * The bridge control register is valid for PCI header
+		 * type BRIDGE, or CARDBUS. Host to PCI controllers use
+		 * PCI header type NORMAL.
+		 */
+		if (bridge
+		    &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+		       ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+			pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+						&config);
+			if (!(config & PCI_BRIDGE_CTL_VGA))
+				return;
+		}
+		bus = bus->parent;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+
+/*
  * Some Toshiba laptops need extra code to enable their TI TSB43AB22/A.
  *
  * We pretend to bring them out of full D3 state, and restore the proper
diff -dupNr linux-2.6.19-rc2-git3.orig/arch/ia64/pci/fixup.c linux-2.6.19-rc2-git3/arch/ia64/pci/fixup.c
--- linux-2.6.19-rc2-git3.orig/arch/ia64/pci/fixup.c	1970-01-01 09:00:00.000000000 +0900
+++ linux-2.6.19-rc2-git3/arch/ia64/pci/fixup.c	2006-10-20 15:46:35.000000000 +0900
@@ -0,0 +1,69 @@
+/*
+ * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
+ * Derived from fixup.c of i386 tree.
+ */
+
+#include <linux/pci.h>
+#include <linux/init.h>
+
+#include <asm/machvec.h>
+
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+
+	if ((strcmp(platform_name, "dig") != 0)
+	    && (strcmp(platform_name, "hpzx1")  != 0))
+		return;
+	/* Maybe, this machine supports legacy memory map. */
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	bus = pdev->bus;
+	while (bus) {
+		bridge = bus->self;
+
+		/*
+		 * From information provided by
+		 * "David Miller" <davem@davemloft.net>
+		 * The bridge control register is valid for PCI header
+		 * type BRIDGE, or CARDBUS. Host to PCI controllers use
+		 * PCI header type NORMAL.
+		 */
+		if (bridge
+		    &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+		       ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+			pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+						&config);
+			if (!(config & PCI_BRIDGE_CTL_VGA))
+				return;
+		}
+		bus = bus->parent;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
diff -dupNr linux-2.6.19-rc2-git3.orig/arch/ia64/pci/Makefile linux-2.6.19-rc2-git3/arch/ia64/pci/Makefile
--- linux-2.6.19-rc2-git3.orig/arch/ia64/pci/Makefile	2006-10-14 01:25:04.000000000 +0900
+++ linux-2.6.19-rc2-git3/arch/ia64/pci/Makefile	2006-10-20 14:47:13.000000000 +0900
@@ -1,4 +1,4 @@
 #
 # Makefile for the ia64-specific parts of the pci bus
 #
-obj-y		:= pci.o
+obj-y		:= pci.o fixup.o
diff -dupNr linux-2.6.19-rc2-git3.orig/drivers/pci/quirks.c linux-2.6.19-rc2-git3/drivers/pci/quirks.c
--- linux-2.6.19-rc2-git3.orig/drivers/pci/quirks.c	2006-10-20 13:48:46.000000000 +0900
+++ linux-2.6.19-rc2-git3/drivers/pci/quirks.c	2006-10-20 14:42:57.000000000 +0900
@@ -1619,52 +1619,6 @@ static void __devinit fixup_rev1_53c810(
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NCR, PCI_DEVICE_ID_NCR_53C810, fixup_rev1_53c810);
 
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
- * is marked here since the boot video device will be the only enabled
- * video device at this point.
- */
-
-static void __devinit fixup_video(struct pci_dev *pdev)
-{
-	struct pci_dev *bridge;
-	struct pci_bus *bus;
-	u16 config;
-
-	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-		return;
-
-	/* Is VGA routed to us? */
-	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;
-	}
-	pci_read_config_word(pdev, PCI_COMMAND, &config);
-	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
-		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
-		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
-	}
-}
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, fixup_video);
-
-
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f, struct pci_fixup *end)
 {
 	while (f < end) {
diff -dupNr linux-2.6.19-rc2-git3.orig/drivers/pci/rom.c linux-2.6.19-rc2-git3/drivers/pci/rom.c
--- linux-2.6.19-rc2-git3.orig/drivers/pci/rom.c	2006-10-20 13:48:46.000000000 +0900
+++ linux-2.6.19-rc2-git3/drivers/pci/rom.c	2006-10-20 15:45:12.000000000 +0900
@@ -72,8 +72,9 @@ void __iomem *pci_map_rom(struct pci_dev
 	int last_image;
 
 	/*
-	 * IORESOURCE_ROM_SHADOW set if the VGA enable bit of the Bridge Control
-	 * register is set for embedded VGA.
+	 * IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy
+	 * memory map if the VGA enable bit of the Bridge Control register is
+	 * set for embedded VGA.
 	 */
 	if (res->flags & IORESOURCE_ROM_SHADOW) {
 		/* primary video rom always starts here */

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: <eiichiro.oiwa.nm@hitachi.com>
Date: Mon, 23 Oct 2006 15:14:07 +0900

> Ok, I fixed, and tested on x86, x86_64 and IA64 dig.
> Thank you.
> 
> From: David Miller <davem@davemloft.net>
> Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>

Greg please apply.

Re: pci_fixup_video change blows up on sparc64

[Greg KH]

On Mon, Oct 23, 2006 at 01:53:53AM -0700, David Miller wrote:
> From: <eiichiro.oiwa.nm@hitachi.com>
> Date: Mon, 23 Oct 2006 15:14:07 +0900
> 
> > Ok, I fixed, and tested on x86, x86_64 and IA64 dig.
> > Thank you.
> > 
> > From: David Miller <davem@davemloft.net>
> > Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>
> 
> Greg please apply.

Ok, but you didn't write this patch, Eiichiro did, right?

Want to get the attribution correct here.

thanks,

greg k-h

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: Greg KH <greg@kroah.com>
Date: Mon, 23 Oct 2006 11:39:49 -0700

> On Mon, Oct 23, 2006 at 01:53:53AM -0700, David Miller wrote:
> > From: <eiichiro.oiwa.nm@hitachi.com>
> > Date: Mon, 23 Oct 2006 15:14:07 +0900
> > 
> > > Ok, I fixed, and tested on x86, x86_64 and IA64 dig.
> > > Thank you.
> > > 
> > > From: David Miller <davem@davemloft.net>
> > > Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>
> > 
> > Greg please apply.
> 
> Ok, but you didn't write this patch, Eiichiro did, right?
> 
> Want to get the attribution correct here.

Yes, Eiichiro did.

patch pci-fix-pci_fixup_video-as-it-blows-up-on-sparc64.patch added to gregkh-2.6 tree

[gregkh]

This is a note to let you know that I've just added the patch titled

     Subject: PCI: fix pci_fixup_video as it blows up on sparc64

to my gregkh-2.6 tree.  Its filename is

     pci-fix-pci_fixup_video-as-it-blows-up-on-sparc64.patch

This tree can be found at 
    http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/


>From eiichiro.oiwa.nm@hitachi.com  Fri Oct 27 10:23:02 2006
MIME-Version: 1.0
Message-ID: <XNM1$9$0$4$$3$3$7$A$9002718U453c5dab@hitachi.com>
Content-Type: text/plain; charset="us-ascii"
To: "David Miller" <davem@davemloft.net>
From: <eiichiro.oiwa.nm@hitachi.com>
Cc: <greg@kroah.com>, <alan@redhat.com>, <jesse.barnes@intel.com>,
	<linux-kernel@vger.kernel.org>, <steven.c.cook@intel.com>,
	<bjorn.helgaas@hp.com>, <tony.luck@intel.com>
Date: Mon, 23 Oct 2006 15:14:07 +0900
Subject: PCI: fix pci_fixup_video as it blows up on sparc64

From: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>

This reverts much of the original pci_fixup_video change and makes it
work for all arches that need it.

fixed, and tested on x86, x86_64 and IA64 dig.

Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>
Acked-by: David Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 arch/i386/pci/fixup.c  |   55 +++++++++++++++++++++++++++++++++++++++
 arch/ia64/pci/Makefile |    2 -
 arch/ia64/pci/fixup.c  |   69 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/quirks.c   |   46 --------------------------------
 drivers/pci/rom.c      |    5 ++-
 5 files changed, 128 insertions(+), 49 deletions(-)

--- gregkh-2.6.orig/arch/i386/pci/fixup.c
+++ gregkh-2.6/arch/i386/pci/fixup.c
@@ -297,6 +297,61 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_IN
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_rootport_aspm_quirk );
 
 /*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	bus = pdev->bus;
+	while (bus) {
+		bridge = bus->self;
+
+		/*
+		 * From information provided by
+		 * "David Miller" <davem@davemloft.net>
+		 * The bridge control register is valid for PCI header
+		 * type BRIDGE, or CARDBUS. Host to PCI controllers use
+		 * PCI header type NORMAL.
+		 */
+		if (bridge
+		    &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+		       ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+			pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+						&config);
+			if (!(config & PCI_BRIDGE_CTL_VGA))
+				return;
+		}
+		bus = bus->parent;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+
+/*
  * Some Toshiba laptops need extra code to enable their TI TSB43AB22/A.
  *
  * We pretend to bring them out of full D3 state, and restore the proper
--- gregkh-2.6.orig/arch/ia64/pci/Makefile
+++ gregkh-2.6/arch/ia64/pci/Makefile
@@ -1,4 +1,4 @@
 #
 # Makefile for the ia64-specific parts of the pci bus
 #
-obj-y		:= pci.o
+obj-y		:= pci.o fixup.o
--- /dev/null
+++ gregkh-2.6/arch/ia64/pci/fixup.c
@@ -0,0 +1,69 @@
+/*
+ * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
+ * Derived from fixup.c of i386 tree.
+ */
+
+#include <linux/pci.h>
+#include <linux/init.h>
+
+#include <asm/machvec.h>
+
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+
+	if ((strcmp(platform_name, "dig") != 0)
+	    && (strcmp(platform_name, "hpzx1")  != 0))
+		return;
+	/* Maybe, this machine supports legacy memory map. */
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	bus = pdev->bus;
+	while (bus) {
+		bridge = bus->self;
+
+		/*
+		 * From information provided by
+		 * "David Miller" <davem@davemloft.net>
+		 * The bridge control register is valid for PCI header
+		 * type BRIDGE, or CARDBUS. Host to PCI controllers use
+		 * PCI header type NORMAL.
+		 */
+		if (bridge
+		    &&((bridge->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+		       ||(bridge->hdr_type == PCI_HEADER_TYPE_CARDBUS))) {
+			pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+						&config);
+			if (!(config & PCI_BRIDGE_CTL_VGA))
+				return;
+		}
+		bus = bus->parent;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
--- gregkh-2.6.orig/drivers/pci/quirks.c
+++ gregkh-2.6/drivers/pci/quirks.c
@@ -1566,52 +1566,6 @@ static void __devinit fixup_rev1_53c810(
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NCR, PCI_DEVICE_ID_NCR_53C810, fixup_rev1_53c810);
 
-/*
- * Fixup to mark boot BIOS video selected by BIOS before it changes
- *
- * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
- *
- * The standard boot ROM sequence for an x86 machine uses the BIOS
- * to select an initial video card for boot display. This boot video
- * card will have it's BIOS copied to C0000 in system RAM.
- * IORESOURCE_ROM_SHADOW is used to associate the boot video
- * card with this copy. On laptops this copy has to be used since
- * the main ROM may be compressed or combined with another image.
- * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
- * is marked here since the boot video device will be the only enabled
- * video device at this point.
- */
-
-static void __devinit fixup_video(struct pci_dev *pdev)
-{
-	struct pci_dev *bridge;
-	struct pci_bus *bus;
-	u16 config;
-
-	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
-		return;
-
-	/* Is VGA routed to us? */
-	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;
-	}
-	pci_read_config_word(pdev, PCI_COMMAND, &config);
-	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
-		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
-		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
-	}
-}
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, fixup_video);
-
-
 static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f, struct pci_fixup *end)
 {
 	while (f < end) {
--- gregkh-2.6.orig/drivers/pci/rom.c
+++ gregkh-2.6/drivers/pci/rom.c
@@ -72,8 +72,9 @@ void __iomem *pci_map_rom(struct pci_dev
 	int last_image;
 
 	/*
-	 * IORESOURCE_ROM_SHADOW set if the VGA enable bit of the Bridge Control
-	 * register is set for embedded VGA.
+	 * IORESOURCE_ROM_SHADOW set on x86, x86_64 and IA64 supports legacy
+	 * memory map if the VGA enable bit of the Bridge Control register is
+	 * set for embedded VGA.
 	 */
 	if (res->flags & IORESOURCE_ROM_SHADOW) {
 		/* primary video rom always starts here */


Patches currently in gregkh-2.6 which might be from davem@davemloft.net are

pci/pci-fix-pci_fixup_video-as-it-blows-up-on-sparc64.patch
pci/pci-use-pci_generic_prep_mwi-on-sparc64.patch

Re[2]: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

>> If an expansion ROM exists on ATI Radeon or ATY128 card, pci_map_rom returns
>> the expansion ROM base address instead of 0xC0000 because fixup_video checks
>> the VGA Enable bit in the Bridge Control register.
>
>It is not valid to expect the bridge control register to return
>anything meaningful on PCI "host bridge".  The Radeon card here sits
>on the root, just under the PCI Host Controller.  The code in
>fixup_video appears to assume that every bus up to the root from
>the VGA device is a PCI-PCI bridge, which is not a valid assumption.
>There can be a PCI host bridge at the root.

Have you ever read the PCI-to-PCI Bridge Architecture Specification?
The default of VGA Enable bit is 0. This mean video ROM doesn't forward
system RAM at 0xC0000. 

There is your VGA card under 0001:00:00.0 Host bridge. The VGA Enable bit
in this host bridge will return 0 and IORESOURCE_ROM_SHADOW won't set.

>Also, and more importantly, you cannot use the 0xc0000 address in a
>raw way like this.  There are multiple PCI domains possible in a
>given system, and the 0xc0000 address you wish to use must be relative
>to that PCI domain.
>
>Therefore, in the presence of multiple PCI domains:
>
>	x = ioremap(0xc0000, ...);
>
>doesn't make any sense, is extremely non-portable, and will crash
>on many non-x86 systems.

It's impossible that multiple VGA cards, which have not the expansion
ROM, exist in a system regardless of multiple PCI domain system.

>All of this pci_fixup_video code was perfectly fine when it was only
>used on x86, where assumptions like this happened to work, but it is
>not possible to continue making these assumptions if this code will
>now run on every single architecture.

pci_fixup_video is also perfectly fine on IA64. And VGA is historical
device of x86 platform.

Re: Re[2]: pci_fixup_video change blows up on sparc64

[Alan Cox]

Ar Iau, 2006-10-19 am 19:01 +0900, ysgrifennodd
eiichiro.oiwa.nm@hitachi.com:
> It's impossible that multiple VGA cards, which have not the expansion
> ROM, exist in a system regardless of multiple PCI domain system.

Strange. I've worked on several machines where it could.

The 0xC0000 is a physical address only as far as the bridge is concerned
(if the bridge even implements it - not all do). The PCI bus or busses
may not even be the root busses of the system. On such systems you can
happily have multiple PCI root bridges each in their own address space
and each with their own idea of where 0xC0000 maps if anywhere.

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

>> It's impossible that multiple VGA cards, which have not the expansion
>> ROM, exist in a system regardless of multiple PCI domain system.
>
>Strange. I've worked on several machines where it could.
>
>The 0xC0000 is a physical address only as far as the bridge is concerned
>(if the bridge even implements it - not all do). The PCI bus or busses
>may not even be the root busses of the system. On such systems you can
>happily have multiple PCI root bridges each in their own address space
>and each with their own idea of where 0xC0000 maps if anywhere.

I am sorry. I mean, for example, if there are two PCI VGA cards and there
is the difference between VGA controllers, only one VGA BIOS at 0xC0000 can't
handle two PCI VGA cards.

My mention was bad. I'm sorry.

Re: pci_fixup_video change blows up on sparc64

[Jesse Barnes]

On Thursday, October 19, 2006 3:01 am, eiichiro.oiwa.nm@hitachi.com 
wrote:
> >> If an expansion ROM exists on ATI Radeon or ATY128 card,
> >> pci_map_rom returns the expansion ROM base address instead of
> >> 0xC0000 because fixup_video checks the VGA Enable bit in the
> >> Bridge Control register.
> >
> >It is not valid to expect the bridge control register to return
> >anything meaningful on PCI "host bridge".  The Radeon card here sits
> >on the root, just under the PCI Host Controller.  The code in
> >fixup_video appears to assume that every bus up to the root from
> >the VGA device is a PCI-PCI bridge, which is not a valid assumption.
> >There can be a PCI host bridge at the root.
>
> Have you ever read the PCI-to-PCI Bridge Architecture Specification?
> The default of VGA Enable bit is 0. This mean video ROM doesn't
> forward system RAM at 0xC0000.
>
> There is your VGA card under 0001:00:00.0 Host bridge. The VGA Enable
> bit in this host bridge will return 0 and IORESOURCE_ROM_SHADOW won't
> set.

I don't think that applies to host->pci bridges though, all bets are off 
as to how their bits are defined.  One check that might make this 
feature a bit more robust is to look for a real PCI ROM on the device.  
If present, we probably don't need to bother with the system copy 
(which probably won't be there anyway).

We should probably also check whether the parent bridge of the device to 
be fixed up is a real pci->pci bridge (if possible).  That would remove 
some ambiguity that's likely to cause problems with other platforms 
too.

Jesse

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: Jesse Barnes <jesse.barnes@intel.com>
Date: Thu, 19 Oct 2006 11:03:11 -0700

> On Thursday, October 19, 2006 3:01 am, eiichiro.oiwa.nm@hitachi.com 
> wrote:
> > >> If an expansion ROM exists on ATI Radeon or ATY128 card,
> > >> pci_map_rom returns the expansion ROM base address instead of
> > >> 0xC0000 because fixup_video checks the VGA Enable bit in the
> > >> Bridge Control register.
> > >
> > >It is not valid to expect the bridge control register to return
> > >anything meaningful on PCI "host bridge".  The Radeon card here sits
> > >on the root, just under the PCI Host Controller.  The code in
> > >fixup_video appears to assume that every bus up to the root from
> > >the VGA device is a PCI-PCI bridge, which is not a valid assumption.
> > >There can be a PCI host bridge at the root.
> >
> > Have you ever read the PCI-to-PCI Bridge Architecture Specification?
> > The default of VGA Enable bit is 0. This mean video ROM doesn't
> > forward system RAM at 0xC0000.
> >
> > There is your VGA card under 0001:00:00.0 Host bridge. The VGA Enable
> > bit in this host bridge will return 0 and IORESOURCE_ROM_SHADOW won't
> > set.
> 
> I don't think that applies to host->pci bridges though, all bets are off 
> as to how their bits are defined.

Yep, that's exactly what I was trying to say.

This is also why the rest of the kernel PCI code, and even things
like "lspci" from PCI utils do not print out the bridge control
register for host->pci bridges.  It's contents are %100 undefined
for host->pci bridges, yet this pci_fixup_video() code is testing
those bits without checking what kind of bridge it is looking at.

> One check that might make this feature a bit more robust is to look
> for a real PCI ROM on the device.  If present, we probably don't
> need to bother with the system copy (which probably won't be there
> anyway).
>
> We should probably also check whether the parent bridge of the device to 
> be fixed up is a real pci->pci bridge (if possible).  That would remove 
> some ambiguity that's likely to cause problems with other platforms 
> too.

At the core of this is the "definition" that 0xc0000 is a location in
physical RAM that the video ROM might be stored.  While this might
be a %100 valid definition on IA64, x86 and x86_64, it simply is not
on other platforms such as sparc64.

In fact even the existence of any RAM at all at that address is
not guarenteed.  I have a few systems where physical RAM starts
at some high physical address, such as 0x80000000.

Even if presence were guarenteed, you can't access this memory using
ioremap() and things like readl() and friends, which is exactly what
callers of pci_map_rom() are doing.  Accesses using readl() use
non-cacheable transactions which result in bus errors, and furthermore
the first argument to ioremap() is not a physical address, it's an
architecture-defined "address cookie" that must be setup by platforms
specific code.

Re: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

From: David Miller <davem@davemloft.net>
>> One check that might make this feature a bit more robust is to look
>> for a real PCI ROM on the device.  If present, we probably don't
>> need to bother with the system copy (which probably won't be there
>> anyway).
>>
>> We should probably also check whether the parent bridge of the device to 
>> be fixed up is a real pci->pci bridge (if possible).  That would remove 
>> some ambiguity that's likely to cause problems with other platforms 
>> too.
>
>At the core of this is the "definition" that 0xc0000 is a location in
>physical RAM that the video ROM might be stored.  While this might
>be a %100 valid definition on IA64, x86 and x86_64, it simply is not
>on other platforms such as sparc64.
>
>In fact even the existence of any RAM at all at that address is
>not guarenteed.  I have a few systems where physical RAM starts
>at some high physical address, such as 0x80000000.
>
>Even if presence were guarenteed, you can't access this memory using
>ioremap() and things like readl() and friends, which is exactly what
>callers of pci_map_rom() are doing.  Accesses using readl() use
>non-cacheable transactions which result in bus errors, and furthermore
>the first argument to ioremap() is not a physical address, it's an
>architecture-defined "address cookie" that must be setup by platforms
>specific code.

Ok, I checked OF specification (PCI Bus IEEE Std 1275-1994 Standard for Boot
Firmware Revision 2.1 p17)
This specification describes the following:
For VGA devices (class-code = 0x000100 or 0x030000), the address ranges are:
0x3B0-0x3BB (I/O, aliased; t=1)
0x3C0-0x3DF (I/O, aliased; t=1)
0xA0000-0xBFFFF (Memory, below 1MB, t=1)

But this specification doesn't define expansion ROM (FCode) address at 0xc0000.
I am sorry. I assumed ROM base address will be 0xc0000 if VGA Enable bit in
Bridge Control Register set. This assuming is incorrect.

IORESOURCE_ROM_SHADOW won't set and pci_map_rom will return PCI ROM base
address if sparc64's VGA Enable bit doesn't set.

David reported pci_map_rom returns 0xc0000. This mean sparc64's VGA Enable bit
set and sparc64 use legacy VGA map. 

As a result, it is possible that expansion ROM address won't point at 0xc0000
if VGA Enable bit set.

Ok, I understood that pci_fixup_video should use on x86, x86_64 and IA64 support
legacy VGA map.

We should have used the following patch:
Signed-off-by: Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>
---

diff -dupNr linux-2.6.18.orig/arch/ia64/pci/Makefile linux-2.6.18/arch/ia64/pci/Makefile
--- linux-2.6.18.orig/arch/ia64/pci/Makefile	2006-09-20 12:42:06.000000000 +0900
+++ linux-2.6.18/arch/ia64/pci/Makefile	2006-09-25 18:36:50.000000000 +0900
@@ -1,4 +1,4 @@
 #
 # Makefile for the ia64-specific parts of the pci bus
 #
-obj-y		:= pci.o
+obj-y		:= pci.o fixup.o
diff -dupNr linux-2.6.18.orig/arch/ia64/pci/fixup.c linux-2.6.18/arch/ia64/pci/fixup.c
--- linux-2.6.18.orig/arch/ia64/pci/fixup.c	1970-01-01 09:00:00.000000000 +0900
+++ linux-2.6.18/arch/ia64/pci/fixup.c	2006-09-25 18:35:12.000000000 +0900
@@ -0,0 +1,56 @@
+/*
+ * Exceptions for specific devices. Usually work-arounds for fatal design flaws.
+ *
+ * Derived from fixup.c of i386 tree.
+ */
+
+#include <linux/delay.h>
+#include <linux/dmi.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+
+
+/*
+ * Fixup to mark boot BIOS video selected by BIOS before it changes
+ *
+ * From information provided by "Jon Smirl" <jonsmirl@gmail.com>
+ *
+ * The standard boot ROM sequence for an x86 machine uses the BIOS
+ * to select an initial video card for boot display. This boot video
+ * card will have it's BIOS copied to C0000 in system RAM.
+ * IORESOURCE_ROM_SHADOW is used to associate the boot video
+ * card with this copy. On laptops this copy has to be used since
+ * the main ROM may be compressed or combined with another image.
+ * See pci_map_rom() for use of this flag. IORESOURCE_ROM_SHADOW
+ * is marked here since the boot video device will be the only enabled
+ * video device at this point.
+ */
+
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
+{
+	struct pci_dev *bridge;
+	struct pci_bus *bus;
+	u16 config;
+
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return;
+
+	/* Is VGA routed to us? */
+	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;
+	}
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_DEBUG "Boot video device is %s\n", pci_name(pdev));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: <eiichiro.oiwa.nm@hitachi.com>
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()).

Re[2]: pci_fixup_video change blows up on sparc64

[eiichiro.oiwa.nm]

From: David Miller <davem@davemloft.net>
>From: <eiichiro.oiwa.nm@hitachi.com>
>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.

Re: pci_fixup_video change blows up on sparc64

[Jesse Barnes]

On Thursday, October 19, 2006 1:37 am, David Miller wrote:
> Also, and more importantly, you cannot use the 0xc0000 address in a
> raw way like this.  There are multiple PCI domains possible in a
> given system, and the 0xc0000 address you wish to use must be
> relative to that PCI domain.
>
> Therefore, in the presence of multiple PCI domains:
>
> 	x = ioremap(0xc0000, ...);
>
> doesn't make any sense, is extremely non-portable, and will crash
> on many non-x86 systems.

Right, I guess we should have been a bit more careful in making this 
code generic.  At least ia64, i386 and x86_64 systems often have video 
BIOSes in system memory at 0xc0000 (note that this isn't in PCI space).  
It sounds like on your system the regular sysfs ROM mapping code should 
be able to see the ROM, and this 0xc0000 mapping/copying shouldn't be 
necessary.

> All of this pci_fixup_video code was perfectly fine when it was only
> used on x86, where assumptions like this happened to work, but it is
> not possible to continue making these assumptions if this code will
> now run on every single architecture.

Maybe we should conditionalize it, making it only available on ia64, 
i386 and x86_64?  Then again, I think there are some embedded platforms 
that could use this code too?

Jesse

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: Jesse Barnes <jesse.barnes@intel.com>
Date: Thu, 19 Oct 2006 09:52:21 -0700

> On Thursday, October 19, 2006 1:37 am, David Miller wrote:
> > Also, and more importantly, you cannot use the 0xc0000 address in a
> > raw way like this.  There are multiple PCI domains possible in a
> > given system, and the 0xc0000 address you wish to use must be
> > relative to that PCI domain.
> >
> > Therefore, in the presence of multiple PCI domains:
> >
> > 	x = ioremap(0xc0000, ...);
> >
> > doesn't make any sense, is extremely non-portable, and will crash
> > on many non-x86 systems.
> 
> Right, I guess we should have been a bit more careful in making this 
> code generic.  At least ia64, i386 and x86_64 systems often have video 
> BIOSes in system memory at 0xc0000 (note that this isn't in PCI space).  

Even if it is in system memory there, accessing physical RAM using
ioremap() and asm/io.h accessors is not exactly legal.  On sparc64,
for example, accessing physical RAM as if it were I/O memory will
result in a BUS ERROR and in fact that's how the bootup crashes
on sparc64 due to this changeset.

Sparc64 systems do not reserve this area of physical ram for video
ROM, and in fact it is very common and possible to have a system
which there is not even physical RAM located at that physical address.

The amount of platforms-specific assumptions made by this code is
impressive, in fact :-)

> It sounds like on your system the regular sysfs ROM mapping code should 
> be able to see the ROM, and this 0xc0000 mapping/copying shouldn't be 
> necessary.

I'm pretty sure it should use the PCI ROM bar area, just like it
always has until this change was installed.

> Maybe we should conditionalize it, making it only available on ia64, 
> i386 and x86_64?  Then again, I think there are some embedded platforms 
> that could use this code too?

This is what should happen, at the very least.

Re: pci_fixup_video change blows up on sparc64

[Jesse Barnes]

On Thursday, October 19, 2006 3:38 pm, David Miller wrote:
> > Right, I guess we should have been a bit more careful in making
> > this code generic.  At least ia64, i386 and x86_64 systems often
> > have video BIOSes in system memory at 0xc0000 (note that this isn't
> > in PCI space).
>
> Even if it is in system memory there, accessing physical RAM using
> ioremap() and asm/io.h accessors is not exactly legal.  On sparc64,
> for example, accessing physical RAM as if it were I/O memory will
> result in a BUS ERROR and in fact that's how the bootup crashes
> on sparc64 due to this changeset.

Good point, we shouldn't use ioremap for the system memory case at all.  
Should be __va or something I guess.

> Sparc64 systems do not reserve this area of physical ram for video
> ROM, and in fact it is very common and possible to have a system
> which there is not even physical RAM located at that physical
> address.
>
> The amount of platforms-specific assumptions made by this code is
> impressive, in fact :-)

No disagreement here...  people often make the mistake when looking at 
the PCI to PCI bridge spec that it also applies to host to PCI bridges.  
And having a video ROM at 0xc0000 is common, but certainly not 
universal.

> > It sounds like on your system the regular sysfs ROM mapping code
> > should be able to see the ROM, and this 0xc0000 mapping/copying
> > shouldn't be necessary.
>
> I'm pretty sure it should use the PCI ROM bar area, just like it
> always has until this change was installed.

Yeah, that's what sysfs will do by default, but this code is overriding 
it's default behavior because some host bridge bit is set, it appears.

> > Maybe we should conditionalize it, making it only available on
> > ia64, i386 and x86_64?  Then again, I think there are some embedded
> > platforms that could use this code too?
>
> This is what should happen, at the very least.

Sounds good to me, though I think we should also add the checks I 
mentioned in my other mail just to be extra robust (of course I don't 
have a system to test on, so I'm hoping Eiichiro can do it. :)

Jesse

Re: pci_fixup_video change blows up on sparc64

[David Miller]

From: Jesse Barnes <jesse.barnes@intel.com>
Date: Thu, 19 Oct 2006 15:58:05 -0700

> On Thursday, October 19, 2006 3:38 pm, David Miller wrote:
> > > Right, I guess we should have been a bit more careful in making
> > > this code generic.  At least ia64, i386 and x86_64 systems often
> > > have video BIOSes in system memory at 0xc0000 (note that this isn't
> > > in PCI space).
> >
> > Even if it is in system memory there, accessing physical RAM using
> > ioremap() and asm/io.h accessors is not exactly legal.  On sparc64,
> > for example, accessing physical RAM as if it were I/O memory will
> > result in a BUS ERROR and in fact that's how the bootup crashes
> > on sparc64 due to this changeset.
> 
> Good point, we shouldn't use ioremap for the system memory case at all.  
> Should be __va or something I guess.

That's one part, but this won't update all the pci_map_rom() callers
who use asm/io.h accessors on the mapping they get back.  Even
pci_read_rom() in the sysfs code this change was aimed at uses
memcpy_fromio().

Also, as an aside, if we're on a system where the x86 BIOS is unlikely
to be executed anyways, we should always use the PCI ROM bar mapping
to access the video card's ROM.