Re: [Qemu-devel] [PATCH] Revert "vfio/pci-quirks.c: Disable stolen memory for igd VFIO"

[Alex Williamson]

On Thu, 30 Mar 2017 18:27:21 +0800
Xiong Zhang <address@hidden> wrote:

> This reverts commit c2b2e158cc7b1cb431bd6039824ec13c3184a775.
> 
> The original patch intend to prevent linux i915 driver from using
> stolen meory. But this patch breaks windows IGD driver loading on
> Gen9+, as IGD HW will use stolen memory on Gen9+, once windows IGD
> driver see zero size stolen memory, it will unload.
> Meanwhile stolen memory will be disabled in 915 when i915 run as
> a guest.

Does this mean that legacy mode IGD assignment is not going to work
on Gen9+ with Windows?  Will it continue to work with Gen8-?

Please clarify Gen9+, is this Kaby Lake?

I assume this patch is intended for QEMU 2.9, it's helpful to make that
explicit during the rc freeze.  Thanks,

Alex

> Signed-off-by: Xiong Zhang <address@hidden>
> ---
>  hw/vfio/pci-quirks.c | 65 
> ++++++++++++++++++++++------------------------------
>  1 file changed, 27 insertions(+), 38 deletions(-)
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index e995e32..e9b493b 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1367,45 +1367,14 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
> *vdev, int nr)
>      uint16_t cmd_orig, cmd;
>      Error *err = NULL;
>  
> -    /* This must be an Intel VGA device. */
> -    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> -        !vfio_is_vga(vdev) || nr != 4) {
> -        return;
> -    }
> -
> -    /*
> -     * IGD is not a standard, they like to change their specs often.  We
> -     * only attempt to support back to SandBridge and we hope that newer
> -     * devices maintain compatibility with generation 8.
> -     */
> -    gen = igd_gen(vdev);
> -    if (gen != 6 && gen != 8) {
> -        error_report("IGD device %s is unsupported by IGD quirks, "
> -                     "try SandyBridge or newer", vdev->vbasedev.name);
> -        return;
> -    }
> -
> -    /*
> -     * Regardless of running in UPT or legacy mode, the guest graphics
> -     * driver may attempt to use stolen memory, however only legacy mode
> -     * has BIOS support for reserving stolen memory in the guest VM.
> -     * Emulate the GMCH register in all cases and zero out the stolen
> -     * memory size here. Legacy mode may request allocation and re-write
> -     * this below.
> -     */
> -    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> -    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> -
> -    /* GMCH is read-only, emulated */
> -    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> -    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> -    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> -
>      /*
> -     * This must be at address 00:02.0 for us to even onsider enabling
> -     * legacy mode.  The vBIOS has dependencies on the PCI bus address.
> +     * This must be an Intel VGA device at address 00:02.0 for us to even
> +     * consider enabling legacy mode.  The vBIOS has dependencies on the
> +     * PCI bus address.
>       */
> -    if (&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
> +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> +        !vfio_is_vga(vdev) || nr != 4 ||
> +        &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
>                                         0, PCI_DEVFN(0x2, 0))) {
>          return;
>      }
> @@ -1425,6 +1394,18 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
> *vdev, int nr)
>      }
>  
>      /*
> +     * IGD is not a standard, they like to change their specs often.  We
> +     * only attempt to support back to SandBridge and we hope that newer
> +     * devices maintain compatibility with generation 8.
> +     */
> +    gen = igd_gen(vdev);
> +    if (gen != 6 && gen != 8) {
> +        error_report("IGD device %s is unsupported in legacy mode, "
> +                     "try SandyBridge or newer", vdev->vbasedev.name);
> +        return;
> +    }
> +
> +    /*
>       * Most of what we're doing here is to enable the ROM to run, so if
>       * there's no ROM, there's no point in setting up this quirk.
>       * NB. We only seem to get BIOS ROMs, so a UEFI VM would need CSM 
> support.
> @@ -1479,6 +1460,8 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
> *vdev, int nr)
>          goto out;
>      }
>  
> +    gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
> +
>      /*
>       * If IGD VGA Disable is clear (expected) and VGA is not already enabled,
>       * try to enable it.  Probably shouldn't be using legacy mode without 
> VGA,
> @@ -1549,11 +1532,12 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
> *vdev, int nr)
>       * when IVD (IGD VGA Disable) is clear, but the claim is that it's 
> unused,
>       * so let's not waste VM memory for it.
>       */
> +    gmch &= ~((gen < 8 ? 0x1f : 0xff) << (gen < 8 ? 3 : 8));
> +
>      if (vdev->igd_gms) {
>          if (vdev->igd_gms <= 0x10) {
>              gms_mb = vdev->igd_gms * 32;
>              gmch |= vdev->igd_gms << (gen < 8 ? 3 : 8);
> -            pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
>          } else {
>              error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
>              vdev->igd_gms = 0;
> @@ -1573,6 +1557,11 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice 
> *vdev, int nr)
>      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
>                      bdsm_size, sizeof(*bdsm_size));
>  
> +    /* GMCH is read-only, emulated */
> +    pci_set_long(vdev->pdev.config + IGD_GMCH, gmch);
> +    pci_set_long(vdev->pdev.wmask + IGD_GMCH, 0);
> +    pci_set_long(vdev->emulated_config_bits + IGD_GMCH, ~0);
> +
>      /* BDSM is read-write, emulated.  The BIOS needs to be able to write it 
> */
>      pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
>      pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);