Re: [Qemu-devel] [SeaBIOS] [RFC PATCH v2] fw/pci: Add support for mapping Intel IGD OpRegion via QEMU

[Alex Williamson]

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:
> > > >   Hi,
> > > >  
> > > > > +static void intel_igd_opregion_setup(struct pci_device *dev, void 
> > > > > *arg)
> > > > > +{
> > > > > +    struct romfile_s *file = romfile_find("etc/igd-opregion");
> > > >  
> > > > Is it possible to have multiple igd devices in a single machine?
> > > > So, should we include the pci address in the file name?
> > > >  
> > > > Guess not needed, it's chipset graphics after all ...
> > >  
> > > Hmm, I think that's probably a pretty good observation, we don't want to
> > > revisit this if vGPUs need/want an OpRegion or if Intel decides to start
> > > allowing more than one per system.  Either could pretty easily introduce
> > > multiple into a VM.
> > 
> > Naming is always more complicated than it seems.  For anything other
> > than a root bus devices, the PCI address doesn't exist until SeaBIOS
> > enumerates devices and assigns bus numbers for bridges.  So unless we
> > want to provide a path to the device like ACPI defines, maybe we should
> > just stick with "etc/igd-opregion".  It seems easily extensible to add
> > more specific files later and default to this one if those aren't found.
> 
> 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).  Looks like pci_enable_default_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.  I'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.  One thing in our favor, maybe, is that there can't be multiple
"etc/igd-opregion" fw_cfg files, QEMU will assert if that happens.  So
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.  We're probably
putting way too much thought into this scenario that Intel likely has no
expectation of supporting though ;)  Thanks,

Alex