| Message ID | 20241203133548.38252-10-tomitamoeko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vfio/igd: Enable legacy mode on more devices | expand |
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); >
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); > > >
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);
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(+)