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

[Laszlo Ersek]

On 02/02/16 05:15, Alex Williamson wrote:
> The proposed IGD OpRegion support in QEMU via vfio maps the host
> OpRegion into VM system memory at the address written to the ASL
> Storage register (0xFC).  The OpRegion contains a 16-byte signature
> followed by a 4-byte size field.  Therefore SeaBIOS can allocate a
> buffer of the typical size (8KB), this results in a matching e820
> reserved entry for the range, then write the buffer address to the ASL
> Storage register, blinking the OpRegion into the VM address space.  At
> that point SeaBIOS can validate the signature and size and remap if
> necessary.  If the OpRegion is larger than 8KB, this would result in
> any conflicting ranges being temporarily mapped with the OpRegion,
> which probably needs further discussion on what that could break.
> Other options might be to use the same algorithm with an obscenely
> sized initial buffer to make sure we do not overlap, always
> re-allocating the proper sized buffer, or perhaps we could pass the
> OpRegion itself or information about the OpRegion through fw_cfg.
> 
> With the posted kernel series[1] and QEMU series[2] (on top of Gerd's
> IGD patches[3]), this makes the OpRegion available to the VM and
> tracing shows that the guest driver does access it.
> 
> [1] https://lkml.org/lkml/2016/2/1/884
> [2] https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg00202.html
> [3] https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00244.html
> 
> Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
> ---
>  src/fw/pciinit.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
> index c31c2fa..4f3251e 100644
> --- a/src/fw/pciinit.c
> +++ b/src/fw/pciinit.c
> @@ -257,6 +257,52 @@ 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)
> +{
> +    u16 bdf = dev->bdf;
> +    u32 orig;
> +    void *opregion;
> +    int size = 8;
> +
> +    if (!CONFIG_QEMU)
> +        return;
> +
> +    orig = pci_config_readl(bdf, 0xFC);
> +
> +realloc:
> +    opregion = malloc_high(size * 1024);
> +    if (!opregion) {
> +        warn_noalloc();
> +        return;
> +    }
> +
> +    /*
> +     * QEMU maps the OpRegion into system memory at the address written here,
> +     * this overlaps our malloc, which marks the range e820 reserved.
> +     */
> +    pci_config_writel(bdf, 0xFC, cpu_to_le32((u32)opregion));
> +
> +    if (memcmp(opregion, "IntelGraphicsMem", 16)) {
> +        pci_config_writel(bdf, 0xFC, orig);
> +        free(opregion);
> +        return; /* the opregion didn't magically appear, not supported */
> +    }
> +
> +    if (size == le32_to_cpu(*(u32 *)(opregion + 16))) {
> +        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));
> +        return; /* success! */
> +    }
> +
> +    pci_config_writel(bdf, 0xFC, orig);
> +    free(opregion);
> +
> +    if (size == 8) { /* try once more with a new size */
> +        size = le32_to_cpu(*(u32 *)(opregion + 16));
> +        goto realloc;

Is this idiomatic SeaBIOS coding style?

How about "for (;;)" -- the code has many instances -- and reworking
this last branch accordingly?

(Apologies, not a very important comment.)

Thanks
Laszlo

> +    }
> +}
> +
>  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 +336,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 at seabios.org
> http://www.seabios.org/mailman/listinfo/seabios
>