From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56410) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3fgu-0001Nn-CJ for qemu-devel@nongnu.org; Thu, 16 Jan 2014 00:40:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W3fgn-0002Br-LV for qemu-devel@nongnu.org; Thu, 16 Jan 2014 00:40:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44842) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3fgn-0002B7-BN for qemu-devel@nongnu.org; Thu, 16 Jan 2014 00:40:25 -0500 Date: Thu, 16 Jan 2014 07:40:06 +0200 From: "Michael S. Tsirkin" Message-ID: <20140116054006.GA8915@redhat.com> References: <9310049934.20140115113729@eikelenboom.it> <20140115115820.GA2167@redhat.com> <79913664.20140115141449@eikelenboom.it> <20140115190315.GA6661@redhat.com> <1177074114.20140115201924@eikelenboom.it> <812514405.20140115233529@eikelenboom.it> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <812514405.20140115233529@eikelenboom.it> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] Secondary VGA passthrough not working, hitting linux kernel's pci_fixup_video List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sander Eikelenboom Cc: Bjorn Helgaas , David Airlie , Jesse Barnes , "qemu-devel@nongnu.org" , Anthony Liguori On Wed, Jan 15, 2014 at 11:35:29PM +0100, Sander Eikelenboom wrote: >=20 > Wednesday, January 15, 2014, 11:10:35 PM, you wrote: >=20 > > [+cc Jesse, David] >=20 > > On Wed, Jan 15, 2014 at 12:19 PM, Sander Eikelenboom > > wrote: >=20 > >>>> >> I understood there is no bridge for a VGA device in you= r virtual machine. > >>>> >> Your emulator for the bridge control register is odd. > >>>> > >>>> > That seems beside the point as there's no bridge here. > >>>> > >>>> >> I guess your virtual machine ignore "PCI-to-PCI Bridge = Architecture > >>>> >> Specification". > >>>> > >>>> > Since there's no pci to pci bridge in this VM, why would this > >>>> > specification apply? >=20 > > My guess is that he is referring to sec 11.3.2 "Initialization > > Algorithm" of the Bridge spec r1.2. That section (and chapter 24 of > > MindShare PCI System Architecture, 4th edition) apparently reiterate = a > > discovery algorithm that appears in the PCI spec. I can't find that > > algorithm in the 3.0 or 2.3 PCI specs, but I assume it was in an > > earlier version. >=20 > >> Yes i have done some more reading .. from what i could find from PCI= specs there is nothing in there > >> that points to a specific boot vga card. > >> The whole agp/pci/pcie bios selection is ACPI stuff .. but if all th= e cards are of the same type .. > >> there is no way to specify a specific card. >=20 > > The algorithm from sec 11.3.2 *does* seem to give a way to identify a > > specific boot VGA card, i.e., the first one you find when searching i= n > > the specified order. There are still issues, such as the fact that > > 11.3.2 says to start at PCI bus 0, while MindShare says to start with > > the largest bus number. >=20 > > Also, sec 12.1.2.1 of the Bridge spec says the PCI_COMMAND memory and > > I/O enables do apply to VGA accesses, so that is potentially a way to > > make two sibling VGA devices work, e.g., by toggling those bits to > > control which device you want to talk to. I also assume that when > > identifying the "boot VGA device," one might ignore devices that the > > BIOS left disabled. >=20 > >>>> >> 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- VGASn= oop- ParErr- Stepping- SERR- FastB2B- DisINTx- > >>>> >> Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dfast= >TAbort- SERR- >>>> >> Latency: 0 > >>>> >> Region 0: Memory at f0000000 (32-bit, prefetchable) [si= ze=3D32M] > >>>> >> Region 1: Memory at f30b0000 (32-bit, non-prefetchable)= [size=3D4K] > >>>> >> Expansion ROM at f30a0000 [disabled] [size=3D64K] > >>>> >> > >>>> >> 00:05.0 VGA compatible controller [0300]: Advanced Micro Device= s [AMD] nee ATI Turks [Radeon HD 6570] [1002:6759] (prog-if 00 [VGA contr= oller]) > >>>> >> Subsystem: PC Partner Limited Device [174b:e193] > >>>> >> Physical Slot: 5 > >>>> >> Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASn= oop- ParErr- Stepping- SERR- FastB2B- DisINTx+ > >>>> >> Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dfast= >TAbort- SERR- >>>> >> Latency: 0 > >>>> >> Interrupt: pin A routed to IRQ 81 > >>>> >> Region 0: Memory at e0000000 (64-bit, prefetchable) [si= ze=3D256M] > >>>> >> Region 2: Memory at f3060000 (64-bit, non-prefetchable)= [size=3D128K] > >>>> >> Region 4: I/O ports at c100 [size=3D256] > >>>> >> Expansion ROM at f3080000 [disabled] [size=3D128K] >=20 > > I am not an expert on this legacy stuff, but this looks like an > > illegal configuration to me: both 00:03.0 and 00:05.0 claim to be VGA > > controllers and both have I/O+ and Mem+, so I would think both would > > respond to the legacy VGA addresses. >=20 > Me neither .. and all pci specs seem to be paywalled, but apart from th= is > the current code of pci_video_fixup doesn't check for I/O+ and Mem+ ei= ther. I see this: pci_read_config_word(pdev, PCI_COMMAND, &config); if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) { pdev->resource[PCI_ROM_RESOURCE].flags |=3D IORESOURCE_RO= M_SHADOW; so it checks that either IO+ or Mem+ is enabled. > =C1nd as i pointed out the kernel at this point has already set vga_def= ault_device (from what it seems > indeed the first device from a ordered scan). So if there should be a b= etter detection mechanism i think it should > be implemented at what sets vga_default_device .. and pci_fixup_video s= hould adhere to that (or fixup if it's still not set) > And that's essentially what the patch does .. and it also doesn't seem = to alter anything for the cases it was implemented for > of i read the comment and commit messages correctly. >=20 > -- > Sander