| Message ID | 1403565068-15229-14-git-send-email-benh@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
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 <benh@kernel.crashing.org> > --- > 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);
Hi, > 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 I was more thinking to add ID6 to indicate the new interface revision with the additional VBE_DISPI_INDEX_ENDIAN_CTRL register. I'm a bit worried that there is no response from the bochs guys yet, I don't want have two incompatible rev6 interfaces. At least nobody seems to have defined one so far, google finds nothing for "bochs dispi 0xB0C6". cheers, Gerd
On Mon, 2014-06-30 at 13:38 +0200, Gerd Hoffmann wrote: > Hi, > > > 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 > > I was more thinking to add ID6 to indicate the new interface revision > with the additional VBE_DISPI_INDEX_ENDIAN_CTRL register. > > I'm a bit worried that there is no response from the bochs guys yet, I > don't want have two incompatible rev6 interfaces. At least nobody seems > to have defined one so far, google finds nothing for "bochs dispi > 0xB0C6". Ah ok, I haven't quite figured out how that DISPI_ID stuff works, I'll dig a bit. Definitely an option though I like the idea of an "capability" register in the new revision so we can easily add features that don't have to be implemented by the host (for example there's little point for Bochs to implement endian control). The lack of response from Bochs worries me too. I've tried hanging out on their IRC as well with no result so far. Cheers, Ben.
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);
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. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- hw/display/vga.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++------ hw/display/vga_int.h | 15 +++++++++++++- 2 files changed, 63 insertions(+), 7 deletions(-)