From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56354) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aR7Dy-0007gO-Lm for qemu-devel@nongnu.org; Wed, 03 Feb 2016 18:52:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aR7Dv-0001gT-D5 for qemu-devel@nongnu.org; Wed, 03 Feb 2016 18:52:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55947) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aR7Dv-0001gO-7N for qemu-devel@nongnu.org; Wed, 03 Feb 2016 18:52:35 -0500 Message-ID: <1454543553.2989.3.camel@redhat.com> From: Alex Williamson Date: Wed, 03 Feb 2016 16:52:33 -0700 In-Reply-To: <20160203220924.GA10055@morn.lan> References: <20160202201023.5353.65948.stgit@gimli.home> <1454490248.4967.51.camel@redhat.com> <1454528580.18969.10.camel@redhat.com> <1454535527.2989.1.camel@redhat.com> <20160203220924.GA10055@morn.lan> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [SeaBIOS] [RFC PATCH v2] fw/pci: Add support for mapping Intel IGD OpRegion via QEMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin O'Connor Cc: seabios@seabios.org, Gerd Hoffmann , qemu-devel@nongnu.org On Wed, 2016-02-03 at 17:09 -0500, Kevin O'Connor wrote: > On Wed, Feb 03, 2016 at 02:38:47PM -0700, Alex Williamson wrote: > > On Wed, 2016-02-03 at 12:43 -0700, Alex Williamson wrote: > > > On Wed, 2016-02-03 at 10:04 +0100, Gerd Hoffmann wrote: > > > > =C2=A0 Hi, > > > > =C2=A0 > > > > > +static void intel_igd_opregion_setup(struct pci_device *dev, v= oid *arg) > > > > > +{ > > > > > +=C2=A0=C2=A0=C2=A0=C2=A0struct romfile_s *file =3D romfile_fin= d("etc/igd-opregion"); > > > > =C2=A0 > > > > Is it possible to have multiple igd devices in a single machine? > > > > So, should we include the pci address in the file name? > > > > =C2=A0 > > > > Guess not needed, it's chipset graphics after all ... > > > =C2=A0 > > > Hmm, I think that's probably a pretty good observation, we don't wa= nt to > > > revisit this if vGPUs need/want an OpRegion or if Intel decides to = start > > > allowing more than one per system.=C2=A0=C2=A0Either could pretty e= asily introduce > > > multiple into a VM. > >=C2=A0 > > Naming is always more complicated than it seems.=C2=A0=C2=A0For anyth= ing other > > than a root bus devices, the PCI address doesn't exist until SeaBIOS > > enumerates devices and assigns bus numbers for bridges.=C2=A0=C2=A0So= unless we > > want to provide a path to the device like ACPI defines, maybe we shou= ld > > just stick with "etc/igd-opregion".=C2=A0=C2=A0It seems easily extens= ible to add > > more specific files later and default to this one if those aren't fou= nd. >=C2=A0 > Perhaps a simpler solution is to just make sure "etc/igd-opregion" is > only deployed for the "active" VGA device (ie, the device that > is_pci_vga() returns true for).=C2=A0=C2=A0Looks like pci_enable_defaul= t_vga() > already has code for something similar. Comments we've seen from Intel suggest they prefer assigning IGD as a secondary VGA device though, so the "active" VGA device might not be IGD at all.=C2=A0=C2=A0I'd also be surprised if QEMU ever sets an active VGA = device itself versus relying on SeaBIOS to do it via pci_enable_default_vga(). Then it seems like we get into the realm of QEMU needing to know in advance how SeaBIOS' search algorithms work to know which is the "next" device.=C2=A0=C2=A0One thing in our favor, maybe, is that there can't be = multiple "etc/igd-opregion" fw_cfg files, QEMU will assert if that happens.=C2=A0=C2= =A0So if we get to SeaBIOS and there are multiple Intel VGA devices, there can only be at most one OpRegion for them and we can either give them all the same or just use it on the first one we encounter.=C2=A0=C2=A0We're p= robably putting way too much thought into this scenario that Intel likely has no expectation of supporting though ;)=C2=A0=C2=A0Thanks, Alex