* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-03 15:32 [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode Marc-André Lureau
@ 2018-08-03 16:48 ` Stefan Berger
2018-08-06 13:39 ` Peter Maydell
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Stefan Berger @ 2018-08-03 16:48 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: berrange, kraxel, Michael S. Tsirkin
On 08/03/2018 11:32 AM, Marc-André Lureau wrote:
> With vga=775 on the Linux command line a first boot of the VM running
> Linux works fine. After a warm reboot it crashes during Linux boot.
>
> Before that, valgrind points out bad memory write to console
> surface. The VGA code is not aware that virtio-gpu got a message
> surface scanout when the display is disabled. Let's reset VGA graphic
> mode when it is the case, so that a new display surface is created
> when doing further VGA operations.
>
> https://bugs.launchpad.net/qemu/+bug/1784900/
>
> Reported-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-03 15:32 [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode Marc-André Lureau
2018-08-03 16:48 ` Stefan Berger
@ 2018-08-06 13:39 ` Peter Maydell
2018-08-06 13:47 ` Marc-André Lureau
2018-08-06 16:55 ` Peter Maydell
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2018-08-06 13:39 UTC (permalink / raw)
To: Marc-André Lureau
Cc: QEMU Developers, Michael S. Tsirkin, Gerd Hoffmann, Stefan Berger
On 3 August 2018 at 16:32, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> With vga=775 on the Linux command line a first boot of the VM running
> Linux works fine. After a warm reboot it crashes during Linux boot.
>
> Before that, valgrind points out bad memory write to console
> surface. The VGA code is not aware that virtio-gpu got a message
> surface scanout when the display is disabled. Let's reset VGA graphic
> mode when it is the case, so that a new display surface is created
> when doing further VGA operations.
>
> https://bugs.launchpad.net/qemu/+bug/1784900/
>
> Reported-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 insertions(+)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9780f755ef..d0321672f4 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -125,6 +125,7 @@ typedef struct VirtIOGPU {
> uint32_t bytes_3d;
> } stats;
>
> + void (*disable_scanout)(struct VirtIOGPU *g, int scanout_id);
> Error *migration_blocker;
> } VirtIOGPU;
I guess for a last-minute 3.0 fix this is ok, but it looks
a bit weird to have a function pointer in a device instance
struct, which the user of the device has reached in and
set in order to modify the behaviour of the object...
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-06 13:39 ` Peter Maydell
@ 2018-08-06 13:47 ` Marc-André Lureau
0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2018-08-06 13:47 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, Michael S. Tsirkin, Gerd Hoffmann, Stefan Berger
On Mon, Aug 6, 2018 at 3:39 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 3 August 2018 at 16:32, Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
>> With vga=775 on the Linux command line a first boot of the VM running
>> Linux works fine. After a warm reboot it crashes during Linux boot.
>>
>> Before that, valgrind points out bad memory write to console
>> surface. The VGA code is not aware that virtio-gpu got a message
>> surface scanout when the display is disabled. Let's reset VGA graphic
>> mode when it is the case, so that a new display surface is created
>> when doing further VGA operations.
>>
>> https://bugs.launchpad.net/qemu/+bug/1784900/
>>
>> Reported-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 insertions(+)
>>
>> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
>> index 9780f755ef..d0321672f4 100644
>> --- a/include/hw/virtio/virtio-gpu.h
>> +++ b/include/hw/virtio/virtio-gpu.h
>> @@ -125,6 +125,7 @@ typedef struct VirtIOGPU {
>> uint32_t bytes_3d;
>> } stats;
>>
>> + void (*disable_scanout)(struct VirtIOGPU *g, int scanout_id);
>> Error *migration_blocker;
>> } VirtIOGPU;
>
> I guess for a last-minute 3.0 fix this is ok, but it looks
> a bit weird to have a function pointer in a device instance
> struct, which the user of the device has reached in and
> set in order to modify the behaviour of the object...
It's not much different than pointing to a static const structure with
function pointers (I thought that was unnecessary for just one
function)
We could also have a VirtioGPUClass with virtual methods, and subclass
TYPE_VIRTIO_GPU. Also a bit over-complicated, and conflicting with my
vhost-user-gpu series. So we may eventually get there in 3.1 though.
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-03 15:32 [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode Marc-André Lureau
2018-08-03 16:48 ` Stefan Berger
2018-08-06 13:39 ` Peter Maydell
@ 2018-08-06 16:55 ` Peter Maydell
2018-08-06 21:16 ` Michael S. Tsirkin
2018-08-07 12:55 ` Gerd Hoffmann
4 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2018-08-06 16:55 UTC (permalink / raw)
To: Marc-André Lureau
Cc: QEMU Developers, Michael S. Tsirkin, Gerd Hoffmann, Stefan Berger
I think this is now our last bug fix needed for rc4, which ideally
I'd like to tag tomorrow (Tuesday). Is there anybody familiar with
virtio-gpu who could review it?
thanks
-- PMM
On 3 August 2018 at 16:32, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> With vga=775 on the Linux command line a first boot of the VM running
> Linux works fine. After a warm reboot it crashes during Linux boot.
>
> Before that, valgrind points out bad memory write to console
> surface. The VGA code is not aware that virtio-gpu got a message
> surface scanout when the display is disabled. Let's reset VGA graphic
> mode when it is the case, so that a new display surface is created
> when doing further VGA operations.
>
> https://bugs.launchpad.net/qemu/+bug/1784900/
>
> Reported-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 insertions(+)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9780f755ef..d0321672f4 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -125,6 +125,7 @@ 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 ec366f4c35..3ddd29c0de 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -421,6 +421,11 @@ 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 2b36f2899a..672b7f9ce2 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -75,6 +75,16 @@ 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,
> @@ -156,6 +166,7 @@ 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.18.0.547.g1d89318c48
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-03 15:32 [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode Marc-André Lureau
` (2 preceding siblings ...)
2018-08-06 16:55 ` Peter Maydell
@ 2018-08-06 21:16 ` Michael S. Tsirkin
2018-08-06 22:43 ` Marc-André Lureau
2018-08-07 12:55 ` Gerd Hoffmann
4 siblings, 1 reply; 11+ messages in thread
From: Michael S. Tsirkin @ 2018-08-06 21:16 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, berrange, kraxel, stefanb
On Fri, Aug 03, 2018 at 05:32:35PM +0200, Marc-André Lureau wrote:
> With vga=775 on the Linux command line a first boot of the VM running
> Linux works fine. After a warm reboot it crashes during Linux boot.
>
> Before that, valgrind points out bad memory write to console
> surface. The VGA code is not aware that virtio-gpu got a message
> surface scanout when the display is disabled. Let's reset VGA graphic
> mode when it is the case, so that a new display surface is created
> when doing further VGA operations.
>
> https://bugs.launchpad.net/qemu/+bug/1784900/
>
> Reported-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 insertions(+)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9780f755ef..d0321672f4 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -125,6 +125,7 @@ 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 ec366f4c35..3ddd29c0de 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -421,6 +421,11 @@ 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 2b36f2899a..672b7f9ce2 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -75,6 +75,16 @@ 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,
Would it make sense to add
vga_disable_scanout() to hw/display/vga_int.h and
poke at graphic_mode there?
I'll leave the decision to you.
> @@ -156,6 +166,7 @@ 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++) {
While I really know very little about vga, it seems
like that's the standard way to force full update so
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
--
MST
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-06 21:16 ` Michael S. Tsirkin
@ 2018-08-06 22:43 ` Marc-André Lureau
0 siblings, 0 replies; 11+ messages in thread
From: Marc-André Lureau @ 2018-08-06 22:43 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Stefan Berger, QEMU, Gerd Hoffmann
Hi
On Mon, Aug 6, 2018 at 11:16 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Aug 03, 2018 at 05:32:35PM +0200, Marc-André Lureau wrote:
>> With vga=775 on the Linux command line a first boot of the VM running
>> Linux works fine. After a warm reboot it crashes during Linux boot.
>>
>> Before that, valgrind points out bad memory write to console
>> surface. The VGA code is not aware that virtio-gpu got a message
>> surface scanout when the display is disabled. Let's reset VGA graphic
>> mode when it is the case, so that a new display surface is created
>> when doing further VGA operations.
>>
>> https://bugs.launchpad.net/qemu/+bug/1784900/
>>
>> Reported-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 insertions(+)
>>
>> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
>> index 9780f755ef..d0321672f4 100644
>> --- a/include/hw/virtio/virtio-gpu.h
>> +++ b/include/hw/virtio/virtio-gpu.h
>> @@ -125,6 +125,7 @@ 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 ec366f4c35..3ddd29c0de 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -421,6 +421,11 @@ 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 2b36f2899a..672b7f9ce2 100644
>> --- a/hw/display/virtio-vga.c
>> +++ b/hw/display/virtio-vga.c
>> @@ -75,6 +75,16 @@ 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,
>
> Would it make sense to add
> vga_disable_scanout() to hw/display/vga_int.h and
> poke at graphic_mode there?
>
> I'll leave the decision to you.
If Gerd doesn't chime in before RC4, I suggest to take that version.
He might want to update it or take a different approach post 3.0.
>
>> @@ -156,6 +166,7 @@ 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++) {
>
> While I really know very little about vga, it seems
> like that's the standard way to force full update so
>
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-03 15:32 [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode Marc-André Lureau
` (3 preceding siblings ...)
2018-08-06 21:16 ` Michael S. Tsirkin
@ 2018-08-07 12:55 ` Gerd Hoffmann
2018-08-07 13:02 ` Marc-André Lureau
4 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2018-08-07 12:55 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, berrange, stefanb, Michael S. Tsirkin
On Fri, Aug 03, 2018 at 05:32:35PM +0200, Marc-André Lureau wrote:
> With vga=775 on the Linux command line a first boot of the VM running
> Linux works fine. After a warm reboot it crashes during Linux boot.
>
> Before that, valgrind points out bad memory write to console
> surface. The VGA code is not aware that virtio-gpu got a message
> surface scanout when the display is disabled. Let's reset VGA graphic
> mode when it is the case, so that a new display surface is created
> when doing further VGA operations.
I guess we should simply call vga_common_reset() in virtio_vga_reset()
instead, can you try that?
thanks,
Gerd
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-07 12:55 ` Gerd Hoffmann
@ 2018-08-07 13:02 ` Marc-André Lureau
2018-08-07 13:41 ` Gerd Hoffmann
0 siblings, 1 reply; 11+ messages in thread
From: Marc-André Lureau @ 2018-08-07 13:02 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Michael S. Tsirkin, QEMU, Stefan Berger
Hi
On Tue, Aug 7, 2018 at 2:55 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Fri, Aug 03, 2018 at 05:32:35PM +0200, Marc-André Lureau wrote:
>> With vga=775 on the Linux command line a first boot of the VM running
>> Linux works fine. After a warm reboot it crashes during Linux boot.
>>
>> Before that, valgrind points out bad memory write to console
>> surface. The VGA code is not aware that virtio-gpu got a message
>> surface scanout when the display is disabled. Let's reset VGA graphic
>> mode when it is the case, so that a new display surface is created
>> when doing further VGA operations.
>
> I guess we should simply call vga_common_reset() in virtio_vga_reset()
> instead, can you try that?
>
Yes I tried, and it still fails. However I think the call is missing
and I was planning to send it for 3.1 later, since I don't think it's
RC material.
The ordering of events is not completely obvious to me, but it's still
crashing/corrupting memory after the reset occurs. The scanout get's
disable after it iirc.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-07 13:02 ` Marc-André Lureau
@ 2018-08-07 13:41 ` Gerd Hoffmann
2018-08-07 15:07 ` Peter Maydell
0 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2018-08-07 13:41 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Michael S. Tsirkin, QEMU, Stefan Berger, Peter Maydell
On Tue, Aug 07, 2018 at 03:02:49PM +0200, Marc-André Lureau wrote:
> Hi
>
> On Tue, Aug 7, 2018 at 2:55 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > On Fri, Aug 03, 2018 at 05:32:35PM +0200, Marc-André Lureau wrote:
> >> With vga=775 on the Linux command line a first boot of the VM running
> >> Linux works fine. After a warm reboot it crashes during Linux boot.
> >>
> >> Before that, valgrind points out bad memory write to console
> >> surface. The VGA code is not aware that virtio-gpu got a message
> >> surface scanout when the display is disabled. Let's reset VGA graphic
> >> mode when it is the case, so that a new display surface is created
> >> when doing further VGA operations.
> >
> > I guess we should simply call vga_common_reset() in virtio_vga_reset()
> > instead, can you try that?
> >
> Yes I tried, and it still fails. However I think the call is missing
> and I was planning to send it for 3.1 later, since I don't think it's
> RC material.
> The ordering of events is not completely obvious to me, but it's still
> crashing/corrupting memory after the reset occurs. The scanout get's
> disable after it iirc.
Ok, lets go with this patch for -rc4 and figure the reset issues later.
It is correct, even though I think there must be a simpler way to do it.
When we have a better fix we can revert it.
Peter, can you apply it directly?
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
thanks,
Gerd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-gpu: fix crashes upon warm reboot with vga mode
2018-08-07 13:41 ` Gerd Hoffmann
@ 2018-08-07 15:07 ` Peter Maydell
0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2018-08-07 15:07 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Marc-André Lureau, Michael S. Tsirkin, QEMU, Stefan Berger
On 7 August 2018 at 14:41, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Tue, Aug 07, 2018 at 03:02:49PM +0200, Marc-André Lureau wrote:
>> Hi
>>
>> On Tue, Aug 7, 2018 at 2:55 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> > On Fri, Aug 03, 2018 at 05:32:35PM +0200, Marc-André Lureau wrote:
>> >> With vga=775 on the Linux command line a first boot of the VM running
>> >> Linux works fine. After a warm reboot it crashes during Linux boot.
>> >>
>> >> Before that, valgrind points out bad memory write to console
>> >> surface. The VGA code is not aware that virtio-gpu got a message
>> >> surface scanout when the display is disabled. Let's reset VGA graphic
>> >> mode when it is the case, so that a new display surface is created
>> >> when doing further VGA operations.
>> >
>> > I guess we should simply call vga_common_reset() in virtio_vga_reset()
>> > instead, can you try that?
>> >
>> Yes I tried, and it still fails. However I think the call is missing
>> and I was planning to send it for 3.1 later, since I don't think it's
>> RC material.
>> The ordering of events is not completely obvious to me, but it's still
>> crashing/corrupting memory after the reset occurs. The scanout get's
>> disable after it iirc.
>
> Ok, lets go with this patch for -rc4 and figure the reset issues later.
> It is correct, even though I think there must be a simpler way to do it.
> When we have a better fix we can revert it.
>
> Peter, can you apply it directly?
>
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Applied to master, thanks.
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread