Re: [Qemu-devel] [PATCH] qxl: create slots on post_load in vga state

[Alon Levy]

On Tue, Oct 25, 2011 at 03:05:08PM +0200, Alon Levy wrote:
> RHBZ 740547
> 

NACK self, dprint contains delta that isn't being used, will resend.

> If we migrate when the device is in vga 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 in post_load for vga mode as well.
> Note that compat does not require any changes because it creates it's
> only slot by a side effect of QXL_IO_SET_MODE.
> 
> Signed-off-by: Alon Levy <address@hidden>
> ---
> v3:
>  no need to create slots in compat mode, they are created by qxl_set_mode.
> 
>  hw/qxl.c |   27 ++++++++++++++++++++-------
>  1 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 03848ed..ab088a9 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -1662,12 +1662,26 @@ static int qxl_pre_load(void *opaque)
>      return 0;
>  }
>  
> +static void qxl_create_memslots(PCIQXLDevice *d)
> +{
> +    int i;
> +
> +    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, 0, QXL_SYNC);
> +    }
> +}
> +
>  static int qxl_post_load(void *opaque, int version)
>  {
>      PCIQXLDevice* d = opaque;
>      uint8_t *ram_start = d->vga.vram_ptr;
>      QXLCommandExt *cmds;
> -    int in, out, i, newmode;
> +    int in, out, newmode;
>  
>      dprint(d, 1, "%s: start\n", __FUNCTION__);
>  
> @@ -1684,19 +1698,16 @@ static int qxl_post_load(void *opaque, int version)
>          qxl_mode_to_string(d->mode));
>      newmode = d->mode;
>      d->mode = QXL_MODE_UNDEFINED;
> +
>      switch (newmode) {
>      case QXL_MODE_UNDEFINED:
>          break;
>      case QXL_MODE_VGA:
> +        qxl_create_memslots(d);
>          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_memslots(d);
>          qxl_create_guest_primary(d, 1, QXL_SYNC);
>  
>          /* replay surface-create and cursor-set commands */
> @@ -1719,6 +1730,8 @@ static int qxl_post_load(void *opaque, int version)
>  
>          break;
>      case QXL_MODE_COMPAT:
> +        /* note: no need to call qxl_create_memslots, qxl_set_mode
> +         * creates the mem slot. */
>          qxl_set_mode(d, d->shadow_rom.mode, 1);
>          break;
>      }
> -- 
> 1.7.7
> 
>