[RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian

BALATON Zoltan posted 1 patch 2 days, 11 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260330140655.31EC55968DE@zero.eik.bme.hu
Maintainers: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Gerd Hoffmann <kraxel@redhat.com>
hw/core/machine.c                | 2 ++
hw/display/cirrus_vga.c          | 6 ++++++
hw/display/cirrus_vga_internal.h | 2 ++
hw/display/cirrus_vga_isa.c      | 2 ++
4 files changed, 12 insertions(+)
[RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian
Posted by BALATON Zoltan 2 days, 11 hours ago
I've tried this but it does not seem to work and leaves default to
auto. Could somebody tell how this should work?

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/core/machine.c                | 2 ++
 hw/display/cirrus_vga.c          | 6 ++++++
 hw/display/cirrus_vga_internal.h | 2 ++
 hw/display/cirrus_vga_isa.c      | 2 ++
 4 files changed, 12 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0aa77a57e9..e573ef46bc 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -41,6 +41,8 @@
 GlobalProperty hw_compat_10_2[] = {
     { "scsi-block", "migrate-pr", "off" },
     { "isa-cirrus-vga", "global-vmstate", "true" },
+    { "isa-cirrus-vga", "x-big-endian-fb", "off" },
+    { "cirrus-vga", "x-big-endian-fb", "off" },
 };
 const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
 
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 629b34fc68..e379f125ae 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2930,6 +2930,10 @@ void cirrus_init_common(CirrusVGAState *s, Object *owner,
     s->vga.cursor_invalidate = cirrus_cursor_invalidate;
     s->vga.cursor_draw_line = cirrus_cursor_draw_line;
 
+    if (s->big_endian_fb != ON_OFF_AUTO_AUTO) {
+        s->vga.big_endian_fb = (s->big_endian_fb == ON_OFF_AUTO_ON);
+    }
+
     qemu_register_reset(cirrus_reset, s);
 }
 
@@ -2987,6 +2991,8 @@ static const Property pci_vga_cirrus_properties[] = {
                        cirrus_vga.vga.vram_size_mb, 4),
     DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
                      cirrus_vga.enable_blitter, true),
+    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
+                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
 };
 
 static void cirrus_vga_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/display/cirrus_vga_internal.h b/hw/display/cirrus_vga_internal.h
index a78ebbd920..7c07201f18 100644
--- a/hw/display/cirrus_vga_internal.h
+++ b/hw/display/cirrus_vga_internal.h
@@ -26,6 +26,7 @@
 #ifndef CIRRUS_VGA_INTERNAL_H
 #define CIRRUS_VGA_INTERNAL_H
 
+#include "hw/core/qdev-properties.h"
 #include "vga_int.h"
 
 /* IDs */
@@ -94,6 +95,7 @@ typedef struct CirrusVGAState {
     int real_vram_size; /* XXX: suppress that */
     int device_id;
     int bustype;
+    OnOffAuto big_endian_fb;
 } CirrusVGAState;
 
 void cirrus_init_common(CirrusVGAState *s, Object *owner,
diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
index 76034a8860..c2b0b61476 100644
--- a/hw/display/cirrus_vga_isa.c
+++ b/hw/display/cirrus_vga_isa.c
@@ -75,6 +75,8 @@ static const Property isa_cirrus_vga_properties[] = {
                      cirrus_vga.enable_blitter, true),
     DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
                      cirrus_vga.vga.global_vmstate, false),
+    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
+                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
 };
 
 static void isa_cirrus_vga_class_init(ObjectClass *klass, const void *data)
-- 
2.41.3
Re: [RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian
Posted by Gerd Hoffmann 2 days, 10 hours ago
  Hi,

> +    { "isa-cirrus-vga", "x-big-endian-fb", "off" },
> +    { "cirrus-vga", "x-big-endian-fb", "off" },

I don't think cirrus needs any of this.  As far I know only standard vga
is used in big endian targets.

IIRC cirrus (as-in: the physical hardware) has 4M vram and a a 8M pci
bar, where the lower 4M offer little-endian access to the vram and the
upper 4M offer big-endian access to the vram.

qemu never emulated the big-endian aperture with the automagic
byteswapping.

take care,
  Gerd
Re: [RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian
Posted by BALATON Zoltan 1 day, 15 hours ago
On Mon, 30 Mar 2026, Gerd Hoffmann wrote:
>  Hi,
>
>> +    { "isa-cirrus-vga", "x-big-endian-fb", "off" },
>> +    { "cirrus-vga", "x-big-endian-fb", "off" },
>
> I don't think cirrus needs any of this.  As far I know only standard vga
> is used in big endian targets.

You can add a cirrus-vga-pci with -device to big endian machine too and 
I've tried an AmigaOS driver which assumes the card is little endian and 
does not work so this fix would be useful. We could just change the 
default as cirrus-vga is relatively unused on big endian machines but 
having the property is safer and may allow users to continue using what 
they use now. Please merge this fix for 11.0 if there are no objections.

> IIRC cirrus (as-in: the physical hardware) has 4M vram and a a 8M pci
> bar, where the lower 4M offer little-endian access to the vram and the
> upper 4M offer big-endian access to the vram.
>
> qemu never emulated the big-endian aperture with the automagic
> byteswapping.

The GD5449 has 16 MB BAR but also may have byte swapping aperture but I 
don't know anything that uses that.

Regards,
BALATON Zoltan
Re: [RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian
Posted by Fabiano Rosas 2 days, 8 hours ago
BALATON Zoltan <balaton@eik.bme.hu> writes:

> I've tried this but it does not seem to work and leaves default to
> auto. Could somebody tell how this should work?
>

This patch sets the hw_compat to "off" so when new code migrates to/from
an old machine type, it will set the endianness to LE due to the
conditional you added.

  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2
  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0

overriding works:

  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=on
  s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
  
  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=off
  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0

The default value is a separate thing, this patch sets it to "auto" so
unless the hw_compat or the user overrides it, that's what you get.

  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus
  s->big_endian_fb: 0
  (s->vga.big_endian_fb: 0, unchanged)
  
  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=on
  s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
  
  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=off
  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0

Maybe what you want is to say the old behavior is auto? Then set the
default to off.

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/core/machine.c                | 2 ++
>  hw/display/cirrus_vga.c          | 6 ++++++
>  hw/display/cirrus_vga_internal.h | 2 ++
>  hw/display/cirrus_vga_isa.c      | 2 ++
>  4 files changed, 12 insertions(+)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 0aa77a57e9..e573ef46bc 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -41,6 +41,8 @@
>  GlobalProperty hw_compat_10_2[] = {
>      { "scsi-block", "migrate-pr", "off" },
>      { "isa-cirrus-vga", "global-vmstate", "true" },
> +    { "isa-cirrus-vga", "x-big-endian-fb", "off" },
> +    { "cirrus-vga", "x-big-endian-fb", "off" },
>  };
>  const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
>  
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 629b34fc68..e379f125ae 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2930,6 +2930,10 @@ void cirrus_init_common(CirrusVGAState *s, Object *owner,
>      s->vga.cursor_invalidate = cirrus_cursor_invalidate;
>      s->vga.cursor_draw_line = cirrus_cursor_draw_line;
>  
> +    if (s->big_endian_fb != ON_OFF_AUTO_AUTO) {
> +        s->vga.big_endian_fb = (s->big_endian_fb == ON_OFF_AUTO_ON);
> +    }
> +
>      qemu_register_reset(cirrus_reset, s);
>  }
>  
> @@ -2987,6 +2991,8 @@ static const Property pci_vga_cirrus_properties[] = {
>                         cirrus_vga.vga.vram_size_mb, 4),
>      DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
>                       cirrus_vga.enable_blitter, true),
> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
> +                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
>  };
>  
>  static void cirrus_vga_class_init(ObjectClass *klass, const void *data)
> diff --git a/hw/display/cirrus_vga_internal.h b/hw/display/cirrus_vga_internal.h
> index a78ebbd920..7c07201f18 100644
> --- a/hw/display/cirrus_vga_internal.h
> +++ b/hw/display/cirrus_vga_internal.h
> @@ -26,6 +26,7 @@
>  #ifndef CIRRUS_VGA_INTERNAL_H
>  #define CIRRUS_VGA_INTERNAL_H
>  
> +#include "hw/core/qdev-properties.h"
>  #include "vga_int.h"
>  
>  /* IDs */
> @@ -94,6 +95,7 @@ typedef struct CirrusVGAState {
>      int real_vram_size; /* XXX: suppress that */
>      int device_id;
>      int bustype;
> +    OnOffAuto big_endian_fb;
>  } CirrusVGAState;
>  
>  void cirrus_init_common(CirrusVGAState *s, Object *owner,
> diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
> index 76034a8860..c2b0b61476 100644
> --- a/hw/display/cirrus_vga_isa.c
> +++ b/hw/display/cirrus_vga_isa.c
> @@ -75,6 +75,8 @@ static const Property isa_cirrus_vga_properties[] = {
>                       cirrus_vga.enable_blitter, true),
>      DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
>                       cirrus_vga.vga.global_vmstate, false),
> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,

s/PCICirrusVGAState/ISACirrusVGAState/

> +                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
>  };
>  
>  static void isa_cirrus_vga_class_init(ObjectClass *klass, const void
>  *data)
Re: [RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian
Posted by BALATON Zoltan 2 days, 4 hours ago
On Mon, 30 Mar 2026, Fabiano Rosas wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
>
>> I've tried this but it does not seem to work and leaves default to
>> auto. Could somebody tell how this should work?
>>
>
> This patch sets the hw_compat to "off" so when new code migrates to/from
> an old machine type, it will set the endianness to LE due to the
> conditional you added.
>
>  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2
>  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0
>
> overriding works:
>
>  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=on
>  s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
>
>  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=off
>  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0
>
> The default value is a separate thing, this patch sets it to "auto" so
> unless the hw_compat or the user overrides it, that's what you get.
>
>  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus
>  s->big_endian_fb: 0
>  (s->vga.big_endian_fb: 0, unchanged)
>
>  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=on
>  s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
>
>  ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=off
>  s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0
>
> Maybe what you want is to say the old behavior is auto? Then set the
> default to off.

Indeed. I was confused about what the compat property should set. Thanks, 
send a corrected patch.

>>      DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
>>                       cirrus_vga.vga.global_vmstate, false),
>> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
>
> s/PCICirrusVGAState/ISACirrusVGAState/

but missed this so will send a v2.

Regards,
BALATON Zoltan
Re: [RFC PATCH] cirrus-vga: Add property to set frame buffer endianness and make it default to little endian
Posted by Philippe Mathieu-Daudé 2 days, 7 hours ago
On 30/3/26 18:22, Fabiano Rosas wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
> 
>> I've tried this but it does not seem to work and leaves default to
>> auto. Could somebody tell how this should work?
>>
> 
> This patch sets the hw_compat to "off" so when new code migrates to/from
> an old machine type, it will set the endianness to LE due to the
> conditional you added.
> 
>    ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2
>    s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0
> 
> overriding works:
> 
>    ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=on
>    s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
>    
>    ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -M pc-q35-10.2 -global cirrus-vga.x-big-endian-fb=off
>    s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0
> 
> The default value is a separate thing, this patch sets it to "auto" so
> unless the hw_compat or the user overrides it, that's what you get.
> 
>    ./qemu-system-x86_64 -display none -nodefaults -vga cirrus
>    s->big_endian_fb: 0
>    (s->vga.big_endian_fb: 0, unchanged)
>    
>    ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=on
>    s->big_endian_fb: 1 --> s->vga.big_endian_fb: 1
>    
>    ./qemu-system-x86_64 -display none -nodefaults -vga cirrus -global cirrus-vga.x-big-endian-fb=off
>    s->big_endian_fb: 2 --> s->vga.big_endian_fb: 0
> 
> Maybe what you want is to say the old behavior is auto? Then set the
> default to off.

Probably better.

> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/core/machine.c                | 2 ++
>>   hw/display/cirrus_vga.c          | 6 ++++++
>>   hw/display/cirrus_vga_internal.h | 2 ++
>>   hw/display/cirrus_vga_isa.c      | 2 ++
>>   4 files changed, 12 insertions(+)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 0aa77a57e9..e573ef46bc 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -41,6 +41,8 @@
>>   GlobalProperty hw_compat_10_2[] = {
>>       { "scsi-block", "migrate-pr", "off" },
>>       { "isa-cirrus-vga", "global-vmstate", "true" },
>> +    { "isa-cirrus-vga", "x-big-endian-fb", "off" },
>> +    { "cirrus-vga", "x-big-endian-fb", "off" },
>>   };
>>   const size_t hw_compat_10_2_len = G_N_ELEMENTS(hw_compat_10_2);
>>   
>> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
>> index 629b34fc68..e379f125ae 100644
>> --- a/hw/display/cirrus_vga.c
>> +++ b/hw/display/cirrus_vga.c
>> @@ -2930,6 +2930,10 @@ void cirrus_init_common(CirrusVGAState *s, Object *owner,
>>       s->vga.cursor_invalidate = cirrus_cursor_invalidate;
>>       s->vga.cursor_draw_line = cirrus_cursor_draw_line;
>>   
>> +    if (s->big_endian_fb != ON_OFF_AUTO_AUTO) {
>> +        s->vga.big_endian_fb = (s->big_endian_fb == ON_OFF_AUTO_ON);
>> +    }
>> +
>>       qemu_register_reset(cirrus_reset, s);
>>   }
>>   
>> @@ -2987,6 +2991,8 @@ static const Property pci_vga_cirrus_properties[] = {
>>                          cirrus_vga.vga.vram_size_mb, 4),
>>       DEFINE_PROP_BOOL("blitter", struct PCICirrusVGAState,
>>                        cirrus_vga.enable_blitter, true),
>> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
>> +                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
>>   };
>>   
>>   static void cirrus_vga_class_init(ObjectClass *klass, const void *data)
>> diff --git a/hw/display/cirrus_vga_internal.h b/hw/display/cirrus_vga_internal.h
>> index a78ebbd920..7c07201f18 100644
>> --- a/hw/display/cirrus_vga_internal.h
>> +++ b/hw/display/cirrus_vga_internal.h
>> @@ -26,6 +26,7 @@
>>   #ifndef CIRRUS_VGA_INTERNAL_H
>>   #define CIRRUS_VGA_INTERNAL_H
>>   
>> +#include "hw/core/qdev-properties.h"

"qapi/qapi-types-common.h" instead.

>>   #include "vga_int.h"
>>   
>>   /* IDs */
>> @@ -94,6 +95,7 @@ typedef struct CirrusVGAState {
>>       int real_vram_size; /* XXX: suppress that */
>>       int device_id;
>>       int bustype;
>> +    OnOffAuto big_endian_fb;
>>   } CirrusVGAState;
>>   
>>   void cirrus_init_common(CirrusVGAState *s, Object *owner,
>> diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
>> index 76034a8860..c2b0b61476 100644
>> --- a/hw/display/cirrus_vga_isa.c
>> +++ b/hw/display/cirrus_vga_isa.c
>> @@ -75,6 +75,8 @@ static const Property isa_cirrus_vga_properties[] = {
>>                        cirrus_vga.enable_blitter, true),
>>       DEFINE_PROP_BOOL("global-vmstate", struct ISACirrusVGAState,
>>                        cirrus_vga.vga.global_vmstate, false),
>> +    DEFINE_PROP_ON_OFF_AUTO("x-big-endian-fb", struct PCICirrusVGAState,
> 
> s/PCICirrusVGAState/ISACirrusVGAState/
> 
>> +                            cirrus_vga.big_endian_fb, ON_OFF_AUTO_AUTO),
>>   };
>>   
>>   static void isa_cirrus_vga_class_init(ObjectClass *klass, const void
>>   *data)