On Fri, Jun 3, 2023, Jim Mattson wrote: > On Fri, Jun 2, 2023 at 3:52 PM Sean Christopherson <seanjc@xxxxxxxxxx <mailto:seanjc@xxxxxxxxxx>> wrote: > > > > On Fri, Jun 02, 2023, Jim Mattson wrote: > > > On Fri, Jun 2, 2023 at 2:48 PM Sean Christopherson <seanjc@xxxxxxxxxx <mailto:seanjc@xxxxxxxxxx>> wrote: > > > > > > > > On Fri, Jun 02, 2023, Jim Mattson wrote: > > > > > On Fri, Jun 2, 2023 at 12:16 PM Sean Christopherson <seanjc@xxxxxxxxxx <mailto:seanjc@xxxxxxxxxx>> wrote: > > > > > > > > > > > > On Fri, Jun 02, 2023, Jim Mattson wrote: > > > > > > > On Fri, Jun 2, 2023 at 12:18 AM Gao Shiyuan <gaoshiyuan@xxxxxxxxx <mailto:gaoshiyuan@xxxxxxxxx>> wrote: > > > > > > > > > > > > > > > > From: Shiyuan Gao <gaoshiyuan@xxxxxxxxx <mailto:gaoshiyuan@xxxxxxxxx>> > > > > > > > > > > > > > > > > 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. > > > > > > > > > > Um... > > > > > > > > > > "You may never migrate this VM to a newer kernel. Sucks to be you." > > > > > > > > Userspace can fudge/fixup state to migrate the VM. > > > > > > Um, yeah. Userspace can clear bit 35 from the saved > > > IA32_PERF_GLOBAL_CTRL MSR so that the migration will complete. But > > > what happens the next time the guest tries to set bit 35 in > > > IA32_PERF_GLOBAL_CTRL, which it will probably do, since it cached > > > CPUID.0AH at boot? > > > > Ah, right. Yeah, guest is hosed. > > > > I'm still not convinced this is KVM's problem to fix. > > > One could argue that userspace should have known better than to > believe KVM_GET_SUPPORTED_CPUID in the first place. Or that it should > have known better than to blindly pass that through to KVM_SET_CPUID2. > I mean, *obviously* KVM didn't really support TOPDOWN.SLOTS. Right? > > > But if userspace can't trust KVM_GET_SUPPORTED_CPUID to tell it about > which fixed counters are supported, how is it supposed to find out? > > > Another way of solving this, which should make everyone happy, is to > add KVM support for TOPDOWN.SLOTS. > Yeah, this way may make everyone happly, but we need guarantee the VM that not support TOPDOWN.SLOTS migrate success. I think this also need be addressed with a quirk like this submmit. I can't find an elegant solution...