qxl: fix guest cursor tracking

Submitted by Yonit Halperin on Oct. 18, 2011, 4:58 p.m.

Details

Message ID 1318957134-2923-1-git-send-email-yhalperi@redhat.com
State New
Headers show

Commit Message

Yonit Halperin Oct. 18, 2011, 4:58 p.m.
(1) If the guest cursor command is empty, don't reload it after migration.
(2) Cleaning the guest cursor when it is released by
    the spice server. In addition, explicitly reset the
    cursor in spice upon destroying the primary surface
    (was done by spice-server implicitly). This will prevent
    access to pci memory that was released.

RHBZ: 744518

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 hw/qxl.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

Comments

Alon Levy Oct. 18, 2011, 5:54 p.m.
On Tue, Oct 18, 2011 at 06:58:54PM +0200, Yonit Halperin wrote:
> (1) If the guest cursor command is empty, don't reload it after migration.
> (2) Cleaning the guest cursor when it is released by
>     the spice server. In addition, explicitly reset the
>     cursor in spice upon destroying the primary surface
>     (was done by spice-server implicitly). This will prevent
>     access to pci memory that was released.
> 

Reviewed-by: Alon Levy <alevy@redhat.com>

> RHBZ: 744518
> 
> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
>  hw/qxl.c |   16 ++++++++++++----
>  1 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 03848ed..c9b60a2 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -238,6 +238,9 @@ void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
>  void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
>  {
>      qxl->ssd.worker->reset_cursor(qxl->ssd.worker);
> +    qemu_mutex_lock(&qxl->track_lock);
> +    qxl->guest_cursor = 0;
> +    qemu_mutex_unlock(&qxl->track_lock);
>  }
>  
>  
> @@ -402,7 +405,9 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
>      {
>          QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
>          if (cmd->type == QXL_CURSOR_SET) {
> +            qemu_mutex_lock(&qxl->track_lock);
>              qxl->guest_cursor = ext->cmd.data;
> +            qemu_mutex_unlock(&qxl->track_lock);
>          }
>          break;
>      }
> @@ -1067,6 +1072,7 @@ static int qxl_destroy_primary(PCIQXLDevice *d, qxl_async_io async)
>  
>      d->mode = QXL_MODE_UNDEFINED;
>      qemu_spice_destroy_primary_surface(&d->ssd, 0, async);
> +    qxl_spice_reset_cursor(d);
>      return 1;
>  }
>  
> @@ -1710,10 +1716,12 @@ static int qxl_post_load(void *opaque, int version)
>              cmds[out].group_id = MEMSLOT_GROUP_GUEST;
>              out++;
>          }
> -        cmds[out].cmd.data = d->guest_cursor;
> -        cmds[out].cmd.type = QXL_CMD_CURSOR;
> -        cmds[out].group_id = MEMSLOT_GROUP_GUEST;
> -        out++;
> +        if (d->guest_cursor) {
> +            cmds[out].cmd.data = d->guest_cursor;
> +            cmds[out].cmd.type = QXL_CMD_CURSOR;
> +            cmds[out].group_id = MEMSLOT_GROUP_GUEST;
> +            out++;
> +        }
>          qxl_spice_loadvm_commands(d, cmds, out);
>          g_free(cmds);
>  
> -- 
> 1.7.6.4
>
Gerd Hoffmann Oct. 19, 2011, 12:48 p.m.
On 10/18/11 18:58, Yonit Halperin wrote:
> (1) If the guest cursor command is empty, don't reload it after migration.
> (2) Cleaning the guest cursor when it is released by
>     the spice server. In addition, explicitly reset the
>     cursor in spice upon destroying the primary surface
>     (was done by spice-server implicitly). This will prevent
>     access to pci memory that was released.

Added to spice patch queue.

thanks,
  Gerd

Patch hide | download patch | download mbox

diff --git a/hw/qxl.c b/hw/qxl.c
index 03848ed..c9b60a2 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -238,6 +238,9 @@  void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
 void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
 {
     qxl->ssd.worker->reset_cursor(qxl->ssd.worker);
+    qemu_mutex_lock(&qxl->track_lock);
+    qxl->guest_cursor = 0;
+    qemu_mutex_unlock(&qxl->track_lock);
 }
 
 
@@ -402,7 +405,9 @@  static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
     {
         QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
         if (cmd->type == QXL_CURSOR_SET) {
+            qemu_mutex_lock(&qxl->track_lock);
             qxl->guest_cursor = ext->cmd.data;
+            qemu_mutex_unlock(&qxl->track_lock);
         }
         break;
     }
@@ -1067,6 +1072,7 @@  static int qxl_destroy_primary(PCIQXLDevice *d, qxl_async_io async)
 
     d->mode = QXL_MODE_UNDEFINED;
     qemu_spice_destroy_primary_surface(&d->ssd, 0, async);
+    qxl_spice_reset_cursor(d);
     return 1;
 }
 
@@ -1710,10 +1716,12 @@  static int qxl_post_load(void *opaque, int version)
             cmds[out].group_id = MEMSLOT_GROUP_GUEST;
             out++;
         }
-        cmds[out].cmd.data = d->guest_cursor;
-        cmds[out].cmd.type = QXL_CMD_CURSOR;
-        cmds[out].group_id = MEMSLOT_GROUP_GUEST;
-        out++;
+        if (d->guest_cursor) {
+            cmds[out].cmd.data = d->guest_cursor;
+            cmds[out].cmd.type = QXL_CMD_CURSOR;
+            cmds[out].group_id = MEMSLOT_GROUP_GUEST;
+            out++;
+        }
         qxl_spice_loadvm_commands(d, cmds, out);
         g_free(cmds);