1. Patchew
  2. QEMU
  3. [PATCH v2 0/9] vfio/igd: Enable legacy mode on more devices
  4. View patch

[PATCH v2 9/9] vfio/igd: add x-igd-gms option back to set DSM region size for guest

Tomita Moeko posted 9 patches 1 year, 2 months ago
There is a newer version of this series
[PATCH v2 9/9] vfio/igd: add x-igd-gms option back to set DSM region size for guest
Posted by Tomita Moeko 1 year, 2 months ago 
DSM region is likely to store framebuffer in Windows, a small DSM
region may cause display issues (e.g. half of the screen is black).
By default, QEMU uses host's original value, which is determined by
DVMT Pre-Allocated option in Intel FSP of host bios. Some vendors
do not expose this config item to users. In such cases, x-igd-gms
option can be used to manually set the data stolen memory size for
guest.

When DVMT Pre-Allocated option is available in host BIOS, user should
set DSM region size there instead of using x-igd-gms option.

Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e464cd6949..0814730f40 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -717,6 +717,23 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
 
     QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next);
 
+    /*
+     * Allow user to override dsm size using x-igd-gms option, in multiples of
+     * 32MiB. This option should only be used when the desired size cannot be
+     * set from DVMT Pre-Allocated option in host BIOS.
+     */
+    if (vdev->igd_gms) {
+        if (gen < 8 && vdev->igd_gms <= 0x10) {
+            gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
+            gmch |= vdev->igd_gms << IGD_GMCH_GEN6_GMS_SHIFT;
+        } else if (vdev->igd_gms <= 0x40) {
+            gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
+            gmch |= vdev->igd_gms << IGD_GMCH_GEN8_GMS_SHIFT;
+        } else {
+            error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
+        }
+    }
+
     ggms_size = igd_gtt_memory_size(gen, gmch);
     gms_size = igd_stolen_memory_size(gen, gmch);
 
-- 
2.45.2
Re: [PATCH v2 9/9] vfio/igd: add x-igd-gms option back to set DSM region size for guest
Posted by Corvin Köhne 1 year, 2 months ago 
On Tue, 2024-12-03 at 21:35 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> DSM region is likely to store framebuffer in Windows, a small DSM
> region may cause display issues (e.g. half of the screen is black).
> By default, QEMU uses host's original value, which is determined by
> DVMT Pre-Allocated option in Intel FSP of host bios. Some vendors
> do not expose this config item to users. In such cases, x-igd-gms
> option can be used to manually set the data stolen memory size for
> guest.
> 
> When DVMT Pre-Allocated option is available in host BIOS, user should
> set DSM region size there instead of using x-igd-gms option.
> 

It might be worth linking the commit which has removed x-igd-gms and mention
that the behaviour changed slightly. Previously, the DSM region size was set to
0 if x-igd-gms was unset. This patch keeps the host value if x-igd-gms is unset.

> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index e464cd6949..0814730f40 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -717,6 +717,23 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int
> nr)
>  
>      QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next);
>  
> +    /*
> +     * Allow user to override dsm size using x-igd-gms option, in multiples
> of
> +     * 32MiB. This option should only be used when the desired size cannot be
> +     * set from DVMT Pre-Allocated option in host BIOS.
> +     */
> +    if (vdev->igd_gms) {
> +        if (gen < 8 && vdev->igd_gms <= 0x10) {

This doesn't work as intended for values larger than 0x10. For those values,
qemu ignores the generation and set GMS as it would be a gen 8 or later device.

> +            gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
> +            gmch |= vdev->igd_gms << IGD_GMCH_GEN6_GMS_SHIFT;
> +        } else if (vdev->igd_gms <= 0x40) {
> +            gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
> +            gmch |= vdev->igd_gms << IGD_GMCH_GEN8_GMS_SHIFT;
> +        } else {
> +            error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);
> +        }
> +    }
> +
>      ggms_size = igd_gtt_memory_size(gen, gmch);
>      gms_size = igd_stolen_memory_size(gen, gmch);
>  

-- 
Kind regards,
Corvin
Re: [PATCH v2 9/9] vfio/igd: add x-igd-gms option back to set DSM region size for guest
Posted by Alex Williamson 1 year, 2 months ago 
On Tue, 3 Dec 2024 16:30:56 +0000
Corvin Köhne <C.Koehne@beckhoff.com> wrote:

> On Tue, 2024-12-03 at 21:35 +0800, Tomita Moeko wrote:
> > CAUTION: External Email!!
> > DSM region is likely to store framebuffer in Windows, a small DSM
> > region may cause display issues (e.g. half of the screen is black).
> > By default, QEMU uses host's original value, which is determined by
> > DVMT Pre-Allocated option in Intel FSP of host bios. Some vendors
> > do not expose this config item to users. In such cases, x-igd-gms
> > option can be used to manually set the data stolen memory size for
> > guest.
> > 
> > When DVMT Pre-Allocated option is available in host BIOS, user should
> > set DSM region size there instead of using x-igd-gms option.
> >   
> 
> It might be worth linking the commit which has removed x-igd-gms and mention
> that the behaviour changed slightly. Previously, the DSM region size was set to
> 0 if x-igd-gms was unset. This patch keeps the host value if x-igd-gms is unset.

Yes, this is really quite confusing.  We never actually removed the
x-igd-gms option as $Subject implies, we only gutted it from doing
anything, *sigh*.  Let's please summarize how 971ca22f041b left the
option but removed any implementation behind that option and this
brings it back to provide some function.

> > Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> > ---
> >  hw/vfio/igd.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> > index e464cd6949..0814730f40 100644
> > --- a/hw/vfio/igd.c
> > +++ b/hw/vfio/igd.c
> > @@ -717,6 +717,23 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int
> > nr)
> >  
> >      QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next);
> >  
> > +    /*
> > +     * Allow user to override dsm size using x-igd-gms option, in multiples
> > of
> > +     * 32MiB. This option should only be used when the desired size cannot be
> > +     * set from DVMT Pre-Allocated option in host BIOS.
> > +     */
> > +    if (vdev->igd_gms) {
> > +        if (gen < 8 && vdev->igd_gms <= 0x10) {  
> 
> This doesn't work as intended for values larger than 0x10. For those values,
> qemu ignores the generation and set GMS as it would be a gen 8 or later device.
> 
> > +            gmch &= ~(IGD_GMCH_GEN6_GMS_MASK << IGD_GMCH_GEN6_GMS_SHIFT);
> > +            gmch |= vdev->igd_gms << IGD_GMCH_GEN6_GMS_SHIFT;
> > +        } else if (vdev->igd_gms <= 0x40) {
> > +            gmch &= ~(IGD_GMCH_GEN8_GMS_MASK << IGD_GMCH_GEN8_GMS_SHIFT);
> > +            gmch |= vdev->igd_gms << IGD_GMCH_GEN8_GMS_SHIFT;
> > +        } else {
> > +            error_report("Unsupported IGD GMS value 0x%x", vdev->igd_gms);

We clearly know the upper limit of valid values for the device
generation, can we include that in the error report?  Thanks,

Alex

> > +        }
> > +    }
> > +
> >      ggms_size = igd_gtt_memory_size(gen, gmch);
> >      gms_size = igd_stolen_memory_size(gen, gmch);
> >    
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.