Re: [Qemu-devel] [RFC 13/14] vga: Add endian control register

[Benjamin Herrenschmidt]

On Tue, 2014-06-24 at 09:11 +1000, Benjamin Herrenschmidt wrote:
> Include the endian state in the migration stream as an optional
> subsection which we only include when the endian isn't the default,
> thus enabling backward compatibility of the common case.

Note: This description is a bit too terse, sorry about that....

This is based on the proposed register addition that I've discussed
in a separate email, adding an "extended capabilities" register and
using it to expose the endian control.

Arguably, this could be split into 2 patches...

At this point however, I'd first want to make sure we all agree on the
approach before going down into the details.

I haven't been successful contacting the Bochs folks, either on their
mailing list or on IRC.

Ben.

> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> ---
>  hw/display/vga.c     | 55 
> ++++++++++++++++++++++++++++++++++++++++++++++------
>  hw/display/vga_int.h | 15 +++++++++++++-
>  2 files changed, 63 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 29d57cf..54b1fbe 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -615,6 +615,11 @@ uint32_t vbe_ioport_read_data(void *opaque, uint32_t 
> addr)
>          }
>      } else if (s->vbe_index == VBE_DISPI_INDEX_VIDEO_MEMORY_64K) {
>          val = s->vram_size / (64 * 1024);
> +    } else if (s->vbe_index == VBE_DISPI_INDEX_EXTENDED_CAPS) {
> +        val = VBE_DISPI_HAS_ENDIAN_CTRL;
> +    } else if (s->vbe_index == VBE_DISPI_INDEX_ENDIAN_CTRL) {
> +        val = s->big_endian_fb ? VBE_DISPI_BIG_ENDIAN :
> +                VBE_DISPI_LITTLE_ENDIAN;
>      } else {
>          val = 0;
>      }
> @@ -634,7 +639,8 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, 
> uint32_t val)
>  {
>      VGACommonState *s = opaque;
>  
> -    if (s->vbe_index <= VBE_DISPI_INDEX_NB) {
> +    if (s->vbe_index <= VBE_DISPI_INDEX_NB ||
> +        s->vbe_index == VBE_DISPI_INDEX_ENDIAN_CTRL) {
>  #ifdef DEBUG_BOCHS_VBE
>          printf("VBE: write index=0x%x val=0x%x\n", s->vbe_index, val);
>  #endif
> @@ -737,7 +743,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, 
> uint32_t val)
>                  s->bank_offset = 0;
>              }
>              s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0;
> -            s->vbe_regs[s->vbe_index] = val;
> +            s->vbe_regs[s->vbe_index] = val | VBE_DISPI_EXTCAPS;
>              vga_update_memory_access(s);
>              break;
>          case VBE_DISPI_INDEX_VIRT_WIDTH:
> @@ -774,6 +780,9 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, 
> uint32_t val)
>                  s->vbe_start_addr >>= 2;
>              }
>              break;
> +     case VBE_DISPI_INDEX_ENDIAN_CTRL:
> +            s->big_endian_fb = !!(val & VBE_DISPI_BIG_ENDIAN);
> +            break;
>          default:
>              break;
>          }
> @@ -1464,7 +1473,8 @@ static void vga_draw_graphic(VGACommonState *s, int 
> full_update)
>      if (s->line_offset != s->last_line_offset ||
>          disp_width != s->last_width ||
>          height != s->last_height ||
> -        s->last_depth != depth) {
> +        s->last_depth != depth ||
> +        s->last_byteswap != byteswap) {
>       if (depth == 32 || depth == 24 ||
>             ((depth == 16 || depth == 15) && !byteswap)) {
>              pixman_format_code_t format =
> @@ -1483,6 +1493,7 @@ static void vga_draw_graphic(VGACommonState *s, int 
> full_update)
>          s->last_height = height;
>          s->last_line_offset = s->line_offset;
>          s->last_depth = depth;
> +        s->last_byteswap = byteswap;
>          full_update = 1;
>      } else if (is_buffer_shared(surface) &&
>                 (full_update || surface_data(surface) != s->vram_ptr
> @@ -1731,6 +1742,7 @@ void vga_common_reset(VGACommonState *s)
>      s->vbe_index = 0;
>      memset(s->vbe_regs, '\0', sizeof(s->vbe_regs));
>      s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
> +    s->vbe_regs[VBE_DISPI_INDEX_ENABLE] = VBE_DISPI_EXTCAPS;
>      s->vbe_start_addr = 0;
>      s->vbe_line_offset = 0;
>      s->vbe_bank_mask = (s->vram_size >> 16) - 1;
> @@ -1751,6 +1763,7 @@ void vga_common_reset(VGACommonState *s)
>      s->cursor_start = 0;
>      s->cursor_end = 0;
>      s->cursor_offset = 0;
> +    s->big_endian_fb = s->default_endian_fb;
>      memset(s->invalidated_y_table, '\0', sizeof(s->invalidated_y_table));
>      memset(s->last_palette, '\0', sizeof(s->last_palette));
>      memset(s->last_ch_attr, '\0', sizeof(s->last_ch_attr));
> @@ -1982,6 +1995,28 @@ static int vga_common_post_load(void *opaque, int 
> version_id)
>      return 0;
>  }
>  
> +static bool vga_endian_state_needed(void *opaque)
> +{
> +    VGACommonState *s = opaque;
> +
> +    /*
> +     * Only send the endian state if it's different from the
> +     * default one, thus ensuring backward compatibility for
> +     * migration of the common case
> +     */
> +    return s->default_endian_fb != s->big_endian_fb;
> +}
> +
> +const VMStateDescription vmstate_vga_endian = {
> +    .name = "vga.endian",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT8_EQUAL(big_endian_fb, VGACommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_vga_common = {
>      .name = "vga",
>      .version_id = 2,
> @@ -2018,6 +2053,14 @@ const VMStateDescription vmstate_vga_common = {
>          VMSTATE_UINT32(vbe_line_offset, VGACommonState),
>          VMSTATE_UINT32(vbe_bank_mask, VGACommonState),
>          VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (VMStateSubsection []) {
> +        {
> +            .vmsd = &vmstate_vga_endian,
> +            .needed = vga_endian_state_needed,
> +        }, {
> +            /* empty */
> +        }
>      }
>  };
>  
> @@ -2084,14 +2127,14 @@ void vga_common_init(VGACommonState *s, Object *obj, 
> bool global_vmstate)
>      }
>  
>      /*
> -     * Set default fb endian based on target, should probably be turned
> +     * Set default fb endian based on target, could probably be turned
>       * into a device attribute set by the machine/platform to remove
>       * all target endian dependencies from this file.
>       */
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    s->big_endian_fb = true;
> +    s->default_endian_fb = true;
>  #else
> -    s->big_endian_fb = false;
> +    s->default_endian_fb = false;
>  #endif
>      vga_dirty_log_start(s);
>  }
> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
> index ae64321..894c6ab 100644
> --- a/hw/display/vga_int.h
> +++ b/hw/display/vga_int.h
> @@ -47,6 +47,8 @@
>  #define VBE_DISPI_INDEX_Y_OFFSET        0x9
>  #define VBE_DISPI_INDEX_NB              0xa /* size of vbe_regs[] */
>  #define VBE_DISPI_INDEX_VIDEO_MEMORY_64K 0xa /* read-only, not in vbe_regs */
> +#define VBE_DISPI_INDEX_EXTENDED_CAPS   0xb /* read-only, not in vbe_regs */
> +#define VBE_DISPI_INDEX_ENDIAN_CTRL     0xc /* not in vbe_regs */
>  
>  #define VBE_DISPI_ID0                   0xB0C0
>  #define VBE_DISPI_ID1                   0xB0C1
> @@ -55,13 +57,22 @@
>  #define VBE_DISPI_ID4                   0xB0C4
>  #define VBE_DISPI_ID5                   0xB0C5
>  
> +/* VBE_DISPI_INDEX_ENABLE fields */
>  #define VBE_DISPI_DISABLED              0x00
>  #define VBE_DISPI_ENABLED               0x01
>  #define VBE_DISPI_GETCAPS               0x02
> +#define VBE_DISPI_EXTCAPS               0x10 /* RO: set when 0xb present */
>  #define VBE_DISPI_8BIT_DAC              0x20
>  #define VBE_DISPI_LFB_ENABLED           0x40
>  #define VBE_DISPI_NOCLEARMEM            0x80
>  
> +/* VBE_DISPI_INDEX_EXTENDED_CAPS fields */
> +#define VBE_DISPI_HAS_ENDIAN_CTRL    0x01 /* RO: has endian control reg */
> +
> +/* VBE_DISPI_INDEX_ENDIAN_CTRL */
> +#define VBE_DISPI_LITTLE_ENDIAN              0x00
> +#define VBE_DISPI_BIG_ENDIAN         0x01
> +
>  #define VBE_DISPI_LFB_PHYSICAL_ADDRESS  0xE0000000
>  
>  #define CH_ATTR_SIZE (160 * 100)
> @@ -148,6 +159,7 @@ typedef struct VGACommonState {
>      uint32_t last_width, last_height; /* in chars or pixels */
>      uint32_t last_scr_width, last_scr_height; /* in pixels */
>      uint32_t last_depth; /* in bits */
> +    bool last_byteswap;
>      uint8_t cursor_start, cursor_end;
>      bool cursor_visible_phase;
>      int64_t cursor_blink_time;
> @@ -155,7 +167,8 @@ typedef struct VGACommonState {
>      const GraphicHwOps *hw_ops;
>      bool full_update_text;
>      bool full_update_gfx;
> -    bool big_endian_fb;
> +    uint8_t big_endian_fb;
> +    uint8_t default_endian_fb;
>      /* hardware mouse cursor support */
>      uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32];
>      void (*cursor_invalidate)(struct VGACommonState *s);