All of lore.kernel.org
 help / color / mirror / Atom feed
* In "pci_fixup_video" check if this is or should be the primary video device to prevent setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card
@ 2014-01-12  4:10 Sander Eikelenboom
  2014-01-12  4:10 ` [PATCH] " Sander Eikelenboom
       [not found] ` <XNM1$9$0$4$$3$3$7$A$9006858U52d4d109@hitachi.com>
  0 siblings, 2 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-12  4:10 UTC (permalink / raw)
  To: Dave Airlie, Eiichiro Oiwa, Greg Kroah-Hartman
  Cc: Sander Eikelenboom, Konrad Rzeszutek Wilk,
	linux-kernel @ vger . kernel . org

Hi Eiichiro / Dave / Greg,

While trying to get secondary PCI/VGA passthrough of a AMD 6570 card to a Xen guest with the radeon driver and modesetting
i'm running into the problem that the driver says the BIOS is a COMBIOS while it expects a ATOMBIOS for the cards.

So the Guest uses both it's normal emulated VGA card provided by Qemu (f.e. cirrus logic) and a real VGA card via
PCI passthrough.

While debugging it turned out that the bios that the driver read was not the AMD bios, but the bios from the emulated card.
(so it wasn't a COMBIOS either ..)

I first thought the culprit was with Xen, Seabios or Qemu ..
So it took quite a while and debugging, but finally my eye fell on this in the guest dmesg:

[    2.545728] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
[    2.545730] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
[    2.558998] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
[    2.559121] pci 0000:00:01.0: PIIX3: Enabling Passive Release
[    2.572412] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
[    2.572415] pci 0000:00:01.0: Activating ISA DMA hang workarounds
[    2.586527] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xd0
[    2.586609] pci 0000:00:03.0: Boot video device
[    2.586696] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xd0
[    2.586827] pci 0000:00:05.0: Boot video device
[    2.586928] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0

It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
which if i read the comment correctly .. shouldn't be the case:

 /*
 * 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.
 */


But the code doesn't check if it's actually the only enabled (or first) video device at that point ..
and it's setting 2 boot video devices and setting both to use the IORESOURCE_ROM_SHADOW at C000 ..
which happens to be the bios from the emulated card.

With this patch applied the passthrough of the card works fine in the guest and dmesg reports:

[    2.167076] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
[    2.167078] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
[    2.179807] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
[    2.179953] pci 0000:00:01.0: PIIX3: Enabling Passive Release
[    2.192953] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
[    2.192955] pci 0000:00:01.0: Activating ISA DMA hang workarounds
[    2.206543] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xe0
[    2.206623] pci 0000:00:03.0: Boot video device
[    2.206710] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xe0
[    2.206842] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0

--
Sander

Sander Eikelenboom (1):
  In "pci_fixup_video" check if this is or should be the primary video
    device to prevent setting the IORESOURCE_ROM_SHADOW flag on a
    secondary VGA card

 arch/x86/pci/fixup.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] In "pci_fixup_video" check if this is or should be the primary video device to prevent setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card
  2014-01-12  4:10 In "pci_fixup_video" check if this is or should be the primary video device to prevent setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card Sander Eikelenboom
@ 2014-01-12  4:10 ` Sander Eikelenboom
       [not found] ` <XNM1$9$0$4$$3$3$7$A$9006858U52d4d109@hitachi.com>
  1 sibling, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-12  4:10 UTC (permalink / raw)
  To: Dave Airlie, Eiichiro Oiwa, Greg Kroah-Hartman
  Cc: Sander Eikelenboom, Konrad Rzeszutek Wilk,
	linux-kernel @ vger . kernel . org

Setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card prevents if from
reading it's own rom. It will get the content of the shadowrom at C000 instead,
which is of the primary VGA card and the driver of the secondary card will bail
out.

Fix this by checking if this is or should be the primary video device before
applying the fix and let the comment reflect this.

Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 arch/x86/pci/fixup.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b046e07..525e49a 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -314,9 +314,9 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_r
  * 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.
+ * See pci_map_rom() for use of this flag. Before we mark the device
+ * with IORESOURCE_ROM_SHADOW we have to check if this is or should become
+ * the primary video card, since this quirk is ran for all video devices.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -347,12 +347,13 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		}
 		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;
-		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
-		if (!vga_default_device())
+	if (!vga_default_device() || pdev == vga_default_device()) {
+		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;
+			dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
 			vga_set_default_device(pdev);
+		}
 	}
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video device t
       [not found] ` <XNM1$9$0$4$$3$3$7$A$9006858U52d4d109@hitachi.com>
@ 2014-01-14 15:26   ` Sander Eikelenboom
       [not found]     ` <XNM1$9$0$4$$3$3$7$A$9006860U52d614ca@hitachi.com>
  0 siblings, 1 reply; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-14 15:26 UTC (permalink / raw)
  To: eiichiro.oiwa.nm; +Cc: airlied, Greg Kroah-Hartman, konrad.wilk, linux-kernel


Tuesday, January 14, 2014, 6:54:17 AM, you wrote:

> Hi Sander,

>>It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
>>which if i read the comment correctly .. shouldn't be the case:

> I think that there is only one bridge VGA Enable bit is set on normal machine.
> I guess two virtual VGA Enable bit are set on your virtual machine.

> static void pci_fixup_video(struct pci_dev *pdev)
> ...
>                          if (!(config & PCI_BRIDGE_CTL_VGA))
>                                  return;
>                  }
> ...

> Eiichiro

I have added some printk stuff .. and that shows that that code never runs for any of the 2 devices ..
since it runs the while loop once but !bridge ...

Here is the complete lspci output, 00:03.0 is the emulated device, 00:05.0 the one passedthrough:

# lspci -vvknn
00:00.0 Host bridge [0600]: Intel Corporation 440FX - 82441FX PMC [Natoma] [8086:1237] (rev 02)
        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0

00:01.0 ISA bridge [0601]: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] [8086:7000]
        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
        Physical Slot: 1
        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0

00:01.1 IDE interface [0101]: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] [8086:7010] (prog-if 80 [Master])
        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
        Physical Slot: 1
        Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Region 0: [virtual] Memory at 000001f0 (32-bit, non-prefetchable) [size=8]
        Region 1: [virtual] Memory at 000003f0 (type 3, non-prefetchable) [size=1]
        Region 2: [virtual] Memory at 00000170 (32-bit, non-prefetchable) [size=8]
        Region 3: [virtual] Memory at 00000370 (type 3, non-prefetchable) [size=1]
        Region 4: I/O ports at c260 [size=16]
        Kernel driver in use: PIIX_IDE

00:01.2 USB controller [0c03]: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] [8086:7020] (rev 01) (prog-if 00 [UHCI])
        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
        Physical Slot: 1
        Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin D routed to IRQ 23
        Region 4: I/O ports at c240 [size=32]
        Kernel driver in use: uhci_hcd

00:01.3 Bridge [0680]: Intel Corporation 82371AB/EB/MB PIIX4 ACPI [8086:7113] (rev 03)
        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
        Physical Slot: 1
        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 9

00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device [5853:0001] (rev 01)
        Subsystem: XenSource, Inc. Xen Platform Device [5853:0001]
        Physical Slot: 2
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 24
        Region 0: I/O ports at c000 [size=256]
        Region 1: Memory at f2000000 (32-bit, prefetchable) [size=16M]
        Kernel driver in use: xen-platform-pci

00:03.0 VGA compatible controller [0300]: Cirrus Logic GD 5446 [1013:00b8] (prog-if 00 [VGA controller])
        Subsystem: Red Hat, Inc Device [1af4:1100]
        Physical Slot: 3
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Region 0: Memory at f0000000 (32-bit, prefetchable) [size=32M]
        Region 1: Memory at f30b0000 (32-bit, non-prefetchable) [size=4K]
        Expansion ROM at f30a0000 [disabled] [size=64K]

00:05.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Turks [Radeon HD 6570] [1002:6759] (prog-if 00 [VGA controller])
        Subsystem: PC Partner Limited Device [174b:e193]
        Physical Slot: 5
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 81
        Region 0: Memory at e0000000 (64-bit, prefetchable) [size=256M]
        Region 2: Memory at f3060000 (64-bit, non-prefetchable) [size=128K]
        Region 4: I/O ports at c100 [size=256]
        Expansion ROM at f3080000 [disabled] [size=128K]
        Capabilities: [50] Power Management version 3
                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
                Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
        Capabilities: [58] Express (v2) Legacy Endpoint, MSI 00
                DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1 unlimited
                        ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
                DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
                        MaxPayload 128 bytes, MaxReadReq 512 bytes
                DevSta: CorrErr+ UncorrErr+ FatalErr- UnsuppReq+ AuxPwr- TransPend-
                LnkCap: Port #0, Speed 5GT/s, Width x16, ASPM L0s L1, Latency L0 <64ns, L1 <1us
                        ClockPM- Surprise- LLActRep- BwNot-
                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
                LnkSta: Speed 2.5GT/s, Width x8, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
                DevCap2: Completion Timeout: Not Supported, TimeoutDis-
                DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
                LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-, Selectable De-emphasis: -6dB
                         Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
                         Compliance De-emphasis: -6dB
                LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1-
                         EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
        Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
                Address: 00000000fee57000  Data: 4300
        Capabilities: [100 v9] #1002
        Kernel driver in use: radeon

>>Hi Eiichiro / Dave / Greg,
>>
>>While trying to get secondary PCI/VGA passthrough of a AMD 6570 card to a Xen guest with the radeon driver and modesetting
>>i'm running into the problem that the driver says the BIOS is a COMBIOS while it expects a ATOMBIOS for the cards.
>>
>>So the Guest uses both it's normal emulated VGA card provided by Qemu (f.e. cirrus logic) and a real VGA card via
>>PCI passthrough.
>>
>>While debugging it turned out that the bios that the driver read was not the AMD bios, but the bios from the emulated card.
>>(so it wasn't a COMBIOS either ..)
>>
>>I first thought the culprit was with Xen, Seabios or Qemu ..
>>So it took quite a while and debugging, but finally my eye fell on this in the guest dmesg:
>>
>>[    2.545728] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
>>[    2.545730] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
>>[    2.558998] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
>>[    2.559121] pci 0000:00:01.0: PIIX3: Enabling Passive Release
>>[    2.572412] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
>>[    2.572415] pci 0000:00:01.0: Activating ISA DMA hang workarounds
>>[    2.586527] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xd0
>>[    2.586609] pci 0000:00:03.0: Boot video device
>>[    2.586696] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xd0
>>[    2.586827] pci 0000:00:05.0: Boot video device
>>[    2.586928] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0
>>
>>It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
>>which if i read the comment correctly .. shouldn't be the case:
>>
>> /*
>> * 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.
>> */
>>
>>
>>But the code doesn't check if it's actually the only enabled (or first) video device at that point ..
>>and it's setting 2 boot video devices and setting both to use the IORESOURCE_ROM_SHADOW at C000 ..
>>which happens to be the bios from the emulated card.
>>
>>With this patch applied the passthrough of the card works fine in the guest and dmesg reports:
>>
>>[    2.167076] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
>>[    2.167078] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
>>[    2.179807] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
>>[    2.179953] pci 0000:00:01.0: PIIX3: Enabling Passive Release
>>[    2.192953] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
>>[    2.192955] pci 0000:00:01.0: Activating ISA DMA hang workarounds
>>[    2.206543] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xe0
>>[    2.206623] pci 0000:00:03.0: Boot video device
>>[    2.206710] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xe0
>>[    2.206842] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0
>>
>>--
>>Sander
>>
>>Sander Eikelenboom (1):
>>  In "pci_fixup_video" check if this is or should be the primary video
>>    device to prevent setting the IORESOURCE_ROM_SHADOW flag on a
>>    secondary VGA card
>>
>> arch/x86/pci/fixup.c |   17 +++++++++--------
>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>
>>-- 
>>1.7.10.4
>>
>>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video devi
       [not found]     ` <XNM1$9$0$4$$3$3$7$A$9006860U52d614ca@hitachi.com>
@ 2014-01-15  8:40       ` Sander Eikelenboom
  2014-01-15 19:36       ` Sander Eikelenboom
  1 sibling, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-15  8:40 UTC (permalink / raw)
  To: eiichiro.oiwa.nm; +Cc: airlied, gregkh, konrad.wilk, linux-kernel


Wednesday, January 15, 2014, 5:55:38 AM, you wrote:

Ok, i will see what the Qemu guys have to say about it, the emulator is the latest Qemu,
but they will probably say .. hey go fix that fixup in the kernel.

--
Sander

> Hi Sander,

> I understood there is no bridge for a VGA device in your virtual machine.
> Your emulator for the bridge control register is odd.
> I guess your virtual machine ignore "PCI-to-PCI Bridge Architecture
> Specification".

> Thank you,

> Eiichiro Oiwa

>>
>>Tuesday, January 14, 2014, 6:54:17 AM, you wrote:
>>
>>> Hi Sander,
>>
>>>>It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
>>>>which if i read the comment correctly .. shouldn't be the case:
>>
>>> I think that there is only one bridge VGA Enable bit is set on normal machine.
>>> I guess two virtual VGA Enable bit are set on your virtual machine.
>>
>>> static void pci_fixup_video(struct pci_dev *pdev)
>>> ...
>>>                          if (!(config & PCI_BRIDGE_CTL_VGA))
>>>                                  return;
>>>                  }
>>> ...
>>
>>> Eiichiro
>>
>>I have added some printk stuff .. and that shows that that code never runs for any of the 2 devices ..
>>since it runs the while loop once but !bridge ...
>>
>>Here is the complete lspci output, 00:03.0 is the emulated device, 00:05.0 the one passedthrough:
>>
>># lspci -vvknn
>>00:00.0 Host bridge [0600]: Intel Corporation 440FX - 82441FX PMC [Natoma] [8086:1237] (rev 02)
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>
>>00:01.0 ISA bridge [0601]: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] [8086:7000]
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>
>>00:01.1 IDE interface [0101]: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] [8086:7010] (prog-if 80 [Master])
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Region 0: [virtual] Memory at 000001f0 (32-bit, non-prefetchable) [size=8]
>>        Region 1: [virtual] Memory at 000003f0 (type 3, non-prefetchable) [size=1]
>>        Region 2: [virtual] Memory at 00000170 (32-bit, non-prefetchable) [size=8]
>>        Region 3: [virtual] Memory at 00000370 (type 3, non-prefetchable) [size=1]
>>        Region 4: I/O ports at c260 [size=16]
>>        Kernel driver in use: PIIX_IDE
>>
>>00:01.2 USB controller [0c03]: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] [8086:7020] (rev 01) (prog-if 00 [UHCI])
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin D routed to IRQ 23
>>        Region 4: I/O ports at c240 [size=32]
>>        Kernel driver in use: uhci_hcd
>>
>>00:01.3 Bridge [0680]: Intel Corporation 82371AB/EB/MB PIIX4 ACPI [8086:7113] (rev 03)
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin A routed to IRQ 9
>>
>>00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device [5853:0001] (rev 01)
>>        Subsystem: XenSource, Inc. Xen Platform Device [5853:0001]
>>        Physical Slot: 2
>>        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin A routed to IRQ 24
>>        Region 0: I/O ports at c000 [size=256]
>>        Region 1: Memory at f2000000 (32-bit, prefetchable) [size=16M]
>>        Kernel driver in use: xen-platform-pci
>>
>>00:03.0 VGA compatible controller [0300]: Cirrus Logic GD 5446 [1013:00b8] (prog-if 00 [VGA controller])
>>        Subsystem: Red Hat, Inc Device [1af4:1100]
>>        Physical Slot: 3
>>        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Region 0: Memory at f0000000 (32-bit, prefetchable) [size=32M]
>>        Region 1: Memory at f30b0000 (32-bit, non-prefetchable) [size=4K]
>>        Expansion ROM at f30a0000 [disabled] [size=64K]
>>
>>00:05.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Turks [Radeon HD 6570] [1002:6759] (prog-if 00 [VGA controller])
>>        Subsystem: PC Partner Limited Device [174b:e193]
>>        Physical Slot: 5
>>        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>>        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin A routed to IRQ 81
>>        Region 0: Memory at e0000000 (64-bit, prefetchable) [size=256M]
>>        Region 2: Memory at f3060000 (64-bit, non-prefetchable) [size=128K]
>>        Region 4: I/O ports at c100 [size=256]
>>        Expansion ROM at f3080000 [disabled] [size=128K]
>>        Capabilities: [50] Power Management version 3
>>                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>                Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
>>        Capabilities: [58] Express (v2) Legacy Endpoint, MSI 00
>>                DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1 unlimited
>>                        ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
>>                DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
>>                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>                        MaxPayload 128 bytes, MaxReadReq 512 bytes
>>                DevSta: CorrErr+ UncorrErr+ FatalErr- UnsuppReq+ AuxPwr- TransPend-
>>                LnkCap: Port #0, Speed 5GT/s, Width x16, ASPM L0s L1, Latency L0 <64ns, L1 <1us
>>                        ClockPM- Surprise- LLActRep- BwNot-
>>                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
>>                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>                LnkSta: Speed 2.5GT/s, Width x8, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>                DevCap2: Completion Timeout: Not Supported, TimeoutDis-
>>                DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
>>                LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-, Selectable De-emphasis: -6dB
>>                         Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
>>                         Compliance De-emphasis: -6dB
>>                LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1-
>>                         EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
>>        Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>                Address: 00000000fee57000  Data: 4300
>>        Capabilities: [100 v9] #1002
>>        Kernel driver in use: radeon
>>
>>>>Hi Eiichiro / Dave / Greg,
>>>>
>>>>While trying to get secondary PCI/VGA passthrough of a AMD 6570 card to a Xen guest with the radeon driver and modesetting
>>>>i'm running into the problem that the driver says the BIOS is a COMBIOS while it expects a ATOMBIOS for the cards.
>>>>
>>>>So the Guest uses both it's normal emulated VGA card provided by Qemu (f.e. cirrus logic) and a real VGA card via
>>>>PCI passthrough.
>>>>
>>>>While debugging it turned out that the bios that the driver read was not the AMD bios, but the bios from the emulated card.
>>>>(so it wasn't a COMBIOS either ..)
>>>>
>>>>I first thought the culprit was with Xen, Seabios or Qemu ..
>>>>So it took quite a while and debugging, but finally my eye fell on this in the guest dmesg:
>>>>
>>>>[    2.545728] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
>>>>[    2.545730] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
>>>>[    2.558998] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
>>>>[    2.559121] pci 0000:00:01.0: PIIX3: Enabling Passive Release
>>>>[    2.572412] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
>>>>[    2.572415] pci 0000:00:01.0: Activating ISA DMA hang workarounds
>>>>[    2.586527] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xd0
>>>>[    2.586609] pci 0000:00:03.0: Boot video device
>>>>[    2.586696] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xd0
>>>>[    2.586827] pci 0000:00:05.0: Boot video device
>>>>[    2.586928] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0
>>>>
>>>>It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
>>>>which if i read the comment correctly .. shouldn't be the case:
>>>>
>>>> /*
>>>> * 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.
>>>> */
>>>>
>>>>
>>>>But the code doesn't check if it's actually the only enabled (or first) video device at that point ..
>>>>and it's setting 2 boot video devices and setting both to use the IORESOURCE_ROM_SHADOW at C000 ..
>>>>which happens to be the bios from the emulated card.
>>>>
>>>>With this patch applied the passthrough of the card works fine in the guest and dmesg reports:
>>>>
>>>>[    2.167076] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
>>>>[    2.167078] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
>>>>[    2.179807] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
>>>>[    2.179953] pci 0000:00:01.0: PIIX3: Enabling Passive Release
>>>>[    2.192953] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
>>>>[    2.192955] pci 0000:00:01.0: Activating ISA DMA hang workarounds
>>>>[    2.206543] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xe0
>>>>[    2.206623] pci 0000:00:03.0: Boot video device
>>>>[    2.206710] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xe0
>>>>[    2.206842] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0
>>>>
>>>>--
>>>>Sander
>>>>
>>>>Sander Eikelenboom (1):
>>>>  In "pci_fixup_video" check if this is or should be the primary video
>>>>    device to prevent setting the IORESOURCE_ROM_SHADOW flag on a
>>>>    secondary VGA card
>>>>
>>>> arch/x86/pci/fixup.c |   17 +++++++++--------
>>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>>
>>>>-- 
>>>>1.7.10.4
>>>>
>>>>
>>
>>
>>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video devi
       [not found]     ` <XNM1$9$0$4$$3$3$7$A$9006860U52d614ca@hitachi.com>
  2014-01-15  8:40       ` In "pci_fixup_video" check if this is or should be the primary video devi Sander Eikelenboom
@ 2014-01-15 19:36       ` Sander Eikelenboom
  2014-01-15 21:50         ` Bjorn Helgaas
  1 sibling, 1 reply; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-15 19:36 UTC (permalink / raw)
  To: eiichiro.oiwa.nm, bhelgaas
  Cc: airlied, gregkh, konrad.wilk, linux-kernel, linux-pci


Wednesday, January 15, 2014, 5:55:38 AM, you wrote:

> Hi Sander,

> I understood there is no bridge for a VGA device in your virtual machine.
> Your emulator for the bridge control register is odd.
> I guess your virtual machine ignore "PCI-to-PCI Bridge Architecture
> Specification".

All the devices are connected to the root bus and by reading from what i could find that would be a valid setup.
(perhaps not common, but hey we are doing a fixup here for other non common cases).

And i don't understand what is wrong with my patch, earlier in the boot the vga_default_device is already set
by the arch and/or vga arbitration code before the fixup is being run.

In other words ..

If we know the vga boot device .. why not use that information to apply the fixup *only* to that vga boot device ?

And that's just what my patch does ..

+       if (!vga_default_device() || pdev == vga_default_device()) {

If we don't know the vga_default_device ... because we don't have that knowlegde
or
if this is actually the vga_default_device  ... because we do have that knowledge ..

and only then .. run the fixup code and set this device as the vga_default_device


So this change actually makes the code adhere to the comment already above it .. saying it should only be applied to the vga_default_device aka
boot video device.

Also added Bjorn and linux-pci to the CC .. should have done that right away ..
sorry for that.

--
Sander


> Thank you,

> Eiichiro Oiwa

>>
>>Tuesday, January 14, 2014, 6:54:17 AM, you wrote:
>>
>>> Hi Sander,
>>
>>>>It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
>>>>which if i read the comment correctly .. shouldn't be the case:
>>
>>> I think that there is only one bridge VGA Enable bit is set on normal machine.
>>> I guess two virtual VGA Enable bit are set on your virtual machine.
>>
>>> static void pci_fixup_video(struct pci_dev *pdev)
>>> ...
>>>                          if (!(config & PCI_BRIDGE_CTL_VGA))
>>>                                  return;
>>>                  }
>>> ...
>>
>>> Eiichiro
>>
>>I have added some printk stuff .. and that shows that that code never runs for any of the 2 devices ..
>>since it runs the while loop once but !bridge ...
>>
>>Here is the complete lspci output, 00:03.0 is the emulated device, 00:05.0 the one passedthrough:
>>
>># lspci -vvknn
>>00:00.0 Host bridge [0600]: Intel Corporation 440FX - 82441FX PMC [Natoma] [8086:1237] (rev 02)
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>
>>00:01.0 ISA bridge [0601]: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] [8086:7000]
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>
>>00:01.1 IDE interface [0101]: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] [8086:7010] (prog-if 80 [Master])
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Region 0: [virtual] Memory at 000001f0 (32-bit, non-prefetchable) [size=8]
>>        Region 1: [virtual] Memory at 000003f0 (type 3, non-prefetchable) [size=1]
>>        Region 2: [virtual] Memory at 00000170 (32-bit, non-prefetchable) [size=8]
>>        Region 3: [virtual] Memory at 00000370 (type 3, non-prefetchable) [size=1]
>>        Region 4: I/O ports at c260 [size=16]
>>        Kernel driver in use: PIIX_IDE
>>
>>00:01.2 USB controller [0c03]: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] [8086:7020] (rev 01) (prog-if 00 [UHCI])
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin D routed to IRQ 23
>>        Region 4: I/O ports at c240 [size=32]
>>        Kernel driver in use: uhci_hcd
>>
>>00:01.3 Bridge [0680]: Intel Corporation 82371AB/EB/MB PIIX4 ACPI [8086:7113] (rev 03)
>>        Subsystem: Red Hat, Inc Qemu virtual machine [1af4:1100]
>>        Physical Slot: 1
>>        Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin A routed to IRQ 9
>>
>>00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device [5853:0001] (rev 01)
>>        Subsystem: XenSource, Inc. Xen Platform Device [5853:0001]
>>        Physical Slot: 2
>>        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin A routed to IRQ 24
>>        Region 0: I/O ports at c000 [size=256]
>>        Region 1: Memory at f2000000 (32-bit, prefetchable) [size=16M]
>>        Kernel driver in use: xen-platform-pci
>>
>>00:03.0 VGA compatible controller [0300]: Cirrus Logic GD 5446 [1013:00b8] (prog-if 00 [VGA controller])
>>        Subsystem: Red Hat, Inc Device [1af4:1100]
>>        Physical Slot: 3
>>        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>>        Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Region 0: Memory at f0000000 (32-bit, prefetchable) [size=32M]
>>        Region 1: Memory at f30b0000 (32-bit, non-prefetchable) [size=4K]
>>        Expansion ROM at f30a0000 [disabled] [size=64K]
>>
>>00:05.0 VGA compatible controller [0300]: Advanced Micro Devices [AMD] nee ATI Turks [Radeon HD 6570] [1002:6759] (prog-if 00 [VGA controller])
>>        Subsystem: PC Partner Limited Device [174b:e193]
>>        Physical Slot: 5
>>        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
>>        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>>        Latency: 0
>>        Interrupt: pin A routed to IRQ 81
>>        Region 0: Memory at e0000000 (64-bit, prefetchable) [size=256M]
>>        Region 2: Memory at f3060000 (64-bit, non-prefetchable) [size=128K]
>>        Region 4: I/O ports at c100 [size=256]
>>        Expansion ROM at f3080000 [disabled] [size=128K]
>>        Capabilities: [50] Power Management version 3
>>                Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-)
>>                Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
>>        Capabilities: [58] Express (v2) Legacy Endpoint, MSI 00
>>                DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s <4us, L1 unlimited
>>                        ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
>>                DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
>>                        RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
>>                        MaxPayload 128 bytes, MaxReadReq 512 bytes
>>                DevSta: CorrErr+ UncorrErr+ FatalErr- UnsuppReq+ AuxPwr- TransPend-
>>                LnkCap: Port #0, Speed 5GT/s, Width x16, ASPM L0s L1, Latency L0 <64ns, L1 <1us
>>                        ClockPM- Surprise- LLActRep- BwNot-
>>                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk-
>>                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>>                LnkSta: Speed 2.5GT/s, Width x8, TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
>>                DevCap2: Completion Timeout: Not Supported, TimeoutDis-
>>                DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-
>>                LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-, Selectable De-emphasis: -6dB
>>                         Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS-
>>                         Compliance De-emphasis: -6dB
>>                LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, EqualizationPhase1-
>>                         EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
>>        Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+
>>                Address: 00000000fee57000  Data: 4300
>>        Capabilities: [100 v9] #1002
>>        Kernel driver in use: radeon
>>
>>>>Hi Eiichiro / Dave / Greg,
>>>>
>>>>While trying to get secondary PCI/VGA passthrough of a AMD 6570 card to a Xen guest with the radeon driver and modesetting
>>>>i'm running into the problem that the driver says the BIOS is a COMBIOS while it expects a ATOMBIOS for the cards.
>>>>
>>>>So the Guest uses both it's normal emulated VGA card provided by Qemu (f.e. cirrus logic) and a real VGA card via
>>>>PCI passthrough.
>>>>
>>>>While debugging it turned out that the bios that the driver read was not the AMD bios, but the bios from the emulated card.
>>>>(so it wasn't a COMBIOS either ..)
>>>>
>>>>I first thought the culprit was with Xen, Seabios or Qemu ..
>>>>So it took quite a while and debugging, but finally my eye fell on this in the guest dmesg:
>>>>
>>>>[    2.545728] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
>>>>[    2.545730] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
>>>>[    2.558998] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
>>>>[    2.559121] pci 0000:00:01.0: PIIX3: Enabling Passive Release
>>>>[    2.572412] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
>>>>[    2.572415] pci 0000:00:01.0: Activating ISA DMA hang workarounds
>>>>[    2.586527] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xd0
>>>>[    2.586609] pci 0000:00:03.0: Boot video device
>>>>[    2.586696] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xd0
>>>>[    2.586827] pci 0000:00:05.0: Boot video device
>>>>[    2.586928] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0
>>>>
>>>>It's calling the "pci_fixup_video" quirk ... and it's calling it twice ..
>>>>which if i read the comment correctly .. shouldn't be the case:
>>>>
>>>> /*
>>>> * 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.
>>>> */
>>>>
>>>>
>>>>But the code doesn't check if it's actually the only enabled (or first) video device at that point ..
>>>>and it's setting 2 boot video devices and setting both to use the IORESOURCE_ROM_SHADOW at C000 ..
>>>>which happens to be the bios from the emulated card.
>>>>
>>>>With this patch applied the passthrough of the card works fine in the guest and dmesg reports:
>>>>
>>>>[    2.167076] pci 0000:00:00.0: calling quirk_natoma+0x0/0x40
>>>>[    2.167078] pci 0000:00:00.0: Limiting direct PCI/PCI transfers
>>>>[    2.179807] pci 0000:00:00.0: calling quirk_passive_release+0x0/0x90
>>>>[    2.179953] pci 0000:00:01.0: PIIX3: Enabling Passive Release
>>>>[    2.192953] pci 0000:00:01.0: calling quirk_isa_dma_hangs+0x0/0x40
>>>>[    2.192955] pci 0000:00:01.0: Activating ISA DMA hang workarounds
>>>>[    2.206543] pci 0000:00:03.0: calling pci_fixup_video+0x0/0xe0
>>>>[    2.206623] pci 0000:00:03.0: Boot video device
>>>>[    2.206710] pci 0000:00:05.0: calling pci_fixup_video+0x0/0xe0
>>>>[    2.206842] pci 0000:00:06.0: calling quirk_e100_interrupt+0x0/0x1c0
>>>>
>>>>--
>>>>Sander
>>>>
>>>>Sander Eikelenboom (1):
>>>>  In "pci_fixup_video" check if this is or should be the primary video
>>>>    device to prevent setting the IORESOURCE_ROM_SHADOW flag on a
>>>>    secondary VGA card
>>>>
>>>> arch/x86/pci/fixup.c |   17 +++++++++--------
>>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>>
>>>>-- 
>>>>1.7.10.4
>>>>
>>>>
>>
>>
>>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video devi
  2014-01-15 19:36       ` Sander Eikelenboom
@ 2014-01-15 21:50         ` Bjorn Helgaas
  2014-01-15 22:25           ` Sander Eikelenboom
  0 siblings, 1 reply; 22+ messages in thread
From: Bjorn Helgaas @ 2014-01-15 21:50 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: eiichiro.oiwa.nm, Dave Airlie, Greg Kroah-Hartman,
	Konrad Rzeszutek Wilk, linux-kernel, linux-pci

On Wed, Jan 15, 2014 at 12:36 PM, Sander Eikelenboom
<linux@eikelenboom.it> wrote:
> ...
> And that's just what my patch does ..
>
> +       if (!vga_default_device() || pdev == vga_default_device()) {
>
> If we don't know the vga_default_device ... because we don't have that knowlegde
> or
> if this is actually the vga_default_device  ... because we do have that knowledge ..
>
> and only then .. run the fixup code and set this device as the vga_default_device
>
>
> So this change actually makes the code adhere to the comment already above it .. saying it should only be applied to the vga_default_device aka
> boot video device.
>
> Also added Bjorn and linux-pci to the CC .. should have done that right away ..
> sorry for that.

Can you resend your patch to linux-pci?  I don't think it made it there.

Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video devi
  2014-01-15 21:50         ` Bjorn Helgaas
@ 2014-01-15 22:25           ` Sander Eikelenboom
       [not found]             ` <XNM1$9$0$4$$3$3$7$A$9006862U52d758ed@hitachi.com>
  2014-01-30  0:08             ` In "pci_fixup_video" check if this is or should be the primary video devi Bjorn Helgaas
  0 siblings, 2 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-15 22:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: eiichiro.oiwa.nm, Dave Airlie, Greg Kroah-Hartman,
	Konrad Rzeszutek Wilk, linux-kernel, linux-pci

[-- Attachment #1: Type: text/plain, Size: 3822 bytes --]


Wednesday, January 15, 2014, 10:50:09 PM, you wrote:

> On Wed, Jan 15, 2014 at 12:36 PM, Sander Eikelenboom
> <linux@eikelenboom.it> wrote:
>> ...
>> And that's just what my patch does ..
>>
>> +       if (!vga_default_device() || pdev == vga_default_device()) {
>>
>> If we don't know the vga_default_device ... because we don't have that knowlegde
>> or
>> if this is actually the vga_default_device  ... because we do have that knowledge ..
>>
>> and only then .. run the fixup code and set this device as the vga_default_device
>>
>>
>> So this change actually makes the code adhere to the comment already above it .. saying it should only be applied to the vga_default_device aka
>> boot video device.
>>
>> Also added Bjorn and linux-pci to the CC .. should have done that right away ..
>> sorry for that.

> Can you resend your patch to linux-pci?  I don't think it made it there.

> Bjorn


Sure .. also attached for the case it gets mangled ..

Date: Sun, 12 Jan 2014 04:49:44 +0100
Subject: [PATCH] In "pci_fixup_video" check if this is or should be the
 primary video device to prevent setting the
 IORESOURCE_ROM_SHADOW flag on a secondary VGA card
To: Dave Airlie <airlied@redhat.com>,
    Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>,
    Greg Kroah-Hartman <gregkh@suse.de>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
    linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>

Setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card prevents if from
reading it's own rom. It will get the content of the shadowrom at C000 instead,
which is of the primary VGA card and the driver of the secondary card will bail
out.

Fix this by checking if this is or should be the primary video device before
applying the fix and let the comment reflect this.

Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 arch/x86/pci/fixup.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b046e07..525e49a 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -314,9 +314,9 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,        PCI_DEVICE_ID_INTEL_MCH_PC1,    pcie_r
  * 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.
+ * See pci_map_rom() for use of this flag. Before we mark the device
+ * with IORESOURCE_ROM_SHADOW we have to check if this is or should become
+ * the primary video card, since this quirk is ran for all video devices.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -347,12 +347,13 @@ static void pci_fixup_video(struct pci_dev *pdev)
                }
                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;
-               dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
-               if (!vga_default_device())
+       if (!vga_default_device() || pdev == vga_default_device()) {
+               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;
+                       dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
                        vga_set_default_device(pdev);
+               }
        }
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-- 
1.7.10.4

[-- Attachment #2: 0001-In-pci_fixup_video-check-if-this-is-or-should-be-the.patch --]
[-- Type: application/octet-stream, Size: 2782 bytes --]

From 59b2928fbdf1197f874f9ee06a42ccd63daf1340 Mon Sep 17 00:00:00 2001
From: Sander Eikelenboom <linux@eikelenboom.it>
Date: Sun, 12 Jan 2014 04:49:44 +0100
Subject: [PATCH] In "pci_fixup_video" check if this is or should be the
 primary video device to prevent setting the
 IORESOURCE_ROM_SHADOW flag on a secondary VGA card
To: Dave Airlie <airlied@redhat.com>,
    Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>,
    Greg Kroah-Hartman <gregkh@suse.de>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
    linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>

Setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card prevents if from
reading it's own rom. It will get the content of the shadowrom at C000 instead,
which is of the primary VGA card and the driver of the secondary card will bail
out.

Fix this by checking if this is or should be the primary video device before
applying the fix and let the comment reflect this.

Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 arch/x86/pci/fixup.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b046e07..525e49a 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -314,9 +314,9 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_r
  * 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.
+ * See pci_map_rom() for use of this flag. Before we mark the device
+ * with IORESOURCE_ROM_SHADOW we have to check if this is or should become
+ * the primary video card, since this quirk is ran for all video devices.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -347,12 +347,13 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		}
 		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;
-		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
-		if (!vga_default_device())
+	if (!vga_default_device() || pdev == vga_default_device()) {
+		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;
+			dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
 			vga_set_default_device(pdev);
+		}
 	}
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video d
       [not found]             ` <XNM1$9$0$4$$3$3$7$A$9006862U52d758ed@hitachi.com>
@ 2014-01-16 13:19                 ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2014-01-16 13:19 UTC (permalink / raw)
  To: eiichiro.oiwa.nm
  Cc: Sander Eikelenboom, Dave Airlie, Greg Kroah-Hartman,
	Konrad Rzeszutek Wilk, linux-kernel, linux-pci,
	Michael S. Tsirkin, Jesse Barnes, David Airlie, qemu-devel

[+cc Michael, Jesse, David, qemu-devel]

On Wed, Jan 15, 2014 at 8:58 PM,  <eiichiro.oiwa.nm@hitachi.com> wrote:
> I suggest you should not break the PCI specification, as a developer of proprietary
> hypervisor, but I think your patch is no problem.
> Your PCI structure is specialized structure for your virtual machine.
> Maybe, your virtual machine will be causing another problem on Linux or other kernels
> because of breaking the PCI specification.

I assume you think qemu is breaking the PCI spec.  What exactly do you
think is broken?  Please give specific references to the spec.  This
conversation is pretty fragmented, and I came in late, so I apologize
if I missed this.

Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] In "pci_fixup_video" check if this is or should be the primary video d
@ 2014-01-16 13:19                 ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2014-01-16 13:19 UTC (permalink / raw)
  To: eiichiro.oiwa.nm
  Cc: Konrad Rzeszutek Wilk, David Airlie, linux-pci,
	Michael S. Tsirkin, linux-kernel, Jesse Barnes, qemu-devel,
	Sander Eikelenboom, Greg Kroah-Hartman, Dave Airlie

[+cc Michael, Jesse, David, qemu-devel]

On Wed, Jan 15, 2014 at 8:58 PM,  <eiichiro.oiwa.nm@hitachi.com> wrote:
> I suggest you should not break the PCI specification, as a developer of proprietary
> hypervisor, but I think your patch is no problem.
> Your PCI structure is specialized structure for your virtual machine.
> Maybe, your virtual machine will be causing another problem on Linux or other kernels
> because of breaking the PCI specification.

I assume you think qemu is breaking the PCI spec.  What exactly do you
think is broken?  Please give specific references to the spec.  This
conversation is pretty fragmented, and I came in late, so I apologize
if I missed this.

Bjorn

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: In "pci_fixup_video" check if this is or should be the primary video devi
  2014-01-15 22:25           ` Sander Eikelenboom
       [not found]             ` <XNM1$9$0$4$$3$3$7$A$9006862U52d758ed@hitachi.com>
@ 2014-01-30  0:08             ` Bjorn Helgaas
  2014-01-31  9:28                 ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f Sander Eikelenboom
  1 sibling, 1 reply; 22+ messages in thread
From: Bjorn Helgaas @ 2014-01-30  0:08 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: eiichiro.oiwa.nm, Dave Airlie, Greg Kroah-Hartman,
	Konrad Rzeszutek Wilk, linux-kernel, linux-pci

On Wed, Jan 15, 2014 at 11:25:28PM +0100, Sander Eikelenboom wrote:
> Date: Sun, 12 Jan 2014 04:49:44 +0100
> Subject: [PATCH] In "pci_fixup_video" check if this is or should be the
>  primary video device to prevent setting the
>  IORESOURCE_ROM_SHADOW flag on a secondary VGA card
> To: Dave Airlie <airlied@redhat.com>,
>     Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com>,
>     Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
>     linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> 
> Setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card prevents if from
> reading it's own rom. It will get the content of the shadowrom at C000 instead,
> which is of the primary VGA card and the driver of the secondary card will bail
> out.
> 
> Fix this by checking if this is or should be the primary video device before
> applying the fix and let the comment reflect this.
> 
> Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
> ---
>  arch/x86/pci/fixup.c |   17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index b046e07..525e49a 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -314,9 +314,9 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,        PCI_DEVICE_ID_INTEL_MCH_PC1,    pcie_r
>   * 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.
> + * See pci_map_rom() for use of this flag. Before we mark the device
> + * with IORESOURCE_ROM_SHADOW we have to check if this is or should become
> + * the primary video card, since this quirk is ran for all video devices.
>   */
>  
>  static void pci_fixup_video(struct pci_dev *pdev)
> @@ -347,12 +347,13 @@ static void pci_fixup_video(struct pci_dev *pdev)
>                 }
>                 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;
> -               dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
> -               if (!vga_default_device())
> +       if (!vga_default_device() || pdev == vga_default_device()) {
> +               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;
> +                       dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
>                         vga_set_default_device(pdev);
> +               }

ia64 also has a pci_fixup_video() that is essentially the same.  Can you
fix that one as well, unless there is a reason why the fix applies only to
x86 and not to ia64?

>         }
>  }
>  DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> -- 
> 1.7.10.4



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out.
  2014-01-30  0:08             ` In "pci_fixup_video" check if this is or should be the primary video devi Bjorn Helgaas
@ 2014-01-31  9:28                 ` Sander Eikelenboom
  0 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-31  9:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Tony Luck, Dave Airlie, Eiichiro Oiwa, Greg Kroah-Hartman
  Cc: Sander Eikelenboom, Konrad Rzeszutek Wilk,
	linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Hi Bjorn / Tony,

I fixed up ia64 as well and brought it inline again with the x86 code,
but i don't have a ia64 machine, so that part is untested.
Perhaps Tony is able to review/test it ?

Sander



Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
prevents it from reading it's own rom. It will get the content of the shadowrom
at C000 instead, which is of the primary VGA card and the driver of the
secondary card will bail out.

Fix this by checking if the arch code or vga-arbitration has already
determined the vga_default_device, if so only apply the fix to this
primary video device and let the comment reflect this.

v2:
    - Fix pci_fixup_video both in x86 and ia64


Sander Eikelenboom (1):
  Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
    primary     prevents it from reading it's own rom. It will get the
    content of the shadowrom     at C000 instead, which is of the
    primary VGA card and the driver of the     secondary card will bail
    out.

 arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
 arch/x86/pci/fixup.c  |   18 ++++++++++--------
 2 files changed, 23 insertions(+), 19 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f
@ 2014-01-31  9:28                 ` Sander Eikelenboom
  0 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-31  9:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Tony Luck, Dave Airlie, Eiichiro Oiwa, Greg Kroah-Hartman
  Cc: Sander Eikelenboom, Konrad Rzeszutek Wilk,
	linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Hi Bjorn / Tony,

I fixed up ia64 as well and brought it inline again with the x86 code,
but i don't have a ia64 machine, so that part is untested.
Perhaps Tony is able to review/test it ?

Sander



Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
prevents it from reading it's own rom. It will get the content of the shadowrom
at C000 instead, which is of the primary VGA card and the driver of the
secondary card will bail out.

Fix this by checking if the arch code or vga-arbitration has already
determined the vga_default_device, if so only apply the fix to this
primary video device and let the comment reflect this.

v2:
    - Fix pci_fixup_video both in x86 and ia64


Sander Eikelenboom (1):
  Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
    primary     prevents it from reading it's own rom. It will get the
    content of the shadowrom     at C000 instead, which is of the
    primary VGA card and the driver of the     secondary card will bail
    out.

 arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
 arch/x86/pci/fixup.c  |   18 ++++++++++--------
 2 files changed, 23 insertions(+), 19 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out.
  2014-01-31  9:28                 ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f Sander Eikelenboom
@ 2014-01-31  9:28                   ` Sander Eikelenboom
  -1 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-31  9:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Tony Luck, Dave Airlie, Eiichiro Oiwa, Greg Kroah-Hartman
  Cc: Sander Eikelenboom, Konrad Rzeszutek Wilk,
	linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Fix this by checking if the arch code or vga-arbitration has already
determined the vga_default_device, if so only apply the fix to this
primary video device and let the comment reflect this.

Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
 arch/x86/pci/fixup.c  |   18 ++++++++++--------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 5dc969d..7762255 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -19,9 +19,10 @@
  * 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.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -35,9 +36,6 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		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) {
@@ -60,10 +58,14 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		}
 		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;
-		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+	if (!vga_default_device() || pdev == vga_default_device()) {
+		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;
+			dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+			vga_set_default_device(pdev);
+		}
 	}
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+				PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b046e07..34528a7 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -314,9 +314,10 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_r
  * 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.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -347,12 +348,13 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		}
 		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;
-		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
-		if (!vga_default_device())
+	if (!vga_default_device() || pdev == vga_default_device()) {
+		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;
+			dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
 			vga_set_default_device(pdev);
+		}
 	}
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f
@ 2014-01-31  9:28                   ` Sander Eikelenboom
  0 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-01-31  9:28 UTC (permalink / raw)
  To: Bjorn Helgaas, Tony Luck, Dave Airlie, Eiichiro Oiwa, Greg Kroah-Hartman
  Cc: Sander Eikelenboom, Konrad Rzeszutek Wilk,
	linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Fix this by checking if the arch code or vga-arbitration has already
determined the vga_default_device, if so only apply the fix to this
primary video device and let the comment reflect this.

Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it>
---
 arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
 arch/x86/pci/fixup.c  |   18 ++++++++++--------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 5dc969d..7762255 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -19,9 +19,10 @@
  * 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.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -35,9 +36,6 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		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) {
@@ -60,10 +58,14 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		}
 		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;
-		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+	if (!vga_default_device() || pdev = vga_default_device()) {
+		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;
+			dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
+			vga_set_default_device(pdev);
+		}
 	}
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+				PCI_CLASS_DISPLAY_VGA, 8, pci_fixup_video);
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b046e07..34528a7 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -314,9 +314,10 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_MCH_PC1,	pcie_r
  * 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.
+ * See pci_map_rom() for use of this flag. Before marking the device
+ * with IORESOURCE_ROM_SHADOW check if a vga_default_device is already set
+ * by either arch cde or vga-arbitration, if so only apply the fixup to this
+ * already determined primary video card.
  */
 
 static void pci_fixup_video(struct pci_dev *pdev)
@@ -347,12 +348,13 @@ static void pci_fixup_video(struct pci_dev *pdev)
 		}
 		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;
-		dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
-		if (!vga_default_device())
+	if (!vga_default_device() || pdev = vga_default_device()) {
+		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;
+			dev_printk(KERN_DEBUG, &pdev->dev, "Boot video device\n");
 			vga_set_default_device(pdev);
+		}
 	}
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out.
  2014-01-31  9:28                 ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f Sander Eikelenboom
@ 2014-02-03 14:52                   ` Konrad Rzeszutek Wilk
  -1 siblings, 0 replies; 22+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-02-03 14:52 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: Bjorn Helgaas, Tony Luck, Dave Airlie, Eiichiro Oiwa,
	Greg Kroah-Hartman, linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
> Hi Bjorn / Tony,
> 
> I fixed up ia64 as well and brought it inline again with the x86 code,
> but i don't have a ia64 machine, so that part is untested.
> Perhaps Tony is able to review/test it ?
> 
> Sander
> 
> 
> 
> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
> prevents it from reading it's own rom. It will get the content of the shadowrom
> at C000 instead, which is of the primary VGA card and the driver of the
> secondary card will bail out.
> 
> Fix this by checking if the arch code or vga-arbitration has already
> determined the vga_default_device, if so only apply the fix to this
> primary video device and let the comment reflect this.
> 
> v2:
>     - Fix pci_fixup_video both in x86 and ia64
> 
> 
> Sander Eikelenboom (1):
>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
>     primary     prevents it from reading it's own rom. It will get the
>     content of the shadowrom     at C000 instead, which is of the
>     primary VGA card and the driver of the     secondary card will bail
>     out.

Your editor mutilated your subject line. It ought to have been just
one line.

Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
on the patch for the x86 part.

The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
so I can't give it a 'Tested-by' yet.

> 
>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
>  2 files changed, 23 insertions(+), 19 deletions(-)
> 
> -- 
> 1.7.10.4
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents
@ 2014-02-03 14:52                   ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 22+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-02-03 14:52 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: Bjorn Helgaas, Tony Luck, Dave Airlie, Eiichiro Oiwa,
	Greg Kroah-Hartman, linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
> Hi Bjorn / Tony,
> 
> I fixed up ia64 as well and brought it inline again with the x86 code,
> but i don't have a ia64 machine, so that part is untested.
> Perhaps Tony is able to review/test it ?
> 
> Sander
> 
> 
> 
> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
> prevents it from reading it's own rom. It will get the content of the shadowrom
> at C000 instead, which is of the primary VGA card and the driver of the
> secondary card will bail out.
> 
> Fix this by checking if the arch code or vga-arbitration has already
> determined the vga_default_device, if so only apply the fix to this
> primary video device and let the comment reflect this.
> 
> v2:
>     - Fix pci_fixup_video both in x86 and ia64
> 
> 
> Sander Eikelenboom (1):
>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
>     primary     prevents it from reading it's own rom. It will get the
>     content of the shadowrom     at C000 instead, which is of the
>     primary VGA card and the driver of the     secondary card will bail
>     out.

Your editor mutilated your subject line. It ought to have been just
one line.

Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
on the patch for the x86 part.

The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
so I can't give it a 'Tested-by' yet.

> 
>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
>  2 files changed, 23 insertions(+), 19 deletions(-)
> 
> -- 
> 1.7.10.4
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out.
  2014-02-03 14:52                   ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Konrad Rzeszutek Wilk
@ 2014-02-07 11:03                     ` Sander Eikelenboom
  -1 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-02-07 11:03 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Tony Luck, Dave Airlie, Eiichiro Oiwa
  Cc: Bjorn Helgaas, Greg Kroah-Hartman,
	linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Hi All,

Still trying to collect tested and acked-by's ..

Dave,
          In commit 6cf20beec4b91c240cf759b4db72669e251f1fc4 you changed pci_fixup_video saying:
          "This fixes the switcheroo on my T410s."
          Do you still have the machine and could you verify if it still works with this patch applied ?

Eiichiro,
          Most of last major overhaul of pci_fixup_video was in commit 6b5c76b8e2ff204fa8d7201acce461188873bf2b saying:
          "PCI: fix pci_fixup_video as it blows up on sparc64"
          Is it possible for you to verify you can still boot on sparc64 with this patch applied ?

Tony,
          Since ia64's pci_fixup_video is also changed to be inline with x86, could you verify it still compiles and boots for you ?


Konrad,
          Thanks for reviewing and testing !

--
Sander




Monday, February 3, 2014, 3:52:05 PM, you wrote:

> On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
>> Hi Bjorn / Tony,
>> 
>> I fixed up ia64 as well and brought it inline again with the x86 code,
>> but i don't have a ia64 machine, so that part is untested.
>> Perhaps Tony is able to review/test it ?
>> 
>> Sander
>> 
>> 
>> 
>> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
>> prevents it from reading it's own rom. It will get the content of the shadowrom
>> at C000 instead, which is of the primary VGA card and the driver of the
>> secondary card will bail out.
>> 
>> Fix this by checking if the arch code or vga-arbitration has already
>> determined the vga_default_device, if so only apply the fix to this
>> primary video device and let the comment reflect this.
>> 
>> v2:
>>     - Fix pci_fixup_video both in x86 and ia64
>> 
>> 
>> Sander Eikelenboom (1):
>>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
>>     primary     prevents it from reading it's own rom. It will get the
>>     content of the shadowrom     at C000 instead, which is of the
>>     primary VGA card and the driver of the     secondary card will bail
>>     out.

> Your editor mutilated your subject line. It ought to have been just
> one line.

> Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
> on the patch for the x86 part.

> The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
> so I can't give it a 'Tested-by' yet.

>> 
>>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
>>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
>>  2 files changed, 23 insertions(+), 19 deletions(-)
>> 
>> -- 
>> 1.7.10.4
>> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents
@ 2014-02-07 11:03                     ` Sander Eikelenboom
  0 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-02-07 11:03 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Tony Luck, Dave Airlie, Eiichiro Oiwa
  Cc: Bjorn Helgaas, Greg Kroah-Hartman,
	linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Hi All,

Still trying to collect tested and acked-by's ..

Dave,
          In commit 6cf20beec4b91c240cf759b4db72669e251f1fc4 you changed pci_fixup_video saying:
          "This fixes the switcheroo on my T410s."
          Do you still have the machine and could you verify if it still works with this patch applied ?

Eiichiro,
          Most of last major overhaul of pci_fixup_video was in commit 6b5c76b8e2ff204fa8d7201acce461188873bf2b saying:
          "PCI: fix pci_fixup_video as it blows up on sparc64"
          Is it possible for you to verify you can still boot on sparc64 with this patch applied ?

Tony,
          Since ia64's pci_fixup_video is also changed to be inline with x86, could you verify it still compiles and boots for you ?


Konrad,
          Thanks for reviewing and testing !

--
Sander




Monday, February 3, 2014, 3:52:05 PM, you wrote:

> On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
>> Hi Bjorn / Tony,
>> 
>> I fixed up ia64 as well and brought it inline again with the x86 code,
>> but i don't have a ia64 machine, so that part is untested.
>> Perhaps Tony is able to review/test it ?
>> 
>> Sander
>> 
>> 
>> 
>> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
>> prevents it from reading it's own rom. It will get the content of the shadowrom
>> at C000 instead, which is of the primary VGA card and the driver of the
>> secondary card will bail out.
>> 
>> Fix this by checking if the arch code or vga-arbitration has already
>> determined the vga_default_device, if so only apply the fix to this
>> primary video device and let the comment reflect this.
>> 
>> v2:
>>     - Fix pci_fixup_video both in x86 and ia64
>> 
>> 
>> Sander Eikelenboom (1):
>>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
>>     primary     prevents it from reading it's own rom. It will get the
>>     content of the shadowrom     at C000 instead, which is of the
>>     primary VGA card and the driver of the     secondary card will bail
>>     out.

> Your editor mutilated your subject line. It ought to have been just
> one line.

> Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
> on the patch for the x86 part.

> The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
> so I can't give it a 'Tested-by' yet.

>> 
>>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
>>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
>>  2 files changed, 23 insertions(+), 19 deletions(-)
>> 
>> -- 
>> 1.7.10.4
>> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out.
  2014-02-03 14:52                   ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Konrad Rzeszutek Wilk
@ 2014-02-13  9:48                     ` Sander Eikelenboom
  -1 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-02-13  9:48 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Konrad Rzeszutek Wilk, Tony Luck, Dave Airlie, Eiichiro Oiwa,
	Greg Kroah-Hartman, linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Hi Bjorn,

I have given it another email and another week, but without gaining any reviewed or acked-by's.
It seems the only way forward is to shovel it in linux-next earlier, give it a good soak and see if
anyone starts to squeal .. or that everything seems to be ok :-)

Would you need a v3 with the acked and reviewed-by from Konrad for x86 in it ?

--

Sander


Monday, February 3, 2014, 3:52:05 PM, you wrote:

> On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
>> Hi Bjorn / Tony,
>> 
>> I fixed up ia64 as well and brought it inline again with the x86 code,
>> but i don't have a ia64 machine, so that part is untested.
>> Perhaps Tony is able to review/test it ?
>> 
>> Sander
>> 
>> 
>> 
>> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
>> prevents it from reading it's own rom. It will get the content of the shadowrom
>> at C000 instead, which is of the primary VGA card and the driver of the
>> secondary card will bail out.
>> 
>> Fix this by checking if the arch code or vga-arbitration has already
>> determined the vga_default_device, if so only apply the fix to this
>> primary video device and let the comment reflect this.
>> 
>> v2:
>>     - Fix pci_fixup_video both in x86 and ia64
>> 
>> 
>> Sander Eikelenboom (1):
>>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
>>     primary     prevents it from reading it's own rom. It will get the
>>     content of the shadowrom     at C000 instead, which is of the
>>     primary VGA card and the driver of the     secondary card will bail
>>     out.

> Your editor mutilated your subject line. It ought to have been just
> one line.

> Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
> on the patch for the x86 part.

> The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
> so I can't give it a 'Tested-by' yet.

>> 
>>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
>>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
>>  2 files changed, 23 insertions(+), 19 deletions(-)
>> 
>> -- 
>> 1.7.10.4
>> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents
@ 2014-02-13  9:48                     ` Sander Eikelenboom
  0 siblings, 0 replies; 22+ messages in thread
From: Sander Eikelenboom @ 2014-02-13  9:48 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Konrad Rzeszutek Wilk, Tony Luck, Dave Airlie, Eiichiro Oiwa,
	Greg Kroah-Hartman, linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

Hi Bjorn,

I have given it another email and another week, but without gaining any reviewed or acked-by's.
It seems the only way forward is to shovel it in linux-next earlier, give it a good soak and see if
anyone starts to squeal .. or that everything seems to be ok :-)

Would you need a v3 with the acked and reviewed-by from Konrad for x86 in it ?

--

Sander


Monday, February 3, 2014, 3:52:05 PM, you wrote:

> On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
>> Hi Bjorn / Tony,
>> 
>> I fixed up ia64 as well and brought it inline again with the x86 code,
>> but i don't have a ia64 machine, so that part is untested.
>> Perhaps Tony is able to review/test it ?
>> 
>> Sander
>> 
>> 
>> 
>> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
>> prevents it from reading it's own rom. It will get the content of the shadowrom
>> at C000 instead, which is of the primary VGA card and the driver of the
>> secondary card will bail out.
>> 
>> Fix this by checking if the arch code or vga-arbitration has already
>> determined the vga_default_device, if so only apply the fix to this
>> primary video device and let the comment reflect this.
>> 
>> v2:
>>     - Fix pci_fixup_video both in x86 and ia64
>> 
>> 
>> Sander Eikelenboom (1):
>>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
>>     primary     prevents it from reading it's own rom. It will get the
>>     content of the shadowrom     at C000 instead, which is of the
>>     primary VGA card and the driver of the     secondary card will bail
>>     out.

> Your editor mutilated your subject line. It ought to have been just
> one line.

> Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
> on the patch for the x86 part.

> The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
> so I can't give it a 'Tested-by' yet.

>> 
>>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
>>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
>>  2 files changed, 23 insertions(+), 19 deletions(-)
>> 
>> -- 
>> 1.7.10.4
>> 



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out.
  2014-02-13  9:48                     ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Sander Eikelenboom
@ 2014-02-14 20:26                       ` Bjorn Helgaas
  -1 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2014-02-14 20:26 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: Konrad Rzeszutek Wilk, Tony Luck, Dave Airlie, Eiichiro Oiwa,
	Greg Kroah-Hartman, linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

On Thu, Feb 13, 2014 at 10:48:24AM +0100, Sander Eikelenboom wrote:
> Hi Bjorn,
> 
> I have given it another email and another week, but without gaining any reviewed or acked-by's.
> It seems the only way forward is to shovel it in linux-next earlier, give it a good soak and see if
> anyone starts to squeal .. or that everything seems to be ok :-)
> 
> Would you need a v3 with the acked and reviewed-by from Konrad for x86 in it ?

No need, I applied this to my pci/misc branch for v3.15.

Tony, let me know if you have any issues with this.

Bjorn

> Monday, February 3, 2014, 3:52:05 PM, you wrote:
> 
> > On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
> >> Hi Bjorn / Tony,
> >> 
> >> I fixed up ia64 as well and brought it inline again with the x86 code,
> >> but i don't have a ia64 machine, so that part is untested.
> >> Perhaps Tony is able to review/test it ?
> >> 
> >> Sander
> >> 
> >> 
> >> 
> >> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
> >> prevents it from reading it's own rom. It will get the content of the shadowrom
> >> at C000 instead, which is of the primary VGA card and the driver of the
> >> secondary card will bail out.
> >> 
> >> Fix this by checking if the arch code or vga-arbitration has already
> >> determined the vga_default_device, if so only apply the fix to this
> >> primary video device and let the comment reflect this.
> >> 
> >> v2:
> >>     - Fix pci_fixup_video both in x86 and ia64
> >> 
> >> 
> >> Sander Eikelenboom (1):
> >>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
> >>     primary     prevents it from reading it's own rom. It will get the
> >>     content of the shadowrom     at C000 instead, which is of the
> >>     primary VGA card and the driver of the     secondary card will bail
> >>     out.
> 
> > Your editor mutilated your subject line. It ought to have been just
> > one line.
> 
> > Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
> > on the patch for the x86 part.
> 
> > The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
> > so I can't give it a 'Tested-by' yet.
> 
> >> 
> >>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
> >>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
> >>  2 files changed, 23 insertions(+), 19 deletions(-)
> >> 
> >> -- 
> >> 1.7.10.4
> >> 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents
@ 2014-02-14 20:26                       ` Bjorn Helgaas
  0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2014-02-14 20:26 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: Konrad Rzeszutek Wilk, Tony Luck, Dave Airlie, Eiichiro Oiwa,
	Greg Kroah-Hartman, linux-kernel @ vger . kernel . org,
	linux-pci @ vger . kernel . org, linux-ia64

On Thu, Feb 13, 2014 at 10:48:24AM +0100, Sander Eikelenboom wrote:
> Hi Bjorn,
> 
> I have given it another email and another week, but without gaining any reviewed or acked-by's.
> It seems the only way forward is to shovel it in linux-next earlier, give it a good soak and see if
> anyone starts to squeal .. or that everything seems to be ok :-)
> 
> Would you need a v3 with the acked and reviewed-by from Konrad for x86 in it ?

No need, I applied this to my pci/misc branch for v3.15.

Tony, let me know if you have any issues with this.

Bjorn

> Monday, February 3, 2014, 3:52:05 PM, you wrote:
> 
> > On Fri, Jan 31, 2014 at 10:28:22AM +0100, Sander Eikelenboom wrote:
> >> Hi Bjorn / Tony,
> >> 
> >> I fixed up ia64 as well and brought it inline again with the x86 code,
> >> but i don't have a ia64 machine, so that part is untested.
> >> Perhaps Tony is able to review/test it ?
> >> 
> >> Sander
> >> 
> >> 
> >> 
> >> Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary
> >> prevents it from reading it's own rom. It will get the content of the shadowrom
> >> at C000 instead, which is of the primary VGA card and the driver of the
> >> secondary card will bail out.
> >> 
> >> Fix this by checking if the arch code or vga-arbitration has already
> >> determined the vga_default_device, if so only apply the fix to this
> >> primary video device and let the comment reflect this.
> >> 
> >> v2:
> >>     - Fix pci_fixup_video both in x86 and ia64
> >> 
> >> 
> >> Sander Eikelenboom (1):
> >>   Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the
> >>     primary     prevents it from reading it's own rom. It will get the
> >>     content of the shadowrom     at C000 instead, which is of the
> >>     primary VGA card and the driver of the     secondary card will bail
> >>     out.
> 
> > Your editor mutilated your subject line. It ought to have been just
> > one line.
> 
> > Anyhow, you can also add 'Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com'
> > on the patch for the x86 part.
> 
> > The ia64 "looks" OK to me, but my ia64 box won't boot v3.11 or later
> > so I can't give it a 'Tested-by' yet.
> 
> >> 
> >>  arch/ia64/pci/fixup.c |   24 +++++++++++++-----------
> >>  arch/x86/pci/fixup.c  |   18 ++++++++++--------
> >>  2 files changed, 23 insertions(+), 19 deletions(-)
> >> 
> >> -- 
> >> 1.7.10.4
> >> 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2014-02-14 20:26 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-12  4:10 In "pci_fixup_video" check if this is or should be the primary video device to prevent setting the IORESOURCE_ROM_SHADOW flag on a secondary VGA card Sander Eikelenboom
2014-01-12  4:10 ` [PATCH] " Sander Eikelenboom
     [not found] ` <XNM1$9$0$4$$3$3$7$A$9006858U52d4d109@hitachi.com>
2014-01-14 15:26   ` In "pci_fixup_video" check if this is or should be the primary video device t Sander Eikelenboom
     [not found]     ` <XNM1$9$0$4$$3$3$7$A$9006860U52d614ca@hitachi.com>
2014-01-15  8:40       ` In "pci_fixup_video" check if this is or should be the primary video devi Sander Eikelenboom
2014-01-15 19:36       ` Sander Eikelenboom
2014-01-15 21:50         ` Bjorn Helgaas
2014-01-15 22:25           ` Sander Eikelenboom
     [not found]             ` <XNM1$9$0$4$$3$3$7$A$9006862U52d758ed@hitachi.com>
2014-01-16 13:19               ` In "pci_fixup_video" check if this is or should be the primary video d Bjorn Helgaas
2014-01-16 13:19                 ` [Qemu-devel] " Bjorn Helgaas
2014-01-30  0:08             ` In "pci_fixup_video" check if this is or should be the primary video devi Bjorn Helgaas
2014-01-31  9:28               ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out Sander Eikelenboom
2014-01-31  9:28                 ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f Sander Eikelenboom
2014-01-31  9:28                 ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out Sander Eikelenboom
2014-01-31  9:28                   ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it f Sander Eikelenboom
2014-02-03 14:52                 ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out Konrad Rzeszutek Wilk
2014-02-03 14:52                   ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Konrad Rzeszutek Wilk
2014-02-07 11:03                   ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out Sander Eikelenboom
2014-02-07 11:03                     ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Sander Eikelenboom
2014-02-13  9:48                   ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out Sander Eikelenboom
2014-02-13  9:48                     ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Sander Eikelenboom
2014-02-14 20:26                     ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents it from reading it's own rom. It will get the content of the shadowrom at C000 instead, which is of the primary VGA card and the driver of the secondary card will bail out Bjorn Helgaas
2014-02-14 20:26                       ` [PATCH v2] Setting the IORESOURCE_ROM_SHADOW flag on a VGA card other than the primary prevents Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.