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

[Alex Williamson]

On Thu, 4 Feb 2016 17:58:23 +0100
Igor Mammedov <address@hidden> wrote:

> On Tue, 02 Feb 2016 13:10:37 -0700
> Alex Williamson <address@hidden> wrote:
> 
> > When assigning Intel IGD graphics via QEMU/vfio, the OpRegion for
> > the device may be exposed as a fw_cfg file.  Allocate space for
> > this, copy the contents and write the ASL Storage register (0xFC)
> > to point to this buffer.  NB, it's possible for QEMU to use the
> > write to the ASL Storage register to map access to the host
> > OpRegion overlapping the allocated buffer, but we shouldn't care if
> > it does.
> > 
> > References:
> > kernel vfio opregion support:
> > https://lkml.org/lkml/2016/2/1/884
> > QEMU vfio opregion support (revised v2 of 7/7 adds fw_cfg):
> > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00202.html
> > Gerd's IGD assignment series:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00244.html
> > 
> > Signed-off-by: Alex Williamson <address@hidden>
> > ---
> >  src/fw/pciinit.c |   30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> > index c31c2fa..92170d5 100644
> > --- a/src/fw/pciinit.c
> > +++ b/src/fw/pciinit.c
> > @@ -257,6 +257,32 @@ static void ich9_smbus_setup(struct pci_device
> > *dev, void *arg) pci_config_writeb(bdf, ICH9_SMB_HOSTC,
> > ICH9_SMB_HOSTC_HST_EN); }
> >  
> > +static void intel_igd_opregion_setup(struct pci_device *dev, void
> > *arg) +{
> > +    struct romfile_s *file = romfile_find("etc/igd-opregion");
> > +    void *opregion;
> > +    u16 bdf = dev->bdf;
> > +
> > +    if (!file || !file->size)
> > +        return;
> > +
> > +    opregion = memalign_high(PAGE_SIZE, file->size);
> > +    if (!opregion) {
> > +        warn_noalloc();
> > +        return;
> > +    }
> > +
> > +    if (file->copy(file, opregion, file->size) < 0) {  
> Is opregion content on host immutable?
> if not then copying it probably wrong and it should be passed-through.

The content is not immutable, but for the first round of things that
we're interested in, it probably is.  It's not clear that we'll ever
move beyond that first level though.  Part of the benefit of this
approach is that SeaBIOS allocates the correct size, copies a static
version of the OpRegion data into place, then effectively tells QEMU
that it has done this by writing to the ASL Storage register.  At that
point QEMU can simply virtualize the register for the guest or it can
map a live version of the OpRegion over top of the SeaBIOS copy.  So we
certainly have the option to go beyond an immutable copy with no
further change to SeaBIOS.  Thanks,

Alex

> > +        free(opregion);
> > +        return;
> > +    }
> > +
> > +    pci_config_writel(bdf, 0xFC, cpu_to_le32((u32)opregion));
> > +
> > +    dprintf(1, "Intel IGD OpRegion enabled on %02x:%02x.%x\n",
> > +            pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf),
> > pci_bdf_to_fn(bdf)); +}
> > +
> >  static const struct pci_device_id pci_device_tbl[] = {
> >      /* PIIX3/PIIX4 PCI to ISA bridge */
> >      PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82371SB_0,
> > @@ -290,6 +316,10 @@ static const struct pci_device_id
> > pci_device_tbl[] = { PCI_DEVICE_CLASS(PCI_VENDOR_ID_APPLE, 0x0017,
> > 0xff00, apple_macio_setup), PCI_DEVICE_CLASS(PCI_VENDOR_ID_APPLE,
> > 0x0022, 0xff00, apple_macio_setup), 
> > +    /* Intel IGD OpRegion setup */
> > +    PCI_DEVICE_CLASS(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> > PCI_CLASS_DISPLAY_VGA,
> > +                     intel_igd_opregion_setup),
> > +
> >      PCI_DEVICE_END,
> >  };
> >  
> > 
> > 
> > _______________________________________________
> > SeaBIOS mailing list
> > address@hidden
> > http://www.seabios.org/mailman/listinfo/seabios  
>