Re: [libvirt PATCH] xenconfig: Add feature gfx_passthru

On Wed, May 13, 2020 at 01:56:52PM +0200, Artur Puzio wrote: > > On 12.05.2020 15:47, Artur Puzio wrote: >> On 08.05.2020 18:41, Jim Fehlig wrote: >>> On 4/30/20 6:07 AM, Artur Puzio wrote: >>>> gfx_passthru xl.cfg option enables GPU specific quirks required for >>>> working >>>> Intel GPU passthru. Qemu (used for device model by xen) will refuse >>>> to start >>>> a VM when an IGD is passed, but this option was not set in Xen. >>> Do we really need to expose this setting to the user? I'm not really >>> sure what to think about it after reading the xl.cfg(5) man page. It >>> starts with >>> >>>    gfx_passthru=BOOLEAN|"STRING" >>> >>> The setting can be a boolean or a string - nice. And it really seems >>> specific to Intel graphics cards. The man page claims that a value of >>> 1 "Enables graphics device PCI passthrough and autodetects the type of >>> device which is being used.". It also says "Note that some graphics >>> cards (AMD/ATI cards, for example) do not necessarily require the >>> gfx_passthru option". >>> >>> Can't libxl just enable this itself if there's a PCI passthrough >>> device and it's detected as type igd? > I have checked possible ways for detection and reusing existing libxl > functions is not possible. Most of relevant functions (including > libxl__detect_gfx_passthru_kind & libxl__is_igd_vga_passthru) use > libxl__gc (libxl garbage collector). AFAIK no libvirt code uses > libxl__gc. We probably shouldn't start doing that(?) Also > libxl__detect_gfx_passthru_kind is static. > > So the only way would be to reimplement detection if IGD is attached. > Even if it would work the same as libxl detection, it would probably > stop in the future... I think just exposing the option to the user is > much, much easier and future proof. Existing strategies: 1. libxl__is_igd_vga_passthru checks for Intel device (0x8086 vendor) with VGA class (0x030000). This should be easy to duplicate using libvirt API, like virPCIDeviceReadID(dev, "vendor") and same for "class". 2. qemu uses a different approach and checks for device being 0000:00:02.0. Again, simple to duplicate into libvirt. As we can see above, there are already two approaches, and it isn't exactly clear which should be used. In fact, I think if those two strategies won't match at some point, the domain may fail to start, because qemu refuses to start with IGD attached (according to its definition) but without the option enabled. Looking at the libxl code, the only thing that option is used for, is to add it to the qemu command line (Artur, have I missed anything?).

In that case, I think ideal solution would be to patch it at the qemu level, to enable the option automatically. I believe this is exactly the case for KVM case (vfio-pci). But I think that's non-trivial approach in Xen path (if it would be easy, I think we wouldn't have this option in the first place). For example the option makes qemu choose a different PCI bridge configuration, which is I think done before checking if IGD is attached. So, while there maybe a technical reason for having this option in qmeu (instead of auto-detection), I don't really see why user needs to provide it manually. Since libxl already has a function to detect IGD, then adding autodetection there would be a better idea. I don't know what libxl maintainers would prefer, but I guess either of "not failing if gfx_passthru=1 but IGD isn't detected" or adding "gfx_passthru=auto".

BTW Here is the discussion when the option was added to qemu: https://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02756.html It may be quite helpful to understand what is it about, but also why (for example I see mentions of some specific Windows drivers).