It's been a long time, I only have vague memories of this :-) But I think it should be ok.
It does definitely make sense in the virtio case and similar where the property is set once for the instance. For bochs and ati, there's a register to configure it as well, so there *may* be an expectation that it gets reset there, I'm less certain. So tentatively... Acked-by: Benjamin Herrenschmidt <[email protected]> On Mon, 2024-12-02 at 20:48 +0100, Philippe Mathieu-Daudé wrote: > ping? > > On 29/11/24 11:17, Philippe Mathieu-Daudé wrote: > > The 'pci-vga' device allow setting a 'big-endian-framebuffer' > > property since commit 3c2784fc864 ("vga: Expose framebuffer > > byteorder as a QOM property"). Similarly, the 'virtio-vga' > > device since commit 8be61ce2ce3 ("virtio-vga: implement > > big-endian-framebuffer property"). > > > > Both call vga_common_reset() in their reset handler, respectively > > pci_secondary_vga_reset() and virtio_vga_base_reset_hold(), which > > reset 'big_endian_fb', overwritting the property. This is not > > correct: the hardware is expected to keep its configured > > endianness during resets. > > > > Move 'big_endian_fb' assignment from vga_common_reset() to > > vga_common_init() which is called once when the common VGA state > > is initialized. > > > > Signed-off-by: Philippe Mathieu-Daudé <[email protected]> > > --- > > hw/display/vga.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/display/vga.c b/hw/display/vga.c > > index 892fedc8dce..b074b58c90d 100644 > > --- a/hw/display/vga.c > > +++ b/hw/display/vga.c > > @@ -1873,7 +1873,6 @@ 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)); > > @@ -2266,6 +2265,7 @@ bool vga_common_init(VGACommonState *s, > > Object *obj, Error **errp) > > * all target endian dependencies from this file. > > */ > > s->default_endian_fb = target_words_bigendian(); > > + s->big_endian_fb = s->default_endian_fb; > > > > vga_dirty_log_start(s); > >
