Here I just mean when Qemu realizes IGD is passed through but without
that appropriate option set, Qemu can post something to explicitly
notify user that this option is needed in his case. But it may be a lazy
idea.
In any case I think the additions of such warnings in qemu are a
separate to the discussion in this thread, so I propose to leave it
alone for now.
Okay.
So now I think I'd better go back handling this on Xen side with your
comments. As you said the Boolean doesn't suffice to indicate that IGD
workarounds are needed. So I think we can reuse that existing bool
'gfx_passthru'.
Firstly we can redefine this as string,
Unfortunately not since libxl's API guarantee requires older clients to
keep working, i.e. those who use libxl_defbool_set on this field.
Probably the best which can be done is to deprecate this field in favour
of a new one (the old field would need to be obeyed only if the new one
was set to its default value).
Probably an Enumeration would be better than a raw string here as well.
This approach doesn't allow for the possibility of multiple such
workarounds though. It's unclear to me if this matters or not.
The other option which I've mentioned is to leave gfx_passthru and have
libxl figure out which workarounds to enable based on the set of PCI
devices passed through. I guess you don't like that approach? (due to
the need to maintain the pci vid:did list?)
No, I like that approach currently :) Please see the below,
- ("gfx_passthru",
libxl_defbool),
+ ("gfx_passthru", string),
Then
+
+ if (libxl__is_igd_vga_passthru(gc, guest_config) ||
This is just working out that way that I already posted previously, and
here I paste this code fragment again,
+static const pci_info fixup_ids[] = {
+ /* Intel HSW Classic */
+ {0x8086, 0x0402}, /* HSWGT1D, HSWD_w7 */
+ {0x8086, 0x0406}, /* HSWGT1M, HSWM_w7 */
+ {0x8086, 0x0412}, /* HSWGT2D, HSWD_w7 */
+ {0x8086, 0x0416}, /* HSWGT2M, HSWM_w7 */
+ {0x8086, 0x041E}, /* HSWGT15D, HSWD_w7 */
+ /* Intel HSW ULT */
+ {0x8086, 0x0A06}, /* HSWGT1UT, HSWM_w7 */
+ {0x8086, 0x0A16}, /* HSWGT2UT, HSWM_w7 */
+ {0x8086, 0x0A26}, /* HSWGT3UT, HSWM_w7 */
+ {0x8086, 0x0A2E}, /* HSWGT3UT28W, HSWM_w7 */
+ {0x8086, 0x0A1E}, /* HSWGT2UX, HSWM_w7 */
+ {0x8086, 0x0A0E}, /* HSWGT1ULX, HSWM_w7 */
+ /* Intel HSW CRW */
+ {0x8086, 0x0D26}, /* HSWGT3CW, HSWM_w7 */
+ {0x8086, 0x0D22}, /* HSWGT3CWDT, HSWD_w7 */
+ /* Intel HSW Server */
+ {0x8086, 0x041A}, /* HSWSVGT2, HSWD_w7 */
+ /* Intel HSW SRVR */
+ {0x8086, 0x040A}, /* HSWSVGT1, HSWD_w7 */
+ /* Intel BSW */
+ {0x8086, 0x1606}, /* BDWULTGT1, BDWM_w7 */
+ {0x8086, 0x1616}, /* BDWULTGT2, BDWM_w7 */
+ {0x8086, 0x1626}, /* BDWULTGT3, BDWM_w7 */
+ {0x8086, 0x160E}, /* BDWULXGT1, BDWM_w7 */
+ {0x8086, 0x161E}, /* BDWULXGT2, BDWM_w7 */
+ {0x8086, 0x1602}, /* BDWHALOGT1, BDWM_w7 */
+ {0x8086, 0x1612}, /* BDWHALOGT2, BDWM_w7 */
+ {0x8086, 0x1622}, /* BDWHALOGT3, BDWM_w7 */
+ {0x8086, 0x162B}, /* BDWHALO28W, BDWM_w7 */
+ {0x8086, 0x162A}, /* BDWGT3WRKS, BDWM_w7 */
+ {0x8086, 0x162D}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+/*
+ * Some devices may need some ways to work well. Here like IGD,
+ * we have to pass a specific option to qemu.
+ */
+int libxl__is_igd_vga_passthru(libxl__gc *gc,
+ const libxl_domain_config *d_config)
+{
+ unsigned int i, j, num = ARRAY_SIZE(fixup_ids);
+ uint16_t vendor, device;
+
+ for (i = 0 ; i < d_config->num_pcidevs ; i++) {
+ libxl_device_pci *pcidev = &d_config->pcidevs[i];
+
+ for (j = 0 ; j < num ; j++) {
+ vendor = fixup_ids[j].vendor;
+ device = fixup_ids[j].device;
+
+ if (sysfs_dev_get_vendor(gc, pcidev) == vendor &&
+ sysfs_dev_get_device(gc, pcidev) == device)
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
Is this expected?
+ (b_info->u.hvm.gfx_passthru &&
+ strncmp(b_info->u.hvm.gfx_passthru, "igd", 3) == 0) ) {
But as you mentioned previously,
"
You might like to optionally consider add a forcing option somehow so
that people with new devices not in the list can control things without
the need to recompile (e.g. gfx_passthru_kind_override?).
"
Here I was trying to convert "gfx_passthru" to address this thing.
According to your comment right now, you prefer we should introduce a
new field instead of the original 'gfx_passthru' to enumerate such a
type. So what about this?
libxl_gfx_passthru_kind_type = Enumeration("gfx_passthru_kind_type", [
(0, "unknown"),
(1, "igd"),
])
Then if we want to override this, just submit the following line into .cfg:
gfx_passthru_kind_override = "igd"
Thanks
Tiejun
+ machinearg = GCSPRINTF("%s,igd-passthru=on",
machinearg);
+ }
+
Of course we need modify something else to align this change.
Thanks
Tiejun
