[Qemu-devel] [PATCH] ui/spice-display: use uintptr_t when casting qxl physical addresses

[Qemu-devel] [PATCH] ui/spice-display: use uintptr_t when casting qxl physical addresses

[Alon Levy]

The current intptr_t casts are a problem when the address's highest
bit is 1, and it is cast to a intptr_t and then to uint64_t, such
as at:
     surface.mem        = (intptr_t)ssd->buf;

This causes the sign bit to be extended which causes a wrong address to
be passed on to spice, which then complains when it gets the wrong
slot_id number, since the slot_id is taken from the higher bits.

The assertion happens early - during the first primary surface creation.

This fixes running "-vga qxl -spice" with 32 bit compiled
qemu-system-i386.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 ui/spice-display.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 35499e2..f5764e9 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -158,7 +158,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     drawable->bbox            = ssd->dirty;
     drawable->clip.type       = SPICE_CLIP_TYPE_NONE;
     drawable->effect          = QXL_EFFECT_OPAQUE;
-    drawable->release_info.id = (intptr_t)update;
+    drawable->release_info.id = (uintptr_t)update;
     drawable->type            = QXL_DRAW_COPY;
     drawable->surfaces_dest[0] = -1;
     drawable->surfaces_dest[1] = -1;
@@ -169,7 +169,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
                       + time_space.tv_nsec / 1000 / 1000;
 
     drawable->u.copy.rop_descriptor  = SPICE_ROPD_OP_PUT;
-    drawable->u.copy.src_bitmap      = (intptr_t)image;
+    drawable->u.copy.src_bitmap      = (uintptr_t)image;
     drawable->u.copy.src_area.right  = bw;
     drawable->u.copy.src_area.bottom = bh;
 
@@ -179,7 +179,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     image->bitmap.stride     = bw * 4;
     image->descriptor.width  = image->bitmap.x = bw;
     image->descriptor.height = image->bitmap.y = bh;
-    image->bitmap.data = (intptr_t)(update->bitmap);
+    image->bitmap.data = (uintptr_t)(update->bitmap);
     image->bitmap.palette = 0;
     image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
 
@@ -200,7 +200,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     }
 
     cmd->type = QXL_CMD_DRAW;
-    cmd->data = (intptr_t)drawable;
+    cmd->data = (uintptr_t)drawable;
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     return update;
@@ -244,7 +244,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
     surface.mouse_mode = true;
     surface.flags      = 0;
     surface.type       = 0;
-    surface.mem        = (intptr_t)ssd->buf;
+    surface.mem        = (uintptr_t)ssd->buf;
     surface.group_id   = MEMSLOT_GROUP_HOST;
 
     qemu_spice_create_primary_surface(ssd, 0, &surface, QXL_SYNC);
-- 
1.7.9.3

Re: [Qemu-devel] [PATCH] ui/spice-display: use uintptr_t when casting qxl physical addresses

[Gerd Hoffmann]

On 03/21/12 17:17, Alon Levy wrote:
> This fixes running "-vga qxl -spice" with 32 bit compiled
> qemu-system-i386.

Patch added to spice patch queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH] ui/spice-display: use uintptr_t when casting qxl physical addresses

[Alon Levy]

On Wed, Mar 21, 2012 at 05:42:25PM +0100, Gerd Hoffmann wrote:
> On 03/21/12 17:17, Alon Levy wrote:
> > This fixes running "-vga qxl -spice" with 32 bit compiled
> > qemu-system-i386.
> 
> Patch added to spice patch queue.

So perhaps you can also ack those:
 http://patchwork.freedesktop.org/patch/9597/
 http://patchwork.freedesktop.org/patch/9598/
 http://patchwork.freedesktop.org/patch/9599/
 http://patchwork.freedesktop.org/patch/9600/
 http://patchwork.freedesktop.org/patch/9601/


> 
> thanks,
>   Gerd

Re: [Qemu-devel] [Spice-devel] [PATCH] ui/spice-display: use uintptr_t when casting qxl physical addresses

[Hans de Goede]

Good one, ACK.

Acked-by: Hans de Goede <hdegoede@redhat.com>

On 03/21/2012 05:17 PM, Alon Levy wrote:
> The current intptr_t casts are a problem when the address's highest
> bit is 1, and it is cast to a intptr_t and then to uint64_t, such
> as at:
>       surface.mem        = (intptr_t)ssd->buf;
>
> This causes the sign bit to be extended which causes a wrong address to
> be passed on to spice, which then complains when it gets the wrong
> slot_id number, since the slot_id is taken from the higher bits.
>
> The assertion happens early - during the first primary surface creation.
>
> This fixes running "-vga qxl -spice" with 32 bit compiled
> qemu-system-i386.
>
> Signed-off-by: Alon Levy<alevy@redhat.com>
> ---
>   ui/spice-display.c |   10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 35499e2..f5764e9 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -158,7 +158,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>       drawable->bbox            = ssd->dirty;
>       drawable->clip.type       = SPICE_CLIP_TYPE_NONE;
>       drawable->effect          = QXL_EFFECT_OPAQUE;
> -    drawable->release_info.id = (intptr_t)update;
> +    drawable->release_info.id = (uintptr_t)update;
>       drawable->type            = QXL_DRAW_COPY;
>       drawable->surfaces_dest[0] = -1;
>       drawable->surfaces_dest[1] = -1;
> @@ -169,7 +169,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>                         + time_space.tv_nsec / 1000 / 1000;
>
>       drawable->u.copy.rop_descriptor  = SPICE_ROPD_OP_PUT;
> -    drawable->u.copy.src_bitmap      = (intptr_t)image;
> +    drawable->u.copy.src_bitmap      = (uintptr_t)image;
>       drawable->u.copy.src_area.right  = bw;
>       drawable->u.copy.src_area.bottom = bh;
>
> @@ -179,7 +179,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>       image->bitmap.stride     = bw * 4;
>       image->descriptor.width  = image->bitmap.x = bw;
>       image->descriptor.height = image->bitmap.y = bh;
> -    image->bitmap.data = (intptr_t)(update->bitmap);
> +    image->bitmap.data = (uintptr_t)(update->bitmap);
>       image->bitmap.palette = 0;
>       image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
>
> @@ -200,7 +200,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>       }
>
>       cmd->type = QXL_CMD_DRAW;
> -    cmd->data = (intptr_t)drawable;
> +    cmd->data = (uintptr_t)drawable;
>
>       memset(&ssd->dirty, 0, sizeof(ssd->dirty));
>       return update;
> @@ -244,7 +244,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
>       surface.mouse_mode = true;
>       surface.flags      = 0;
>       surface.type       = 0;
> -    surface.mem        = (intptr_t)ssd->buf;
> +    surface.mem        = (uintptr_t)ssd->buf;
>       surface.group_id   = MEMSLOT_GROUP_HOST;
>
>       qemu_spice_create_primary_surface(ssd, 0,&surface, QXL_SYNC);