From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37195) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXNZv-0000Ep-33 for qemu-devel@nongnu.org; Fri, 26 Sep 2014 00:56:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XXNZm-0008MP-8o for qemu-devel@nongnu.org; Fri, 26 Sep 2014 00:56:23 -0400 Received: from ozlabs.org ([103.22.144.67]:46262) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXNZl-0008K4-Mk for qemu-devel@nongnu.org; Fri, 26 Sep 2014 00:56:14 -0400 Date: Fri, 26 Sep 2014 14:34:22 +1000 From: David Gibson Message-ID: <20140926043422.GA20209@voom.redhat.com> References: <1411475457-10942-1-git-send-email-kraxel@redhat.com> <1411475457-10942-3-git-send-email-kraxel@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="gBBFr7Ir9EOA20Yy" Content-Disposition: inline In-Reply-To: <1411475457-10942-3-git-send-email-kraxel@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/3] vga: Add endian control register List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: David Gibson , qemu-devel@nongnu.org --gBBFr7Ir9EOA20Yy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 23, 2014 at 02:30:56PM +0200, Gerd Hoffmann wrote: > From: Benjamin Herrenschmidt >=20 > 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. >=20 > Signed-off-by: Benjamin Herrenschmidt >=20 > Changes by kraxel: > * Remove bochs dispi interface changes. We'll do that in > a different way to make sure we don't conflict with > possible future bochs dispi interface changes. > * keep live migration bits. >=20 > Signed-off-by: Gerd Hoffmann > --- > hw/display/vga.c | 41 +++++++++++++++++++++++++++++++++++++---- > hw/display/vga_int.h | 4 +++- > 2 files changed, 40 insertions(+), 5 deletions(-) >=20 > diff --git a/hw/display/vga.c b/hw/display/vga.c > index 3f02984..fd16806 100644 > --- a/hw/display/vga.c > +++ b/hw/display/vga.c > @@ -1508,7 +1508,8 @@ static void vga_draw_graphic(VGACommonState *s, int= full_update) > if (s->line_offset !=3D s->last_line_offset || > disp_width !=3D s->last_width || > height !=3D s->last_height || > - s->last_depth !=3D depth) { > + s->last_depth !=3D depth || > + s->last_byteswap !=3D byteswap) { > if (depth =3D=3D 32 || (depth =3D=3D 16 && !byteswap)) { > pixman_format_code_t format =3D > qemu_default_pixman_format(depth, !byteswap); > @@ -1526,6 +1527,7 @@ static void vga_draw_graphic(VGACommonState *s, int= full_update) > s->last_height =3D height; > s->last_line_offset =3D s->line_offset; > s->last_depth =3D depth; > + s->last_byteswap =3D byteswap; > full_update =3D 1; > } else if (is_buffer_shared(surface) && > (full_update || surface_data(surface) !=3D s->vram_ptr > @@ -1789,6 +1791,7 @@ void vga_common_reset(VGACommonState *s) > s->cursor_start =3D 0; > s->cursor_end =3D 0; > s->cursor_offset =3D 0; > + s->big_endian_fb =3D 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)); > @@ -2020,6 +2023,28 @@ static int vga_common_post_load(void *opaque, int = version_id) > return 0; > } > =20 > +static bool vga_endian_state_needed(void *opaque) > +{ > + VGACommonState *s =3D 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 !=3D s->big_endian_fb; > +} > + > +const VMStateDescription vmstate_vga_endian =3D { > + .name =3D "vga.endian", > + .version_id =3D 1, > + .minimum_version_id =3D 1, > + .fields =3D (VMStateField[]) { > + VMSTATE_UINT8_EQUAL(big_endian_fb, VGACommonState), I don't think this is right. If I'm remembering how the vmstate macros work, this will abort if big_endian_fb isn't equal on source and destination. Which means as soon as it's possible to actually change the big_endian_fb value, migrations will start failing/ I think this should instead be VMSTATE_BOOL() [snip] > - 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); In which case you also don't need to change the type of the big_endian_fb field. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --gBBFr7Ir9EOA20Yy Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUJOzOAAoJEGw4ysog2bOSLMAP/1g9rd0RNUiO/iGqjE/6yJ0G AGE6z+BmgAosPN6b+vQkPAZYEwoiEeHDpQeFguqDgFMTWNWGTCbKCutYE0cWCGF/ uZeSx/vxdTmCkbark5QRDpy7ylZxy48raY7ApdZ+olDHSuhzC5O1Sa7VBpTKTAKM 6QGgl1uZPq9io4/kdrnvlFLV2QT4H9laXSDhx7NrRVKWuzn/J0pOfPVjsd8qu8p6 L8PQbuf0FeagU5Rl9vRMvV3MyvnRJ907hdukGZJE4+cYhAcMtgjb256+9HvlsISJ 9LnruSU9RVpZ3ahKBuH4c0krWN3pnJBc1UyNkJBAg154y6lV7gY68AmPW84AtJL1 X5Rly3KUgqvdgH7EFBElb8qJIbQD3kZxJHAM/2E1MuSHne4uX5wC8vp1jYvxvmE7 Dq/UD00mQWpiLNoRqAcmlbNEkzj/TPBXkNAhx+AxXTH/qU3v39ZzmAct4lb3uv0z /vglMEF5UIErY6LcMfQWNAUVoB0GCGS6PNR5fDu068vq9RjBjhCH0TcUa8NK1PkN FMBEHQRnqGbdLuvMbahgrj/fNZ0nA3hxGz//PsXa/cccbr29+tEwEWn/SywL2pJy ldFzctKnr8fucDZDVJghvHeYFFIHH9Nmlc93H/K0W+uyj6bSWnzq5eRfyaRi8sCM 85EVfq1M1uUoDzFbjG6U =LVzD -----END PGP SIGNATURE----- --gBBFr7Ir9EOA20Yy--