Re: [Xen-devel] [PATCH v2] vga: fix byteswapping.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 20 Feb 2013, Gerd Hoffmann wrote:
> In case host and guest endianness differ the vga code first creates
> a shared surface (using qemu_create_displaysurface_from), then goes
> patch the surface format to indicate that the bytes must be swapped.
> 
> The switch to pixman broke that hack as the format patching isn't
> propagated into the pixman image, so ui code using the pixman image
> directly (such as vnc) uses the wrong format.
> 
> Fix that by adding a byteswap parameter to
> qemu_create_displaysurface_from, so we'll use the correct format
> when creating the surface (and the pixman image) and don't have
> to patch the format afterwards.
> 
> [ v2: unbreak xen build ]
> 
> Cc: qemu-stable@xxxxxxxxxx
> Cc: mark.cave-ayland@xxxxxxxxxxxx
> Cc: agraf@xxxxxxx
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>

the patch looks good to me


>  hw/qxl-render.c      |    3 ++-
>  hw/vga.c             |   18 ++++++++----------
>  hw/vmware_vga.c      |    2 +-
>  hw/xenfb.c           |    3 ++-
>  include/ui/console.h |    3 ++-
>  ui/console.c         |    9 +++++++--
>  6 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/qxl-render.c b/hw/qxl-render.c
> index 88e63f8..455fb91 100644
> --- a/hw/qxl-render.c
> +++ b/hw/qxl-render.c
> @@ -118,7 +118,8 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice 
> *qxl)
>                   qxl->guest_primary.surface.height,
>                   qxl->guest_primary.bits_pp,
>                   qxl->guest_primary.abs_stride,
> -                 qxl->guest_primary.data);
> +                 qxl->guest_primary.data,
> +                 false);
>          } else {
>              qemu_resize_displaysurface(vga->ds,
>                      qxl->guest_primary.surface.width,
> diff --git a/hw/vga.c b/hw/vga.c
> index e2ba7f2..1caf23d 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -1643,6 +1643,11 @@ static void vga_draw_graphic(VGACommonState *s, int 
> full_update)
>      uint8_t *d;
>      uint32_t v, addr1, addr;
>      vga_draw_line_func *vga_draw_line;
> +#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
> +    static const bool byteswap = false;
> +#else
> +    static const bool byteswap = true;
> +#endif
>  
>      full_update |= update_basic_params(s);
>  
> @@ -1685,18 +1690,11 @@ static void vga_draw_graphic(VGACommonState *s, int 
> full_update)
>          disp_width != s->last_width ||
>          height != s->last_height ||
>          s->last_depth != depth) {
> -#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
> -        if (depth == 16 || depth == 32) {
> -#else
> -        if (depth == 32) {
> -#endif
> +        if (depth == 32 || (depth == 16 && !byteswap)) {
>              qemu_free_displaysurface(s->ds);
>              s->ds->surface = qemu_create_displaysurface_from(disp_width, 
> height, depth,
>                      s->line_offset,
> -                    s->vram_ptr + (s->start_addr * 4));
> -#if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN)
> -            s->ds->surface->pf = 
> qemu_different_endianness_pixelformat(depth);
> -#endif
> +                    s->vram_ptr + (s->start_addr * 4), byteswap);
>              dpy_gfx_resize(s->ds);
>          } else {
>              qemu_console_resize(s->ds, disp_width, height);
> @@ -1715,7 +1713,7 @@ static void vga_draw_graphic(VGACommonState *s, int 
> full_update)
>          s->ds->surface = qemu_create_displaysurface_from(disp_width,
>                  height, depth,
>                  s->line_offset,
> -                s->vram_ptr + (s->start_addr * 4));
> +                s->vram_ptr + (s->start_addr * 4), byteswap);
>          dpy_gfx_setdata(s->ds);
>      }
>  
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index cd15ee4..8fc201b 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -1074,7 +1074,7 @@ static void vmsvga_screen_dump(void *opaque, const char 
> *filename, bool cswitch,
>                                   ds_get_height(s->vga.ds),
>                                   32,
>                                   ds_get_linesize(s->vga.ds),
> -                                 s->vga.vram_ptr);
> +                                 s->vga.vram_ptr, false);
>          ppm_save(filename, ds, errp);
>          g_free(ds);
>      }
> diff --git a/hw/xenfb.c b/hw/xenfb.c
> index 903efd3..7f1f6b4 100644
> --- a/hw/xenfb.c
> +++ b/hw/xenfb.c
> @@ -756,7 +756,8 @@ static void xenfb_update(void *opaque)
>              qemu_free_displaysurface(xenfb->c.ds);
>              xenfb->c.ds->surface = qemu_create_displaysurface_from
>                  (xenfb->width, xenfb->height, xenfb->depth,
> -                 xenfb->row_stride, xenfb->pixels + xenfb->offset);
> +                 xenfb->row_stride, xenfb->pixels + xenfb->offset,
> +                 false);
>              break;
>          default:
>              /* we must convert stuff */
> diff --git a/include/ui/console.h b/include/ui/console.h
> index fc23baa..18012f1 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -184,7 +184,8 @@ struct DisplayState {
>  void register_displaystate(DisplayState *ds);
>  DisplayState *get_displaystate(void);
>  DisplaySurface* qemu_create_displaysurface_from(int width, int height, int 
> bpp,
> -                                                int linesize, uint8_t *data);
> +                                                int linesize, uint8_t *data,
> +                                                bool byteswap);
>  PixelFormat qemu_different_endianness_pixelformat(int bpp);
>  PixelFormat qemu_default_pixelformat(int bpp);
>  
> diff --git a/ui/console.c b/ui/console.c
> index 0a68836..25e06a5 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1339,11 +1339,16 @@ DisplaySurface 
> *qemu_resize_displaysurface(DisplayState *ds,
>  }
>  
>  DisplaySurface *qemu_create_displaysurface_from(int width, int height, int 
> bpp,
> -                                                int linesize, uint8_t *data)
> +                                                int linesize, uint8_t *data,
> +                                                bool byteswap)
>  {
>      DisplaySurface *surface = g_new0(DisplaySurface, 1);
>  
> -    surface->pf = qemu_default_pixelformat(bpp);
> +    if (byteswap) {
> +        surface->pf = qemu_different_endianness_pixelformat(bpp);
> +    } else {
> +        surface->pf = qemu_default_pixelformat(bpp);
> +    }
>  
>      surface->format = qemu_pixman_get_format(&surface->pf);
>      assert(surface->format != 0);
> -- 
> 1.7.9.7
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel