PMU virtualization and AMD erratum 1292

PMU virtualization and AMD erratum 1292

[Jim Mattson]

From AMD erratum 1292:

The processor may experience sampling inaccuracies that cause the
following performance counters to overcount retire-based events.
 • PMCx0C0 [Retired Instructions]
 • PMCx0C1 [Retired Uops]
 • PMCx0C2 [Retired Branch Instructions]
 • PMCx0C3 [Retired Branch Instructions Mispredicted]
 • PMCx0C4 [Retired Taken Branch Instructions]
 • PMCx0C5 [Retired Taken Branch Instructions Mispredicted]
 • PMCx0C8 [Retired Near Returns]
 • PMCx0C9 [Retired Near Returns Mispredicted]
 • PMCx0CA [Retired Indirect Branch Instructions Mispredicted]
• PMCx0CC [Retired Indirect Branch Instructions]
 • PMCx0D1 [Retired Conditional Branch Instructions]
 • PMCx1C7 [Retired Mispredicted Branch Instructions due to Direction Mismatch]
 • PMCx1D0 [Retired Fused Branch Instructions]

The recommended workaround is:

To count the non-FP affected PMC events correctly:
 • Use Core::X86::Msr::PERF_CTL2 to count the events, and
 • Program Core::X86::Msr::PERF_CTL2[43] to 1b, and
 • Program Core::X86::Msr::PERF_CTL2[20] to 0b.

It's unfortunate that kvm's PMU virtualization completely circumvents
any attempt to employ the recommended workaround. Admittedly, bit 43
is "reserved," and it would be foolish for a hypervisor to let a guest
set a reserved bit in a host MSR. But, even the first recommendation
is impossible under KVM, because the host's perf subsystem actually
decides which hardware counter is going to be used, regardless of what
the guest asks for.

Am I the only one bothered by this?

Re: PMU virtualization and AMD erratum 1292

[Like Xu]

On 15/1/2022 4:02 am, Jim Mattson wrote:
>  From AMD erratum 1292:

I see quite a few errata in AMD's products in terms of PMU counters.

Considering the number of this type of machines in real world,
there is a real need to think about it. Thanks for pointing out.

> 
> The processor may experience sampling inaccuracies that cause the
> following performance counters to overcount retire-based events.
>   • PMCx0C0 [Retired Instructions]
>   • PMCx0C1 [Retired Uops]
>   • PMCx0C2 [Retired Branch Instructions]
>   • PMCx0C3 [Retired Branch Instructions Mispredicted]
>   • PMCx0C4 [Retired Taken Branch Instructions]
>   • PMCx0C5 [Retired Taken Branch Instructions Mispredicted]
>   • PMCx0C8 [Retired Near Returns]
>   • PMCx0C9 [Retired Near Returns Mispredicted]
>   • PMCx0CA [Retired Indirect Branch Instructions Mispredicted]
> • PMCx0CC [Retired Indirect Branch Instructions]
>   • PMCx0D1 [Retired Conditional Branch Instructions]
>   • PMCx1C7 [Retired Mispredicted Branch Instructions due to Direction Mismatch]
>   • PMCx1D0 [Retired Fused Branch Instructions]
> 
> The recommended workaround is:

Or to set the BIOS Setup Option "IBS hardware workaround."
(not recommended for production due to negative performance impact)

> 
> To count the non-FP affected PMC events correctly:
>   • Use Core::X86::Msr::PERF_CTL2 to count the events, and
>   • Program Core::X86::Msr::PERF_CTL2[43] to 1b, and
>   • Program Core::X86::Msr::PERF_CTL2[20] to 0b.

diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index 12d8b301065a..6a7638043066 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -18,6 +18,13 @@
  #include "pmu.h"
  #include "svm.h"

+/* AMD erratum 1292 */
+static inline bool cpu_overcount_retire_events(struct kvm_vcpu *vcpu)
+{
+	return guest_cpuid_family(vcpu) == 0x19 &&
+		guest_cpuid_model(vcpu) < 0x10;
+}
+
  enum pmu_type {
  	PMU_TYPE_COUNTER = 0,
  	PMU_TYPE_EVNTSEL,
@@ -252,6 +259,7 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct 
msr_data *msr_info)
  	struct kvm_pmc *pmc;
  	u32 msr = msr_info->index;
  	u64 data = msr_info->data;
+	u64 reserved_bits = pmu->reserved_bits;

  	/* MSR_PERFCTRn */
  	pmc = get_gp_pmc_amd(pmu, msr, PMU_TYPE_COUNTER);
@@ -264,7 +272,9 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct 
msr_data *msr_info)
  	if (pmc) {
  		if (data == pmc->eventsel)
  			return 0;
-		if (!(data & pmu->reserved_bits)) {
+		if (pmc->idx == 2 && cpu_overcount_retire_events(vcpu))
+			reserved_bits &= ~BIT_ULL(43);
+		if (!(data & reserved_bits)) {
  			reprogram_gp_counter(pmc, data);
  			return 0;
  		}

> 
> It's unfortunate that kvm's PMU virtualization completely circumvents
> any attempt to employ the recommended workaround. Admittedly, bit 43
> is "reserved," and it would be foolish for a hypervisor to let a guest
> set a reserved bit in a host MSR. 

It's easy for KVM to clear the reserved bit PERF_CTL2[43]
for only (AMD Family 19h Models 00h-0Fh) guests.

Obviously, such guests need to be updated and the reserved bit can
be accessed safely. Don't worry about the legacy guest, see below.

> But, even the first recommendation
> is impossible under KVM, because the host's perf subsystem actually
> decides which hardware counter is going to be used, regardless of what
> the guest asks for.

First, the host perf subsystem needs to be patched to implement this workaround.
  (AMD guys have been notified)

The patched host perf will schedule all retire events to counter 2 as long as
the requested event_select and unit_mask are matched in the workaround table.

It works for both host-created perf_events and KVM-created perf_events, so that
all legacy (retire event) guests counters will use the specific host counter 2 and,
the sampling (w/o host counter multiplexing) will be kept accurate.

> 
> Am I the only one bothered by this?
With this workaround, it is easier to trigger multiplexing, which the guest
does not correctly perceive even now.

Re: PMU virtualization and AMD erratum 1292

[Jim Mattson]

On Sun, Jan 16, 2022 at 8:26 PM Like Xu <like.xu.linux@gmail.com> wrote:
...
> It's easy for KVM to clear the reserved bit PERF_CTL2[43]
> for only (AMD Family 19h Models 00h-0Fh) guests.

KVM is currently *way* too aggressive about synthesizing #GP for
"reserved" bits on AMD hardware. Note that "reserved" generally has a
much weaker definition in AMD documentation than in Intel
documentation. When Intel says that an MSR bit is "reserved," it means
that an attempt to set the bit will raise #GP. When AMD says that an
MSR bit is "reserved," it does not necessarily mean the same thing.
(Usually, AMD will write MBZ to indicate that the bit must be zero.)

On my Zen3 CPU, I can write 0xffffffffffffffff to MSR 0xc0010204,
without getting a #GP. Hence, KVM should not synthesize a #GP for any
writes to this MSR.

Note that the value I get back from rdmsr is 0x30fffdfffff, so there
appears to be no storage behind bit 43. If KVM allows this bit to be
set, it should ensure that reads of this bit always return 0, as they
do on hardware.

Re: PMU virtualization and AMD erratum 1292

[Jim Mattson]

On Mon, Jan 17, 2022 at 12:57 PM Jim Mattson <jmattson@google.com> wrote:
>
> On Sun, Jan 16, 2022 at 8:26 PM Like Xu <like.xu.linux@gmail.com> wrote:
> ...
> > It's easy for KVM to clear the reserved bit PERF_CTL2[43]
> > for only (AMD Family 19h Models 00h-0Fh) guests.
>
> KVM is currently *way* too aggressive about synthesizing #GP for
> "reserved" bits on AMD hardware. Note that "reserved" generally has a
> much weaker definition in AMD documentation than in Intel
> documentation. When Intel says that an MSR bit is "reserved," it means
> that an attempt to set the bit will raise #GP. When AMD says that an
> MSR bit is "reserved," it does not necessarily mean the same thing.
> (Usually, AMD will write MBZ to indicate that the bit must be zero.)
>
> On my Zen3 CPU, I can write 0xffffffffffffffff to MSR 0xc0010204,
> without getting a #GP. Hence, KVM should not synthesize a #GP for any
> writes to this MSR.
>
> Note that the value I get back from rdmsr is 0x30fffdfffff, so there
> appears to be no storage behind bit 43. If KVM allows this bit to be
> set, it should ensure that reads of this bit always return 0, as they
> do on hardware.

Bit 19 (Intel's old Pin Control bit) seems to have storage behind it.
It is interesting that in Figure 13-7 "Core Performance Event-Select
Register (PerfEvtSeln)" of the APM volume 2, this "reserved" bit is
not marked in grey. The remaining "reserved" bits (which are marked in
grey), should probably be annotated with "RAZ."

Re: PMU virtualization and AMD erratum 1292

[Like Xu]

On 18/1/2022 12:08 pm, Jim Mattson wrote:
> On Mon, Jan 17, 2022 at 12:57 PM Jim Mattson <jmattson@google.com> wrote:
>>
>> On Sun, Jan 16, 2022 at 8:26 PM Like Xu <like.xu.linux@gmail.com> wrote:
>> ...
>>> It's easy for KVM to clear the reserved bit PERF_CTL2[43]
>>> for only (AMD Family 19h Models 00h-0Fh) guests.
>>
>> KVM is currently *way* too aggressive about synthesizing #GP for
>> "reserved" bits on AMD hardware. Note that "reserved" generally has a
>> much weaker definition in AMD documentation than in Intel
>> documentation. When Intel says that an MSR bit is "reserved," it means
>> that an attempt to set the bit will raise #GP. When AMD says that an
>> MSR bit is "reserved," it does not necessarily mean the same thing.

I agree. And I'm curious as to why there are hardly any guest user complaints.

The term "reserved" is described in the AMD "Conventions and Definitions":

	Fields marked as reserved may be used at some future time.
	To preserve compatibility with future processors, reserved fields require 
special handling when
	read or written by software. Software must not depend on the state of a 
reserved field (unless
	qualified as RAZ), nor upon the ability of such fields to return a previously 
written state.

	If a field is marked reserved *without qualification*, software must not change 
the state of
	that field; it must reload that field with the same value returned from a prior 
read.
	
	Reserved fields may be qualified as IGN, MBZ, RAZ, or SBZ.

For AMD, #GP comes from "Writing 1 to any bit that must be zero (MBZ) in the MSR."

>> (Usually, AMD will write MBZ to indicate that the bit must be zero.)
>>
>> On my Zen3 CPU, I can write 0xffffffffffffffff to MSR 0xc0010204,
>> without getting a #GP. Hence, KVM should not synthesize a #GP for any
>> writes to this MSR.
>>

; storage behind bit 43 test
; CPU family:          25
; Model:               1

wrmsr -p 0 0xc0010204 0x80000000000
rdmsr -p 0 0xc0010204 # return 0x80000000000

>> Note that the value I get back from rdmsr is 0x30fffdfffff, so there
>> appears to be no storage behind bit 43. If KVM allows this bit to be
>> set, it should ensure that reads of this bit always return 0, as they
>> do on hardware.

The PERF_CTL2[43] is marked reserved without qualification in the in Figure 13-7.

I'm not sure we really need a cleanup storm of #GP for all SVM's non-MBZ 
reserved bits.

> 
> Bit 19 (Intel's old Pin Control bit) seems to have storage behind it.
> It is interesting that in Figure 13-7 "Core Performance Event-Select
> Register (PerfEvtSeln)" of the APM volume 2, this "reserved" bit is
> not marked in grey. The remaining "reserved" bits (which are marked in
> grey), should probably be annotated with "RAZ."
> 

In any diagram, we at least have three types of "reservation":

- Reserved + grey
- Reserved, MBZ + grey
- Reserved + no grey

So it is better not to think of "Reserved + grey" as "Reserved, MBZ + grey".

Re: PMU virtualization and AMD erratum 1292

[Jim Mattson]

On Mon, Jan 17, 2022 at 10:25 PM Like Xu <like.xu.linux@gmail.com> wrote:
>
> On 18/1/2022 12:08 pm, Jim Mattson wrote:
> > On Mon, Jan 17, 2022 at 12:57 PM Jim Mattson <jmattson@google.com> wrote:
> >>
> >> On Sun, Jan 16, 2022 at 8:26 PM Like Xu <like.xu.linux@gmail.com> wrote:
> >> ...
> >>> It's easy for KVM to clear the reserved bit PERF_CTL2[43]
> >>> for only (AMD Family 19h Models 00h-0Fh) guests.
> >>
> >> KVM is currently *way* too aggressive about synthesizing #GP for
> >> "reserved" bits on AMD hardware. Note that "reserved" generally has a
> >> much weaker definition in AMD documentation than in Intel
> >> documentation. When Intel says that an MSR bit is "reserved," it means
> >> that an attempt to set the bit will raise #GP. When AMD says that an
> >> MSR bit is "reserved," it does not necessarily mean the same thing.
>
> I agree. And I'm curious as to why there are hardly any guest user complaints.
>
> The term "reserved" is described in the AMD "Conventions and Definitions":
>
>         Fields marked as reserved may be used at some future time.
>         To preserve compatibility with future processors, reserved fields require
> special handling when
>         read or written by software. Software must not depend on the state of a
> reserved field (unless
>         qualified as RAZ), nor upon the ability of such fields to return a previously
> written state.
>
>         If a field is marked reserved *without qualification*, software must not change
> the state of
>         that field; it must reload that field with the same value returned from a prior
> read.
>
>         Reserved fields may be qualified as IGN, MBZ, RAZ, or SBZ.
>
> For AMD, #GP comes from "Writing 1 to any bit that must be zero (MBZ) in the MSR."
>
> >> (Usually, AMD will write MBZ to indicate that the bit must be zero.)
> >>
> >> On my Zen3 CPU, I can write 0xffffffffffffffff to MSR 0xc0010204,
> >> without getting a #GP. Hence, KVM should not synthesize a #GP for any
> >> writes to this MSR.
> >>
>
> ; storage behind bit 43 test
> ; CPU family:          25
> ; Model:               1
>
> wrmsr -p 0 0xc0010204 0x80000000000
> rdmsr -p 0 0xc0010204 # return 0x80000000000

Oops. You're right. The host that I thought was a Zen3 was actually a
Zen2. Switching to an actual Zen3, I find that there is storage behind
bits 42 and 43, both of which are indicated as reserved.


> >> Note that the value I get back from rdmsr is 0x30fffdfffff, so there
> >> appears to be no storage behind bit 43. If KVM allows this bit to be
> >> set, it should ensure that reads of this bit always return 0, as they
> >> do on hardware.
>
> The PERF_CTL2[43] is marked reserved without qualification in the in Figure 13-7.
>
> I'm not sure we really need a cleanup storm of #GP for all SVM's non-MBZ
> reserved bits.

OTOH, we wouldn't need to have this discussion if these MSRs had been
implemented correctly to begin with.

> >
> > Bit 19 (Intel's old Pin Control bit) seems to have storage behind it.
> > It is interesting that in Figure 13-7 "Core Performance Event-Select
> > Register (PerfEvtSeln)" of the APM volume 2, this "reserved" bit is
> > not marked in grey. The remaining "reserved" bits (which are marked in
> > grey), should probably be annotated with "RAZ."
> >
>
> In any diagram, we at least have three types of "reservation":
>
> - Reserved + grey
> - Reserved, MBZ + grey
> - Reserved + no grey
>
> So it is better not to think of "Reserved + grey" as "Reserved, MBZ + grey".

Right. None of these bits MBZ. I was observing that the grey fields
RAZ. However, that observation was on Zen2. Zen3 is different. Now,
it's not clear to me what the grey highlights mean. Perhaps nothing at
all.

Re: PMU virtualization and AMD erratum 1292

[Like Xu]

On 19/1/2022 2:22 am, Jim Mattson wrote:
> On Mon, Jan 17, 2022 at 10:25 PM Like Xu <like.xu.linux@gmail.com> wrote:
>>
>> On 18/1/2022 12:08 pm, Jim Mattson wrote:
>>> On Mon, Jan 17, 2022 at 12:57 PM Jim Mattson <jmattson@google.com> wrote:
>>>>
>>>> On Sun, Jan 16, 2022 at 8:26 PM Like Xu <like.xu.linux@gmail.com> wrote:
>>>> ...
>>>>> It's easy for KVM to clear the reserved bit PERF_CTL2[43]
>>>>> for only (AMD Family 19h Models 00h-0Fh) guests.
>>>>
>>>> KVM is currently *way* too aggressive about synthesizing #GP for
>>>> "reserved" bits on AMD hardware. Note that "reserved" generally has a
>>>> much weaker definition in AMD documentation than in Intel
>>>> documentation. When Intel says that an MSR bit is "reserved," it means
>>>> that an attempt to set the bit will raise #GP. When AMD says that an
>>>> MSR bit is "reserved," it does not necessarily mean the same thing.
>>
>> I agree. And I'm curious as to why there are hardly any guest user complaints.
>>
>> The term "reserved" is described in the AMD "Conventions and Definitions":
>>
>>          Fields marked as reserved may be used at some future time.
>>          To preserve compatibility with future processors, reserved fields require
>> special handling when
>>          read or written by software. Software must not depend on the state of a
>> reserved field (unless
>>          qualified as RAZ), nor upon the ability of such fields to return a previously
>> written state.
>>
>>          If a field is marked reserved *without qualification*, software must not change
>> the state of
>>          that field; it must reload that field with the same value returned from a prior
>> read.
>>
>>          Reserved fields may be qualified as IGN, MBZ, RAZ, or SBZ.
>>
>> For AMD, #GP comes from "Writing 1 to any bit that must be zero (MBZ) in the MSR."
>>
>>>> (Usually, AMD will write MBZ to indicate that the bit must be zero.)
>>>>
>>>> On my Zen3 CPU, I can write 0xffffffffffffffff to MSR 0xc0010204,
>>>> without getting a #GP. Hence, KVM should not synthesize a #GP for any
>>>> writes to this MSR.
>>>>
>>
>> ; storage behind bit 43 test
>> ; CPU family:          25
>> ; Model:               1
>>
>> wrmsr -p 0 0xc0010204 0x80000000000
>> rdmsr -p 0 0xc0010204 # return 0x80000000000
> 
> Oops. You're right. The host that I thought was a Zen3 was actually a
> Zen2. Switching to an actual Zen3, I find that there is storage behind
> bits 42 and 43, both of which are indicated as reserved.
> 
> 
>>>> Note that the value I get back from rdmsr is 0x30fffdfffff, so there
>>>> appears to be no storage behind bit 43. If KVM allows this bit to be
>>>> set, it should ensure that reads of this bit always return 0, as they
>>>> do on hardware.
>>
>> The PERF_CTL2[43] is marked reserved without qualification in the in Figure 13-7.
>>
>> I'm not sure we really need a cleanup storm of #GP for all SVM's non-MBZ
>> reserved bits.
> 
> OTOH, we wouldn't need to have this discussion if these MSRs had been
> implemented correctly to begin with.

So should KVM remove all #GP for AMD's non-MBZ reserved bits?

Not a small amount of work, plus almost none guest user complaints.

> 
>>>
>>> Bit 19 (Intel's old Pin Control bit) seems to have storage behind it.
>>> It is interesting that in Figure 13-7 "Core Performance Event-Select
>>> Register (PerfEvtSeln)" of the APM volume 2, this "reserved" bit is
>>> not marked in grey. The remaining "reserved" bits (which are marked in
>>> grey), should probably be annotated with "RAZ."
>>>
>>
>> In any diagram, we at least have three types of "reservation":
>>
>> - Reserved + grey
>> - Reserved, MBZ + grey
>> - Reserved + no grey
>>
>> So it is better not to think of "Reserved + grey" as "Reserved, MBZ + grey".
> 
> Right. None of these bits MBZ. I was observing that the grey fields
> RAZ. However, that observation was on Zen2. Zen3 is different. Now,
> it's not clear to me what the grey highlights mean. Perhaps nothing at
> all.

Anyway, does this fix [0] help with this issue, assuming AMD guys would come
up with a workaround for the host perf scheduler as usual ?

[0] https://lore.kernel.org/kvm/20220117055703.52020-1-likexu@tencent.com/

Re: PMU virtualization and AMD erratum 1292

[Ananth Narayan]

On 19-01-2022 09:24 am, Like Xu wrote:

...

>>> In any diagram, we at least have three types of "reservation":
>>>
>>> - Reserved + grey
>>> - Reserved, MBZ + grey
>>> - Reserved + no grey
>>>
>>> So it is better not to think of "Reserved + grey" as "Reserved, MBZ + grey".
>>
>> Right. None of these bits MBZ. I was observing that the grey fields
>> RAZ. However, that observation was on Zen2. Zen3 is different. Now,
>> it's not clear to me what the grey highlights mean. Perhaps nothing at
>> all.
> 
> Anyway, does this fix [0] help with this issue, assuming AMD guys would come
> up with a workaround for the host perf scheduler as usual ?

Yes, that is WIP. You'll see a patch from Ravi.

Regards,
Ananth