[PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines

[PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines

[Thomas Huth]

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.0

Re: [PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines

[Fabiano Rosas]

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>

Re: [PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines

[Philippe Mathieu-Daudé]

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>

Re: VGA default endianness (was: [PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines)

[BALATON Zoltan]

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. Anybody wants to make a patch?

Regards,
BALATON Zoltan

Re: VGA default endianness (was: [PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines)

[Gerd Hoffmann]

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

Re: VGA default endianness

[Philippe Mathieu-Daudé]

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.

Re: VGA default endianness (was: [PATCH] hw/display/cirrus_vga_isa: Disable global_vmstate by default for new machines)

[BALATON Zoltan]

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