Re: [PATCH v1 5/6] hw/arm/virt: Enable backup bitmap for dirty ring

[Peter Maydell]

On Thu, 23 Feb 2023 at 00:52, Gavin Shan <gshan@redhat.com> wrote:
>
> On 2/23/23 2:54 AM, Peter Maydell wrote:
> > But we might have to for other boards we add later. We shouldn't
> > put code in per-board if it's not really board specific.
> >
> > Moreover, I think "we need the backup bitmap if the kernel is
> > using its GICv3 or ITS implementation" is a kernel implementation
> > detail. It seems to me that it would be cleaner if QEMU didn't
> > have to hardcode "we happen to know that these are the situations
> > when we need to do that". A better API would be "ask the kernel
> > 'do we need this?' and enable it if it says 'yes'". The kernel
> > knows what its implementations of ITS and GICv3 (and perhaps
> > future in-kernel memory-using devices) require, after all.
> >
>
> Well, As we know so far, the backup bitmap extension is only required by 
> 'kvm-arm-gicv3'
> and 'arm-its-kvm' device. Those two devices are only used by virt machine at 
> present.
> So it's a board specific requirement. I'm not sure about the future. We may 
> need to
> enable the extension for other devices and other boards. That time, the 
> requirement
> isn't board specific any more. However, we're uncertain for the future.

Most boards using KVM are likely to want a GICv3, and
probably an ITS too. A board with no interrupt controller
is useless, and the GICv2 is obsolete.

> In order to cover the future requirement, the extension is needed by other 
> boards,
> the best way I can figure out is to enable the extension in generic path in 
> kvm_init()
> if the extension is supported by the host kernel. In this way, the 
> unnecessary overhead
> is introduced for those boards where 'kvm-arm-vgic3' and 'arm-its-kvm' aren't 
> used.
> The overhead should be very small and acceptable. Note that the host kernel 
> don't know
> if 'kvm-arm-vgic3' or 'arm-its-kvm' device is needed by the board in 
> kvm_init(), which
> is the generic path.

We can have a generic hook that happens after board init is
done, if we want to do non-board-specific stuff that happens
later. However I suspect that anybody who cares about migration
performance is likely using a GICv3 at least anyway,
so "enable always" should be fine.

thanks
-- PMM