Re: [PATCH v3] vfio/igd: Make MTL/ARL guests fall back to BAR-based framebuffer access

[Tomita Moeko <tomitamoeko@gmail.com>]

On 2026-09-21 15:38, Yuan Wang wrote:
> Meteor Lake and Arrow Lake GOP can read BAR0 MMIO offset 0x138914 to
> detect whether direct framebuffer access via DSM is available.
> 
> In a VFIO passthrough VM, the guest cannot access the host DSM memory
> region via such a pointer. If the guest GOP enables that path, it may
> hand off a DSM-based framebuffer address that the guest cannot actually
> use.
> 
> Intercept reads from the detection register and return 0 so the guest
> does not enable the DSM-based path and instead keeps using the standard
> BAR-based framebuffer address.
> 
> Suggested-by: Tomita Moeko <tomitamoeko@gmail.com>
> Reviewed-by: Bosheng Xue <bosheng.xue@intel.com>
> Reviewed-by: Junjie Cao <junjie.cao@intel.com>
> Signed-off-by: Yuan Wang <yuan1.wang@intel.com>
> ---
> Changes:
> v3:
> - Emulated register 0x138914 to return 0x0, which signals the guest
>   GOP driver to fall back to BAR-based mapping since DSM is
>   unavailable in the VM.
> 
> v2:
> - Resending because the v1 patch was sent with an incorrect future
>   system timestamp due to an unsynchronized local clock. No code
>   changes.
> 
> Notes:
> MTL/ARL device IDs remain in igd_gen().
> This is required because vfio_probe_igd_bar0_quirk() checks the
> generation before installing BAR0 quirks, and the 0x138914 quirk
> is added afterward.
> Meanwhile, we also plan to remove the bdsm-size check in the OVMF
> offline patches.
> ---
>  hw/vfio/igd.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 413a49aae9..f6a2eab805 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -96,6 +96,8 @@ static int igd_gen(VFIOPCIDevice *vdev)
>      case 0x4C00:    /* Rocket Lake */
>      case 0x4600:    /* Alder Lake */
>      case 0xA700:    /* Raptor Lake */
> +    case 0x7D00:    /* Meteor Lake / Arrow Lake */
> +    case 0xB600:    /* Meteor Lake */
>          return 12;
>      }
>  

0xB600 is for Arrow Lake according to include/drm/intel/pciids.h.

    #define INTEL_ARL_S_IDS(MACRO__, ...) \
        MACRO__(0x7D67, ## __VA_ARGS__), \
        MACRO__(0xB640, ## __VA_ARGS__)


Meteor Lake is Gen 12.70, while igd_gen() only tracks the major generation.
Here Meteor/Arrow Lake and existing devices are all Gen 12, but different
quirks are applied, making it look a bit awkward.

Maybe the generation could be extended to 4 digits, or use flags instead.

    case 0x4600:    /* Alder Lake */
    case 0xA700:    /* Raptor Lake */
        return 1200;
        return IGD_FLAG_GMCH_GEN8 | IGD_FLAG_GMS_GEN9 | IGD_FLAG_QUIRK_BDSM_GEN11;
    case 0x7D00:    /* Meteor Lake / Arrow Lake */
    case 0xB600:    /* Arrow Lake */
        return 1270;
        return IGD_FLAG_QUIRK_MMIO_138914;

Anyway, this is outside the scope of this patch, just a note for possible
future cleanup.

> @@ -454,10 +456,32 @@ static bool vfio_pci_igd_override_gms(int gen, uint32_t gms, uint32_t *gmch)
>  
>  #define IGD_GGC_MMIO_OFFSET     0x108040
>  #define IGD_BDSM_MMIO_OFFSET    0x1080C0
> +#define IGD_BDSM_ACCESS         0x138914

nit: perhaps rename this to IGD_MTL_PCODE_STOLEN_ACCESS to match i915.

> +
> +#define IGD_IS_MTL_OR_ARL(vdev) \
> +    ((((vdev)->device_id & 0xff00) == 0x7d00) || \
> +     (((vdev)->device_id & 0xff00) == 0xb600))
> +
> +static uint64_t vfio_igd_bdsm_access_read(void *opaque, hwaddr addr,
> +                                          unsigned size)
> +{
> +    return 0;
> +}
> +
> +static void vfio_igd_bdsm_access_write(void *opaque, hwaddr addr,
> +                                       uint64_t data, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps vfio_igd_bdsm_access_quirk = {
> +    .read = vfio_igd_bdsm_access_read,
> +    .write = vfio_igd_bdsm_access_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
>  
>  void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
>  {
> -    VFIOQuirk *ggc_quirk, *bdsm_quirk;
> +    VFIOQuirk *ggc_quirk, *bdsm_quirk, *bdsm_access_quirk;
>      VFIOConfigMirrorQuirk *ggc_mirror, *bdsm_mirror;
>      int gen;
>  
> @@ -472,6 +496,23 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
>          return;
>      }
>  
> +    /*
> +     * MTL/ARL guests must keep using the BAR-based framebuffer address.
> +     * Return 0 at 0x138914 so GOP stays on the standard access path in the VM.
> +     */
> +    if (IGD_IS_MTL_OR_ARL(vdev)) {
> +        bdsm_access_quirk = vfio_quirk_alloc(1);
> +        memory_region_init_io(bdsm_access_quirk->mem, OBJECT(vdev),
> +                              &vfio_igd_bdsm_access_quirk, vdev,
> +                              "vfio-igd-bdsm-access-quirk", 4);
> +        memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
> +                                            IGD_BDSM_ACCESS,
> +                                            bdsm_access_quirk->mem,
> +                                            1);
> +
> +        QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_access_quirk, next);
> +    }
> +
>      if (vdev->igd_gms) {
>          ggc_quirk = vfio_quirk_alloc(1);
>          ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));

Overall it looks good to me, with the minor nits above.

Reviewed-by: Tomita Moeko <tomitamoeko@gmail.com>

Thanks,
Moeko