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.