2014-01-12 04:11:14

by Sander Eikelenboom

[permalink] [raw]
Subject: 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

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" <[email protected]>
*
* 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


2014-01-12 04:11:09

by Sander Eikelenboom

[permalink] [raw]
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

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 <[email protected]>
---
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