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

2016-02-04 Thread Alex Williamson
On Thu, 4 Feb 2016 17:58:23 +0100
Igor Mammedov  wrote:

> On Tue, 02 Feb 2016 13:10:37 -0700
> Alex Williamson  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 
> > ---
> >  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
> > SeaBIOS@seabios.org
> > http://www.seabios.org/mailman/listinfo/seabios  
> 


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2016-02-04 Thread Igor Mammedov
On Tue, 02 Feb 2016 13:10:37 -0700
Alex Williamson  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 
> ---
>  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.

> +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
> SeaBIOS@seabios.org
> http://www.seabios.org/mailman/listinfo/seabios


___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios


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

2016-02-04 Thread Kevin O'Connor
On Thu, Feb 04, 2016 at 08:52:44AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > 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,
> 
> Yea, lets stick with the plain simple "igd-opregion".  Even if intel
> allows multiple igd devices per host some day things continue to work as
> long as you assign only one of them to a guest.

Sounds good.  The patch looks fine to me.  I do think we should commit
to SeaBIOS only after the QEMU support is accepted though.

-Kevin

___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios