[Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)

[Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)

[Alon Levy]

If we migrate when the device is not in a native state the guest
still believes the slots are created, and will cause operations
that reference the slots, causing a "panic: virtual address out of range"
on the first of them. Easy to see by migrating in vga mode (with
a driver loaded, for instance windows cmd window in full screen mode)
and then exiting vga mode back to native mode will cause said panic.

Fixed by doing the slot recreation unconditionally at post_load

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 6db2f1a..c5204d8 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1684,6 +1684,12 @@ static int qxl_post_load(void *opaque, int version)
         qxl_mode_to_string(d->mode));
     newmode = d->mode;
     d->mode = QXL_MODE_UNDEFINED;
+    for (i = 0; i < NUM_MEMSLOTS; i++) {
+        if (!d->guest_slots[i].active) {
+            continue;
+        }
+        qxl_add_memslot(d, i, 0, QXL_SYNC);
+    }
     switch (newmode) {
     case QXL_MODE_UNDEFINED:
         break;
@@ -1691,12 +1697,6 @@ static int qxl_post_load(void *opaque, int version)
         qxl_enter_vga_mode(d);
         break;
     case QXL_MODE_NATIVE:
-        for (i = 0; i < NUM_MEMSLOTS; i++) {
-            if (!d->guest_slots[i].active) {
-                continue;
-            }
-            qxl_add_memslot(d, i, 0, QXL_SYNC);
-        }
         qxl_create_guest_primary(d, 1, QXL_SYNC);
 
         /* replay surface-create and cursor-set commands */
-- 
1.7.6.2

Re: [Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)

[Gerd Hoffmann]

On 09/22/11 14:33, Alon Levy wrote:
> If we migrate when the device is not in a native state the guest
> still believes the slots are created, and will cause operations
> that reference the slots, causing a "panic: virtual address out of range"
> on the first of them. Easy to see by migrating in vga mode (with
> a driver loaded, for instance windows cmd window in full screen mode)
> and then exiting vga mode back to native mode will cause said panic.
>
> Fixed by doing the slot recreation unconditionally at post_load

Added to spice patch queue.

thanks,
   Gerd

[Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)

[Alon Levy]

If we migrate when the device is not in a native state the guest
still believes the slots are created, and will cause operations
that reference the slots, causing a "panic: virtual address out of range"
on the first of them. Easy to see by migrating in vga mode (with
a driver loaded, for instance windows cmd window in full screen mode)
and then exiting vga mode back to native mode will cause said panic.

Fixed by doing the slot recreation unconditionally at post_load

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 03848ed..4e9f39f 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1684,6 +1684,14 @@ static int qxl_post_load(void *opaque, int version)
         qxl_mode_to_string(d->mode));
     newmode = d->mode;
     d->mode = QXL_MODE_UNDEFINED;
+    for (i = 0; i < NUM_MEMSLOTS; i++) {
+        if (!d->guest_slots[i].active) {
+            continue;
+        }
+        dprint(d, 1, "%s: restoring guest slot %d delta %"PRIu64"\n",
+               __func__, i, d->guest_slots[i].delta);
+        qxl_add_memslot(d, i, d->guest_slots[i].delta, QXL_SYNC);
+    }
     switch (newmode) {
     case QXL_MODE_UNDEFINED:
         break;
@@ -1691,12 +1699,6 @@ static int qxl_post_load(void *opaque, int version)
         qxl_enter_vga_mode(d);
         break;
     case QXL_MODE_NATIVE:
-        for (i = 0; i < NUM_MEMSLOTS; i++) {
-            if (!d->guest_slots[i].active) {
-                continue;
-            }
-            qxl_add_memslot(d, i, 0, QXL_SYNC);
-        }
         qxl_create_guest_primary(d, 1, QXL_SYNC);
 
         /* replay surface-create and cursor-set commands */
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)

[Yonit Halperin]

ACK
On 10/17/2011 12:24 PM, Alon Levy wrote:
> If we migrate when the device is not in a native state the guest
> still believes the slots are created, and will cause operations
> that reference the slots, causing a "panic: virtual address out of range"
> on the first of them. Easy to see by migrating in vga mode (with
> a driver loaded, for instance windows cmd window in full screen mode)
> and then exiting vga mode back to native mode will cause said panic.
>
> Fixed by doing the slot recreation unconditionally at post_load
>
> Signed-off-by: Alon Levy<alevy@redhat.com>
> ---
>   hw/qxl.c |   14 ++++++++------
>   1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 03848ed..4e9f39f 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -1684,6 +1684,14 @@ static int qxl_post_load(void *opaque, int version)
>           qxl_mode_to_string(d->mode));
>       newmode = d->mode;
>       d->mode = QXL_MODE_UNDEFINED;
> +    for (i = 0; i<  NUM_MEMSLOTS; i++) {
> +        if (!d->guest_slots[i].active) {
> +            continue;
> +        }
> +        dprint(d, 1, "%s: restoring guest slot %d delta %"PRIu64"\n",
> +               __func__, i, d->guest_slots[i].delta);
> +        qxl_add_memslot(d, i, d->guest_slots[i].delta, QXL_SYNC);
> +    }
>       switch (newmode) {
>       case QXL_MODE_UNDEFINED:
>           break;
> @@ -1691,12 +1699,6 @@ static int qxl_post_load(void *opaque, int version)
>           qxl_enter_vga_mode(d);
>           break;
>       case QXL_MODE_NATIVE:
> -        for (i = 0; i<  NUM_MEMSLOTS; i++) {
> -            if (!d->guest_slots[i].active) {
> -                continue;
> -            }
> -            qxl_add_memslot(d, i, 0, QXL_SYNC);
> -        }
>           qxl_create_guest_primary(d, 1, QXL_SYNC);
>
>           /* replay surface-create and cursor-set commands */

Re: [Qemu-devel] [PATCH] qxl: create slots on post_load in any state (fix RHBZ 740547)

[Alon Levy]

On Mon, Oct 17, 2011 at 12:24:20PM +0200, Alon Levy wrote:
> If we migrate when the device is not in a native state the guest
> still believes the slots are created, and will cause operations
> that reference the slots, causing a "panic: virtual address out of range"
> on the first of them. Easy to see by migrating in vga mode (with
> a driver loaded, for instance windows cmd window in full screen mode)
> and then exiting vga mode back to native mode will cause said panic.
> 

Self NACK, doesn't work with compat mode. We don't save delta in
migration.

> Fixed by doing the slot recreation unconditionally at post_load
> 
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  hw/qxl.c |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 03848ed..4e9f39f 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -1684,6 +1684,14 @@ static int qxl_post_load(void *opaque, int version)
>          qxl_mode_to_string(d->mode));
>      newmode = d->mode;
>      d->mode = QXL_MODE_UNDEFINED;
> +    for (i = 0; i < NUM_MEMSLOTS; i++) {
> +        if (!d->guest_slots[i].active) {
> +            continue;
> +        }
> +        dprint(d, 1, "%s: restoring guest slot %d delta %"PRIu64"\n",
> +               __func__, i, d->guest_slots[i].delta);
> +        qxl_add_memslot(d, i, d->guest_slots[i].delta, QXL_SYNC);
> +    }
>      switch (newmode) {
>      case QXL_MODE_UNDEFINED:
>          break;
> @@ -1691,12 +1699,6 @@ static int qxl_post_load(void *opaque, int version)
>          qxl_enter_vga_mode(d);
>          break;
>      case QXL_MODE_NATIVE:
> -        for (i = 0; i < NUM_MEMSLOTS; i++) {
> -            if (!d->guest_slots[i].active) {
> -                continue;
> -            }
> -            qxl_add_memslot(d, i, 0, QXL_SYNC);
> -        }
>          qxl_create_guest_primary(d, 1, QXL_SYNC);
>  
>          /* replay surface-create and cursor-set commands */
> -- 
> 1.7.6.4
> 
>