hw/core/machine.c | 1 + hw/display/cirrus_vga_isa.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
From: Thomas Huth <thuth@redhat.com>
In the long run, we would like to get rid of the code that allows to
register migration state globally, so set global_vmstate to false when
using the isa-cirrus-vga device with new machines, and only enable it
for older machines to avoid breaking the migration there.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/core/machine.c | 1 +
hw/display/cirrus_vga_isa.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 6cf0e2f404e..0aa77a57e95 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,7 @@
GlobalProperty hw_compat_10_2[] = {
{ "scsi-block", "migrate-pr", "off" },
+ { "isa-cirrus-vga", "global-vmstate", "true" },
};
const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
index bad9ec7599c..76034a88605 100644
--- a/hw/display/cirrus_vga_isa.c
+++ b/hw/display/cirrus_vga_isa.c
@@ -56,7 +56,6 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, Error **errp)
s->vram_size_mb);
return;
}
- s->global_vmstate = true;
if (!vga_common_init(s, OBJECT(dev), errp)) {
return;
}
@@ -74,6 +73,8 @@ static const Property isa_cirrus_vga_properties[] = {
cirrus_vga.vga.vram_size_mb, 4),
DEFINE_PROP_BOOL("blitter", struct ISACirrusVGAState,
cirrus_vga.enable_blitter, true),
+ DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
+ cirrus_vga.vga.global_vmstate, false),
};
static void isa_cirrus_vga_class_init(ObjectClass *klass, const void *data)
--
2.53.0On Thu, 26 Mar 2026, Thomas Huth wrote:
> In the long run, we would like to get rid of the code that allows to
Not related to this patch but there's another hack in vga that might be
good to get rid of similarly. At hw/display/vga.c:2263 it says:
/*
* 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.
*/
s->default_endian_fb = target_big_endian();
s->big_endian_fb = s->default_endian_fb;
I think this does not make much sense as VGA is little endian but since
this hack is there since forever just changing it may break some guests.
I've tried that changing the default_endian_fb to false at least breaks
OpenBIOS and its QEMU VGA NDRV for MacOS. So at least those might need
some backward compatibility property until they are fixed. I don't know
what else could it break but maybe it would be a good opportunity to
change it with new machine types with a property that could also be set by
users if something breaks just to find out what might need fixing and
eventually be able to remove it. Anybody wants to make a patch?
Regards,
BALATON Zoltan
On Sun, Mar 29, 2026 at 09:07:42PM +0200, BALATON Zoltan wrote: > On Thu, 26 Mar 2026, Thomas Huth wrote: > > In the long run, we would like to get rid of the code that allows to > > Not related to this patch but there's another hack in vga that might be good > to get rid of similarly. At hw/display/vga.c:2263 it says: > > /* > * 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. > */ > s->default_endian_fb = target_big_endian(); > s->big_endian_fb = s->default_endian_fb; > > I think this does not make much sense as VGA is little endian but since this > hack is there since forever just changing it may break some guests. Short history lesson: Loooong ago vga used to have hard-coded endianess. Then vga got support for switching endianness at runtime, and this was added for backward compatibility. IIRC the arrival of little endian powerpc support triggered all that, and I had my share of endian bugs in different places during that time. Some guest drivers have been fixed since, the linux driver explicitly sets endianess since years, but ... > I've tried that changing the default_endian_fb to false at least > breaks OpenBIOS and its QEMU VGA NDRV for MacOS. ... apparently not all guest drivers. > So at least those might need some backward compatibility property > until they are fixed. I don't know what else could it break Only the colors will break. Guest writes big-endian data, host interprets that as little-endian data, and you get a funny looking screen. > but maybe it would be a good opportunity to change it with new machine > types with a property that could also be set by users if something breaks > just to find out what might need fixing and eventually be able to remove it. I think it makes sense to try fix the remaining guest drivers first. take care, Gerd
On Mon, 30 Mar 2026, Gerd Hoffmann wrote: > On Sun, Mar 29, 2026 at 09:07:42PM +0200, BALATON Zoltan wrote: >> On Thu, 26 Mar 2026, Thomas Huth wrote: >>> In the long run, we would like to get rid of the code that allows to >> >> Not related to this patch but there's another hack in vga that might be good >> to get rid of similarly. At hw/display/vga.c:2263 it says: >> >> /* >> * 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. >> */ >> s->default_endian_fb = target_big_endian(); >> s->big_endian_fb = s->default_endian_fb; >> >> I think this does not make much sense as VGA is little endian but since this >> hack is there since forever just changing it may break some guests. > > Short history lesson: Loooong ago vga used to have hard-coded > endianess. Then vga got support for switching endianness at runtime, > and this was added for backward compatibility. Not all VGA that use hw/display/vga.c can switch endianness. It seems only stdvga-pci, bochs-display and ati-vga can but not the others. There are even some sysbus vga devices that might need some changes. > IIRC the arrival of little endian powerpc support triggered all that, > and I had my share of endian bugs in different places during that time. > > Some guest drivers have been fixed since, the linux driver explicitly > sets endianess since years, but ... > >> I've tried that changing the default_endian_fb to false at least >> breaks OpenBIOS and its QEMU VGA NDRV for MacOS. > > ... apparently not all guest drivers. > >> So at least those might need some backward compatibility property >> until they are fixed. I don't know what else could it break > > Only the colors will break. Guest writes big-endian data, host > interprets that as little-endian data, and you get a funny looking > screen. That's enough to make it unusable in practice so we'd need to do something about that. >> but maybe it would be a good opportunity to change it with new machine >> types with a property that could also be set by users if something breaks >> just to find out what might need fixing and eventually be able to remove it. > > I think it makes sense to try fix the remaining guest drivers first. Problem is we don't know what those guests are. I've randomly found OpenBIOS and the MacOS NDRV because I know they use big endian but don't know how many others are there. That's why I thought maybe adding a property and changing the default could work as users could just add the property to their command until the drivers are adapted. Regards. BALATON Zoltan
On 29/3/26 21:07, BALATON Zoltan wrote: > On Thu, 26 Mar 2026, Thomas Huth wrote: >> In the long run, we would like to get rid of the code that allows to > > Not related to this patch but there's another hack in vga that might be > good to get rid of similarly. At hw/display/vga.c:2263 it says: > > /* > * 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. > */ > s->default_endian_fb = target_big_endian(); > s->big_endian_fb = s->default_endian_fb; > > I think this does not make much sense as VGA is little endian but since > this hack is there since forever just changing it may break some guests. > I've tried that changing the default_endian_fb to false at least breaks > OpenBIOS and its QEMU VGA NDRV for MacOS. So at least those might need > some backward compatibility property until they are fixed. I don't know > what else could it break but maybe it would be a good opportunity to > change it with new machine types with a property that could also be set > by users if something breaks just to find out what might need fixing and > eventually be able to remove it. On the Jazz machine there is a bus bridge chipset handling such endianness conversion, but we don't model it. I suppose something similar is used on those machines, and the VGA chipset behind the bridge is indeed a pure little endian one. Mostly because it is cheaper for manufacturer to use that setup than synthetize a big endian VGA chipset.
On 26/3/26 16:48, Thomas Huth wrote: > From: Thomas Huth <thuth@redhat.com> > > In the long run, we would like to get rid of the code that allows to > register migration state globally, so set global_vmstate to false when > using the isa-cirrus-vga device with new machines, and only enable it > for older machines to avoid breaking the migration there. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > hw/core/machine.c | 1 + > hw/display/cirrus_vga_isa.c | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thomas Huth <thuth@redhat.com> writes:
> From: Thomas Huth <thuth@redhat.com>
>
> In the long run, we would like to get rid of the code that allows to
> register migration state globally, so set global_vmstate to false when
> using the isa-cirrus-vga device with new machines, and only enable it
> for older machines to avoid breaking the migration there.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/core/machine.c | 1 +
> hw/display/cirrus_vga_isa.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 6cf0e2f404e..0aa77a57e95 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -40,6 +40,7 @@
>
> GlobalProperty hw_compat_10_2[] = {
> { "scsi-block", "migrate-pr", "off" },
> + { "isa-cirrus-vga", "global-vmstate", "true" },
> };
> const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
>
> diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
> index bad9ec7599c..76034a88605 100644
> --- a/hw/display/cirrus_vga_isa.c
> +++ b/hw/display/cirrus_vga_isa.c
> @@ -56,7 +56,6 @@ static void isa_cirrus_vga_realizefn(DeviceState *dev, Error **errp)
> s->vram_size_mb);
> return;
> }
> - s->global_vmstate = true;
> if (!vga_common_init(s, OBJECT(dev), errp)) {
> return;
> }
> @@ -74,6 +73,8 @@ static const Property isa_cirrus_vga_properties[] = {
> cirrus_vga.vga.vram_size_mb, 4),
> DEFINE_PROP_BOOL("blitter", struct ISACirrusVGAState,
> cirrus_vga.enable_blitter, true),
> + DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
> + cirrus_vga.vga.global_vmstate, false),
> };
>
> static void isa_cirrus_vga_class_init(ObjectClass *klass, const void *data)
Reviewed-by: Fabiano Rosas <farosas@suse.de>
© 2016 - 2026 Red Hat, Inc.