x-igd-gms is used for overriding DSM region size in GGC register in
both config space and MMIO BAR0, by default host value is used.
There is no need to emulate it in default case.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
hw/vfio/igd.c | 49 ++++++++++++++++++++++++++-----------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 7f289a62a3..5d12f753ab 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -477,22 +477,24 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
- ggc_quirk = vfio_quirk_alloc(1);
- ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
- ggc_mirror->mem = ggc_quirk->mem;
- ggc_mirror->vdev = vdev;
- ggc_mirror->bar = nr;
- ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
- ggc_mirror->config_offset = IGD_GMCH;
-
- memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
- &vfio_generic_mirror_quirk, ggc_mirror,
- "vfio-igd-ggc-quirk", 2);
- memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
- ggc_mirror->offset, ggc_mirror->mem,
- 1);
+ if (vdev->igd_gms) {
+ ggc_quirk = vfio_quirk_alloc(1);
+ ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
+ ggc_mirror->mem = ggc_quirk->mem;
+ ggc_mirror->vdev = vdev;
+ ggc_mirror->bar = nr;
+ ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
+ ggc_mirror->config_offset = IGD_GMCH;
- QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
+ memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
+ &vfio_generic_mirror_quirk, ggc_mirror,
+ "vfio-igd-ggc-quirk", 2);
+ memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
+ ggc_mirror->offset, ggc_mirror->mem,
+ 1);
+
+ QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
+ }
bdsm_quirk = vfio_quirk_alloc(1);
bdsm_mirror = bdsm_quirk->data = g_malloc0(sizeof(*bdsm_mirror));
@@ -631,9 +633,15 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
* 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 &&
- !vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
- return false;
+ if (vdev->igd_gms) {
+ if (vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
+ /* 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);
+ } else {
+ return false;
+ }
}
gms_size = igd_stolen_memory_size(gen, gmch);
@@ -651,11 +659,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
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 */
if (gen < 11) {
pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
--
2.47.2On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
> CAUTION: External Email!!
> x-igd-gms is used for overriding DSM region size in GGC register in
> both config space and MMIO BAR0, by default host value is used.
> There is no need to emulate it in default case.
>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
> hw/vfio/igd.c | 49 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 7f289a62a3..5d12f753ab 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -477,22 +477,24 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int
> nr)
> return;
> }
>
> - ggc_quirk = vfio_quirk_alloc(1);
> - ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
> - ggc_mirror->mem = ggc_quirk->mem;
> - ggc_mirror->vdev = vdev;
> - ggc_mirror->bar = nr;
> - ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
> - ggc_mirror->config_offset = IGD_GMCH;
> -
> - memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
> - &vfio_generic_mirror_quirk, ggc_mirror,
> - "vfio-igd-ggc-quirk", 2);
> - memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
> - ggc_mirror->offset, ggc_mirror->mem,
> - 1);
> + if (vdev->igd_gms) {
> + ggc_quirk = vfio_quirk_alloc(1);
> + ggc_mirror = ggc_quirk->data = g_malloc0(sizeof(*ggc_mirror));
> + ggc_mirror->mem = ggc_quirk->mem;
> + ggc_mirror->vdev = vdev;
> + ggc_mirror->bar = nr;
> + ggc_mirror->offset = IGD_GGC_MMIO_OFFSET;
> + ggc_mirror->config_offset = IGD_GMCH;
>
> - QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
> + memory_region_init_io(ggc_mirror->mem, OBJECT(vdev),
> + &vfio_generic_mirror_quirk, ggc_mirror,
> + "vfio-igd-ggc-quirk", 2);
> + memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
> + ggc_mirror->offset, ggc_mirror-
> >mem,
> + 1);
> +
> + QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, ggc_quirk, next);
> + }
>
> bdsm_quirk = vfio_quirk_alloc(1);
> bdsm_mirror = bdsm_quirk->data = g_malloc0(sizeof(*bdsm_mirror));
> @@ -631,9 +633,15 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
> * 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 &&
> - !vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
> - return false;
> + if (vdev->igd_gms) {
> + if (vfio_pci_igd_override_gms(gen, vdev->igd_gms, &gmch)) {
> + /* 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);
> + } else {
> + return false;
> + }
nit suggestion: You could avoid a level of indentation by using this style:
if (!vfio_pci_igd_override_gms(...)) {
return false;
}
/* GMCH is read-only, emulated */
pci_set_long(...)
> }
>
> gms_size = igd_stolen_memory_size(gen, gmch);
> @@ -651,11 +659,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice
> *vdev, Error **errp)
> 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
> */
> if (gen < 11) {
> pci_set_long(vdev->pdev.config + IGD_BDSM, 0);
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
--
Kind regards,
Corvin
On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote: > CAUTION: External Email!! > x-igd-gms is used for overriding DSM region size in GGC register in > both config space and MMIO BAR0, by default host value is used. > There is no need to emulate it in default case. > > Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com> > Is the GGC register writeable after UEFI or is it locked? If it's writable, I'm wondering what might happen if the guest writes to the GGC register to increase DSM region size. -- Kind regards, Corvin
On 2025/4/29 14:28, Corvin Köhne wrote:
> On Tue, 2025-04-29 at 00:10 +0800, Tomita Moeko wrote:
>> CAUTION: External Email!!
>> x-igd-gms is used for overriding DSM region size in GGC register in
>> both config space and MMIO BAR0, by default host value is used.
>> There is no need to emulate it in default case.
>>
>> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
>>
>
> Is the GGC register writeable after UEFI or is it locked? If it's writable, I'm
> wondering what might happen if the guest writes to the GGC register to increase
> DSM region size.
>
The document says its is read-only mirror to the Graphics Control register
in Host bridge (00:00.0). In guest, the register being mirrored does not
exist, and GCC seems to be a scratch register that only used by software.
GOP driver setup DSM according to the value emulated by QEMU, this can be
confirmed with intel_gtt tool in intel-gpu-tools.
Setting GMS to 128MB do solves the strange issue that screen flickers on
4k60Hz display when host value is 64MB. Since the GOP driver is closed-
source, I am unable to explain this, but it works.
Memory view of IGD passthrough looks like below. The emulated GGC register
is responsible for the size of Guest DSM region, there is no impact to the
host side.
IGD Addr Space Host Addr Space Guest Addr Space
+-------------+ +-------------+ +-------------+
| | | | | |
| | | | | |
| | +-------------+ +-------------+
| | | Data Stolen | | Data Stolen |
| | | (Guest) | | (Guest) |
| | +------------>+-------------+<------->+-------------+<--Guest BDSM
| | | Passthrough | | EPT | | Emulated by QEMU
DSMSIZE+-------------+ | with IOMMU | | Mapping | | Programmed by guest FW
| | | | | | |
| | | | | | |
0+-------------+--+ | | | |
| +-------------+ | |
| | Data Stolen | +-------------+
| | (Host) |
+------------>+-------------+<--Host BDSM
Non- | | "real" one in HW
Passthrough | | Programmed by host FW
+-------------+
Thanks,
Moeko
© 2016 - 2026 Red Hat, Inc.