| Message ID | 20210510132051.2208563-13-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/25] qemu-edid: use qemu_edid_size() | expand |
On 5/10/21 3:20 PM, Gerd Hoffmann wrote: > Move device init (realize) and properties. > > Drop the virgl property, the virtio-gpu-gl-device has virgl enabled no > matter what. Just use virtio-gpu-device instead if you don't want > enable virgl and opengl. This simplifies the logic and reduces the test > matrix. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Message-id: 20210430113547.1816178-1-kraxel@redhat.com > Message-Id: <20210430113547.1816178-4-kraxel@redhat.com> > --- > include/hw/virtio/virtio-gpu.h | 1 + > hw/display/virtio-gpu-gl.c | 33 +++++++++++++++++++++++++++++++++ > hw/display/virtio-gpu.c | 23 +---------------------- > 3 files changed, 35 insertions(+), 22 deletions(-) > > @@ -1251,12 +1236,6 @@ static Property virtio_gpu_properties[] = { > VIRTIO_GPU_BASE_PROPERTIES(VirtIOGPU, parent_obj.conf), > DEFINE_PROP_SIZE("max_hostmem", VirtIOGPU, conf_max_hostmem, > 256 * MiB), > -#ifdef CONFIG_VIRGL > - DEFINE_PROP_BIT("virgl", VirtIOGPU, parent_obj.conf.flags, > - VIRTIO_GPU_FLAG_VIRGL_ENABLED, true), > - DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, > - VIRTIO_GPU_FLAG_STATS_ENABLED, false), > -#endif > DEFINE_PROP_END_OF_LIST(), > }; > > Sorry for catching this a bit late, but libvirt is looking for "virgl" property when guest XML has 3D acceleration enabled: <video> <model type='virtio' heads='1' primary='yes'> <acceleration accel3d='yes'/> </model> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> This is the corresponding part of cmd line: -device virtio-vga,id=video0,virgl=on,max_outputs=1,bus=pci.0,addr=0x2 The commit message suggests that virtio-gpu-gl-device should be used instead. Fair enough, so IIUC the cmd line should be changed to: -device virtio-gpu-gl-device,id=video0,max_outputs=1,bus=pci.0,addr=0x2 Michal
Hi On Fri, May 21, 2021 at 1:34 PM Michal Prívozník <mprivozn@redhat.com> wrote: > On 5/10/21 3:20 PM, Gerd Hoffmann wrote: > > Move device init (realize) and properties. > > > > Drop the virgl property, the virtio-gpu-gl-device has virgl enabled no > > matter what. Just use virtio-gpu-device instead if you don't want > > enable virgl and opengl. This simplifies the logic and reduces the test > > matrix. > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > Message-id: 20210430113547.1816178-1-kraxel@redhat.com > > Message-Id: <20210430113547.1816178-4-kraxel@redhat.com> > > --- > > include/hw/virtio/virtio-gpu.h | 1 + > > hw/display/virtio-gpu-gl.c | 33 +++++++++++++++++++++++++++++++++ > > hw/display/virtio-gpu.c | 23 +---------------------- > > 3 files changed, 35 insertions(+), 22 deletions(-) > > > > > @@ -1251,12 +1236,6 @@ static Property virtio_gpu_properties[] = { > > VIRTIO_GPU_BASE_PROPERTIES(VirtIOGPU, parent_obj.conf), > > DEFINE_PROP_SIZE("max_hostmem", VirtIOGPU, conf_max_hostmem, > > 256 * MiB), > > -#ifdef CONFIG_VIRGL > > - DEFINE_PROP_BIT("virgl", VirtIOGPU, parent_obj.conf.flags, > > - VIRTIO_GPU_FLAG_VIRGL_ENABLED, true), > > - DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, > > - VIRTIO_GPU_FLAG_STATS_ENABLED, false), > > -#endif > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > > > Sorry for catching this a bit late, but libvirt is looking for "virgl" > property when guest XML has 3D acceleration enabled: > > <video> > <model type='virtio' heads='1' primary='yes'> > <acceleration accel3d='yes'/> > </model> > <address type='pci' domain='0x0000' bus='0x00' slot='0x02' > function='0x0'/> > </video> > > This is the corresponding part of cmd line: > > -device virtio-vga,id=video0,virgl=on,max_outputs=1,bus=pci.0,addr=0x2 > > The commit message suggests that virtio-gpu-gl-device should be used > instead. Fair enough, so IIUC the cmd line should be changed to: > > -device virtio-gpu-gl-device,id=video0,max_outputs=1,bus=pci.0,addr=0x2 > Should be with virtio-vga-gl instead. And I think virtio-gpu-gl-pci for secondary devices. (it's not clear to me if virtio-gpu*-device should be user_creatable on x86 at least)
Hi, > (it's not clear to me if virtio-gpu*-device should be user_creatable on x86 > at least) Yes (microvm uses virtio-mmio). take care, Gerd
Hi, > Sorry for catching this a bit late, but libvirt is looking for "virgl" > property when guest XML has 3D acceleration enabled: Yes, libvirt must be adapted to this. https://gitlab.com/libvirt/libvirt/-/issues/167 As far I know libvirt checks whenever the virgl property exists to figure whenever virgl support is available (as you can compile qemu without virgl support). So without changes libvirt will simply think there is no 3d support and configurations without virgl enables should continue to work fine. Configurations with virgl enabled will break though, and unfortunaly there is no easy way to avoid that. > The commit message suggests that virtio-gpu-gl-device should be used > instead. Fair enough, so IIUC the cmd line should be changed to: > > -device virtio-gpu-gl-device,id=video0,max_outputs=1,bus=pci.0,addr=0x2 virtio-gpu-gl-device is the mmio variant, for pci you need virtio-gpu-gl-pci. But otherwise yes, this is what libvirt should use in case it figures qemu supports the virtio-gpu-gl-pci device (again, when compiling with virgl disabled the device will not be there). take care, Gerd
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 5b2d011b49d5..7430f3cd9958 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -221,6 +221,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g, void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g, struct iovec *iov, uint32_t count); void virtio_gpu_process_cmdq(VirtIOGPU *g); +void virtio_gpu_device_realize(DeviceState *qdev, Error **errp); /* virtio-gpu-3d.c */ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g, diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index a477cbe186d3..9b7b5f00d7e6 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -16,14 +16,47 @@ #include "qemu/module.h" #include "qemu/error-report.h" #include "qapi/error.h" +#include "sysemu/sysemu.h" #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-gpu.h" #include "hw/virtio/virtio-gpu-bswap.h" #include "hw/virtio/virtio-gpu-pixman.h" #include "hw/qdev-properties.h" +static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) +{ + VirtIOGPU *g = VIRTIO_GPU(qdev); + +#if defined(HOST_WORDS_BIGENDIAN) + error_setg(errp, "virgl is not supported on bigendian platforms"); + return; +#endif + + if (!display_opengl) { + error_setg(errp, "opengl is not available"); + return; + } + + g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED); + VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = + virtio_gpu_virgl_get_num_capsets(g); + + virtio_gpu_device_realize(qdev, errp); +} + +static Property virtio_gpu_gl_properties[] = { + DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, + VIRTIO_GPU_FLAG_STATS_ENABLED, false), + DEFINE_PROP_END_OF_LIST(), +}; + static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) { + DeviceClass *dc = DEVICE_CLASS(klass); + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass); + + vdc->realize = virtio_gpu_gl_device_realize; + device_class_set_props(dc, virtio_gpu_gl_properties); } static const TypeInfo virtio_gpu_gl_info = { diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 6f3791deb3ae..461b7769b457 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1121,25 +1121,10 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, return 0; } -static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) +void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(qdev); VirtIOGPU *g = VIRTIO_GPU(qdev); - bool have_virgl; - -#if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN) - have_virgl = false; -#else - have_virgl = display_opengl; -#endif - if (!have_virgl) { - g->parent_obj.conf.flags &= ~(1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED); - } else { -#if defined(CONFIG_VIRGL) - VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = - virtio_gpu_virgl_get_num_capsets(g); -#endif - } if (!virtio_gpu_base_device_realize(qdev, virtio_gpu_handle_ctrl_cb, @@ -1251,12 +1236,6 @@ static Property virtio_gpu_properties[] = { VIRTIO_GPU_BASE_PROPERTIES(VirtIOGPU, parent_obj.conf), DEFINE_PROP_SIZE("max_hostmem", VirtIOGPU, conf_max_hostmem, 256 * MiB), -#ifdef CONFIG_VIRGL - DEFINE_PROP_BIT("virgl", VirtIOGPU, parent_obj.conf.flags, - VIRTIO_GPU_FLAG_VIRGL_ENABLED, true), - DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, - VIRTIO_GPU_FLAG_STATS_ENABLED, false), -#endif DEFINE_PROP_END_OF_LIST(), };
Move device init (realize) and properties. Drop the virgl property, the virtio-gpu-gl-device has virgl enabled no matter what. Just use virtio-gpu-device instead if you don't want enable virgl and opengl. This simplifies the logic and reduces the test matrix. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20210430113547.1816178-1-kraxel@redhat.com Message-Id: <20210430113547.1816178-4-kraxel@redhat.com> --- include/hw/virtio/virtio-gpu.h | 1 + hw/display/virtio-gpu-gl.c | 33 +++++++++++++++++++++++++++++++++ hw/display/virtio-gpu.c | 23 +---------------------- 3 files changed, 35 insertions(+), 22 deletions(-)