On Wed, Jul 20, 2022, Duan, Zhenzhong wrote:
> >On Tue, Jul 19, 2022, Paolo Bonzini wrote:
> >> On 7/18/22 22:12, Sean Christopherson wrote:
> >> > On Mon, Jul 18, 2022, Paolo Bonzini wrote:
> >> > > This needs to be fixed in the kernel because old QEMU/new KVM is
> >> > > supported.
> >> >
> >> > I can't object to adding a quirk for this since KVM is breaking
> >> > userspace,
> >> > but on the KVM side we really need to stop "sanitizing" userspace inputs
> >> > unless it puts the host at risk, because inevitably it leads to needing
> >> > a quirk.
> >>
> >> The problem is not the sanitizing, it's that userspace literally
> >> cannot know that this needs to be done because the feature bits are
> >> "backwards" (1 = unavailable).
> >
> >Yes, the bits being inverted contributed to KVM not providing a way for
> >userspace to enumerate PEBS and BTS support, but lack of enumeration is a
> >seperate issue.
> >
> >If KVM had simply ignored invalid guest state from the get go, then
> >userspace would never have gained a dependency on KVM sanitizing guest
> >state. The fact that KVM didn't enumerate support in any way is an
> >orthogonal problem. To play nice with older userspace, KVM will need to
> >add a quirk to restore the sanizting code, but that doesn't solve the
> >enumeration issue. And vice versa, solving the enuemaration problem
> >doesn't magically fix old userspace.
> Hi,
>
> I didn't clearly understand the boundary of when to use quirk and when to fix
> it directly, appreciate your guide. My previous understanding for quirk is
> about backward compatibility, old behavior vs. new behavior. But this issue
> is more like a regression or bug, and the sanitizing code is only in kvm/next
> branch, not in kernel upstream yet, why bother to use a quirk?
Oh! Yay! You're absolutely right. And now I understand why Paolo is saying
the
problem has nothing to do with sanitizing...
I was thinking that KVM had been doing the sanitizing before the PEBS enabling,
but that bad behavior was introduced by bef6ecca46ac ("KVM: x86/pmu: Set
MSR_IA32_MISC_ENABLE_EMON bit when vPMU is enabled").
Sorry for the noise.
