Re: [PATCH 9/9] vfio/igd: Remove generation limitation for IGD passthrough

["Corvin Köhne" <C.Koehne@beckhoff.com>]

[-- Attachment #1: Type: text/plain, Size: 8560 bytes --]

On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> Starting from Intel Core Ultra Series (Meteor Lake), Data Stolen Memory
> has became a part of LMEMBAR (MMIO BAR2) [1][2], meaning that BDSM and
> GGC register quirks are no longer needed on these platforms.
> 
> To support Meteor/Arrow/Lunar Lake and future IGD devices, remove the
> generation limitation in IGD passthrough, and apply BDSM and GGC quirks
> only to known Gen6-12 devices.
> 
> [1]
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGeu0AAADvIwGqJzMNfJw1SGDaz4T4NOe0pQjgcXIl8aa6EBISsE_mEsZ1x35XrJZk1uUAVgzAGUkZWFMG--4B2zOo1pBHv9oVATF-lJkWnY1dOUOHqYlAt4T5EfdMCovIk0M0ZxBIgFBnJEE3wNG6NOh1mPjge5-M1OW80X-Dp9n6iSKirvdFiYnh9VLEHlff9BdoT5IJ8JjnKnoVVAT7iuWwkFDayl2MoMIuAKFMrDxfrXsbkPQYuHMP0b_bdAgRcors5TKTBFPsQ1IKC7wICpETvUXKQnqex7TU1gzMwYVj2iEC9PKyiY8RBfLgXlCWhE81
>  
> [2]
> https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGetYAAACNfuvceS_GGt4XbczrWqbmNztfDneI6ldPTQC3yZpB9711r9-sHzWe45iikAQRMr99rOo7ZX_vc6L9JWHxUovjxtqg88ymadBor_RtfcJUH6gTjN4bCIsufZ84hsdPPZ4VCkMbFxROFqERsVxQpR_kPhvdqbni1CwWW3rGeBkifKTUC4rH-OmGNSww_6COlh2arRPZR899bXdYf1SFGjDc6zbSFE36nMBDW9jc3tBgb2VaMcERYIL-TVSLQoYnRxeybcAqQAE41ZyIoFEH8FHyGaikRWLl0
>  
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  docs/igd-assign.txt |  6 +++++
>  hw/vfio/igd.c       | 58 ++++++++++++++++-----------------------------
>  2 files changed, 27 insertions(+), 37 deletions(-)
> 
> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> index fc444503ff..af4e8391fc 100644
> --- a/docs/igd-assign.txt
> +++ b/docs/igd-assign.txt
> @@ -157,6 +157,12 @@ fw_cfg requirements on the VM firmware:
>     it's expected that this fw_cfg file is only relevant to a single PCI
>     class VGA device with Intel vendor ID, appearing at PCI bus address
> 00:02.0.
>  
> +   Starting from Meteor Lake, IGD devices access stolen memory via its MMIO
> +   BAR2 (LMEMBAR) and removed the BDSM register in config space. There is
> +   no need for guest firmware to allocate data stolen memory in guest address
> +   space and write it to BDSM register. Value of this fw_cfg file is 0 in
> +   such case.
> +
>  Upstream Seabios has OpRegion and BDSM (pre-Gen11 device only) support.
>  However, the support is not accepted by upstream EDK2/OVMF. A recommended
>  solution is to create a virtual OpRom with following DXE drivers:
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 5d12f753ab..2584861ae6 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -103,6 +103,7 @@ static int igd_gen(VFIOPCIDevice *vdev)
>      /*
>       * Unfortunately, Intel changes it's specification quite often. This
> makes
>       * it impossible to use a suitable default value for unknown devices.
> +     * Return -1 for not applying any generation-specific quirks.
>       */
>      return -1;
>  }
> @@ -458,20 +459,12 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int
> nr)
>      VFIOConfigMirrorQuirk *ggc_mirror, *bdsm_mirror;
>      int gen;
>  
> -    /*
> -     * This must be an Intel VGA device at address 00:02.0 for us to even
> -     * consider enabling legacy mode. Some driver have dependencies on the
> PCI
> -     * bus address.
> -     */
>      if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
>          !vfio_is_vga(vdev) || nr != 0) {
>          return;
>      }
>  
> -    /*
> -     * Only on IGD devices of gen 11 and above, the BDSM register is mirrored
> -     * into MMIO space and read from MMIO space by the Windows driver.
> -     */
> +    /* Only on IGD Gen6-12 device needs quirks in BAR 0 */
>      gen = igd_gen(vdev);
>      if (gen < 6) {
>          return;
> @@ -518,7 +511,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev,
> Error **errp)
>  {
>      g_autofree struct vfio_region_info *opregion = NULL;
>      int ret, gen;
> -    uint64_t gms_size;
> +    uint64_t gms_size = 0;
>      uint64_t *bdsm_size;
>      uint32_t gmch;
>      bool legacy_mode_enabled = false;
> @@ -535,18 +528,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>      }
>      info_report("OpRegion detected on Intel display %x.", vdev->device_id);
>  
> -    /*
> -     * 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 == -1) {
> -        error_report("IGD device %s is unsupported in legacy mode, "
> -                     "try SandyBridge or newer", vdev-
> >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGemwAAAAwg
> gXVbgmULncwUjdD3WRW6m3nKghz9vudEZhl9xzeICl7FUK5O-
> hjEdzY8nxw3ASLDUPNCoEiymJZadffJUCslCcwoArfPIlRFLV9huLvwU-
> 6mMTuTItplXGJHszjVRgrc7pHIkf98_n1wyM1 );
> -        return true;
> -    }
> -
>      gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
>  
>      /*
> @@ -644,32 +626,34 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
>          }
>      }
>  
> -    gms_size = igd_stolen_memory_size(gen, gmch);
> +    if (gen > 0) {
> +        gms_size = igd_stolen_memory_size(gen, gmch);
> +
> +        /* BDSM is read-write, emulated. BIOS needs to be able to write it */
> +        if (gen < 11) {
> +            pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
> +            pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
> +            pci_set_long(vdev->emulated_config_bits + IGD_BDSM, ~0);
> +        } else {
> +            pci_set_quad(vdev->pdev.config + IGD_BDSM_GEN11, 0);
> +            pci_set_quad(vdev->pdev.wmask + IGD_BDSM_GEN11, ~0);
> +            pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
> +        }
> +    }
>  
>      /*
>       * Request reserved memory for stolen memory via fw_cfg.  VM firmware
>       * must allocate a 1MB aligned reserved memory region below 4GB with
> -     * the requested size (in bytes) for use by the Intel PCI class VGA
> -     * device at VM address 00:02.0.  The base address of this reserved
> -     * memory region must be written to the device BDSM register at PCI
> -     * config offset 0x5C.
> +     * the requested size (in bytes) for use by the IGD device. The base
> +     * address of this reserved memory region must be written to the
> +     * device BDSM register.
> +     * For newer device without BDSM register, this fw_cfg item is 0.
>       */
>      bdsm_size = g_malloc(sizeof(*bdsm_size));
>      *bdsm_size = cpu_to_le64(gms_size);
>      fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
>                      bdsm_size, sizeof(*bdsm_size));
>  
> -    /* BDSM is read-write, emulated.  The BIOS needs to be able to write it
> */
> -    if (gen < 11) {
> -        pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
> -        pci_set_long(vdev->pdev.wmask + IGD_BDSM, ~0);
> -        pci_set_long(vdev->emulated_config_bits + IGD_BDSM, ~0);
> -    } else {
> -        pci_set_quad(vdev->pdev.config + IGD_BDSM_GEN11, 0);
> -        pci_set_quad(vdev->pdev.wmask + IGD_BDSM_GEN11, ~0);
> -        pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
> -    }
> -
>      trace_vfio_pci_igd_bdsm_enabled(vdev-
> >https://nospamproxywebp.beckhoff.com/enQsig/link?id=BAgAAACrZmEBKRIGemwAAAAwg
> gXVbgmULncwUjdD3WRW6m3nKghz9vudEZhl9xzeICl7FUK5O-
> hjEdzY8nxw3ASLDUPNCoEiymJZadffJUCslCcwoArfPIlRFLV9huLvwU-
> 6mMTuTItplXGJHszjVRgrc7pHIkf98_n1wyM1 , (gms_size / MiB));
>  
>      return true;

This commit looks good. However, you haven't tested it yet, right? I'm also
unable to test it. It would be good if someone could test this patch before
merging it.

Not sure what upstream thinks about it. From my side:

Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>


-- 
Kind regards,
Corvin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]