* [PULL 01/21] vfio/igd: Remove GTT write quirk in IO BAR 4
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 02/21] vfio/igd: Do not include GTT stolen size in etc/igd-bdsm-size Cédric Le Goater
` (20 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
The IO BAR4 of IGD devices contains a pair of 32-bit address/data
registers, MMIO_Index (0x0) and MMIO_Data (0x4), which provide access
to the MMIO BAR0 (GTTMMADR) from IO space. These registers are probably
only used by the VBIOS, and are not documented by intel. The observed
layout of MMIO_Index register is:
31 2 1 0
+-------------------------------------------------------------------+
| Offset | Rsvd | Sel |
+-------------------------------------------------------------------+
- Offset: Byte offset in specified region, 4-byte aligned.
- Sel: Region selector
0: MMIO register region (first half of MMIO BAR0)
1: GTT region (second half of MMIO BAR0). Pre Gen11 only.
Currently, QEMU implements a quirk that adjusts the guest Data Stolen
Memory (DSM) region address to be (addr - host BDSM + guest BDSM) when
programming GTT entries via IO BAR4, assuming guest still programs GTT
with host DSM address, which is not the case. Guest's BDSM register is
emulated and initialized to 0 at startup by QEMU, then SeaBIOS programs
its value[1]. As result, the address programmed to GTT entries by VBIOS
running in guest are valid GPA, and this unnecessary adjustment brings
inconsistency.
[1] https://gitlab.com/qemu-project/seabios/-/blob/1.12-stable/src/fw/pciinit.c#L319-332
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-2-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/igd.c | 191 +-------------------------------------------------
1 file changed, 1 insertion(+), 190 deletions(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index b1a237edd6608aab71c03036abef5c5d3cbcf12f..ca3a32f4f2b8a01d5c82225a0354c2e9ce1bb3b2 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -106,12 +106,6 @@ static int igd_gen(VFIOPCIDevice *vdev)
return -1;
}
-typedef struct VFIOIGDQuirk {
- struct VFIOPCIDevice *vdev;
- uint32_t index;
- uint64_t bdsm;
-} VFIOIGDQuirk;
-
#define IGD_GMCH 0x50 /* Graphics Control Register */
#define IGD_BDSM 0x5c /* Base Data of Stolen Memory */
#define IGD_BDSM_GEN11 0xc0 /* Base Data of Stolen Memory of gen 11 and later */
@@ -300,129 +294,6 @@ static int vfio_pci_igd_lpc_init(VFIOPCIDevice *vdev,
return ret;
}
-/*
- * IGD Gen8 and newer support up to 8MB for the GTT and use a 64bit PTE
- * entry, older IGDs use 2MB and 32bit. Each PTE maps a 4k page. Therefore
- * we either have 2M/4k * 4 = 2k or 8M/4k * 8 = 16k as the maximum iobar index
- * for programming the GTT.
- *
- * See linux:include/drm/i915_drm.h for shift and mask values.
- */
-static int vfio_igd_gtt_max(VFIOPCIDevice *vdev)
-{
- uint32_t gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, sizeof(gmch));
- int gen = igd_gen(vdev);
- uint64_t ggms_size = igd_gtt_memory_size(gen, gmch);
-
- return (ggms_size / (4 * KiB)) * (gen < 8 ? 4 : 8);
-}
-
-/*
- * The IGD ROM will make use of stolen memory (GGMS) for support of VESA modes.
- * Somehow the host stolen memory range is used for this, but how the ROM gets
- * it is a mystery, perhaps it's hardcoded into the ROM. Thankfully though, it
- * reprograms the GTT through the IOBAR where we can trap it and transpose the
- * programming to the VM allocated buffer. That buffer gets reserved by the VM
- * firmware via the fw_cfg entry added below. Here we're just monitoring the
- * IOBAR address and data registers to detect a write sequence targeting the
- * GTTADR. This code is developed by observed behavior and doesn't have a
- * direct spec reference, unfortunately.
- */
-static uint64_t vfio_igd_quirk_data_read(void *opaque,
- hwaddr addr, unsigned size)
-{
- VFIOIGDQuirk *igd = opaque;
- VFIOPCIDevice *vdev = igd->vdev;
-
- igd->index = ~0;
-
- return vfio_region_read(&vdev->bars[4].region, addr + 4, size);
-}
-
-static void vfio_igd_quirk_data_write(void *opaque, hwaddr addr,
- uint64_t data, unsigned size)
-{
- VFIOIGDQuirk *igd = opaque;
- VFIOPCIDevice *vdev = igd->vdev;
- uint64_t val = data;
- int gen = igd_gen(vdev);
-
- /*
- * Programming the GGMS starts at index 0x1 and uses every 4th index (ie.
- * 0x1, 0x5, 0x9, 0xd,...). For pre-Gen8 each 4-byte write is a whole PTE
- * entry, with 0th bit enable set. For Gen8 and up, PTEs are 64bit, so
- * entries 0x5 & 0xd are the high dword, in our case zero. Each PTE points
- * to a 4k page, which we translate to a page from the VM allocated region,
- * pointed to by the BDSM register. If this is not set, we fail.
- *
- * We trap writes to the full configured GTT size, but we typically only
- * see the vBIOS writing up to (nearly) the 1MB barrier. In fact it often
- * seems to miss the last entry for an even 1MB GTT. Doing a gratuitous
- * write of that last entry does work, but is hopefully unnecessary since
- * we clear the previous GTT on initialization.
- */
- if ((igd->index % 4 == 1) && igd->index < vfio_igd_gtt_max(vdev)) {
- if (gen < 8 || (igd->index % 8 == 1)) {
- uint64_t base;
-
- if (gen < 11) {
- base = pci_get_long(vdev->pdev.config + IGD_BDSM);
- } else {
- base = pci_get_quad(vdev->pdev.config + IGD_BDSM_GEN11);
- }
- if (!base) {
- hw_error("vfio-igd: Guest attempted to program IGD GTT before "
- "BIOS reserved stolen memory. Unsupported BIOS?");
- }
-
- val = data - igd->bdsm + base;
- } else {
- val = 0; /* upper 32bits of pte, we only enable below 4G PTEs */
- }
-
- trace_vfio_pci_igd_bar4_write(vdev->vbasedev.name,
- igd->index, data, val);
- }
-
- vfio_region_write(&vdev->bars[4].region, addr + 4, val, size);
-
- igd->index = ~0;
-}
-
-static const MemoryRegionOps vfio_igd_data_quirk = {
- .read = vfio_igd_quirk_data_read,
- .write = vfio_igd_quirk_data_write,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static uint64_t vfio_igd_quirk_index_read(void *opaque,
- hwaddr addr, unsigned size)
-{
- VFIOIGDQuirk *igd = opaque;
- VFIOPCIDevice *vdev = igd->vdev;
-
- igd->index = ~0;
-
- return vfio_region_read(&vdev->bars[4].region, addr, size);
-}
-
-static void vfio_igd_quirk_index_write(void *opaque, hwaddr addr,
- uint64_t data, unsigned size)
-{
- VFIOIGDQuirk *igd = opaque;
- VFIOPCIDevice *vdev = igd->vdev;
-
- igd->index = data;
-
- vfio_region_write(&vdev->bars[4].region, addr, data, size);
-}
-
-static const MemoryRegionOps vfio_igd_index_quirk = {
- .read = vfio_igd_quirk_index_read,
- .write = vfio_igd_quirk_index_write,
- .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
#define IGD_GGC_MMIO_OFFSET 0x108040
#define IGD_BDSM_MMIO_OFFSET 0x1080C0
@@ -494,14 +365,11 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
g_autofree struct vfio_region_info *opregion = NULL;
g_autofree struct vfio_region_info *host = NULL;
g_autofree struct vfio_region_info *lpc = NULL;
- VFIOQuirk *quirk;
- VFIOIGDQuirk *igd;
PCIDevice *lpc_bridge;
- int i, ret, gen;
+ int ret, gen;
uint64_t ggms_size, gms_size;
uint64_t *bdsm_size;
uint32_t gmch;
- uint16_t cmd_orig, cmd;
Error *err = NULL;
/*
@@ -634,32 +502,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
- /* Setup our quirk to munge GTT addresses to the VM allocated buffer */
- quirk = vfio_quirk_alloc(2);
- igd = quirk->data = g_malloc0(sizeof(*igd));
- igd->vdev = vdev;
- igd->index = ~0;
- if (gen < 11) {
- igd->bdsm = vfio_pci_read_config(&vdev->pdev, IGD_BDSM, 4);
- } else {
- igd->bdsm = vfio_pci_read_config(&vdev->pdev, IGD_BDSM_GEN11, 4);
- igd->bdsm |=
- (uint64_t)vfio_pci_read_config(&vdev->pdev, IGD_BDSM_GEN11 + 4, 4) << 32;
- }
- igd->bdsm &= ~((1 * MiB) - 1); /* 1MB aligned */
-
- memory_region_init_io(&quirk->mem[0], OBJECT(vdev), &vfio_igd_index_quirk,
- igd, "vfio-igd-index-quirk", 4);
- memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
- 0, &quirk->mem[0], 1);
-
- memory_region_init_io(&quirk->mem[1], OBJECT(vdev), &vfio_igd_data_quirk,
- igd, "vfio-igd-data-quirk", 4);
- memory_region_add_subregion_overlap(vdev->bars[nr].region.mem,
- 4, &quirk->mem[1], 1);
-
- 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
@@ -717,37 +559,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
}
- /*
- * This IOBAR gives us access to GTTADR, which allows us to write to
- * the GTT itself. So let's go ahead and write zero to all the GTT
- * entries to avoid spurious DMA faults. Be sure I/O access is enabled
- * before talking to the device.
- */
- if (pread(vdev->vbasedev.fd, &cmd_orig, sizeof(cmd_orig),
- vdev->config_offset + PCI_COMMAND) != sizeof(cmd_orig)) {
- error_report("IGD device %s - failed to read PCI command register",
- vdev->vbasedev.name);
- }
-
- cmd = cmd_orig | PCI_COMMAND_IO;
-
- if (pwrite(vdev->vbasedev.fd, &cmd, sizeof(cmd),
- vdev->config_offset + PCI_COMMAND) != sizeof(cmd)) {
- error_report("IGD device %s - failed to write PCI command register",
- vdev->vbasedev.name);
- }
-
- for (i = 1; i < vfio_igd_gtt_max(vdev); i += 4) {
- vfio_region_write(&vdev->bars[4].region, 0, i, 4);
- vfio_region_write(&vdev->bars[4].region, 4, 0, 4);
- }
-
- if (pwrite(vdev->vbasedev.fd, &cmd_orig, sizeof(cmd_orig),
- vdev->config_offset + PCI_COMMAND) != sizeof(cmd_orig)) {
- error_report("IGD device %s - failed to restore PCI command register",
- vdev->vbasedev.name);
- }
-
trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name,
(ggms_size + gms_size) / MiB);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 02/21] vfio/igd: Do not include GTT stolen size in etc/igd-bdsm-size
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
2025-03-11 18:13 ` [PULL 01/21] vfio/igd: Remove GTT write quirk in IO BAR 4 Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 03/21] vfio/igd: Consolidate OpRegion initialization into a single function Cédric Le Goater
` (19 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
Though GTT Stolen Memory (GSM) is right below Data Stolen Memory (DSM)
in host address space, direct access to GSM is prohibited, and it is
not mapped to guest address space. Both host and guest accesses GSM
indirectly through the second half of MMIO BAR0 (GTTMMADR).
Guest firmware only need to reserve a memory region for DSM and program
the BDSM register with the base address of that region, that's actually
what both SeaBIOS[1] and IgdAssignmentDxe does now.
[1] https://gitlab.com/qemu-project/seabios/-/blob/1.12-stable/src/fw/pciinit.c#L319-332
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-3-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/igd.c | 28 +++-------------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index ca3a32f4f2b8a01d5c82225a0354c2e9ce1bb3b2..dda4c7bb5df8bd056c3e06eae38533da7586c6c9 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -112,28 +112,8 @@ static int igd_gen(VFIOPCIDevice *vdev)
#define IGD_GMCH_GEN6_GMS_SHIFT 3 /* SNB_GMCH in i915 */
#define IGD_GMCH_GEN6_GMS_MASK 0x1f
-#define IGD_GMCH_GEN6_GGMS_SHIFT 8
-#define IGD_GMCH_GEN6_GGMS_MASK 0x3
#define IGD_GMCH_GEN8_GMS_SHIFT 8 /* BDW_GMCH in i915 */
#define IGD_GMCH_GEN8_GMS_MASK 0xff
-#define IGD_GMCH_GEN8_GGMS_SHIFT 6
-#define IGD_GMCH_GEN8_GGMS_MASK 0x3
-
-static uint64_t igd_gtt_memory_size(int gen, uint16_t gmch)
-{
- uint64_t ggms;
-
- if (gen < 8) {
- ggms = (gmch >> IGD_GMCH_GEN6_GGMS_SHIFT) & IGD_GMCH_GEN6_GGMS_MASK;
- } else {
- ggms = (gmch >> IGD_GMCH_GEN8_GGMS_SHIFT) & IGD_GMCH_GEN8_GGMS_MASK;
- if (ggms != 0) {
- ggms = 1ULL << ggms;
- }
- }
-
- return ggms * MiB;
-}
static uint64_t igd_stolen_memory_size(int gen, uint32_t gmch)
{
@@ -367,7 +347,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
g_autofree struct vfio_region_info *lpc = NULL;
PCIDevice *lpc_bridge;
int ret, gen;
- uint64_t ggms_size, gms_size;
+ uint64_t gms_size;
uint64_t *bdsm_size;
uint32_t gmch;
Error *err = NULL;
@@ -527,7 +507,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
}
}
- ggms_size = igd_gtt_memory_size(gen, gmch);
gms_size = igd_stolen_memory_size(gen, gmch);
/*
@@ -539,7 +518,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
* config offset 0x5C.
*/
bdsm_size = g_malloc(sizeof(*bdsm_size));
- *bdsm_size = cpu_to_le64(ggms_size + gms_size);
+ *bdsm_size = cpu_to_le64(gms_size);
fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
bdsm_size, sizeof(*bdsm_size));
@@ -559,6 +538,5 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
pci_set_quad(vdev->emulated_config_bits + IGD_BDSM_GEN11, ~0);
}
- trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name,
- (ggms_size + gms_size) / MiB);
+ trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, (gms_size / MiB));
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 03/21] vfio/igd: Consolidate OpRegion initialization into a single function
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
2025-03-11 18:13 ` [PULL 01/21] vfio/igd: Remove GTT write quirk in IO BAR 4 Cédric Le Goater
2025-03-11 18:13 ` [PULL 02/21] vfio/igd: Do not include GTT stolen size in etc/igd-bdsm-size Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 04/21] vfio/igd: Move LPC bridge initialization to a separate function Cédric Le Goater
` (18 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
Both x-igd-opregion option and legacy mode require identical steps to
set up OpRegion for IGD devices. Consolidate these steps into a single
vfio_pci_igd_setup_opregion function.
The function call in pci.c is wrapped with ifdef temporarily to prevent
build error for non-x86 archs, it will be removed after we decouple it
from legacy mode.
Additionally, move vfio_pci_igd_opregion_init to igd.c to prevent it
from being compiled in non-x86 builds.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-4-tomitamoeko@gmail.com
[ clg: Fixed spelling in vfio_pci_igd_setup_opregion() ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.h | 4 +-
hw/vfio/igd.c | 101 +++++++++++++++++++++++++++++++++++--------
hw/vfio/pci-quirks.c | 50 ---------------------
hw/vfio/pci.c | 22 ++--------
4 files changed, 88 insertions(+), 89 deletions(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index d638c781f6f168a71d3a44b48ba0c4edfd334f44..f660b0d80fcfcb81129d443732fe887b084d7a52 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -227,9 +227,7 @@ int vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp);
-bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
- struct vfio_region_info *info,
- Error **errp);
+bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp);
void vfio_display_reset(VFIOPCIDevice *vdev);
bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index dda4c7bb5df8bd056c3e06eae38533da7586c6c9..113ad56ad4206e29198717d944ffe944e1c2e27c 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -106,6 +106,7 @@ static int igd_gen(VFIOPCIDevice *vdev)
return -1;
}
+#define IGD_ASLS 0xfc /* ASL Storage Register */
#define IGD_GMCH 0x50 /* Graphics Control Register */
#define IGD_BDSM 0x5c /* Base Data of Stolen Memory */
#define IGD_BDSM_GEN11 0xc0 /* Base Data of Stolen Memory of gen 11 and later */
@@ -138,6 +139,82 @@ static uint64_t igd_stolen_memory_size(int gen, uint32_t gmch)
return 0;
}
+/*
+ * The OpRegion includes the Video BIOS Table, which seems important for
+ * telling the driver what sort of outputs it has. Without this, the device
+ * may work in the guest, but we may not get output. This also requires BIOS
+ * support to reserve and populate a section of guest memory sufficient for
+ * the table and to write the base address of that memory to the ASLS register
+ * of the IGD device.
+ */
+static bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
+ struct vfio_region_info *info,
+ Error **errp)
+{
+ int ret;
+
+ vdev->igd_opregion = g_malloc0(info->size);
+ ret = pread(vdev->vbasedev.fd, vdev->igd_opregion,
+ info->size, info->offset);
+ if (ret != info->size) {
+ error_setg(errp, "failed to read IGD OpRegion");
+ g_free(vdev->igd_opregion);
+ vdev->igd_opregion = NULL;
+ return false;
+ }
+
+ /*
+ * Provide fw_cfg with a copy of the OpRegion which the VM firmware is to
+ * allocate 32bit reserved memory for, copy these contents into, and write
+ * the reserved memory base address to the device ASLS register at 0xFC.
+ * Alignment of this reserved region seems flexible, but using a 4k page
+ * alignment seems to work well. This interface assumes a single IGD
+ * device, which may be at VM address 00:02.0 in legacy mode or another
+ * address in UPT mode.
+ *
+ * NB, there may be future use cases discovered where the VM should have
+ * direct interaction with the host OpRegion, in which case the write to
+ * the ASLS register would trigger MemoryRegion setup to enable that.
+ */
+ fw_cfg_add_file(fw_cfg_find(), "etc/igd-opregion",
+ vdev->igd_opregion, info->size);
+
+ trace_vfio_pci_igd_opregion_enabled(vdev->vbasedev.name);
+
+ pci_set_long(vdev->pdev.config + IGD_ASLS, 0);
+ pci_set_long(vdev->pdev.wmask + IGD_ASLS, ~0);
+ pci_set_long(vdev->emulated_config_bits + IGD_ASLS, ~0);
+
+ return true;
+}
+
+bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
+{
+ g_autofree struct vfio_region_info *opregion = NULL;
+ int ret;
+
+ /* Hotplugging is not supported for opregion access */
+ if (vdev->pdev.qdev.hotplugged) {
+ error_setg(errp, "IGD OpRegion is not supported on hotplugged device");
+ return false;
+ }
+
+ ret = vfio_get_dev_region_info(&vdev->vbasedev,
+ VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
+ VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
+ if (ret) {
+ error_setg_errno(errp, -ret,
+ "Device does not supports IGD OpRegion feature");
+ return false;
+ }
+
+ if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
+ return false;
+ }
+
+ return true;
+}
+
/*
* The rather short list of registers that we copy from the host devices.
* The LPC/ISA bridge values are definitely needed to support the vBIOS, the
@@ -342,7 +419,6 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
{
g_autofree struct vfio_region_info *rom = NULL;
- g_autofree struct vfio_region_info *opregion = NULL;
g_autofree struct vfio_region_info *host = NULL;
g_autofree struct vfio_region_info *lpc = NULL;
PCIDevice *lpc_bridge;
@@ -418,15 +494,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
* Check whether we have all the vfio device specific regions to
* support legacy mode (added in Linux v4.6). If not, bail.
*/
- ret = vfio_get_dev_region_info(&vdev->vbasedev,
- VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
- VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
- if (ret) {
- error_report("IGD device %s does not support OpRegion access,"
- "legacy mode disabled", vdev->vbasedev.name);
- return;
- }
-
ret = vfio_get_dev_region_info(&vdev->vbasedev,
VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG, &host);
@@ -459,6 +526,13 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
+ /* Setup OpRegion access */
+ if (!vfio_pci_igd_setup_opregion(vdev, &err)) {
+ error_append_hint(&err, "IGD legacy mode disabled\n");
+ error_report_err(err);
+ return;
+ }
+
/* Create our LPC/ISA bridge */
ret = vfio_pci_igd_lpc_init(vdev, lpc);
if (ret) {
@@ -475,13 +549,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
- /* Setup OpRegion access */
- if (!vfio_pci_igd_opregion_init(vdev, opregion, &err)) {
- error_append_hint(&err, "IGD legacy mode disabled\n");
- error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
- return;
- }
-
/*
* 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
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index c53591fe2ba5f71364befb7c916ae4f7abde4cbf..37966e17f0178bbf2d82b2d84da40ba2e261d9f3 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1114,56 +1114,6 @@ static void vfio_probe_rtl8168_bar2_quirk(VFIOPCIDevice *vdev, int nr)
trace_vfio_quirk_rtl8168_probe(vdev->vbasedev.name);
}
-#define IGD_ASLS 0xfc /* ASL Storage Register */
-
-/*
- * The OpRegion includes the Video BIOS Table, which seems important for
- * telling the driver what sort of outputs it has. Without this, the device
- * may work in the guest, but we may not get output. This also requires BIOS
- * support to reserve and populate a section of guest memory sufficient for
- * the table and to write the base address of that memory to the ASLS register
- * of the IGD device.
- */
-bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
- struct vfio_region_info *info, Error **errp)
-{
- int ret;
-
- vdev->igd_opregion = g_malloc0(info->size);
- ret = pread(vdev->vbasedev.fd, vdev->igd_opregion,
- info->size, info->offset);
- if (ret != info->size) {
- error_setg(errp, "failed to read IGD OpRegion");
- g_free(vdev->igd_opregion);
- vdev->igd_opregion = NULL;
- return false;
- }
-
- /*
- * Provide fw_cfg with a copy of the OpRegion which the VM firmware is to
- * allocate 32bit reserved memory for, copy these contents into, and write
- * the reserved memory base address to the device ASLS register at 0xFC.
- * Alignment of this reserved region seems flexible, but using a 4k page
- * alignment seems to work well. This interface assumes a single IGD
- * device, which may be at VM address 00:02.0 in legacy mode or another
- * address in UPT mode.
- *
- * NB, there may be future use cases discovered where the VM should have
- * direct interaction with the host OpRegion, in which case the write to
- * the ASLS register would trigger MemoryRegion setup to enable that.
- */
- fw_cfg_add_file(fw_cfg_find(), "etc/igd-opregion",
- vdev->igd_opregion, info->size);
-
- trace_vfio_pci_igd_opregion_enabled(vdev->vbasedev.name);
-
- pci_set_long(vdev->pdev.config + IGD_ASLS, 0);
- pci_set_long(vdev->pdev.wmask + IGD_ASLS, ~0);
- pci_set_long(vdev->emulated_config_bits + IGD_ASLS, ~0);
-
- return true;
-}
-
/*
* Common quirk probe entry points.
*/
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index fdbc15885d442ee20343e6b7127d54a70b1688ce..419dc2c4c85ffeed3b794e65f8040e11ba58bb0e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3136,30 +3136,14 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
vfio_bar_quirk_setup(vdev, i);
}
+#ifdef CONFIG_VFIO_IGD
if (!vdev->igd_opregion &&
vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) {
- g_autofree struct vfio_region_info *opregion = NULL;
-
- if (vdev->pdev.qdev.hotplugged) {
- error_setg(errp,
- "cannot support IGD OpRegion feature on hotplugged "
- "device");
- goto out_unset_idev;
- }
-
- ret = vfio_get_dev_region_info(vbasedev,
- VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
- VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
- if (ret) {
- error_setg_errno(errp, -ret,
- "does not support requested IGD OpRegion feature");
- goto out_unset_idev;
- }
-
- if (!vfio_pci_igd_opregion_init(vdev, opregion, errp)) {
+ if (!vfio_pci_igd_setup_opregion(vdev, errp)) {
goto out_unset_idev;
}
}
+#endif
/* QEMU emulates all of MSI & MSIX */
if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 04/21] vfio/igd: Move LPC bridge initialization to a separate function
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (2 preceding siblings ...)
2025-03-11 18:13 ` [PULL 03/21] vfio/igd: Consolidate OpRegion initialization into a single function Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 05/21] vfio/pci: Add placeholder for device-specific config space quirks Cédric Le Goater
` (17 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
A new option will soon be introduced to decouple the LPC bridge/Host
bridge ID quirk from legacy mode. To prepare for this, move the LPC
bridge initialization into a separate function.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-5-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/igd.c | 122 +++++++++++++++++++++++++++++---------------------
1 file changed, 70 insertions(+), 52 deletions(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 113ad56ad4206e29198717d944ffe944e1c2e27c..7feed7dfa920725d1d550b9225c7075453bf7b88 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -351,6 +351,72 @@ static int vfio_pci_igd_lpc_init(VFIOPCIDevice *vdev,
return ret;
}
+static bool vfio_pci_igd_setup_lpc_bridge(VFIOPCIDevice *vdev, Error **errp)
+{
+ g_autofree struct vfio_region_info *host = NULL;
+ g_autofree struct vfio_region_info *lpc = NULL;
+ PCIDevice *lpc_bridge;
+ int ret;
+
+ /*
+ * Copying IDs or creating new devices are not supported on hotplug
+ */
+ if (vdev->pdev.qdev.hotplugged) {
+ error_setg(errp, "IGD LPC is not supported on hotplugged device");
+ return false;
+ }
+
+ /*
+ * We need to create an LPC/ISA bridge at PCI bus address 00:1f.0 that we
+ * can stuff host values into, so if there's already one there and it's not
+ * one we can hack on, this quirk is no-go. Sorry Q35.
+ */
+ lpc_bridge = pci_find_device(pci_device_root_bus(&vdev->pdev),
+ 0, PCI_DEVFN(0x1f, 0));
+ if (lpc_bridge && !object_dynamic_cast(OBJECT(lpc_bridge),
+ "vfio-pci-igd-lpc-bridge")) {
+ error_setg(errp,
+ "Cannot create LPC bridge due to existing device at 1f.0");
+ return false;
+ }
+
+ /*
+ * Check whether we have all the vfio device specific regions to
+ * support LPC quirk (added in Linux v4.6).
+ */
+ ret = vfio_get_dev_region_info(&vdev->vbasedev,
+ VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
+ VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG, &lpc);
+ if (ret) {
+ error_setg(errp, "IGD LPC bridge access is not supported by kernel");
+ return false;
+ }
+
+ ret = vfio_get_dev_region_info(&vdev->vbasedev,
+ VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
+ VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG, &host);
+ if (ret) {
+ error_setg(errp, "IGD host bridge access is not supported by kernel");
+ return false;
+ }
+
+ /* Create/modify LPC bridge */
+ ret = vfio_pci_igd_lpc_init(vdev, lpc);
+ if (ret) {
+ error_setg(errp, "Failed to create/modify LPC bridge for IGD");
+ return false;
+ }
+
+ /* Stuff some host values into the VM PCI host bridge */
+ ret = vfio_pci_igd_host_init(vdev, host);
+ if (ret) {
+ error_setg(errp, "Failed to modify host bridge for IGD");
+ return false;
+ }
+
+ return true;
+}
+
#define IGD_GGC_MMIO_OFFSET 0x108040
#define IGD_BDSM_MMIO_OFFSET 0x1080C0
@@ -419,9 +485,6 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
{
g_autofree struct vfio_region_info *rom = NULL;
- g_autofree struct vfio_region_info *host = NULL;
- g_autofree struct vfio_region_info *lpc = NULL;
- PCIDevice *lpc_bridge;
int ret, gen;
uint64_t gms_size;
uint64_t *bdsm_size;
@@ -440,20 +503,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
- /*
- * We need to create an LPC/ISA bridge at PCI bus address 00:1f.0 that we
- * can stuff host values into, so if there's already one there and it's not
- * one we can hack on, legacy mode is no-go. Sorry Q35.
- */
- lpc_bridge = pci_find_device(pci_device_root_bus(&vdev->pdev),
- 0, PCI_DEVFN(0x1f, 0));
- if (lpc_bridge && !object_dynamic_cast(OBJECT(lpc_bridge),
- "vfio-pci-igd-lpc-bridge")) {
- error_report("IGD device %s cannot support legacy mode due to existing "
- "devices at address 1f.0", vdev->vbasedev.name);
- return;
- }
-
/*
* 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
@@ -490,28 +539,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
- /*
- * Check whether we have all the vfio device specific regions to
- * support legacy mode (added in Linux v4.6). If not, bail.
- */
- ret = vfio_get_dev_region_info(&vdev->vbasedev,
- VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
- VFIO_REGION_SUBTYPE_INTEL_IGD_HOST_CFG, &host);
- if (ret) {
- error_report("IGD device %s does not support host bridge access,"
- "legacy mode disabled", vdev->vbasedev.name);
- return;
- }
-
- ret = vfio_get_dev_region_info(&vdev->vbasedev,
- VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
- VFIO_REGION_SUBTYPE_INTEL_IGD_LPC_CFG, &lpc);
- if (ret) {
- error_report("IGD device %s does not support LPC bridge access,"
- "legacy mode disabled", vdev->vbasedev.name);
- return;
- }
-
gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
/*
@@ -533,19 +560,10 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
return;
}
- /* Create our LPC/ISA bridge */
- ret = vfio_pci_igd_lpc_init(vdev, lpc);
- if (ret) {
- error_report("IGD device %s failed to create LPC bridge, "
- "legacy mode disabled", vdev->vbasedev.name);
- return;
- }
-
- /* Stuff some host values into the VM PCI host bridge */
- ret = vfio_pci_igd_host_init(vdev, host);
- if (ret) {
- error_report("IGD device %s failed to modify host bridge, "
- "legacy mode disabled", vdev->vbasedev.name);
+ /* Setup LPC bridge / Host bridge PCI IDs */
+ if (!vfio_pci_igd_setup_lpc_bridge(vdev, &err)) {
+ error_append_hint(&err, "IGD legacy mode disabled\n");
+ error_report_err(err);
return;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 05/21] vfio/pci: Add placeholder for device-specific config space quirks
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (3 preceding siblings ...)
2025-03-11 18:13 ` [PULL 04/21] vfio/igd: Move LPC bridge initialization to a separate function Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 06/21] vfio/igd: Refactor vfio_probe_igd_bar4_quirk into pci config quirk Cédric Le Goater
` (16 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
IGD devices require device-specific quirk to be applied to their PCI
config space. Currently, it is put in the BAR4 quirk that does nothing
to BAR4 itself. Add a placeholder for PCI config space quirks to hold
that quirk later.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-6-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.h | 1 +
hw/vfio/pci-quirks.c | 5 +++++
hw/vfio/pci.c | 4 ++++
3 files changed, 10 insertions(+)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index f660b0d80fcfcb81129d443732fe887b084d7a52..d125e738655a43ec01348b784af90f7e121bd695 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -204,6 +204,7 @@ uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
+bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
void vfio_vga_quirk_setup(VFIOPCIDevice *vdev);
void vfio_vga_quirk_exit(VFIOPCIDevice *vdev);
void vfio_vga_quirk_finalize(VFIOPCIDevice *vdev);
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 37966e17f0178bbf2d82b2d84da40ba2e261d9f3..78aef7d60eff7f8e776c030942439523b2197ded 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1117,6 +1117,11 @@ static void vfio_probe_rtl8168_bar2_quirk(VFIOPCIDevice *vdev, int nr)
/*
* Common quirk probe entry points.
*/
+bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp)
+{
+ return true;
+}
+
void vfio_vga_quirk_setup(VFIOPCIDevice *vdev)
{
vfio_vga_probe_ati_3c3_quirk(vdev);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 419dc2c4c85ffeed3b794e65f8040e11ba58bb0e..ff1e720dbadc2251d76e4c71c026757e2cef1a8c 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3128,6 +3128,10 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
goto out_unset_idev;
}
+ if (!vfio_config_quirk_setup(vdev, errp)) {
+ goto out_unset_idev;
+ }
+
if (vdev->vga) {
vfio_vga_quirk_setup(vdev);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 06/21] vfio/igd: Refactor vfio_probe_igd_bar4_quirk into pci config quirk
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (4 preceding siblings ...)
2025-03-11 18:13 ` [PULL 05/21] vfio/pci: Add placeholder for device-specific config space quirks Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 07/21] vfio/igd: Decouple common quirks from legacy mode Cédric Le Goater
` (15 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
The actual IO BAR4 write quirk in vfio_probe_igd_bar4_quirk was removed
in previous change, leaving the function not matching its name, so move
it into the newly introduced vfio_config_quirk_setup. There is no
functional change in this commit.
For now, to align with current legacy mode behavior, it returns and
proceeds on error. Later it will fail on error after decoupling the
quirks from legacy mode.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-7-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.h | 2 +-
hw/vfio/igd.c | 21 ++++++++++++---------
hw/vfio/pci-quirks.c | 6 +++++-
3 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index d125e738655a43ec01348b784af90f7e121bd695..fc7ead7727b37ed57f159416bb73698a44a38348 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -216,7 +216,7 @@ bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp);
void vfio_quirk_reset(VFIOPCIDevice *vdev);
VFIOQuirk *vfio_quirk_alloc(int nr_mem);
void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr);
-void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr);
+bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp);
extern const PropertyInfo qdev_prop_nv_gpudirect_clique;
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 7feed7dfa920725d1d550b9225c7075453bf7b88..65a3dbb5afd073404a960d3eed309368e1448950 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -482,7 +482,8 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_quirk, next);
}
-void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
+bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev,
+ Error **errp G_GNUC_UNUSED)
{
g_autofree struct vfio_region_info *rom = NULL;
int ret, gen;
@@ -497,10 +498,10 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
* PCI bus address.
*/
if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
- !vfio_is_vga(vdev) || nr != 4 ||
+ !vfio_is_vga(vdev) ||
&vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
0, PCI_DEVFN(0x2, 0))) {
- return;
+ return true;
}
/*
@@ -512,7 +513,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
if (gen == -1) {
error_report("IGD device %s is unsupported in legacy mode, "
"try SandyBridge or newer", vdev->vbasedev.name);
- return;
+ return true;
}
/*
@@ -525,7 +526,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
if ((ret || !rom->size) && !vdev->pdev.romfile) {
error_report("IGD device %s has no ROM, legacy mode disabled",
vdev->vbasedev.name);
- return;
+ return true;
}
/*
@@ -536,7 +537,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
error_report("IGD device %s hotplugged, ROM disabled, "
"legacy mode disabled", vdev->vbasedev.name);
vdev->rom_read_failed = true;
- return;
+ return true;
}
gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
@@ -550,21 +551,21 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
error_report("IGD device %s failed to enable VGA access, "
"legacy mode disabled", vdev->vbasedev.name);
- return;
+ return true;
}
/* Setup OpRegion access */
if (!vfio_pci_igd_setup_opregion(vdev, &err)) {
error_append_hint(&err, "IGD legacy mode disabled\n");
error_report_err(err);
- return;
+ return true;
}
/* Setup LPC bridge / Host bridge PCI IDs */
if (!vfio_pci_igd_setup_lpc_bridge(vdev, &err)) {
error_append_hint(&err, "IGD legacy mode disabled\n");
error_report_err(err);
- return;
+ return true;
}
/*
@@ -624,4 +625,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
}
trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, (gms_size / MiB));
+
+ return true;
}
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 78aef7d60eff7f8e776c030942439523b2197ded..f998761abca2d1c4c366a876838f1bc05b877574 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1119,6 +1119,11 @@ static void vfio_probe_rtl8168_bar2_quirk(VFIOPCIDevice *vdev, int nr)
*/
bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp)
{
+#ifdef CONFIG_VFIO_IGD
+ if (!vfio_probe_igd_config_quirk(vdev, errp)) {
+ return false;
+ }
+#endif
return true;
}
@@ -1170,7 +1175,6 @@ void vfio_bar_quirk_setup(VFIOPCIDevice *vdev, int nr)
vfio_probe_rtl8168_bar2_quirk(vdev, nr);
#ifdef CONFIG_VFIO_IGD
vfio_probe_igd_bar0_quirk(vdev, nr);
- vfio_probe_igd_bar4_quirk(vdev, nr);
#endif
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 07/21] vfio/igd: Decouple common quirks from legacy mode
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (5 preceding siblings ...)
2025-03-11 18:13 ` [PULL 06/21] vfio/igd: Refactor vfio_probe_igd_bar4_quirk into pci config quirk Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 08/21] vfio/igd: Handle x-igd-opregion option in config quirk Cédric Le Goater
` (14 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
So far, IGD-specific quirks all require enabling legacy mode, which is
toggled by assigning IGD to 00:02.0. However, some quirks, like the BDSM
and GGC register quirks, should be applied to all supported IGD devices.
A new config option, x-igd-legacy-mode=[on|off|auto], is introduced to
control the legacy mode only quirks. The default value is "auto", which
keeps current behavior that enables legacy mode implicitly and continues
on error when all following conditions are met.
* Machine type is i440fx
* IGD device is at guest BDF 00:02.0
If any one of the conditions above is not met, the default behavior is
equivalent to "off", QEMU will fail immediately if any error occurs.
Users can also use "on" to force enabling legacy mode. It checks if all
the conditions above are met and set up legacy mode. QEMU will also fail
immediately on error in this case.
Additionally, the hotplug check in legacy mode is removed as hotplugging
IGD device is never supported, and it will be checked when enabling the
OpRegion quirk.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-8-tomitamoeko@gmail.com
[ clg: - Changed warn_report() by info_report() in
vfio_probe_igd_config_quirk() as suggested by Alex W.
- Fixed spelling in vfio_probe_igd_config_quirk () ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.h | 1 +
hw/vfio/igd.c | 127 +++++++++++++++++++++++++++++---------------------
hw/vfio/pci.c | 2 +
3 files changed, 77 insertions(+), 53 deletions(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index fc7ead7727b37ed57f159416bb73698a44a38348..3e66b19d8f24fd8c0f44ca26d63e511edbc4318a 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -158,6 +158,7 @@ struct VFIOPCIDevice {
uint32_t display_xres;
uint32_t display_yres;
int32_t bootindex;
+ OnOffAuto igd_legacy_mode;
uint32_t igd_gms;
OffAutoPCIBAR msix_relo;
uint8_t nv_gpudirect_clique;
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 65a3dbb5afd073404a960d3eed309368e1448950..ee36875310b4728ac0049327a1f71021d5e6f770 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -15,6 +15,7 @@
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
+#include "hw/boards.h"
#include "hw/hw.h"
#include "hw/nvram/fw_cfg.h"
#include "pci.h"
@@ -432,9 +433,7 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
* bus address.
*/
if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
- !vfio_is_vga(vdev) || nr != 0 ||
- &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
- 0, PCI_DEVFN(0x2, 0))) {
+ !vfio_is_vga(vdev) || nr != 0) {
return;
}
@@ -482,14 +481,13 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_quirk, next);
}
-bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev,
- Error **errp G_GNUC_UNUSED)
+bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
{
- g_autofree struct vfio_region_info *rom = NULL;
int ret, gen;
uint64_t gms_size;
uint64_t *bdsm_size;
uint32_t gmch;
+ bool legacy_mode_enabled = false;
Error *err = NULL;
/*
@@ -498,9 +496,7 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev,
* PCI bus address.
*/
if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
- !vfio_is_vga(vdev) ||
- &vdev->pdev != pci_find_device(pci_device_root_bus(&vdev->pdev),
- 0, PCI_DEVFN(0x2, 0))) {
+ !vfio_is_vga(vdev)) {
return true;
}
@@ -516,56 +512,67 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev,
return true;
}
- /*
- * Most of what we're doing here is to enable the ROM to run, so if
- * there's no ROM, there's no point in setting up this quirk.
- * NB. We only seem to get BIOS ROMs, so a UEFI VM would need CSM support.
- */
- ret = vfio_get_region_info(&vdev->vbasedev,
- VFIO_PCI_ROM_REGION_INDEX, &rom);
- if ((ret || !rom->size) && !vdev->pdev.romfile) {
- error_report("IGD device %s has no ROM, legacy mode disabled",
- vdev->vbasedev.name);
- return true;
- }
-
- /*
- * Ignore the hotplug corner case, mark the ROM failed, we can't
- * create the devices we need for legacy mode in the hotplug scenario.
- */
- if (vdev->pdev.qdev.hotplugged) {
- error_report("IGD device %s hotplugged, ROM disabled, "
- "legacy mode disabled", vdev->vbasedev.name);
- vdev->rom_read_failed = true;
- return true;
- }
-
gmch = vfio_pci_read_config(&vdev->pdev, IGD_GMCH, 4);
/*
- * If IGD VGA Disable is clear (expected) and VGA is not already enabled,
- * try to enable it. Probably shouldn't be using legacy mode without VGA,
- * but also no point in us enabling VGA if disabled in hardware.
+ * For backward compatibility, enable legacy mode when
+ * - Machine type is i440fx (pc_piix)
+ * - IGD device is at guest BDF 00:02.0
+ * - Not manually disabled by x-igd-legacy-mode=off
*/
- if (!(gmch & 0x2) && !vdev->vga && !vfio_populate_vga(vdev, &err)) {
- error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
- error_report("IGD device %s failed to enable VGA access, "
- "legacy mode disabled", vdev->vbasedev.name);
- return true;
- }
+ if ((vdev->igd_legacy_mode != ON_OFF_AUTO_OFF) &&
+ !strcmp(MACHINE_GET_CLASS(qdev_get_machine())->family, "pc_piix") &&
+ (&vdev->pdev == pci_find_device(pci_device_root_bus(&vdev->pdev),
+ 0, PCI_DEVFN(0x2, 0)))) {
+ /*
+ * IGD legacy mode requires:
+ * - VBIOS in ROM BAR or file
+ * - VGA IO/MMIO ranges are claimed by IGD
+ * - OpRegion
+ * - Same LPC bridge and Host bridge VID/DID/SVID/SSID as host
+ */
+ g_autofree struct vfio_region_info *rom = NULL;
+
+ legacy_mode_enabled = true;
+ info_report("IGD legacy mode enabled, "
+ "use x-igd-legacy-mode=off to disable it if unwanted.");
+
+ /*
+ * Most of what we're doing here is to enable the ROM to run, so if
+ * there's no ROM, there's no point in setting up this quirk.
+ * NB. We only seem to get BIOS ROMs, so UEFI VM would need CSM support.
+ */
+ ret = vfio_get_region_info(&vdev->vbasedev,
+ VFIO_PCI_ROM_REGION_INDEX, &rom);
+ if ((ret || !rom->size) && !vdev->pdev.romfile) {
+ error_setg(&err, "Device has no ROM");
+ goto error;
+ }
- /* Setup OpRegion access */
- if (!vfio_pci_igd_setup_opregion(vdev, &err)) {
- error_append_hint(&err, "IGD legacy mode disabled\n");
- error_report_err(err);
- return true;
- }
+ /*
+ * If IGD VGA Disable is clear (expected) and VGA is not already
+ * enabled, try to enable it. Probably shouldn't be using legacy mode
+ * without VGA, but also no point in us enabling VGA if disabled in
+ * hardware.
+ */
+ if (!(gmch & 0x2) && !vdev->vga && !vfio_populate_vga(vdev, &err)) {
+ error_setg(&err, "Unable to enable VGA access");
+ goto error;
+ }
- /* Setup LPC bridge / Host bridge PCI IDs */
- if (!vfio_pci_igd_setup_lpc_bridge(vdev, &err)) {
- error_append_hint(&err, "IGD legacy mode disabled\n");
- error_report_err(err);
- return true;
+ /* Setup OpRegion access */
+ if (!vfio_pci_igd_setup_opregion(vdev, &err)) {
+ goto error;
+ }
+
+ /* Setup LPC bridge / Host bridge PCI IDs */
+ if (!vfio_pci_igd_setup_lpc_bridge(vdev, &err)) {
+ goto error;
+ }
+ } else if (vdev->igd_legacy_mode == ON_OFF_AUTO_ON) {
+ error_setg(&err,
+ "Machine is not i440fx or assigned BDF is not 00:02.0");
+ goto error;
}
/*
@@ -627,4 +634,18 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev,
trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, (gms_size / MiB));
return true;
+
+error:
+ /*
+ * When legacy mode is implicity enabled, continue on error,
+ * to keep compatibility
+ */
+ if (legacy_mode_enabled && (vdev->igd_legacy_mode == ON_OFF_AUTO_AUTO)) {
+ error_report_err(err);
+ error_report("IGD legacy mode disabled");
+ return true;
+ }
+
+ error_propagate(errp, err);
+ return false;
}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ff1e720dbadc2251d76e4c71c026757e2cef1a8c..444a33d94b7ee56311e828f69f29c724b91812c3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3369,6 +3369,8 @@ static const Property vfio_pci_dev_properties[] = {
VFIO_FEATURE_ENABLE_REQ_BIT, true),
DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
+ DEFINE_PROP_ON_OFF_AUTO("x-igd-legacy-mode", VFIOPCIDevice,
+ igd_legacy_mode, ON_OFF_AUTO_AUTO),
DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
vbasedev.enable_migration, ON_OFF_AUTO_AUTO),
DEFINE_PROP("x-migration-multifd-transfer", VFIOPCIDevice,
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 08/21] vfio/igd: Handle x-igd-opregion option in config quirk
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (6 preceding siblings ...)
2025-03-11 18:13 ` [PULL 07/21] vfio/igd: Decouple common quirks from legacy mode Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 09/21] vfio/igd: Introduce x-igd-lpc option for LPC bridge ID quirk Cédric Le Goater
` (13 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
Both enable OpRegion option (x-igd-opregion) and legacy mode require
setting up OpRegion copy for IGD devices. As the config quirk no longer
depends on legacy mode, we can now handle x-igd-opregion option there
instead of in vfio_realize.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-9-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.h | 2 --
hw/vfio/igd.c | 14 +++++++++-----
hw/vfio/pci.c | 9 ---------
3 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 3e66b19d8f24fd8c0f44ca26d63e511edbc4318a..816bdbf844dc17e7cbac9a6f42daf9a448ab9cc0 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -229,8 +229,6 @@ int vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp);
-bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp);
-
void vfio_display_reset(VFIOPCIDevice *vdev);
bool vfio_display_probe(VFIOPCIDevice *vdev, Error **errp);
void vfio_display_finalize(VFIOPCIDevice *vdev);
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index ee36875310b4728ac0049327a1f71021d5e6f770..12e07517b4a09ef87791acc09f6a6120abf17ae5 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -189,7 +189,7 @@ static bool vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
return true;
}
-bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_pci_igd_setup_opregion(VFIOPCIDevice *vdev, Error **errp)
{
g_autofree struct vfio_region_info *opregion = NULL;
int ret;
@@ -560,10 +560,8 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
goto error;
}
- /* Setup OpRegion access */
- if (!vfio_pci_igd_setup_opregion(vdev, &err)) {
- goto error;
- }
+ /* Enable OpRegion quirk */
+ vdev->features |= VFIO_FEATURE_ENABLE_IGD_OPREGION;
/* Setup LPC bridge / Host bridge PCI IDs */
if (!vfio_pci_igd_setup_lpc_bridge(vdev, &err)) {
@@ -575,6 +573,12 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
goto error;
}
+ /* Setup OpRegion access */
+ if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
+ !vfio_pci_igd_setup_opregion(vdev, errp)) {
+ goto error;
+ }
+
/*
* 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
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 444a33d94b7ee56311e828f69f29c724b91812c3..e2897bdcd58d092d64689b5921a34c133139b56d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3140,15 +3140,6 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
vfio_bar_quirk_setup(vdev, i);
}
-#ifdef CONFIG_VFIO_IGD
- if (!vdev->igd_opregion &&
- vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) {
- if (!vfio_pci_igd_setup_opregion(vdev, errp)) {
- goto out_unset_idev;
- }
- }
-#endif
-
/* QEMU emulates all of MSI & MSIX */
if (pdev->cap_present & QEMU_PCI_CAP_MSIX) {
memset(vdev->emulated_config_bits + pdev->msix_cap, 0xff,
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 09/21] vfio/igd: Introduce x-igd-lpc option for LPC bridge ID quirk
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (7 preceding siblings ...)
2025-03-11 18:13 ` [PULL 08/21] vfio/igd: Handle x-igd-opregion option in config quirk Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 10/21] vfio/igd: Fix broken KVMGT OpRegion support Cédric Le Goater
` (12 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
The LPC bridge/Host bridge IDs quirk is also not dependent on legacy
mode. Recent Windows driver no longer depends on these IDs, as well as
Linux i915 driver, while UEFI GOP seems still needs them. Make it an
option to allow users enabling and disabling it as needed.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-10-tomitamoeko@gmail.com
[ clg: - Fixed spelling in vfio_probe_igd_config_quirk() ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.h | 3 +++
hw/vfio/igd.c | 14 ++++++++------
hw/vfio/pci.c | 2 ++
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 816bdbf844dc17e7cbac9a6f42daf9a448ab9cc0..d94ecaba689c4681687c0a6796ffbcda522ae179 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -154,6 +154,9 @@ struct VFIOPCIDevice {
#define VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT 2
#define VFIO_FEATURE_ENABLE_IGD_OPREGION \
(1 << VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT)
+#define VFIO_FEATURE_ENABLE_IGD_LPC_BIT 3
+#define VFIO_FEATURE_ENABLE_IGD_LPC \
+ (1 << VFIO_FEATURE_ENABLE_IGD_LPC_BIT)
OnOffAuto display;
uint32_t display_xres;
uint32_t display_yres;
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 12e07517b4a09ef87791acc09f6a6120abf17ae5..8a88dbab13ede764d0610a043132a3acfe208134 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -560,13 +560,9 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
goto error;
}
- /* Enable OpRegion quirk */
+ /* Enable OpRegion and LPC bridge quirk */
vdev->features |= VFIO_FEATURE_ENABLE_IGD_OPREGION;
-
- /* Setup LPC bridge / Host bridge PCI IDs */
- if (!vfio_pci_igd_setup_lpc_bridge(vdev, &err)) {
- goto error;
- }
+ vdev->features |= VFIO_FEATURE_ENABLE_IGD_LPC;
} else if (vdev->igd_legacy_mode == ON_OFF_AUTO_ON) {
error_setg(&err,
"Machine is not i440fx or assigned BDF is not 00:02.0");
@@ -579,6 +575,12 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
goto error;
}
+ /* Setup LPC bridge / Host bridge PCI IDs */
+ if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_LPC) &&
+ !vfio_pci_igd_setup_lpc_bridge(vdev, errp)) {
+ goto error;
+ }
+
/*
* 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
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e2897bdcd58d092d64689b5921a34c133139b56d..3cb7806f2f219174ffb7d28595e9430b41ee40ae 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3360,6 +3360,8 @@ static const Property vfio_pci_dev_properties[] = {
VFIO_FEATURE_ENABLE_REQ_BIT, true),
DEFINE_PROP_BIT("x-igd-opregion", VFIOPCIDevice, features,
VFIO_FEATURE_ENABLE_IGD_OPREGION_BIT, false),
+ DEFINE_PROP_BIT("x-igd-lpc", VFIOPCIDevice, features,
+ VFIO_FEATURE_ENABLE_IGD_LPC_BIT, false),
DEFINE_PROP_ON_OFF_AUTO("x-igd-legacy-mode", VFIOPCIDevice,
igd_legacy_mode, ON_OFF_AUTO_AUTO),
DEFINE_PROP_ON_OFF_AUTO("enable-migration", VFIOPCIDevice,
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 10/21] vfio/igd: Fix broken KVMGT OpRegion support
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (8 preceding siblings ...)
2025-03-11 18:13 ` [PULL 09/21] vfio/igd: Introduce x-igd-lpc option for LPC bridge ID quirk Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 11/21] vfio/migration: Use BE byte order for device state wire packets Cédric Le Goater
` (11 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Tomita Moeko, Corvin Köhne,
Cédric Le Goater
From: Tomita Moeko <tomitamoeko@gmail.com>
The KVMGT/GVT-g vGPU also exposes OpRegion. But unlike IGD passthrough,
it only needs the OpRegion quirk. A previous change moved x-igd-opregion
handling to config quirk breaks KVMGT functionality as it brings extra
checks and applied other quirks. Here we check if the device is mdev
(KVMGT) or not (passthrough), and then applies corresponding quirks.
As before, users must manually specify x-igd-opregion=on to enable it
on KVMGT devices. In the future, we may check the VID/DID and enable
OpRegion automatically.
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Corvin Köhne <c.koehne@beckhoff.com>
Link: https://lore.kernel.org/qemu-devel/20250306180131.32970-11-tomitamoeko@gmail.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/igd.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 8a88dbab13ede764d0610a043132a3acfe208134..265fffc2aa52d032ba619b1c3759ecebad438033 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -481,7 +481,7 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_quirk, next);
}
-bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
+static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
{
int ret, gen;
uint64_t gms_size;
@@ -655,3 +655,28 @@ error:
error_propagate(errp, err);
return false;
}
+
+/*
+ * KVMGT/GVT-g vGPU exposes an emulated OpRegion. So far, users have to specify
+ * x-igd-opregion=on to enable the access.
+ * TODO: Check VID/DID and enable opregion access automatically
+ */
+static bool vfio_pci_kvmgt_config_quirk(VFIOPCIDevice *vdev, Error **errp)
+{
+ if ((vdev->features & VFIO_FEATURE_ENABLE_IGD_OPREGION) &&
+ !vfio_pci_igd_setup_opregion(vdev, errp)) {
+ return false;
+ }
+
+ return true;
+}
+
+bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
+{
+ /* KVMGT/GVT-g vGPU is exposed as mdev */
+ if (vdev->vbasedev.mdev) {
+ return vfio_pci_kvmgt_config_quirk(vdev, errp);
+ }
+
+ return vfio_pci_igd_config_quirk(vdev, errp);
+}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 11/21] vfio/migration: Use BE byte order for device state wire packets
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (9 preceding siblings ...)
2025-03-11 18:13 ` [PULL 10/21] vfio/igd: Fix broken KVMGT OpRegion support Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 12/21] system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h' Cédric Le Goater
` (10 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Maciej S. Szmigiero, Avihai Horon,
Cédric Le Goater
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
Wire data commonly use BE byte order (including in the existing migration
protocol), use it also for for VFIO device state packets.
This will allow VFIO multifd device state transfer between hosts with
different endianness.
Although currently there is no such use case, it's good to have it now
for completeness.
Reviewed-by: Avihai Horon <avihaih@nvidia.com>
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Link: https://lore.kernel.org/qemu-devel/dcfc04cc1a50655650dbac8398e2742ada84ee39.1741611079.git.maciej.szmigiero@oracle.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/migration-multifd.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/vfio/migration-multifd.c b/hw/vfio/migration-multifd.c
index 233724710b377afd40aabe5ef5b846f654f9865d..378f6f3bf01f6a4155fb424f8028cb5380f27f02 100644
--- a/hw/vfio/migration-multifd.c
+++ b/hw/vfio/migration-multifd.c
@@ -13,6 +13,7 @@
#include "hw/vfio/vfio-common.h"
#include "migration/misc.h"
#include "qapi/error.h"
+#include "qemu/bswap.h"
#include "qemu/error-report.h"
#include "qemu/lockable.h"
#include "qemu/main-loop.h"
@@ -155,12 +156,16 @@ bool vfio_multifd_load_state_buffer(void *opaque, char *data, size_t data_size,
return false;
}
+ packet->version = be32_to_cpu(packet->version);
if (packet->version != VFIO_DEVICE_STATE_PACKET_VER_CURRENT) {
error_setg(errp, "%s: packet has unknown version %" PRIu32,
vbasedev->name, packet->version);
return false;
}
+ packet->idx = be32_to_cpu(packet->idx);
+ packet->flags = be32_to_cpu(packet->flags);
+
if (packet->idx == UINT32_MAX) {
error_setg(errp, "%s: packet index is invalid", vbasedev->name);
return false;
@@ -558,9 +563,9 @@ vfio_save_complete_precopy_thread_config_state(VFIODevice *vbasedev,
packet_len = sizeof(*packet) + bioc->usage;
packet = g_malloc0(packet_len);
- packet->version = VFIO_DEVICE_STATE_PACKET_VER_CURRENT;
- packet->idx = idx;
- packet->flags = VFIO_DEVICE_STATE_CONFIG_STATE;
+ packet->version = cpu_to_be32(VFIO_DEVICE_STATE_PACKET_VER_CURRENT);
+ packet->idx = cpu_to_be32(idx);
+ packet->flags = cpu_to_be32(VFIO_DEVICE_STATE_CONFIG_STATE);
memcpy(&packet->data, bioc->data, bioc->usage);
if (!multifd_queue_device_state(idstr, instance_id,
@@ -610,7 +615,7 @@ vfio_multifd_save_complete_precopy_thread(SaveLiveCompletePrecopyThreadData *d,
}
packet = g_malloc0(sizeof(*packet) + migration->data_buffer_size);
- packet->version = VFIO_DEVICE_STATE_PACKET_VER_CURRENT;
+ packet->version = cpu_to_be32(VFIO_DEVICE_STATE_PACKET_VER_CURRENT);
for (idx = 0; ; idx++) {
ssize_t data_size;
@@ -631,7 +636,7 @@ vfio_multifd_save_complete_precopy_thread(SaveLiveCompletePrecopyThreadData *d,
break;
}
- packet->idx = idx;
+ packet->idx = cpu_to_be32(idx);
packet_size = sizeof(*packet) + data_size;
if (!multifd_queue_device_state(d->idstr, d->instance_id,
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 12/21] system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h'
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (10 preceding siblings ...)
2025-03-11 18:13 ` [PULL 11/21] vfio/migration: Use BE byte order for device state wire packets Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 13/21] hw/vfio/spapr: Do not include <linux/kvm.h> Cédric Le Goater
` (9 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Eric Auger, David Hildenbrand, Cédric Le Goater
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Both qemu_minrampagesize() and qemu_maxrampagesize() are
related to host memory backends, having the following call
stack:
qemu_minrampagesize()
-> find_min_backend_pagesize()
-> object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)
qemu_maxrampagesize()
-> find_max_backend_pagesize()
-> object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)
Having TYPE_MEMORY_BACKEND defined in "system/hostmem.h":
include/system/hostmem.h:23:#define TYPE_MEMORY_BACKEND "memory-backend"
Move their prototype declaration to "system/hostmem.h".
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250308230917.18907-7-philmd@linaro.org>
Acked-by: David Hildenbrand <david@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-2-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
include/exec/ram_addr.h | 3 ---
include/system/hostmem.h | 3 +++
hw/ppc/spapr_caps.c | 1 +
hw/s390x/s390-virtio-ccw.c | 1 +
hw/vfio/spapr.c | 1 +
5 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 3d8df4edf15cd916d477250444ac6833c76f0286..e4c28fbec9b470f0f64eb07ad5fcba6b43a5fcc4 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -102,9 +102,6 @@ static inline unsigned long int ramblock_recv_bitmap_offset(void *host_addr,
bool ramblock_is_pmem(RAMBlock *rb);
-long qemu_minrampagesize(void);
-long qemu_maxrampagesize(void);
-
/**
* qemu_ram_alloc_from_file,
* qemu_ram_alloc_from_fd: Allocate a ram block from the specified backing
diff --git a/include/system/hostmem.h b/include/system/hostmem.h
index 5c21ca55c0176af7356905e1b04358c52fc8a5f8..62642e602ca97dffa4baec29f9b73ccf56c29700 100644
--- a/include/system/hostmem.h
+++ b/include/system/hostmem.h
@@ -93,4 +93,7 @@ bool host_memory_backend_is_mapped(HostMemoryBackend *backend);
size_t host_memory_backend_pagesize(HostMemoryBackend *memdev);
char *host_memory_backend_get_name(HostMemoryBackend *backend);
+long qemu_minrampagesize(void);
+long qemu_maxrampagesize(void);
+
#endif
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 904bff87ce125cd0d69560f2b2ec54ed0a5e94e9..9e53d0c1fd1cccf366f7306a83f8859e22f39da3 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -34,6 +34,7 @@
#include "kvm_ppc.h"
#include "migration/vmstate.h"
#include "system/tcg.h"
+#include "system/hostmem.h"
#include "hw/ppc/spapr.h"
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index a9b3db19f63d5b50a87ed751d856294f2c0c17d5..75b32182eb01991fdc63fa1d38c838d5dc95d82b 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -41,6 +41,7 @@
#include "hw/s390x/tod.h"
#include "system/system.h"
#include "system/cpus.h"
+#include "system/hostmem.h"
#include "target/s390x/kvm/pv.h"
#include "migration/blocker.h"
#include "qapi/visitor.h"
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index ad4c499eafea27906a028d0674944fbe63e3f03a..237f96dd3fa2b7e1c859471fee014d5a3d31f1f0 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -15,6 +15,7 @@
#include <linux/kvm.h>
#endif
#include "system/kvm.h"
+#include "system/hostmem.h"
#include "exec/address-spaces.h"
#include "hw/vfio/vfio-common.h"
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 13/21] hw/vfio/spapr: Do not include <linux/kvm.h>
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (11 preceding siblings ...)
2025-03-11 18:13 ` [PULL 12/21] system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h' Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 14/21] hw/vfio/common: Include missing 'system/tcg.h' header Cédric Le Goater
` (8 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Richard Henderson, Cédric Le Goater, Eric Auger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
<linux/kvm.h> is already included by "system/kvm.h" in the next line.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250307180337.14811-3-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-3-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/spapr.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 237f96dd3fa2b7e1c859471fee014d5a3d31f1f0..1a5d1611f2cdc8fa6254a77b8ec3d1f56d28e3a6 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -11,9 +11,6 @@
#include "qemu/osdep.h"
#include <sys/ioctl.h>
#include <linux/vfio.h>
-#ifdef CONFIG_KVM
-#include <linux/kvm.h>
-#endif
#include "system/kvm.h"
#include "system/hostmem.h"
#include "exec/address-spaces.h"
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 14/21] hw/vfio/common: Include missing 'system/tcg.h' header
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (12 preceding siblings ...)
2025-03-11 18:13 ` [PULL 13/21] hw/vfio/spapr: Do not include <linux/kvm.h> Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 15/21] hw/vfio/common: Get target page size using runtime helpers Cédric Le Goater
` (7 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Richard Henderson, Cédric Le Goater, Eric Auger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Always include necessary headers explicitly, to avoid
when refactoring unrelated ones:
hw/vfio/common.c:1176:45: error: implicit declaration of function ‘tcg_enabled’;
1176 | tcg_enabled() ? DIRTY_CLIENTS_ALL :
| ^~~~~~~~~~~
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250307180337.14811-2-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-4-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7a4010ef4ee6a10bc1f9f2f8e1b12c60362a575c..b1596b6bf647e9e3c5f4701e61ddd9323d49e880 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -42,6 +42,7 @@
#include "migration/misc.h"
#include "migration/blocker.h"
#include "migration/qemu-file.h"
+#include "system/tcg.h"
#include "system/tpm.h"
VFIODeviceList vfio_device_list =
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 15/21] hw/vfio/common: Get target page size using runtime helpers
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (13 preceding siblings ...)
2025-03-11 18:13 ` [PULL 14/21] hw/vfio/common: Include missing 'system/tcg.h' header Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 16/21] hw/vfio: Compile some common objects once Cédric Le Goater
` (6 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Richard Henderson,
Cédric Le Goater
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Prefer runtime helpers to get target page size.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20250305153929.43687-3-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-5-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/common.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index b1596b6bf647e9e3c5f4701e61ddd9323d49e880..1a0d9290f88c9774a98f65087a36b86922b21a73 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -30,6 +30,7 @@
#include "exec/address-spaces.h"
#include "exec/memory.h"
#include "exec/ram_addr.h"
+#include "exec/target_page.h"
#include "hw/hw.h"
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
@@ -393,13 +394,14 @@ static void vfio_register_ram_discard_listener(VFIOContainerBase *bcontainer,
MemoryRegionSection *section)
{
RamDiscardManager *rdm = memory_region_get_ram_discard_manager(section->mr);
+ int target_page_size = qemu_target_page_size();
VFIORamDiscardListener *vrdl;
/* Ignore some corner cases not relevant in practice. */
- g_assert(QEMU_IS_ALIGNED(section->offset_within_region, TARGET_PAGE_SIZE));
+ g_assert(QEMU_IS_ALIGNED(section->offset_within_region, target_page_size));
g_assert(QEMU_IS_ALIGNED(section->offset_within_address_space,
- TARGET_PAGE_SIZE));
- g_assert(QEMU_IS_ALIGNED(int128_get64(section->size), TARGET_PAGE_SIZE));
+ target_page_size));
+ g_assert(QEMU_IS_ALIGNED(int128_get64(section->size), target_page_size));
vrdl = g_new0(VFIORamDiscardListener, 1);
vrdl->bcontainer = bcontainer;
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 16/21] hw/vfio: Compile some common objects once
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (14 preceding siblings ...)
2025-03-11 18:13 ` [PULL 15/21] hw/vfio/common: Get target page size using runtime helpers Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 17/21] hw/vfio: Compile more " Cédric Le Goater
` (5 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Richard Henderson, Cédric Le Goater, Eric Auger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Some files don't rely on any target-specific knowledge
and can be compiled once:
- helpers.c
- container-base.c
- migration.c (removing unnecessary "exec/ram_addr.h")
- migration-multifd.c
- cpr.c
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250308230917.18907-4-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-6-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/migration.c | 1 -
hw/vfio/meson.build | 13 ++++++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 416643ddd69ab64c252eadd07702a9d5dad0db47..fbff46cfc35e0ee69e9599c9f8efc7437bbe3370 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -27,7 +27,6 @@
#include "qapi/error.h"
#include "qapi/qapi-events-vfio.h"
#include "exec/ramlist.h"
-#include "exec/ram_addr.h"
#include "pci.h"
#include "trace.h"
#include "hw/hw.h"
diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 260d65febd6bcd1935cfa9f59cc08aae21fc97fd..8e376cfcbf89a822961a1a76c3030eb670dd0513 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -1,12 +1,7 @@
vfio_ss = ss.source_set()
vfio_ss.add(files(
- 'helpers.c',
'common.c',
- 'container-base.c',
'container.c',
- 'migration.c',
- 'migration-multifd.c',
- 'cpr.c',
))
vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
@@ -25,3 +20,11 @@ vfio_ss.add(when: 'CONFIG_VFIO_AP', if_true: files('ap.c'))
vfio_ss.add(when: 'CONFIG_VFIO_IGD', if_true: files('igd.c'))
specific_ss.add_all(when: 'CONFIG_VFIO', if_true: vfio_ss)
+
+system_ss.add(when: 'CONFIG_VFIO', if_true: files(
+ 'helpers.c',
+ 'container-base.c',
+ 'migration.c',
+ 'migration-multifd.c',
+ 'cpr.c',
+))
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 17/21] hw/vfio: Compile more objects once
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (15 preceding siblings ...)
2025-03-11 18:13 ` [PULL 16/21] hw/vfio: Compile some common objects once Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 18/21] hw/vfio: Compile iommufd.c once Cédric Le Goater
` (4 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Richard Henderson, Cédric Le Goater, Eric Auger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
These files depend on the VFIO symbol in their Kconfig
definition. They don't rely on target specific definitions,
move them to system_ss[] to build them once.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250308230917.18907-5-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-7-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 8e376cfcbf89a822961a1a76c3030eb670dd0513..784eae4b55994a53d16b5af600b06b0f77f2b36f 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -14,13 +14,13 @@ vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
))
vfio_ss.add(when: 'CONFIG_VFIO_CCW', if_true: files('ccw.c'))
vfio_ss.add(when: 'CONFIG_VFIO_PLATFORM', if_true: files('platform.c'))
-vfio_ss.add(when: 'CONFIG_VFIO_XGMAC', if_true: files('calxeda-xgmac.c'))
-vfio_ss.add(when: 'CONFIG_VFIO_AMD_XGBE', if_true: files('amd-xgbe.c'))
vfio_ss.add(when: 'CONFIG_VFIO_AP', if_true: files('ap.c'))
vfio_ss.add(when: 'CONFIG_VFIO_IGD', if_true: files('igd.c'))
specific_ss.add_all(when: 'CONFIG_VFIO', if_true: vfio_ss)
+system_ss.add(when: 'CONFIG_VFIO_XGMAC', if_true: files('calxeda-xgmac.c'))
+system_ss.add(when: 'CONFIG_VFIO_AMD_XGBE', if_true: files('amd-xgbe.c'))
system_ss.add(when: 'CONFIG_VFIO', if_true: files(
'helpers.c',
'container-base.c',
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 18/21] hw/vfio: Compile iommufd.c once
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (16 preceding siblings ...)
2025-03-11 18:13 ` [PULL 17/21] hw/vfio: Compile more " Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 19/21] hw/vfio: Compile display.c once Cédric Le Goater
` (3 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Richard Henderson, Cédric Le Goater, Eric Auger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
Removing unused "exec/ram_addr.h" header allow to compile
iommufd.c once for all targets.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250308230917.18907-6-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-8-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/iommufd.c | 1 -
hw/vfio/meson.build | 6 +++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index df61edffc084360d9f332ae1d122518c0ecbb955..42c8412bbf50724dddb43f9b19a3aa40c8bc311d 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -25,7 +25,6 @@
#include "qemu/cutils.h"
#include "qemu/chardev_open.h"
#include "pci.h"
-#include "exec/ram_addr.h"
static int iommufd_cdev_map(const VFIOContainerBase *bcontainer, hwaddr iova,
ram_addr_t size, void *vaddr, bool readonly)
diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 784eae4b55994a53d16b5af600b06b0f77f2b36f..5c9ec7e8971bc6e8146d173b6b957e5ed88f73b6 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -4,9 +4,6 @@ vfio_ss.add(files(
'container.c',
))
vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
-vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
- 'iommufd.c',
-))
vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
'display.c',
'pci-quirks.c',
@@ -28,3 +25,6 @@ system_ss.add(when: 'CONFIG_VFIO', if_true: files(
'migration-multifd.c',
'cpr.c',
))
+system_ss.add(when: ['CONFIG_VFIO', 'CONFIG_IOMMUFD'], if_true: files(
+ 'iommufd.c',
+))
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 19/21] hw/vfio: Compile display.c once
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (17 preceding siblings ...)
2025-03-11 18:13 ` [PULL 18/21] hw/vfio: Compile iommufd.c once Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 20/21] vfio/pci-quirks: Exclude non-ioport BAR from ATI quirk Cédric Le Goater
` (2 subsequent siblings)
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Williamson, Philippe Mathieu-Daudé, Pierrick Bouvier,
Richard Henderson, Cédric Le Goater, Eric Auger
From: Philippe Mathieu-Daudé <philmd@linaro.org>
display.c doesn't rely on target specific definitions,
move it to system_ss[] to build it once.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20250308230917.18907-8-philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/20250311085743.21724-9-philmd@linaro.org
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/meson.build | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index 5c9ec7e8971bc6e8146d173b6b957e5ed88f73b6..a8939c838657b09c38f93ad69d541df5aea30a6f 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -5,7 +5,6 @@ vfio_ss.add(files(
))
vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
vfio_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
- 'display.c',
'pci-quirks.c',
'pci.c',
))
@@ -28,3 +27,6 @@ system_ss.add(when: 'CONFIG_VFIO', if_true: files(
system_ss.add(when: ['CONFIG_VFIO', 'CONFIG_IOMMUFD'], if_true: files(
'iommufd.c',
))
+system_ss.add(when: 'CONFIG_VFIO_PCI', if_true: files(
+ 'display.c',
+))
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 20/21] vfio/pci-quirks: Exclude non-ioport BAR from ATI quirk
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (18 preceding siblings ...)
2025-03-11 18:13 ` [PULL 19/21] hw/vfio: Compile display.c once Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-11 18:13 ` [PULL 21/21] vfio/pci: Drop debug commentary from x-device-dirty-page-tracking Cédric Le Goater
2025-03-13 7:05 ` [PULL 00/21] vfio queue Stefan Hajnoczi
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Williamson, Vasilis Liaskovitis, Cédric Le Goater
From: Vasilis Liaskovitis <vliaskovitis@suse.com>
The ATI BAR4 quirk is targeting an ioport BAR. Older devices may
have a BAR4 which is not an ioport, causing a segfault here. Test
the BAR type to skip these devices.
Similar to
"8f419c5b: vfio/pci-quirks: Exclude non-ioport BAR from NVIDIA quirk"
Untested, as I don't have the card to test.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2856
Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
Reviewed-by: Alex Williamson <alex.williamson@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250310235833.41026-1-vliaskovitis@suse.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci-quirks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index f998761abca2d1c4c366a876838f1bc05b877574..3f002252acfb7ac809107c99bdbdbaf66d56a50d 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -403,7 +403,7 @@ static void vfio_probe_ati_bar4_quirk(VFIOPCIDevice *vdev, int nr)
/* This windows doesn't seem to be used except by legacy VGA code */
if (!vfio_pci_is(vdev, PCI_VENDOR_ID_ATI, PCI_ANY_ID) ||
- !vdev->vga || nr != 4) {
+ !vdev->vga || nr != 4 || !vdev->bars[4].ioport) {
return;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 21/21] vfio/pci: Drop debug commentary from x-device-dirty-page-tracking
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (19 preceding siblings ...)
2025-03-11 18:13 ` [PULL 20/21] vfio/pci-quirks: Exclude non-ioport BAR from ATI quirk Cédric Le Goater
@ 2025-03-11 18:13 ` Cédric Le Goater
2025-03-13 7:05 ` [PULL 00/21] vfio queue Stefan Hajnoczi
21 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2025-03-11 18:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Williamson, Joao Martins, Cédric Le Goater
From: Joao Martins <joao.m.martins@oracle.com>
The intent behind the x-device-dirty-page-tracking option is twofold:
1) development/testing in the presence of VFs with VF dirty page tracking
2) deliberately choosing platform dirty tracker over the VF one.
Item 2) scenario is useful when VF dirty tracker is not as fast as
IOMMU, or there's some limitations around it (e.g. number of them is
limited; aggregated address space under tracking is limited),
efficiency/scalability (e.g. 1 pagetable in IOMMU dirty tracker to scan
vs N VFs) or just troubleshooting. Given item 2 it is not restricted to
debugging, hence drop the debug parenthesis from the option description.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/qemu-devel/20250311174807.79825-1-joao.m.martins@oracle.com
[ clg: Fixed subject spelling ]
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
hw/vfio/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 3cb7806f2f219174ffb7d28595e9430b41ee40ae..7f1532fbed9aed2eae2c98f6fd79a9056ff1e84f 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3532,7 +3532,7 @@ static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
object_class_property_set_description(klass, /* 9.1 */
"x-device-dirty-page-tracking",
"Disable device dirty page tracking and use "
- "container-based dirty page tracking (DEBUG)");
+ "container-based dirty page tracking");
object_class_property_set_description(klass, /* 9.1 */
"migration-events",
"Emit VFIO migration QAPI event when a VFIO device "
--
2.48.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PULL 00/21] vfio queue
2025-03-11 18:13 [PULL 00/21] vfio queue Cédric Le Goater
` (20 preceding siblings ...)
2025-03-11 18:13 ` [PULL 21/21] vfio/pci: Drop debug commentary from x-device-dirty-page-tracking Cédric Le Goater
@ 2025-03-13 7:05 ` Stefan Hajnoczi
21 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2025-03-13 7:05 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-devel, Alex Williamson, Cédric Le Goater
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/10.0 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread