[Qemu-devel] [PATCH 0/2] virtio-vga: fix reset

[Qemu-devel] [PATCH 0/2] virtio-vga: fix reset

[Gerd Hoffmann]

Gerd Hoffmann (2):
  virtio-vga: fix reset
  Revert "virtio-gpu: fix crashes upon warm reboot with vga mode"

 include/hw/virtio/virtio-gpu.h |  2 +-
 hw/display/virtio-gpu.c        |  7 +------
 hw/display/virtio-vga.c        | 17 +++++------------
 3 files changed, 7 insertions(+), 19 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 1/2] virtio-vga: fix reset

[Gerd Hoffmann]

We must call the reset functions for both virtio-gpu
and vga to properly reset the combo device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/virtio/virtio-gpu.h | 1 +
 hw/display/virtio-gpu.c        | 2 +-
 hw/display/virtio-vga.c        | 6 +++++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index d0321672f4..a718b7233f 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -150,6 +150,7 @@ extern const GraphicHwOps virtio_gpu_ops;
     } while (0)
 
 /* virtio-gpu.c */
+void virtio_gpu_reset(VirtIODevice *vdev);
 void virtio_gpu_ctrl_response(VirtIOGPU *g,
                               struct virtio_gpu_ctrl_command *cmd,
                               struct virtio_gpu_ctrl_hdr *resp,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 3ddd29c0de..d6c9b02b86 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1289,7 +1289,7 @@ static void virtio_gpu_instance_init(Object *obj)
 {
 }
 
-static void virtio_gpu_reset(VirtIODevice *vdev)
+void virtio_gpu_reset(VirtIODevice *vdev)
 {
     VirtIOGPU *g = VIRTIO_GPU(vdev);
     struct virtio_gpu_simple_resource *res, *tmp;
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 701d980872..50c72f26f4 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -179,8 +179,12 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 static void virtio_vga_reset(DeviceState *dev)
 {
     VirtIOVGA *vvga = VIRTIO_VGA(dev);
-    vvga->vdev.enable = 0;
 
+    /* reset virtio-gpu */
+    virtio_gpu_reset(VIRTIO_DEVICE(&vvga->vdev));
+
+    /* reset vga */
+    vga_common_reset(&vvga->vga);
     vga_dirty_log_start(&vvga->vga);
 }
 
-- 
2.9.3

[Qemu-devel] [PATCH 2/2] Revert "virtio-gpu: fix crashes upon warm reboot with vga mode"

[Gerd Hoffmann]

This reverts commit 93f874fe9dbe0b997b5a9459840957efd13d7191.

Now with virtio-vga being resetted properly the
crash workaround is not needed any more.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/virtio/virtio-gpu.h |  1 -
 hw/display/virtio-gpu.c        |  5 -----
 hw/display/virtio-vga.c        | 11 -----------
 3 files changed, 17 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a718b7233f..2374f022e8 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -125,7 +125,6 @@ typedef struct VirtIOGPU {
         uint32_t bytes_3d;
     } stats;
 
-    void (*disable_scanout)(struct VirtIOGPU *g, int scanout_id);
     Error *migration_blocker;
 } VirtIOGPU;
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index d6c9b02b86..1cc953006f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -421,11 +421,6 @@ static void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
                                          scanout->height ?: 480,
                                          "Guest disabled display.");
     }
-
-    if (g->disable_scanout) {
-        g->disable_scanout(g, scanout_id);
-    }
-
     dpy_gfx_replace_surface(scanout->con, ds);
     scanout->resource_id = 0;
     scanout->ds = NULL;
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 50c72f26f4..1e601c1a3b 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -75,16 +75,6 @@ static void virtio_vga_gl_block(void *opaque, bool block)
     }
 }
 
-static void virtio_vga_disable_scanout(VirtIOGPU *g, int scanout_id)
-{
-    VirtIOVGA *vvga = container_of(g, VirtIOVGA, vdev);
-
-    if (scanout_id == 0) {
-        /* reset surface if needed */
-        vvga->vga.graphic_mode = -1;
-    }
-}
-
 static const GraphicHwOps virtio_vga_ops = {
     .invalidate = virtio_vga_invalidate_display,
     .gfx_update = virtio_vga_update_display,
@@ -166,7 +156,6 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
                                  vvga->vga_mrs, true);
 
     vga->con = g->scanout[0].con;
-    g->disable_scanout = virtio_vga_disable_scanout;
     graphic_console_set_hwops(vga->con, &virtio_vga_ops, vvga);
 
     for (i = 0; i < g->conf.max_outputs; i++) {
-- 
2.9.3

Re: [Qemu-devel] [PATCH 1/2] virtio-vga: fix reset

[Marc-André Lureau]

Hi

On Tue, Aug 21, 2018 at 1:13 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> We must call the reset functions for both virtio-gpu
> and vga to properly reset the combo device.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/virtio/virtio-gpu.h | 1 +
>  hw/display/virtio-gpu.c        | 2 +-
>  hw/display/virtio-vga.c        | 6 +++++-
>  3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index d0321672f4..a718b7233f 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -150,6 +150,7 @@ extern const GraphicHwOps virtio_gpu_ops;
>      } while (0)
>
>  /* virtio-gpu.c */
> +void virtio_gpu_reset(VirtIODevice *vdev);
>  void virtio_gpu_ctrl_response(VirtIOGPU *g,
>                                struct virtio_gpu_ctrl_command *cmd,
>                                struct virtio_gpu_ctrl_hdr *resp,
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 3ddd29c0de..d6c9b02b86 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1289,7 +1289,7 @@ static void virtio_gpu_instance_init(Object *obj)
>  {
>  }
>
> -static void virtio_gpu_reset(VirtIODevice *vdev)
> +void virtio_gpu_reset(VirtIODevice *vdev)
>  {
>      VirtIOGPU *g = VIRTIO_GPU(vdev);
>      struct virtio_gpu_simple_resource *res, *tmp;
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index 701d980872..50c72f26f4 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -179,8 +179,12 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>  static void virtio_vga_reset(DeviceState *dev)
>  {
>      VirtIOVGA *vvga = VIRTIO_VGA(dev);
> -    vvga->vdev.enable = 0;
>
> +    /* reset virtio-gpu */
> +    virtio_gpu_reset(VIRTIO_DEVICE(&vvga->vdev));
> +
> +    /* reset vga */
> +    vga_common_reset(&vvga->vga);

Ah, I only tested with vga_common_reset(). I was missing the
virtio_gpu_reset() part.

I assume you tested it?

>      vga_dirty_log_start(&vvga->vga);
>  }
>
> --
> 2.9.3
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/2] virtio-vga: fix reset

[Gerd Hoffmann]

> > @@ -179,8 +179,12 @@ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> >  static void virtio_vga_reset(DeviceState *dev)
> >  {
> >      VirtIOVGA *vvga = VIRTIO_VGA(dev);
> > -    vvga->vdev.enable = 0;
> >
> > +    /* reset virtio-gpu */
> > +    virtio_gpu_reset(VIRTIO_DEVICE(&vvga->vdev));
> > +
> > +    /* reset vga */
> > +    vga_common_reset(&vvga->vga);
> 
> Ah, I only tested with vga_common_reset(). I was missing the
> virtio_gpu_reset() part.

/me too (at least back then when discussing -rc4), took me some
debugging to figure ...

> I assume you tested it?

Sure.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/2] virtio-vga: fix reset

[Marc-André Lureau]

On Tue, Aug 21, 2018 at 1:13 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>
>
> Gerd Hoffmann (2):
>   virtio-vga: fix reset
>   Revert "virtio-gpu: fix crashes upon warm reboot with vga mode"

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>
>  include/hw/virtio/virtio-gpu.h |  2 +-
>  hw/display/virtio-gpu.c        |  7 +------
>  hw/display/virtio-vga.c        | 17 +++++------------
>  3 files changed, 7 insertions(+), 19 deletions(-)
>
> --
> 2.9.3
>


-- 
Marc-André Lureau