Re: [libvirt] [PATCH] vfio/pci: Default display option to "off"
[Cc +Erik,libvirt] Sorry, should have cc'd libvirt with this initially since display support is under development. I think "off" is the better compatibility option, but perhaps the damage is done since it was the 2.12 default. Thanks, Alex On Tue, 29 May 2018 09:18:10 -0600 Alex Williamson <alex.williamson@redhat.com> wrote:
Commit a9994687cb9b ("vfio/display: core & wireup") added display support to vfio-pci with the default being "auto", which breaks existing VMs when the vGPU requires GL support but had no previous requirement for a GL compatible configuration. "Off" is the safer default as we impose no new requirements to VM configurations.
Fixes: a9994687cb9b ("vfio/display: core & wireup") Cc: qemu-stable@nongnu.org Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Alex Williamson <alex.williamson@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 84e27c7bb2d1..18c493b49ec1 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -3160,7 +3160,7 @@ static Property vfio_pci_dev_properties[] = { DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host), DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev), DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice, - display, ON_OFF_AUTO_AUTO), + display, ON_OFF_AUTO_OFF), DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice, intx.mmap_timeout, 1100), DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
On Tue, May 29, 2018 at 09:24:08AM -0600, Alex Williamson wrote:
[Cc +Erik,libvirt]
Sorry, should have cc'd libvirt with this initially since display support is under development. I think "off" is the better compatibility option, but perhaps the damage is done since it was the 2.12 default. Thanks,
Thanks for CC'ing me. So, the approach I took with the initial design to be sent as an RFC is to default to display=off in libvirt. My implementation is lacking support for display=auto, solely because the way libvirt would need to get all the information about devices was cumbersome - whether it uses dmabuf or vfio regions ergo not guessing whether a missing gl=on signals dmabuf or it was just a mistake. Therefore, we're going to default to use egl-headless unless explicitly asked to turn on OpenGL. Back to the 'auto' value, it would also mean for libvirt to make policy decisions which is not what libvirt's supposed to do plus in this case it's difficult for any layer to guess what user's intention with the device was, whether it was supposed to be used for graphics or not. We can add support for 'auto' once there are compelling reasons for us to do so and the overall consensus is in favour of implementing that, however, we wouldn't change the display=off default anyway, since it's much safer for us. Erik
Alex
On Tue, 29 May 2018 09:18:10 -0600 Alex Williamson <alex.williamson@redhat.com> wrote:
Commit a9994687cb9b ("vfio/display: core & wireup") added display support to vfio-pci with the default being "auto", which breaks existing VMs when the vGPU requires GL support but had no previous requirement for a GL compatible configuration. "Off" is the safer default as we impose no new requirements to VM configurations.
Fixes: a9994687cb9b ("vfio/display: core & wireup") Cc: qemu-stable@nongnu.org Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Alex Williamson <alex.williamson@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 84e27c7bb2d1..18c493b49ec1 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -3160,7 +3160,7 @@ static Property vfio_pci_dev_properties[] = { DEFINE_PROP_PCI_HOST_DEVADDR("host", VFIOPCIDevice, host), DEFINE_PROP_STRING("sysfsdev", VFIOPCIDevice, vbasedev.sysfsdev), DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice, - display, ON_OFF_AUTO_AUTO), + display, ON_OFF_AUTO_OFF), DEFINE_PROP_UINT32("x-intx-mmap-timeout-ms", VFIOPCIDevice, intx.mmap_timeout, 1100), DEFINE_PROP_BIT("x-vga", VFIOPCIDevice, features,
participants (2)
-
Alex Williamson -
Erik Skultety