Re: [PATCH] macfb: fix VRAM dirty memory region logging

[Laurent Vivier]

Le 08/01/2022 à 18:15, Mark Cave-Ayland a écrit :
> On 08/01/2022 16:53, Laurent Vivier wrote:
>
> > Le 08/01/2022 à 17:41, Mark Cave-Ayland a écrit :
> > > The macfb VRAM memory region was configured with coalescing rather than dirty
> > > memory logging enabled, causing some areas of the screen not to redraw after
> > > a full screen update.
> > >
> > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > > Fixes: 8ac919a065 ("hw/m68k: add Nubus macfb video card")
> > > ---
> > >   hw/display/macfb.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> > > index 277d3e6633..4bd7c3ad6a 100644
> > > --- a/hw/display/macfb.c
> > > +++ b/hw/display/macfb.c
> > > @@ -661,9 +661,9 @@ static bool macfb_common_realize(DeviceState *dev, 
> > > MacfbState *s, Error **errp)
> > >       memory_region_init_ram(&s->mem_vram, OBJECT(dev), "macfb-vram",
> > >                              MACFB_VRAM_SIZE, &error_abort);
> > > +    memory_region_set_log(&s->mem_vram, true, DIRTY_MEMORY_VGA);
> > >       s->vram = memory_region_get_ram_ptr(&s->mem_vram);
> > >       s->vram_bit_mask = MACFB_VRAM_SIZE - 1;
> > > -    memory_region_set_coalescing(&s->mem_vram);
> > >       s->vbl_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, macfb_vbl_timer, s);
> > >       macfb_update_mode(s);
> >
> > I understant why you add memory_region_set_log() but I don't understand why you remove 
> > memory_region_set_coalescing().
>
> Looking at the other display devices, only VGA and cirrus use memory_region_set_coalescing() and 
> that's on the IO ports rather than the VRAM.
>
> Based upon this my suspicion is that this is mainly a vmexit optimisation when using KVM which isn't 
> relevant here for macfb.

You're right.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Thanks,
Laurent