* [PATCH 1/2] hw/display: report an error if virgl initialization failed @ 2021-07-01 7:18 marcandre.lureau 2021-07-01 7:18 ` [PATCH 2/2] hw/display: fail early when multiple virgl devices are requested marcandre.lureau 0 siblings, 1 reply; 4+ messages in thread From: marcandre.lureau @ 2021-07-01 7:18 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, kraxel From: Marc-André Lureau <marcandre.lureau@redhat.com> Currently, virgl initialization error are silently ignored. This is likely going to crash later on, as the device isn't fully initialized then. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/display/virtio-gpu-virgl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 092c6dc380..46b56f94d9 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -605,6 +605,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs); if (ret != 0) { + error_report("virgl could not be initialized: %d", ret); return ret; } -- 2.29.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] hw/display: fail early when multiple virgl devices are requested 2021-07-01 7:18 [PATCH 1/2] hw/display: report an error if virgl initialization failed marcandre.lureau @ 2021-07-01 7:18 ` marcandre.lureau 2021-07-03 5:14 ` Mark Cave-Ayland 0 siblings, 1 reply; 4+ messages in thread From: marcandre.lureau @ 2021-07-01 7:18 UTC (permalink / raw) To: qemu-devel; +Cc: Marc-André Lureau, kraxel From: Marc-André Lureau <marcandre.lureau@redhat.com> This avoids failing to initialize virgl and crashing later on, and clear the user expectations. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/display/virtio-gpu-gl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index d971b48080..c973d4824b 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -25,6 +25,8 @@ #include <virglrenderer.h> +static int virgl_count = 0; + static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g, struct virtio_gpu_scanout *s, uint32_t resource_id) @@ -113,6 +115,11 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) return; #endif + if (virgl_count++ > 0) { + error_setg(errp, "multiple virgl devices aren't supported yet"); + return; + } + if (!display_opengl) { error_setg(errp, "opengl is not available"); return; @@ -124,6 +131,10 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) virtio_gpu_device_realize(qdev, errp); } +static void virtio_gpu_gl_device_unrealize(DeviceState *dev) +{ + virgl_count--; +} static Property virtio_gpu_gl_properties[] = { DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, @@ -144,6 +155,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data; vdc->realize = virtio_gpu_gl_device_realize; + vdc->unrealize = virtio_gpu_gl_device_unrealize; vdc->reset = virtio_gpu_gl_reset; device_class_set_props(dc, virtio_gpu_gl_properties); } -- 2.29.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] hw/display: fail early when multiple virgl devices are requested 2021-07-01 7:18 ` [PATCH 2/2] hw/display: fail early when multiple virgl devices are requested marcandre.lureau @ 2021-07-03 5:14 ` Mark Cave-Ayland 2021-07-05 10:42 ` Marc-André Lureau 0 siblings, 1 reply; 4+ messages in thread From: Mark Cave-Ayland @ 2021-07-03 5:14 UTC (permalink / raw) To: marcandre.lureau, qemu-devel; +Cc: kraxel On 01/07/2021 08:18, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > This avoids failing to initialize virgl and crashing later on, and clear > the user expectations. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > hw/display/virtio-gpu-gl.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c > index d971b48080..c973d4824b 100644 > --- a/hw/display/virtio-gpu-gl.c > +++ b/hw/display/virtio-gpu-gl.c > @@ -25,6 +25,8 @@ > > #include <virglrenderer.h> > > +static int virgl_count = 0; > + > static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g, > struct virtio_gpu_scanout *s, > uint32_t resource_id) > @@ -113,6 +115,11 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) > return; > #endif > > + if (virgl_count++ > 0) { > + error_setg(errp, "multiple virgl devices aren't supported yet"); > + return; > + } > + > if (!display_opengl) { > error_setg(errp, "opengl is not available"); > return; > @@ -124,6 +131,10 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) > > virtio_gpu_device_realize(qdev, errp); > } > +static void virtio_gpu_gl_device_unrealize(DeviceState *dev) > +{ > + virgl_count--; > +} > > static Property virtio_gpu_gl_properties[] = { > DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, > @@ -144,6 +155,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) > vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data; > > vdc->realize = virtio_gpu_gl_device_realize; > + vdc->unrealize = virtio_gpu_gl_device_unrealize; > vdc->reset = virtio_gpu_gl_reset; > device_class_set_props(dc, virtio_gpu_gl_properties); > } FWIW I think the best way to prevent instantiation of multiple devices is to use the QOM API to detect if more than one instance of a class exists within the QOM tree. Have a look at fw_cfg_find() and its usage from fw_cfg_common_realize() in hw/nvram/fw_cfg.c for an example of this. ATB, Mark. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] hw/display: fail early when multiple virgl devices are requested 2021-07-03 5:14 ` Mark Cave-Ayland @ 2021-07-05 10:42 ` Marc-André Lureau 0 siblings, 0 replies; 4+ messages in thread From: Marc-André Lureau @ 2021-07-05 10:42 UTC (permalink / raw) To: Mark Cave-Ayland; +Cc: QEMU, Gerd Hoffmann [-- Attachment #1: Type: text/plain, Size: 2739 bytes --] Hi Mark On Sat, Jul 3, 2021 at 9:15 AM Mark Cave-Ayland < mark.cave-ayland@ilande.co.uk> wrote: > On 01/07/2021 08:18, marcandre.lureau@redhat.com wrote: > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > This avoids failing to initialize virgl and crashing later on, and clear > > the user expectations. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > hw/display/virtio-gpu-gl.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c > > index d971b48080..c973d4824b 100644 > > --- a/hw/display/virtio-gpu-gl.c > > +++ b/hw/display/virtio-gpu-gl.c > > @@ -25,6 +25,8 @@ > > > > #include <virglrenderer.h> > > > > +static int virgl_count = 0; > > + > > static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g, > > struct virtio_gpu_scanout > *s, > > uint32_t resource_id) > > @@ -113,6 +115,11 @@ static void > virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) > > return; > > #endif > > > > + if (virgl_count++ > 0) { > > + error_setg(errp, "multiple virgl devices aren't supported yet"); > > + return; > > + } > > + > > if (!display_opengl) { > > error_setg(errp, "opengl is not available"); > > return; > > @@ -124,6 +131,10 @@ static void > virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) > > > > virtio_gpu_device_realize(qdev, errp); > > } > > +static void virtio_gpu_gl_device_unrealize(DeviceState *dev) > > +{ > > + virgl_count--; > > +} > > > > static Property virtio_gpu_gl_properties[] = { > > DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, > > @@ -144,6 +155,7 @@ static void virtio_gpu_gl_class_init(ObjectClass > *klass, void *data) > > vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data; > > > > vdc->realize = virtio_gpu_gl_device_realize; > > + vdc->unrealize = virtio_gpu_gl_device_unrealize; > > vdc->reset = virtio_gpu_gl_reset; > > device_class_set_props(dc, virtio_gpu_gl_properties); > > } > > FWIW I think the best way to prevent instantiation of multiple devices is > to use the > QOM API to detect if more than one instance of a class exists within the > QOM tree. > > Have a look at fw_cfg_find() and its usage from fw_cfg_common_realize() in > hw/nvram/fw_cfg.c for an example of this. > Good idea, I forgot about that trick. Sent "[PATCH v2] hw/display: fail early when multiple virgl devices are requested". thanks -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 3900 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-05 10:44 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-07-01 7:18 [PATCH 1/2] hw/display: report an error if virgl initialization failed marcandre.lureau 2021-07-01 7:18 ` [PATCH 2/2] hw/display: fail early when multiple virgl devices are requested marcandre.lureau 2021-07-03 5:14 ` Mark Cave-Ayland 2021-07-05 10:42 ` Marc-André Lureau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).