Re: [PATCH v6 04/16] KVM: x86/pmu: Set MSR_IA32_MISC_ENABLE_EMON bit when vPMU is enabled

[Xu, Like]

On 2021/5/18 7:51, Sean Christopherson wrote:
> On Mon, May 17, 2021, Sean Christopherson wrote:
> > On Thu, May 13, 2021, Xu, Like wrote:
> > > On 2021/5/12 23:18, Sean Christopherson wrote:
> > > > On Wed, May 12, 2021, Xu, Like wrote:
> > > > > Hi Venkatesh Srinivas,
> > > > >
> > > > > On 2021/5/12 9:58, Venkatesh Srinivas wrote:
> > > > > > On 5/10/21, Like Xu <like.xu@xxxxxxxxxxxxxxx> wrote:
> > > > > > > On Intel platforms, the software can use the IA32_MISC_ENABLE[7] bit to
> > > > > > > detect whether the processor supports performance monitoring facility.
> > > > > > >
> > > > > > > It depends on the PMU is enabled for the guest, and a software write
> > > > > > > operation to this available bit will be ignored.
> > > > > > Is the behavior that writes to IA32_MISC_ENABLE[7] are ignored (rather than #GP)
> > > > > > documented someplace?
> > > > > The bit[7] behavior of the real hardware on the native host is quite
> > > > > suspicious.
> > > > Ugh.  Can you file an SDM bug to get the wording and accessibility updated?  The
> > > > current phrasing is a mess:
> > > >
> > > >     Performance Monitoring Available (R)
> > > >     1 = Performance monitoring enabled.
> > > >     0 = Performance monitoring disabled.
> > > >
> > > > The (R) is ambiguous because most other entries that are read-only use (RO), and
> > > > the "enabled vs. disabled" implies the bit is writable and really does control
> > > > the PMU.  But on my Haswell system, it's read-only.
> > > On your Haswell system, does it cause #GP or just silent if you change this
> > > bit ?
> > Attempting to clear the bit generates a #GP.
> *sigh*
>
> Venkatesh and I are exhausting our brown paper bag supply.
>
> Attempting to clear bit 7 is ignored on both Haswell and Goldmont.  This _no_ #GP,
> the toggle is simply ignored.  I forgot to specify hex format (multiple times),
> and Venkatesh accessed the wrong MSR (0x10a instead of 0x1a0).

*sigh*

> So your proposal to ignore the toggle in KVM is the way to go, but please
> document in the changelog that that behavior matches bare metal.

Thank you, I will clearly state it in the commit message.

> It would be nice to get the SDM cleaned up to use "supported/unsupported", and to
> pick one of (R), (RO), and (R/O) for all MSRs entries for consistency, but that
> may be a pipe dream.

Glad you could review my code. I have reported this issue internally.

> Sorry for the run-around :-/