Message ID | 20210701071837.738897-2-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | [1/2] hw/display: report an error if virgl initialization failed | expand |
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.
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
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); }