Re: [PATCH] KVM: x86/vPMU: ignore the check of IA32_PERF_GLOBAL_CTRL bit35

[Sean Christopherson <seanjc@google.com>]

On Fri, Jun 02, 2023, Jim Mattson wrote:
> On Fri, Jun 2, 2023 at 12:18 AM Gao Shiyuan <gaoshiyuan@baidu.com> wrote:
> >
> > From: Shiyuan Gao <gaoshiyuan@baidu.com>
> >
> > When live-migrate VM on icelake microarchitecture, if the source
> > host kernel before commit 2e8cd7a3b828 ("kvm: x86: limit the maximum
> > number of vPMU fixed counters to 3") and the dest host kernel after this
> > commit, the migration will fail.
> >
> > The source VM's CPUID.0xA.edx[0..4]=4 that is reported by KVM and
> > the IA32_PERF_GLOBAL_CTRL MSR is 0xf000000ff. However the dest VM's
> > CPUID.0xA.edx[0..4]=3 and the IA32_PERF_GLOBAL_CTRL MSR is 0x7000000ff.
> > This inconsistency leads to migration failure.

IMO, this is a userspace bug.  KVM provided userspace all the information it needed
to know that the target is incompatible (3 counters instead of 4), it's userspace's
fault for not sanity checking that the target is compatible.

I agree that KVM isn't blame free, but hacking KVM to cover up userspace mistakes
everytime a feature appears or disappears across kernel versions or configs isn't
maintainable.

> > The QEMU limits the maximum number of vPMU fixed counters to 3, so ignore
> > the check of IA32_PERF_GLOBAL_CTRL bit35.

Unconditionally allowing the bit is unsafe, e.g. will cause KVM to miss consistency
checks on PERF_GLOBAL_CTRL when emulating nested VM-Enter.  They should eventually
be caught by hardware, but it's still undesirable.

> Today, the fixed counters are limited to 3, but I hope we get support
> for top down slots soon.

Is there any reason KVM can't simply add support for the fourth fixed counter?
Perf is already aware of the topdown slots event, so isn't this "just" a matter
of adding the appropriate entries?

> Perhaps this inconsistency is best addressed with a quirk?

*If* we ended up adding a hack to KVM, I'd prefer not to add a quirk, this shouldn't
be something KVM needs to carry long term.