* [PATCH 1/7] perf/x86/core: Update x86_pmu.pebs_capable for ICELAKE_{X,D}
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-13 12:25 ` [PATCH 2/7] perf/x86/core: Completely disable guest PEBS via guest's global_ctrl Like Xu
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu, Kan Liang
From: Like Xu <likexu@tencent.com>
Ice Lake microarchitecture with EPT-Friendly PEBS capability also support
the Extended feature, which means that all counters (both fixed function
and general purpose counters) can be used for PEBS events.
Update x86_pmu.pebs_capable like SPR to apply PEBS_ALL semantics.
Cc: Kan Liang <kan.liang@linux.intel.com>
Fixes: fb358e0b811e ("perf/x86/intel: Add EPT-Friendly PEBS for Ice Lake Server")
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/events/intel/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 4e9b7af9cc45..e46fd496187b 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -6239,6 +6239,7 @@ __init int intel_pmu_init(void)
case INTEL_FAM6_ICELAKE_X:
case INTEL_FAM6_ICELAKE_D:
x86_pmu.pebs_ept = 1;
+ x86_pmu.pebs_capable = ~0ULL;
pmem = true;
fallthrough;
case INTEL_FAM6_ICELAKE_L:
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/7] perf/x86/core: Completely disable guest PEBS via guest's global_ctrl
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
2022-07-13 12:25 ` [PATCH 1/7] perf/x86/core: Update x86_pmu.pebs_capable for ICELAKE_{X,D} Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-13 12:25 ` [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask Like Xu
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu
From: Like Xu <likexu@tencent.com>
When a guest PEBS counter is cross-mapped by a host counter, software
will remove the corresponding bit in the arr[global_ctrl].guest and
expect hardware to perform a change of state "from enable to disable"
via the msr_slot[] switch during the vmx transaction.
The real world is that if user adjust the counter overflow value small
enough, it still opens a tiny race window for the previously PEBS-enabled
counter to write cross-mapped PEBS records into the guest's PEBS buffer,
when arr[global_ctrl].guest has been prioritised (switch_msr_special stuff)
to switch into the enabled state, while the arr[pebs_enable].guest has not.
Close this window by clearing invalid bits in the arr[global_ctrl].guest.
Fixes: 854250329c02 ("KVM: x86/pmu: Disable guest PEBS temporarily in two rare situations")
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/events/intel/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index e46fd496187b..495ac447bb3a 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4052,8 +4052,9 @@ static struct perf_guest_switch_msr *intel_guest_get_msrs(int *nr, void *data)
/* Disable guest PEBS if host PEBS is enabled. */
arr[pebs_enable].guest = 0;
} else {
- /* Disable guest PEBS for cross-mapped PEBS counters. */
+ /* Disable guest PEBS thoroughly for cross-mapped PEBS counters. */
arr[pebs_enable].guest &= ~kvm_pmu->host_cross_mapped_mask;
+ arr[global_ctrl].guest &= ~kvm_pmu->host_cross_mapped_mask;
/* Set hw GLOBAL_CTRL bits for PEBS counter when it runs for guest */
arr[global_ctrl].guest |= arr[pebs_enable].guest;
}
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
2022-07-13 12:25 ` [PATCH 1/7] perf/x86/core: Update x86_pmu.pebs_capable for ICELAKE_{X,D} Like Xu
2022-07-13 12:25 ` [PATCH 2/7] perf/x86/core: Completely disable guest PEBS via guest's global_ctrl Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-21 0:45 ` Sean Christopherson
2022-07-13 12:25 ` [PATCH 4/7] KVM: x86/pmu: Not to generate PEBS records for emulated instructions Like Xu
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu
From: Like Xu <likexu@tencent.com>
In the extreme case of host counters multiplexing and contention, the
perf_event requested by the guest's pebs counter is not allocated to any
actual physical counter, in which case hw.idx is bookkept as -1,
resulting in an out-of-bounds access to host_cross_mapped_mask.
Fixes: 854250329c02 ("KVM: x86/pmu: Disable guest PEBS temporarily in two rare situations")
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/kvm/vmx/pmu_intel.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 53ccba896e77..1588627974fa 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -783,20 +783,19 @@ static void intel_pmu_cleanup(struct kvm_vcpu *vcpu)
void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu)
{
struct kvm_pmc *pmc = NULL;
- int bit;
+ int bit, hw_idx;
for_each_set_bit(bit, (unsigned long *)&pmu->global_ctrl,
X86_PMC_IDX_MAX) {
pmc = intel_pmc_idx_to_pmc(pmu, bit);
if (!pmc || !pmc_speculative_in_use(pmc) ||
- !intel_pmc_is_enabled(pmc))
+ !intel_pmc_is_enabled(pmc) || !pmc->perf_event)
continue;
- if (pmc->perf_event && pmc->idx != pmc->perf_event->hw.idx) {
- pmu->host_cross_mapped_mask |=
- BIT_ULL(pmc->perf_event->hw.idx);
- }
+ hw_idx = pmc->perf_event->hw.idx;
+ if (hw_idx != pmc->idx && hw_idx != -1)
+ pmu->host_cross_mapped_mask |= BIT_ULL(hw_idx);
}
}
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask
2022-07-13 12:25 ` [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask Like Xu
@ 2022-07-21 0:45 ` Sean Christopherson
2022-07-21 2:02 ` Like Xu
0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2022-07-21 0:45 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm, Like Xu
On Wed, Jul 13, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
>
> In the extreme case of host counters multiplexing and contention, the
> perf_event requested by the guest's pebs counter is not allocated to any
> actual physical counter, in which case hw.idx is bookkept as -1,
> resulting in an out-of-bounds access to host_cross_mapped_mask.
>
> Fixes: 854250329c02 ("KVM: x86/pmu: Disable guest PEBS temporarily in two rare situations")
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> arch/x86/kvm/vmx/pmu_intel.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 53ccba896e77..1588627974fa 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -783,20 +783,19 @@ static void intel_pmu_cleanup(struct kvm_vcpu *vcpu)
> void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu)
> {
> struct kvm_pmc *pmc = NULL;
> - int bit;
> + int bit, hw_idx;
>
> for_each_set_bit(bit, (unsigned long *)&pmu->global_ctrl,
> X86_PMC_IDX_MAX) {
> pmc = intel_pmc_idx_to_pmc(pmu, bit);
>
> if (!pmc || !pmc_speculative_in_use(pmc) ||
> - !intel_pmc_is_enabled(pmc))
> + !intel_pmc_is_enabled(pmc) || !pmc->perf_event)
> continue;
>
> - if (pmc->perf_event && pmc->idx != pmc->perf_event->hw.idx) {
> - pmu->host_cross_mapped_mask |=
> - BIT_ULL(pmc->perf_event->hw.idx);
> - }
> + hw_idx = pmc->perf_event->hw.idx;
> + if (hw_idx != pmc->idx && hw_idx != -1)
How about "hw_idx > 0" so that KVM is a little less dependent on perf's exact
behavior? A comment here would be nice too.
> + pmu->host_cross_mapped_mask |= BIT_ULL(hw_idx);
> }
> }
>
> --
> 2.37.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask
2022-07-21 0:45 ` Sean Christopherson
@ 2022-07-21 2:02 ` Like Xu
2022-07-21 18:27 ` Sean Christopherson
0 siblings, 1 reply; 17+ messages in thread
From: Like Xu @ 2022-07-21 2:02 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm, Like Xu
On 21/7/2022 8:45 am, Sean Christopherson wrote:
> On Wed, Jul 13, 2022, Like Xu wrote:
>> From: Like Xu <likexu@tencent.com>
>>
>> In the extreme case of host counters multiplexing and contention, the
>> perf_event requested by the guest's pebs counter is not allocated to any
>> actual physical counter, in which case hw.idx is bookkept as -1,
>> resulting in an out-of-bounds access to host_cross_mapped_mask.
>>
>> Fixes: 854250329c02 ("KVM: x86/pmu: Disable guest PEBS temporarily in two rare situations")
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> ---
>> arch/x86/kvm/vmx/pmu_intel.c | 11 +++++------
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
>> index 53ccba896e77..1588627974fa 100644
>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
>> @@ -783,20 +783,19 @@ static void intel_pmu_cleanup(struct kvm_vcpu *vcpu)
>> void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu)
>> {
>> struct kvm_pmc *pmc = NULL;
>> - int bit;
>> + int bit, hw_idx;
>>
>> for_each_set_bit(bit, (unsigned long *)&pmu->global_ctrl,
>> X86_PMC_IDX_MAX) {
>> pmc = intel_pmc_idx_to_pmc(pmu, bit);
>>
>> if (!pmc || !pmc_speculative_in_use(pmc) ||
>> - !intel_pmc_is_enabled(pmc))
>> + !intel_pmc_is_enabled(pmc) || !pmc->perf_event)
>> continue;
>>
>> - if (pmc->perf_event && pmc->idx != pmc->perf_event->hw.idx) {
>> - pmu->host_cross_mapped_mask |=
>> - BIT_ULL(pmc->perf_event->hw.idx);
>> - }
>> + hw_idx = pmc->perf_event->hw.idx;
>> + if (hw_idx != pmc->idx && hw_idx != -1)
>
> How about "hw_idx > 0" so that KVM is a little less dependent on perf's exact
> behavior? A comment here would be nice too.
The "hw->idx = 0" means that it occupies counter 0, so this part will look like
this:
hw_idx = pmc->perf_event->hw.idx;
/* make it a little less dependent on perf's exact behavior */
if (hw_idx != pmc->idx && hw_idx > -1)
pmu->host_cross_mapped_mask |= BIT_ULL(hw_idx);
, what do you think ?
>
>> + pmu->host_cross_mapped_mask |= BIT_ULL(hw_idx);
>> }
>> }
>>
>> --
>> 2.37.0
>>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask
2022-07-21 2:02 ` Like Xu
@ 2022-07-21 18:27 ` Sean Christopherson
0 siblings, 0 replies; 17+ messages in thread
From: Sean Christopherson @ 2022-07-21 18:27 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm, Like Xu
On Thu, Jul 21, 2022, Like Xu wrote:
> On 21/7/2022 8:45 am, Sean Christopherson wrote:
> > On Wed, Jul 13, 2022, Like Xu wrote:
> > > From: Like Xu <likexu@tencent.com>
> > >
> > > In the extreme case of host counters multiplexing and contention, the
> > > perf_event requested by the guest's pebs counter is not allocated to any
> > > actual physical counter, in which case hw.idx is bookkept as -1,
> > > resulting in an out-of-bounds access to host_cross_mapped_mask.
> > >
> > > Fixes: 854250329c02 ("KVM: x86/pmu: Disable guest PEBS temporarily in two rare situations")
> > > Signed-off-by: Like Xu <likexu@tencent.com>
> > > ---
> > > arch/x86/kvm/vmx/pmu_intel.c | 11 +++++------
> > > 1 file changed, 5 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> > > index 53ccba896e77..1588627974fa 100644
> > > --- a/arch/x86/kvm/vmx/pmu_intel.c
> > > +++ b/arch/x86/kvm/vmx/pmu_intel.c
> > > @@ -783,20 +783,19 @@ static void intel_pmu_cleanup(struct kvm_vcpu *vcpu)
> > > void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu)
> > > {
> > > struct kvm_pmc *pmc = NULL;
> > > - int bit;
> > > + int bit, hw_idx;
> > > for_each_set_bit(bit, (unsigned long *)&pmu->global_ctrl,
> > > X86_PMC_IDX_MAX) {
> > > pmc = intel_pmc_idx_to_pmc(pmu, bit);
> > > if (!pmc || !pmc_speculative_in_use(pmc) ||
> > > - !intel_pmc_is_enabled(pmc))
> > > + !intel_pmc_is_enabled(pmc) || !pmc->perf_event)
> > > continue;
> > > - if (pmc->perf_event && pmc->idx != pmc->perf_event->hw.idx) {
> > > - pmu->host_cross_mapped_mask |=
> > > - BIT_ULL(pmc->perf_event->hw.idx);
> > > - }
> > > + hw_idx = pmc->perf_event->hw.idx;
> > > + if (hw_idx != pmc->idx && hw_idx != -1)
> >
> > How about "hw_idx > 0" so that KVM is a little less dependent on perf's exact
> > behavior? A comment here would be nice too.
>
> The "hw->idx = 0" means that it occupies counter 0, so this part will look
> like this:
Doh, typo on my part, meant "hw_idx >= 0". "> -1" is ok, though it's definitely
less idiomatic:
$ git grep ">= 0" | wc -l
5679
$ git grep "> -1" | wc -l
66
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] KVM: x86/pmu: Not to generate PEBS records for emulated instructions
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
` (2 preceding siblings ...)
2022-07-13 12:25 ` [PATCH 3/7] KVM: x86/pmu: Avoid setting BIT_ULL(-1) to pmu->host_cross_mapped_mask Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-21 0:51 ` Sean Christopherson
2022-07-13 12:25 ` [PATCH 5/7] KVM: x86/pmu: Avoid using PEBS perf_events for normal counters Like Xu
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu
From: Like Xu <likexu@tencent.com>
The KVM accumulate an enabeld counter for at least INSTRUCTIONS or
BRANCH_INSTRUCTION hw event from any KVM emulated instructions,
generating emulated overflow interrupt on counter overflow, which
in theory should also happen when the PEBS counter overflows but
it currently lacks this part of the underlying support (e.g. through
software injection of records in the irq context or a lazy approach).
In this case, KVM skips the injection of this BUFFER_OVF PMI (effectively
dropping one PEBS record) and let the overflow counter move on. The loss
of a single sample does not introduce a loss of accuracy, but is easily
noticeable for certain specific instructions.
This issue is expected to be addressed along with the issue
of PEBS cross-mapped counters with a slow-path proposal.
Fixes: 79f3e3b58386 ("KVM: x86/pmu: Reprogram PEBS event to emulate guest PEBS counter")
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/kvm/pmu.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 02f9e4f245bd..08ee0fed63d5 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -106,9 +106,14 @@ static inline void __kvm_perf_overflow(struct kvm_pmc *pmc, bool in_pmi)
return;
if (pmc->perf_event && pmc->perf_event->attr.precise_ip) {
- /* Indicate PEBS overflow PMI to guest. */
- skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
- (unsigned long *)&pmu->global_status);
+ if (!in_pmi) {
+ /* The emulated instructions does not generate PEBS records. */
+ skip_pmi = true;
+ } else {
+ /* Indicate PEBS overflow PMI to guest. */
+ skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
+ (unsigned long *)&pmu->global_status);
+ }
} else {
__set_bit(pmc->idx, (unsigned long *)&pmu->global_status);
}
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/7] KVM: x86/pmu: Not to generate PEBS records for emulated instructions
2022-07-13 12:25 ` [PATCH 4/7] KVM: x86/pmu: Not to generate PEBS records for emulated instructions Like Xu
@ 2022-07-21 0:51 ` Sean Christopherson
2022-07-21 2:22 ` Like Xu
0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2022-07-21 0:51 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm, Like Xu
"Don't" instead of "Not to". Not is an adverb, not a verb itself.
On Wed, Jul 13, 2022, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
>
> The KVM accumulate an enabeld counter for at least INSTRUCTIONS or
Probably just "KVM" instead of "the KVM"?
s/enabeld/enabled
> BRANCH_INSTRUCTION hw event from any KVM emulated instructions,
> generating emulated overflow interrupt on counter overflow, which
> in theory should also happen when the PEBS counter overflows but
> it currently lacks this part of the underlying support (e.g. through
> software injection of records in the irq context or a lazy approach).
>
> In this case, KVM skips the injection of this BUFFER_OVF PMI (effectively
> dropping one PEBS record) and let the overflow counter move on. The loss
> of a single sample does not introduce a loss of accuracy, but is easily
> noticeable for certain specific instructions.
>
> This issue is expected to be addressed along with the issue
> of PEBS cross-mapped counters with a slow-path proposal.
>
> Fixes: 79f3e3b58386 ("KVM: x86/pmu: Reprogram PEBS event to emulate guest PEBS counter")
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> arch/x86/kvm/pmu.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 02f9e4f245bd..08ee0fed63d5 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -106,9 +106,14 @@ static inline void __kvm_perf_overflow(struct kvm_pmc *pmc, bool in_pmi)
> return;
>
> if (pmc->perf_event && pmc->perf_event->attr.precise_ip) {
> - /* Indicate PEBS overflow PMI to guest. */
> - skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
> - (unsigned long *)&pmu->global_status);
> + if (!in_pmi) {
> + /* The emulated instructions does not generate PEBS records. */
This needs a better comment. IIUC, it's not that they don't generate records,
it's that KVM is _choosing_ to not generate records to hack around a different
bug(s). If that's true a TODO or FIXME would also be nice.
> + skip_pmi = true;
> + } else {
> + /* Indicate PEBS overflow PMI to guest. */
> + skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
> + (unsigned long *)&pmu->global_status);
> + }
> } else {
> __set_bit(pmc->idx, (unsigned long *)&pmu->global_status);
> }
> --
> 2.37.0
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/7] KVM: x86/pmu: Not to generate PEBS records for emulated instructions
2022-07-21 0:51 ` Sean Christopherson
@ 2022-07-21 2:22 ` Like Xu
0 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-21 2:22 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm
On 21/7/2022 8:51 am, Sean Christopherson wrote:
> "Don't" instead of "Not to". Not is an adverb, not a verb itself.
>
> On Wed, Jul 13, 2022, Like Xu wrote:
>> From: Like Xu <likexu@tencent.com>
>>
>> The KVM accumulate an enabeld counter for at least INSTRUCTIONS or
>
> Probably just "KVM" instead of "the KVM"?
>
> s/enabeld/enabled
Applied, thanks.
>
>> BRANCH_INSTRUCTION hw event from any KVM emulated instructions,
>> generating emulated overflow interrupt on counter overflow, which
>> in theory should also happen when the PEBS counter overflows but
>> it currently lacks this part of the underlying support (e.g. through
>> software injection of records in the irq context or a lazy approach).
>>
>> In this case, KVM skips the injection of this BUFFER_OVF PMI (effectively
>> dropping one PEBS record) and let the overflow counter move on. The loss
>> of a single sample does not introduce a loss of accuracy, but is easily
>> noticeable for certain specific instructions.
>>
>> This issue is expected to be addressed along with the issue
>> of PEBS cross-mapped counters with a slow-path proposal.
>>
>> Fixes: 79f3e3b58386 ("KVM: x86/pmu: Reprogram PEBS event to emulate guest PEBS counter")
>> Signed-off-by: Like Xu <likexu@tencent.com>
>> ---
>> arch/x86/kvm/pmu.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
>> index 02f9e4f245bd..08ee0fed63d5 100644
>> --- a/arch/x86/kvm/pmu.c
>> +++ b/arch/x86/kvm/pmu.c
>> @@ -106,9 +106,14 @@ static inline void __kvm_perf_overflow(struct kvm_pmc *pmc, bool in_pmi)
>> return;
>>
>> if (pmc->perf_event && pmc->perf_event->attr.precise_ip) {
>> - /* Indicate PEBS overflow PMI to guest. */
>> - skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
>> - (unsigned long *)&pmu->global_status);
>> + if (!in_pmi) {
>> + /* The emulated instructions does not generate PEBS records. */
>
> This needs a better comment. IIUC, it's not that they don't generate records,
> it's that KVM is _choosing_ to not generate records to hack around a different
> bug(s). If that's true a TODO or FIXME would also be nice.
Indeed, to understand more of the context, this part will look like this:
if (!in_pmi) {
/*
* TODO: KVM is currently _choosing_ to not generate records
* for emulated instructions, avoiding BUFFER_OVF PMI when
* there are no records. Strictly speaking, it should be done
* as well in the right context to improve sampling accuracy.
*/
skip_pmi = true;
} else {
/* Indicate PEBS overflow PMI to guest. */
skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
(unsigned long *)&pmu->global_status);
}
, what do you think ?
>
>> + skip_pmi = true;
>> + } else {
>> + /* Indicate PEBS overflow PMI to guest. */
>> + skip_pmi = __test_and_set_bit(GLOBAL_STATUS_BUFFER_OVF_BIT,
>> + (unsigned long *)&pmu->global_status);
>> + }
>> } else {
>> __set_bit(pmc->idx, (unsigned long *)&pmu->global_status);
>> }
>> --
>> 2.37.0
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/7] KVM: x86/pmu: Avoid using PEBS perf_events for normal counters
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
` (3 preceding siblings ...)
2022-07-13 12:25 ` [PATCH 4/7] KVM: x86/pmu: Not to generate PEBS records for emulated instructions Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-13 12:25 ` [PATCH 6/7] KVM: x86/pmu: Defer reprogram_counter() to kvm_pmu_handle_event() Like Xu
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu
From: Like Xu <likexu@tencent.com>
The check logic in the pmc_resume_counter() to determine whether
a perf_event is reusable is partial and flawed, especially when it
comes to a pseudocode sequence (not correct but clearly valid) like:
- enabling a counter and its PEBS bit
- enable global_ctrl
- run workload
- disable only the PEBS bit, leaving the global_ctrl bit enabled
In this corner case, a perf_event created for PEBS can be reused by
a normal counter before it has been released and recreated, and when this
normal counter overflows, it triggers a PEBS interrupt (precise_ip != 0).
To address this issue, the reuse check has been revamped and KVM will
go back to do reprogram_counter() when any bit of guest PEBS_ENABLE
msr has changed, which is similar to what global_ctrl_changed() does.
Fixes: 79f3e3b58386 ("KVM: x86/pmu: Reprogram PEBS event to emulate guest PEBS counter")
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/kvm/pmu.c | 4 ++--
arch/x86/kvm/vmx/pmu_intel.c | 14 +++++++-------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 08ee0fed63d5..2c03fe208093 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -232,8 +232,8 @@ static bool pmc_resume_counter(struct kvm_pmc *pmc)
get_sample_period(pmc, pmc->counter)))
return false;
- if (!test_bit(pmc->idx, (unsigned long *)&pmc_to_pmu(pmc)->pebs_enable) &&
- pmc->perf_event->attr.precise_ip)
+ if (test_bit(pmc->idx, (unsigned long *)&pmc_to_pmu(pmc)->pebs_enable) !=
+ (!!pmc->perf_event->attr.precise_ip))
return false;
/* reuse perf_event to serve as pmc_reprogram_counter() does*/
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 1588627974fa..5f6b9f596f16 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -68,15 +68,11 @@ static struct kvm_pmc *intel_pmc_idx_to_pmc(struct kvm_pmu *pmu, int pmc_idx)
}
}
-/* function is called when global control register has been updated. */
-static void global_ctrl_changed(struct kvm_pmu *pmu, u64 data)
+static void reprogram_counters(struct kvm_pmu *pmu, u64 diff)
{
int bit;
- u64 diff = pmu->global_ctrl ^ data;
struct kvm_pmc *pmc;
- pmu->global_ctrl = data;
-
for_each_set_bit(bit, (unsigned long *)&diff, X86_PMC_IDX_MAX) {
pmc = intel_pmc_idx_to_pmc(pmu, bit);
if (pmc)
@@ -404,7 +400,7 @@ static int intel_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;
+ u64 reserved_bits, diff;
switch (msr) {
case MSR_CORE_PERF_FIXED_CTR_CTRL:
@@ -425,7 +421,9 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
if (pmu->global_ctrl == data)
return 0;
if (kvm_valid_perf_global_ctrl(pmu, data)) {
- global_ctrl_changed(pmu, data);
+ diff = pmu->global_ctrl ^ data;
+ pmu->global_ctrl = data;
+ reprogram_counters(pmu, diff);
return 0;
}
break;
@@ -440,7 +438,9 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
if (pmu->pebs_enable == data)
return 0;
if (!(data & pmu->pebs_enable_mask)) {
+ diff = pmu->pebs_enable ^ data;
pmu->pebs_enable = data;
+ reprogram_counters(pmu, diff);
return 0;
}
break;
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/7] KVM: x86/pmu: Defer reprogram_counter() to kvm_pmu_handle_event()
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
` (4 preceding siblings ...)
2022-07-13 12:25 ` [PATCH 5/7] KVM: x86/pmu: Avoid using PEBS perf_events for normal counters Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-13 12:25 ` [PATCH 7/7] KVM: x86/pmu: Defer counter emulated overflow via pmc->stale_counter Like Xu
2022-07-13 12:25 ` [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS) Like Xu
7 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu
From: Like Xu <likexu@tencent.com>
During a KVM-trap from vm-exit to vm-entry, requests from different
sources will try to create one or more perf_events via reprogram_counter(),
which will allow some predecessor actions to be undone posteriorly,
especially repeated calls to some perf subsystem interfaces. These
repetitive calls can be omitted because only the final state of the
perf_event and the hardware resources it occupies will take effect
for the guest right before the vm-entry.
To realize this optimization, KVM marks the creation requirements via
reprogram_pmi, and then defers the actual execution with the help of
vcpu KVM_REQ_PMU request.
Opportunistically update a comment for pmu->reprogram_pmi.
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/kvm/pmu.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 2c03fe208093..681d3ac8d75c 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -101,7 +101,7 @@ static inline void __kvm_perf_overflow(struct kvm_pmc *pmc, bool in_pmi)
struct kvm_pmu *pmu = pmc_to_pmu(pmc);
bool skip_pmi = false;
- /* Ignore counters that have been reprogrammed already. */
+ /* Ignore counters that have not been reprogrammed. */
if (test_and_set_bit(pmc->idx, pmu->reprogram_pmi))
return;
@@ -289,6 +289,13 @@ static bool check_pmu_event_filter(struct kvm_pmc *pmc)
}
void reprogram_counter(struct kvm_pmc *pmc)
+{
+ __set_bit(pmc->idx, pmc_to_pmu(pmc)->reprogram_pmi);
+ kvm_make_request(KVM_REQ_PMU, pmc->vcpu);
+}
+EXPORT_SYMBOL_GPL(reprogram_counter);
+
+static void __reprogram_counter(struct kvm_pmc *pmc)
{
struct kvm_pmu *pmu = pmc_to_pmu(pmc);
u64 eventsel = pmc->eventsel;
@@ -330,7 +337,6 @@ void reprogram_counter(struct kvm_pmc *pmc)
!(eventsel & ARCH_PERFMON_EVENTSEL_OS),
eventsel & ARCH_PERFMON_EVENTSEL_INT);
}
-EXPORT_SYMBOL_GPL(reprogram_counter);
void kvm_pmu_handle_event(struct kvm_vcpu *vcpu)
{
@@ -340,11 +346,12 @@ void kvm_pmu_handle_event(struct kvm_vcpu *vcpu)
for_each_set_bit(bit, pmu->reprogram_pmi, X86_PMC_IDX_MAX) {
struct kvm_pmc *pmc = static_call(kvm_x86_pmu_pmc_idx_to_pmc)(pmu, bit);
- if (unlikely(!pmc || !pmc->perf_event)) {
+ if (unlikely(!pmc)) {
clear_bit(bit, pmu->reprogram_pmi);
continue;
}
- reprogram_counter(pmc);
+
+ __reprogram_counter(pmc);
}
/*
@@ -522,7 +529,7 @@ static void kvm_pmu_incr_counter(struct kvm_pmc *pmc)
prev_count = pmc->counter;
pmc->counter = (pmc->counter + 1) & pmc_bitmask(pmc);
- reprogram_counter(pmc);
+ __reprogram_counter(pmc);
if (pmc->counter < prev_count)
__kvm_perf_overflow(pmc, false);
}
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 7/7] KVM: x86/pmu: Defer counter emulated overflow via pmc->stale_counter
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
` (5 preceding siblings ...)
2022-07-13 12:25 ` [PATCH 6/7] KVM: x86/pmu: Defer reprogram_counter() to kvm_pmu_handle_event() Like Xu
@ 2022-07-13 12:25 ` Like Xu
2022-07-13 12:25 ` [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS) Like Xu
7 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu, Wanpeng Li
From: Like Xu <likexu@tencent.com>
There are contextual restrictions on the functions that can be called
in the *_exit_handlers_fastpath path, for example calling
pmc_reprogram_counter() brings up a host complaint like:
[*] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:580
[*] in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 2981888, name: CPU 15/KVM
[*] preempt_count: 1, expected: 0
[*] RCU nest depth: 0, expected: 0
[*] INFO: lockdep is turned off.
[*] irq event stamp: 0
[*] hardirqs last enabled at (0): [<0000000000000000>] 0x0
[*] hardirqs last disabled at (0): [<ffffffff8121222a>] copy_process+0x146a/0x62d0
[*] softirqs last enabled at (0): [<ffffffff81212269>] copy_process+0x14a9/0x62d0
[*] softirqs last disabled at (0): [<0000000000000000>] 0x0
[*] Preemption disabled at:
[*] [<ffffffffc2063fc1>] vcpu_enter_guest+0x1001/0x3dc0 [kvm]
[*] CPU: 17 PID: 2981888 Comm: CPU 15/KVM Kdump: 5.19.0-rc1-g239111db364c-dirty #2
[*] Call Trace:
[*] <TASK>
[*] dump_stack_lvl+0x6c/0x9b
[*] __might_resched.cold+0x22e/0x297
[*] __mutex_lock+0xc0/0x23b0
[*] perf_event_ctx_lock_nested+0x18f/0x340
[*] perf_event_pause+0x1a/0x110
[*] reprogram_counter+0x2af/0x1490 [kvm]
[*] kvm_pmu_trigger_event+0x429/0x950 [kvm]
[*] kvm_skip_emulated_instruction+0x48/0x90 [kvm]
[*] handle_fastpath_set_msr_irqoff+0x349/0x3b0 [kvm]
[*] vmx_vcpu_run+0x268e/0x3b80 [kvm_intel]
[*] vcpu_enter_guest+0x1d22/0x3dc0 [kvm]
A new stale_counter field is introduced to keep this part of the semantics
invariant. It records the current counter value and it's used to determine
whether to inject an emulated overflow interrupt in the later
kvm_pmu_handle_event(), given that the internal count value from its
perf_event has not been added to pmc->counter in time, or the guest
will update the value of a running counter directly.
Opportunistically shrink sizeof(struct kvm_pmc) a bit.
Suggested-by: Wanpeng Li <wanpengli@tencent.com>
Fixes: 9cd803d496e7 ("KVM: x86: Update vPMCs when retiring instructions")
Signed-off-by: Like Xu <likexu@tencent.com>
---
arch/x86/include/asm/kvm_host.h | 5 +++--
arch/x86/kvm/pmu.c | 15 ++++++++-------
arch/x86/kvm/svm/pmu.c | 2 +-
arch/x86/kvm/vmx/pmu_intel.c | 4 ++--
4 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index de5a149d0971..4d85d189d082 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -489,7 +489,10 @@ enum pmc_type {
struct kvm_pmc {
enum pmc_type type;
u8 idx;
+ bool is_paused;
+ bool intr;
u64 counter;
+ u64 stale_counter;
u64 eventsel;
struct perf_event *perf_event;
struct kvm_vcpu *vcpu;
@@ -498,8 +501,6 @@ struct kvm_pmc {
* ctrl value for fixed counters.
*/
u64 current_config;
- bool is_paused;
- bool intr;
};
#define KVM_PMC_MAX_FIXED 3
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 681d3ac8d75c..b43561bd8daf 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -352,6 +352,12 @@ void kvm_pmu_handle_event(struct kvm_vcpu *vcpu)
}
__reprogram_counter(pmc);
+
+ if (pmc->stale_counter) {
+ if (pmc->counter < pmc->stale_counter)
+ __kvm_perf_overflow(pmc, false);
+ pmc->stale_counter = 0;
+ }
}
/*
@@ -524,14 +530,9 @@ void kvm_pmu_destroy(struct kvm_vcpu *vcpu)
static void kvm_pmu_incr_counter(struct kvm_pmc *pmc)
{
- u64 prev_count;
-
- prev_count = pmc->counter;
+ pmc->stale_counter = pmc->counter;
pmc->counter = (pmc->counter + 1) & pmc_bitmask(pmc);
-
- __reprogram_counter(pmc);
- if (pmc->counter < prev_count)
- __kvm_perf_overflow(pmc, false);
+ reprogram_counter(pmc);
}
static inline bool eventsel_match_perf_hw_id(struct kvm_pmc *pmc,
diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index f24613a108c5..e9c66dd659a6 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -290,7 +290,7 @@ static void amd_pmu_reset(struct kvm_vcpu *vcpu)
struct kvm_pmc *pmc = &pmu->gp_counters[i];
pmc_stop_counter(pmc);
- pmc->counter = pmc->eventsel = 0;
+ pmc->counter = pmc->stale_counter = pmc->eventsel = 0;
}
}
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 5f6b9f596f16..ca4b55085c87 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -654,14 +654,14 @@ static void intel_pmu_reset(struct kvm_vcpu *vcpu)
pmc = &pmu->gp_counters[i];
pmc_stop_counter(pmc);
- pmc->counter = pmc->eventsel = 0;
+ pmc->counter = pmc->stale_counter = pmc->eventsel = 0;
}
for (i = 0; i < KVM_PMC_MAX_FIXED; i++) {
pmc = &pmu->fixed_counters[i];
pmc_stop_counter(pmc);
- pmc->counter = 0;
+ pmc->counter = pmc->stale_counter = 0;
}
pmu->fixed_ctr_ctrl = pmu->global_ctrl = pmu->global_status = 0;
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS)
2022-07-13 12:24 [PATCH 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
` (6 preceding siblings ...)
2022-07-13 12:25 ` [PATCH 7/7] KVM: x86/pmu: Defer counter emulated overflow via pmc->stale_counter Like Xu
@ 2022-07-13 12:25 ` Like Xu
7 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-13 12:25 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Jim Mattson, linux-kernel, kvm, Like Xu
From: Like Xu <likexu@tencent.com>
This unit-test is intended to test the KVM's support for
the Processor Event Based Sampling (PEBS) which is another
PMU feature on Intel processors (start from Ice Lake Server).
If a bit in PEBS_ENABLE is set to 1, its corresponding counter will
write at least one PEBS records (including partial state of the vcpu
at the time of the current hardware event) to the guest memory on
counter overflow, and trigger an interrupt at a specific DS state.
The format of a PEBS record can be configured by another register.
These tests cover most usage scenarios, for example there are some
specially constructed scenarios (not a typical behaviour of Linux
PEBS driver). It lowers the threshold for others to understand this
feature and opens up more exploration of KVM implementation or
hw feature itself.
Signed-off-by: Like Xu <likexu@tencent.com>
---
lib/x86/msr.h | 1 +
x86/Makefile.x86_64 | 1 +
x86/pmu_pebs.c | 511 ++++++++++++++++++++++++++++++++++++++++++++
x86/unittests.cfg | 7 +
4 files changed, 520 insertions(+)
create mode 100644 x86/pmu_pebs.c
diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index fa1c0c8..252e041 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -52,6 +52,7 @@
#define MSR_IA32_MCG_CTL 0x0000017b
#define MSR_IA32_PEBS_ENABLE 0x000003f1
+#define MSR_PEBS_DATA_CFG 0x000003f2
#define MSR_IA32_DS_AREA 0x00000600
#define MSR_IA32_PERF_CAPABILITIES 0x00000345
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index e19284a..c82c274 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -33,6 +33,7 @@ tests += $(TEST_DIR)/vmware_backdoors.$(exe)
tests += $(TEST_DIR)/rdpru.$(exe)
tests += $(TEST_DIR)/pks.$(exe)
tests += $(TEST_DIR)/pmu_lbr.$(exe)
+tests += $(TEST_DIR)/pmu_pebs.$(exe)
ifeq ($(CONFIG_EFI),y)
tests += $(TEST_DIR)/amd_sev.$(exe)
diff --git a/x86/pmu_pebs.c b/x86/pmu_pebs.c
new file mode 100644
index 0000000..5498bb0
--- /dev/null
+++ b/x86/pmu_pebs.c
@@ -0,0 +1,511 @@
+#include "x86/msr.h"
+#include "x86/processor.h"
+#include "x86/isr.h"
+#include "x86/apic.h"
+#include "x86/apic-defs.h"
+#include "x86/desc.h"
+#include "alloc.h"
+
+#include "vm.h"
+#include "types.h"
+#include "processor.h"
+#include "vmalloc.h"
+#include "alloc_page.h"
+
+#define PC_VECTOR 32
+
+#define X86_FEATURE_PDCM (CPUID(0x1, 0, ECX, 15))
+
+#define PERF_CAP_PEBS_FORMAT 0xf00
+#define PMU_CAP_FW_WRITES (1ULL << 13)
+
+#define INTEL_PMC_IDX_FIXED 32
+
+#define GLOBAL_STATUS_BUFFER_OVF_BIT 62
+#define GLOBAL_STATUS_BUFFER_OVF BIT_ULL(GLOBAL_STATUS_BUFFER_OVF_BIT)
+
+#define EVNTSEL_USR_SHIFT 16
+#define EVNTSEL_OS_SHIFT 17
+#define EVNTSEL_EN_SHIF 22
+
+#define EVNTSEL_EN (1 << EVNTSEL_EN_SHIF)
+#define EVNTSEL_USR (1 << EVNTSEL_USR_SHIFT)
+#define EVNTSEL_OS (1 << EVNTSEL_OS_SHIFT)
+
+#define PEBS_DATACFG_MEMINFO BIT_ULL(0)
+#define PEBS_DATACFG_GP BIT_ULL(1)
+#define PEBS_DATACFG_XMMS BIT_ULL(2)
+#define PEBS_DATACFG_LBRS BIT_ULL(3)
+
+#define ICL_EVENTSEL_ADAPTIVE (1ULL << 34)
+#define PEBS_DATACFG_LBR_SHIFT 24
+#define MAX_NUM_LBR_ENTRY 32
+
+union perf_capabilities {
+ struct {
+ u64 lbr_format:6;
+ u64 pebs_trap:1;
+ u64 pebs_arch_reg:1;
+ u64 pebs_format:4;
+ u64 smm_freeze:1;
+ /*
+ * PMU supports separate counter range for writing
+ * values > 32bit.
+ */
+ u64 full_width_write:1;
+ u64 pebs_baseline:1;
+ u64 perf_metrics:1;
+ u64 pebs_output_pt_available:1;
+ u64 anythread_deprecated:1;
+ };
+ u64 capabilities;
+};
+
+union cpuid10_eax {
+ struct {
+ unsigned int version_id:8;
+ unsigned int num_counters:8;
+ unsigned int bit_width:8;
+ unsigned int mask_length:8;
+ } split;
+ unsigned int full;
+} pmu_eax;
+
+union cpuid10_edx {
+ struct {
+ unsigned int num_counters_fixed:5;
+ unsigned int bit_width_fixed:8;
+ unsigned int reserved:19;
+ } split;
+ unsigned int full;
+} pmu_edx;
+
+static u64 gp_counter_base = MSR_IA32_PERFCTR0;
+static union perf_capabilities perf;
+static unsigned int max_nr_gp_events;
+static unsigned long *ds_bufer;
+static unsigned long *pebs_buffer;
+static u64 ctr_start_val;
+
+struct debug_store {
+ u64 bts_buffer_base;
+ u64 bts_index;
+ u64 bts_absolute_maximum;
+ u64 bts_interrupt_threshold;
+ u64 pebs_buffer_base;
+ u64 pebs_index;
+ u64 pebs_absolute_maximum;
+ u64 pebs_interrupt_threshold;
+ u64 pebs_event_reset[64];
+};
+
+struct pebs_basic {
+ u64 format_size;
+ u64 ip;
+ u64 applicable_counters;
+ u64 tsc;
+};
+
+struct pebs_meminfo {
+ u64 address;
+ u64 aux;
+ u64 latency;
+ u64 tsx_tuning;
+};
+
+struct pebs_gprs {
+ u64 flags, ip, ax, cx, dx, bx, sp, bp, si, di;
+ u64 r8, r9, r10, r11, r12, r13, r14, r15;
+};
+
+struct pebs_xmm {
+ u64 xmm[16*2]; /* two entries for each register */
+};
+
+struct lbr_entry {
+ u64 from;
+ u64 to;
+ u64 info;
+};
+
+enum pmc_type {
+ GP = 0,
+ FIXED,
+};
+
+static uint32_t intel_arch_events[] = {
+ 0x00c4, /* PERF_COUNT_HW_BRANCH_INSTRUCTIONS */
+ 0x00c5, /* PERF_COUNT_HW_BRANCH_MISSES */
+ 0x0300, /* PERF_COUNT_HW_REF_CPU_CYCLES */
+ 0x003c, /* PERF_COUNT_HW_CPU_CYCLES */
+ 0x00c0, /* PERF_COUNT_HW_INSTRUCTIONS */
+ 0x013c, /* PERF_COUNT_HW_BUS_CYCLES */
+ 0x4f2e, /* PERF_COUNT_HW_CACHE_REFERENCES */
+ 0x412e, /* PERF_COUNT_HW_CACHE_MISSES */
+};
+
+static u64 pebs_data_cfgs[] = {
+ PEBS_DATACFG_MEMINFO,
+ PEBS_DATACFG_GP,
+ PEBS_DATACFG_XMMS,
+ PEBS_DATACFG_LBRS | ((MAX_NUM_LBR_ENTRY -1) << PEBS_DATACFG_LBR_SHIFT),
+};
+
+/* Iterating each counter value is a waste of time, pick a few typical values. */
+static u64 counter_start_values[] = {
+ /* if PEBS counter doesn't overflow at all */
+ 0,
+ 0xfffffffffff0,
+ /* normal counter overflow to have PEBS records */
+ 0xfffffffffffe,
+ /* test whether emulated instructions should trigger PEBS */
+ 0xffffffffffff,
+};
+
+static unsigned int get_adaptive_pebs_record_size(u64 pebs_data_cfg)
+{
+ unsigned int sz = sizeof(struct pebs_basic);
+
+ if (!perf.pebs_baseline)
+ return sz;
+
+ if (pebs_data_cfg & PEBS_DATACFG_MEMINFO)
+ sz += sizeof(struct pebs_meminfo);
+ if (pebs_data_cfg & PEBS_DATACFG_GP)
+ sz += sizeof(struct pebs_gprs);
+ if (pebs_data_cfg & PEBS_DATACFG_XMMS)
+ sz += sizeof(struct pebs_xmm);
+ if (pebs_data_cfg & PEBS_DATACFG_LBRS)
+ sz += MAX_NUM_LBR_ENTRY * sizeof(struct lbr_entry);
+
+ return sz;
+}
+
+static void cnt_overflow(isr_regs_t *regs)
+{
+ apic_write(APIC_EOI, 0);
+}
+
+static inline void workload(void)
+{
+ asm volatile(
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "label2:\n"
+ :
+ :
+ : "eax", "ebx", "ecx", "edx");
+}
+
+static inline void workload2(void)
+{
+ asm volatile(
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "label3:\n"
+ :
+ :
+ : "eax", "ebx", "ecx", "edx");
+}
+
+static void alloc_buffers(void)
+{
+ ds_bufer = alloc_page();
+ force_4k_page(ds_bufer);
+ memset(ds_bufer, 0x0, PAGE_SIZE);
+
+ pebs_buffer = alloc_page();
+ force_4k_page(pebs_buffer);
+ memset(pebs_buffer, 0x0, PAGE_SIZE);
+}
+
+static void free_buffers(void)
+{
+ if (ds_bufer)
+ free_page(ds_bufer);
+
+ if (pebs_buffer)
+ free_page(pebs_buffer);
+}
+
+static void pebs_enable(u64 bitmask, u64 pebs_data_cfg)
+{
+ static struct debug_store *ds;
+ u64 baseline_extra_ctrl, fixed_ctr_ctrl = 0;
+ unsigned int idx;
+
+ if (perf.pebs_baseline)
+ wrmsr(MSR_PEBS_DATA_CFG, pebs_data_cfg);
+
+ ds = (struct debug_store *)ds_bufer;
+ ds->pebs_index = ds->pebs_buffer_base = (unsigned long)pebs_buffer;
+ ds->pebs_absolute_maximum = (unsigned long)pebs_buffer + PAGE_SIZE;
+ ds->pebs_interrupt_threshold = ds->pebs_buffer_base +
+ get_adaptive_pebs_record_size(pebs_data_cfg);
+
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++) {
+ if (!(BIT_ULL(INTEL_PMC_IDX_FIXED + idx) & bitmask))
+ continue;
+ baseline_extra_ctrl = perf.pebs_baseline ?
+ (1ULL << (INTEL_PMC_IDX_FIXED + idx * 4)) : 0;
+ wrmsr(MSR_CORE_PERF_FIXED_CTR0 + idx, ctr_start_val);
+ fixed_ctr_ctrl |= (0xbULL << (idx * 4) | baseline_extra_ctrl);
+ }
+ if (fixed_ctr_ctrl)
+ wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, fixed_ctr_ctrl);
+
+ for (idx = 0; idx < max_nr_gp_events; idx++) {
+ if (!(BIT_ULL(idx) & bitmask))
+ continue;
+ baseline_extra_ctrl = perf.pebs_baseline ?
+ ICL_EVENTSEL_ADAPTIVE : 0;
+ wrmsr(MSR_P6_EVNTSEL0 + idx,
+ EVNTSEL_EN | EVNTSEL_OS | EVNTSEL_USR |
+ intel_arch_events[idx] | baseline_extra_ctrl);
+ wrmsr(gp_counter_base + idx, ctr_start_val);
+ }
+
+ wrmsr(MSR_IA32_DS_AREA, (unsigned long)ds_bufer);
+ wrmsr(MSR_IA32_PEBS_ENABLE, bitmask);
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, bitmask);
+}
+
+static void pmu_env_cleanup(void)
+{
+ unsigned int idx;
+
+ memset(ds_bufer, 0x0, PAGE_SIZE);
+ memset(pebs_buffer, 0x0, PAGE_SIZE);
+ wrmsr(MSR_IA32_PEBS_ENABLE, 0);
+ wrmsr(MSR_IA32_DS_AREA, 0);
+ if (perf.pebs_baseline)
+ wrmsr(MSR_PEBS_DATA_CFG, 0);
+
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+
+ wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++) {
+ wrmsr(MSR_CORE_PERF_FIXED_CTR0 + idx, 0);
+ }
+
+ for (idx = 0; idx < pmu_eax.split.num_counters; idx++) {
+ wrmsr(MSR_P6_EVNTSEL0 + idx, 0);
+ wrmsr(MSR_IA32_PERFCTR0 + idx, 0);
+ }
+
+ wrmsr(MSR_CORE_PERF_GLOBAL_OVF_CTRL, rdmsr(MSR_CORE_PERF_GLOBAL_STATUS));
+}
+
+static inline void pebs_disable_1(void)
+{
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+}
+
+static inline void pebs_disable_2(void)
+{
+ wrmsr(MSR_IA32_PEBS_ENABLE, 0);
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+}
+
+static void pebs_disable(unsigned int idx)
+{
+ if (idx % 2) {
+ pebs_disable_1();
+ } else {
+ pebs_disable_2();
+ }
+}
+
+static void check_pebs_records(u64 bitmask, u64 pebs_data_cfg)
+{
+ struct pebs_basic *pebs_rec = (struct pebs_basic *)pebs_buffer;
+ struct debug_store *ds = (struct debug_store *)ds_bufer;
+ unsigned int pebs_record_size = get_adaptive_pebs_record_size(pebs_data_cfg);
+ unsigned int count = 0;
+ bool expected, pebs_idx_match, pebs_size_match, data_cfg_match;
+ void *vernier;
+
+ expected = (ds->pebs_index == ds->pebs_buffer_base) && !pebs_rec->format_size;
+ if (!(rdmsr(MSR_CORE_PERF_GLOBAL_STATUS) & GLOBAL_STATUS_BUFFER_OVF)) {
+ report(expected, "No OVF irq, none PEBS records.");
+ return;
+ }
+
+ if (expected) {
+ report(!expected, "A OVF irq, but none PEBS records.");
+ return;
+ }
+
+ expected = ds->pebs_index >= ds->pebs_interrupt_threshold;
+ vernier = (void *)pebs_buffer;
+ do {
+ pebs_rec = (struct pebs_basic *)vernier;
+ pebs_record_size = pebs_rec->format_size >> 48;
+ pebs_idx_match =
+ pebs_rec->applicable_counters & bitmask;
+ pebs_size_match =
+ pebs_record_size == get_adaptive_pebs_record_size(pebs_data_cfg);
+ data_cfg_match =
+ (pebs_rec->format_size & 0xffffffffffff) == pebs_data_cfg;
+ expected = pebs_idx_match && pebs_size_match && data_cfg_match;
+ report(expected,
+ "PEBS record (written seq %d) is verified (inclduing size, counters and cfg).", count);
+ vernier = vernier + pebs_record_size;
+ count++;
+ } while (expected && (void *)vernier < (void *)ds->pebs_index);
+
+ if (!expected) {
+ if (!pebs_idx_match)
+ printf("FAIL: The applicable_counters (0x%lx) doesn't match with pmc_bitmask (0x%lx).\n",
+ pebs_rec->applicable_counters, bitmask);
+ if (!pebs_size_match)
+ printf("FAIL: The pebs_record_size (%d) doesn't match with MSR_PEBS_DATA_CFG (%d).\n",
+ pebs_record_size, get_adaptive_pebs_record_size(pebs_data_cfg));
+ if (!data_cfg_match)
+ printf("FAIL: The pebs_data_cfg (0x%lx) doesn't match with MSR_PEBS_DATA_CFG (0x%lx).\n",
+ pebs_rec->format_size & 0xffffffffffff, pebs_data_cfg);
+ }
+}
+
+static void check_one_counter(enum pmc_type type,
+ unsigned int idx, u64 pebs_data_cfg)
+{
+ report_prefix_pushf("%s counter %d (0x%lx)",
+ type == FIXED ? "Extended Fixed" : "GP", idx, ctr_start_val);
+ pmu_env_cleanup();
+ pebs_enable(BIT_ULL(type == FIXED ? INTEL_PMC_IDX_FIXED + idx : idx), pebs_data_cfg);
+ workload();
+ pebs_disable(idx);
+ check_pebs_records(BIT_ULL(type == FIXED ? INTEL_PMC_IDX_FIXED + idx : idx), pebs_data_cfg);
+ report_prefix_pop();
+}
+
+static void check_multiple_counters(u64 bitmask, u64 pebs_data_cfg)
+{
+ pmu_env_cleanup();
+ pebs_enable(bitmask, pebs_data_cfg);
+ workload2();
+ pebs_disable(0);
+ check_pebs_records(bitmask, pebs_data_cfg);
+}
+
+static void check_pebs_counters(u64 pebs_data_cfg)
+{
+ unsigned int idx;
+ u64 bitmask = 0;
+
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++)
+ check_one_counter(FIXED, idx, pebs_data_cfg);
+
+ for (idx = 0; idx < max_nr_gp_events; idx++)
+ check_one_counter(GP, idx, pebs_data_cfg);
+
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++)
+ bitmask |= BIT_ULL(INTEL_PMC_IDX_FIXED + idx);
+ for (idx = 0; idx < max_nr_gp_events; idx += 2)
+ bitmask |= BIT_ULL(idx);
+ report_prefix_pushf("Multiple (0x%lx)", bitmask);
+ check_multiple_counters(bitmask, pebs_data_cfg);
+ report_prefix_pop();
+}
+
+int main(int ac, char **av)
+{
+ struct cpuid id;
+ unsigned int i, j;
+
+ setup_vm();
+ id = cpuid(10);
+
+ pmu_eax.full = id.a;
+ pmu_edx.full = id.d;
+ max_nr_gp_events = MIN(pmu_eax.split.num_counters, ARRAY_SIZE(intel_arch_events));
+
+ printf("PMU version: %d\n", pmu_eax.split.version_id);
+ if (this_cpu_has(X86_FEATURE_PDCM))
+ perf.capabilities = rdmsr(MSR_IA32_PERF_CAPABILITIES);
+
+ if (perf.capabilities & PMU_CAP_FW_WRITES)
+ gp_counter_base = MSR_IA32_PMC0;
+
+ if (!is_intel() || (pmu_eax.split.version_id < 2) ||
+ !(perf.capabilities & PERF_CAP_PEBS_FORMAT) ||
+ (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL)) {
+ report_skip("This platform doesn't support guest PEBS.");
+ return 0;
+ }
+
+ printf("PEBS format: %d\n", perf.pebs_format);
+ printf("PEBS GP counters: %d\n", pmu_eax.split.num_counters);
+ printf("PEBS Fixed counters: %d\n", pmu_edx.split.num_counters_fixed);
+ printf("PEBS baseline (Adaptive PEBS): %d\n", perf.pebs_baseline);
+
+ printf("Known reasons for none PEBS records:\n");
+ printf("1. The selected event does not support PEBS;\n");
+ printf("2. From a core pmu perspective, the vCPU and pCPU models are not same;\n");
+ printf("3. Guest counter has not yet overflowed or been cross-mapped by the host;\n");
+
+ handle_irq(PC_VECTOR, cnt_overflow);
+ alloc_buffers();
+
+ for (i = 0; i < ARRAY_SIZE(counter_start_values); i++) {
+ ctr_start_val = counter_start_values[i];
+ check_pebs_counters(0);
+ if (!perf.pebs_baseline)
+ continue;
+
+ for (j = 0; j < ARRAY_SIZE(pebs_data_cfgs); j++) {
+ report_prefix_pushf("Adaptive (0x%lx)", pebs_data_cfgs[j]);
+ check_pebs_counters(pebs_data_cfgs[j]);
+ report_prefix_pop();
+ }
+ }
+
+ free_buffers();
+
+ return report_summary();
+}
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index d6dc19f..5731454 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -198,6 +198,13 @@ check = /sys/module/kvm/parameters/ignore_msrs=N
check = /proc/sys/kernel/nmi_watchdog=0
accel = kvm
+[pmu_pebs]
+arch = x86_64
+file = pmu_pebs.flat
+extra_params = -cpu host,migratable=no
+check = /proc/sys/kernel/nmi_watchdog=0
+accel = kvm
+
[pmu_emulation]
file = pmu.flat
arch = x86_64
--
2.37.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS)
2022-07-21 10:35 [PATCH v2 0/7] KVM: x86/pmu: Fix some corner cases including Intel PEBS Like Xu
@ 2022-07-21 10:35 ` Like Xu
2022-07-27 22:42 ` Sean Christopherson
0 siblings, 1 reply; 17+ messages in thread
From: Like Xu @ 2022-07-21 10:35 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson; +Cc: Jim Mattson, linux-kernel, kvm
This unit-test is intended to test the KVM's support for
the Processor Event Based Sampling (PEBS) which is another
PMU feature on Intel processors (start from Ice Lake Server).
If a bit in PEBS_ENABLE is set to 1, its corresponding counter will
write at least one PEBS records (including partial state of the vcpu
at the time of the current hardware event) to the guest memory on
counter overflow, and trigger an interrupt at a specific DS state.
The format of a PEBS record can be configured by another register.
These tests cover most usage scenarios, for example there are some
specially constructed scenarios (not a typical behaviour of Linux
PEBS driver). It lowers the threshold for others to understand this
feature and opens up more exploration of KVM implementation or
hw feature itself.
Signed-off-by: Like Xu <likexu@tencent.com>
---
lib/x86/msr.h | 1 +
x86/Makefile.x86_64 | 1 +
x86/pmu_pebs.c | 511 ++++++++++++++++++++++++++++++++++++++++++++
x86/unittests.cfg | 7 +
4 files changed, 520 insertions(+)
create mode 100644 x86/pmu_pebs.c
diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index fa1c0c8..252e041 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -52,6 +52,7 @@
#define MSR_IA32_MCG_CTL 0x0000017b
#define MSR_IA32_PEBS_ENABLE 0x000003f1
+#define MSR_PEBS_DATA_CFG 0x000003f2
#define MSR_IA32_DS_AREA 0x00000600
#define MSR_IA32_PERF_CAPABILITIES 0x00000345
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index e19284a..c82c274 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -33,6 +33,7 @@ tests += $(TEST_DIR)/vmware_backdoors.$(exe)
tests += $(TEST_DIR)/rdpru.$(exe)
tests += $(TEST_DIR)/pks.$(exe)
tests += $(TEST_DIR)/pmu_lbr.$(exe)
+tests += $(TEST_DIR)/pmu_pebs.$(exe)
ifeq ($(CONFIG_EFI),y)
tests += $(TEST_DIR)/amd_sev.$(exe)
diff --git a/x86/pmu_pebs.c b/x86/pmu_pebs.c
new file mode 100644
index 0000000..5498bb0
--- /dev/null
+++ b/x86/pmu_pebs.c
@@ -0,0 +1,511 @@
+#include "x86/msr.h"
+#include "x86/processor.h"
+#include "x86/isr.h"
+#include "x86/apic.h"
+#include "x86/apic-defs.h"
+#include "x86/desc.h"
+#include "alloc.h"
+
+#include "vm.h"
+#include "types.h"
+#include "processor.h"
+#include "vmalloc.h"
+#include "alloc_page.h"
+
+#define PC_VECTOR 32
+
+#define X86_FEATURE_PDCM (CPUID(0x1, 0, ECX, 15))
+
+#define PERF_CAP_PEBS_FORMAT 0xf00
+#define PMU_CAP_FW_WRITES (1ULL << 13)
+
+#define INTEL_PMC_IDX_FIXED 32
+
+#define GLOBAL_STATUS_BUFFER_OVF_BIT 62
+#define GLOBAL_STATUS_BUFFER_OVF BIT_ULL(GLOBAL_STATUS_BUFFER_OVF_BIT)
+
+#define EVNTSEL_USR_SHIFT 16
+#define EVNTSEL_OS_SHIFT 17
+#define EVNTSEL_EN_SHIF 22
+
+#define EVNTSEL_EN (1 << EVNTSEL_EN_SHIF)
+#define EVNTSEL_USR (1 << EVNTSEL_USR_SHIFT)
+#define EVNTSEL_OS (1 << EVNTSEL_OS_SHIFT)
+
+#define PEBS_DATACFG_MEMINFO BIT_ULL(0)
+#define PEBS_DATACFG_GP BIT_ULL(1)
+#define PEBS_DATACFG_XMMS BIT_ULL(2)
+#define PEBS_DATACFG_LBRS BIT_ULL(3)
+
+#define ICL_EVENTSEL_ADAPTIVE (1ULL << 34)
+#define PEBS_DATACFG_LBR_SHIFT 24
+#define MAX_NUM_LBR_ENTRY 32
+
+union perf_capabilities {
+ struct {
+ u64 lbr_format:6;
+ u64 pebs_trap:1;
+ u64 pebs_arch_reg:1;
+ u64 pebs_format:4;
+ u64 smm_freeze:1;
+ /*
+ * PMU supports separate counter range for writing
+ * values > 32bit.
+ */
+ u64 full_width_write:1;
+ u64 pebs_baseline:1;
+ u64 perf_metrics:1;
+ u64 pebs_output_pt_available:1;
+ u64 anythread_deprecated:1;
+ };
+ u64 capabilities;
+};
+
+union cpuid10_eax {
+ struct {
+ unsigned int version_id:8;
+ unsigned int num_counters:8;
+ unsigned int bit_width:8;
+ unsigned int mask_length:8;
+ } split;
+ unsigned int full;
+} pmu_eax;
+
+union cpuid10_edx {
+ struct {
+ unsigned int num_counters_fixed:5;
+ unsigned int bit_width_fixed:8;
+ unsigned int reserved:19;
+ } split;
+ unsigned int full;
+} pmu_edx;
+
+static u64 gp_counter_base = MSR_IA32_PERFCTR0;
+static union perf_capabilities perf;
+static unsigned int max_nr_gp_events;
+static unsigned long *ds_bufer;
+static unsigned long *pebs_buffer;
+static u64 ctr_start_val;
+
+struct debug_store {
+ u64 bts_buffer_base;
+ u64 bts_index;
+ u64 bts_absolute_maximum;
+ u64 bts_interrupt_threshold;
+ u64 pebs_buffer_base;
+ u64 pebs_index;
+ u64 pebs_absolute_maximum;
+ u64 pebs_interrupt_threshold;
+ u64 pebs_event_reset[64];
+};
+
+struct pebs_basic {
+ u64 format_size;
+ u64 ip;
+ u64 applicable_counters;
+ u64 tsc;
+};
+
+struct pebs_meminfo {
+ u64 address;
+ u64 aux;
+ u64 latency;
+ u64 tsx_tuning;
+};
+
+struct pebs_gprs {
+ u64 flags, ip, ax, cx, dx, bx, sp, bp, si, di;
+ u64 r8, r9, r10, r11, r12, r13, r14, r15;
+};
+
+struct pebs_xmm {
+ u64 xmm[16*2]; /* two entries for each register */
+};
+
+struct lbr_entry {
+ u64 from;
+ u64 to;
+ u64 info;
+};
+
+enum pmc_type {
+ GP = 0,
+ FIXED,
+};
+
+static uint32_t intel_arch_events[] = {
+ 0x00c4, /* PERF_COUNT_HW_BRANCH_INSTRUCTIONS */
+ 0x00c5, /* PERF_COUNT_HW_BRANCH_MISSES */
+ 0x0300, /* PERF_COUNT_HW_REF_CPU_CYCLES */
+ 0x003c, /* PERF_COUNT_HW_CPU_CYCLES */
+ 0x00c0, /* PERF_COUNT_HW_INSTRUCTIONS */
+ 0x013c, /* PERF_COUNT_HW_BUS_CYCLES */
+ 0x4f2e, /* PERF_COUNT_HW_CACHE_REFERENCES */
+ 0x412e, /* PERF_COUNT_HW_CACHE_MISSES */
+};
+
+static u64 pebs_data_cfgs[] = {
+ PEBS_DATACFG_MEMINFO,
+ PEBS_DATACFG_GP,
+ PEBS_DATACFG_XMMS,
+ PEBS_DATACFG_LBRS | ((MAX_NUM_LBR_ENTRY -1) << PEBS_DATACFG_LBR_SHIFT),
+};
+
+/* Iterating each counter value is a waste of time, pick a few typical values. */
+static u64 counter_start_values[] = {
+ /* if PEBS counter doesn't overflow at all */
+ 0,
+ 0xfffffffffff0,
+ /* normal counter overflow to have PEBS records */
+ 0xfffffffffffe,
+ /* test whether emulated instructions should trigger PEBS */
+ 0xffffffffffff,
+};
+
+static unsigned int get_adaptive_pebs_record_size(u64 pebs_data_cfg)
+{
+ unsigned int sz = sizeof(struct pebs_basic);
+
+ if (!perf.pebs_baseline)
+ return sz;
+
+ if (pebs_data_cfg & PEBS_DATACFG_MEMINFO)
+ sz += sizeof(struct pebs_meminfo);
+ if (pebs_data_cfg & PEBS_DATACFG_GP)
+ sz += sizeof(struct pebs_gprs);
+ if (pebs_data_cfg & PEBS_DATACFG_XMMS)
+ sz += sizeof(struct pebs_xmm);
+ if (pebs_data_cfg & PEBS_DATACFG_LBRS)
+ sz += MAX_NUM_LBR_ENTRY * sizeof(struct lbr_entry);
+
+ return sz;
+}
+
+static void cnt_overflow(isr_regs_t *regs)
+{
+ apic_write(APIC_EOI, 0);
+}
+
+static inline void workload(void)
+{
+ asm volatile(
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "jne label2\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "label2:\n"
+ :
+ :
+ : "eax", "ebx", "ecx", "edx");
+}
+
+static inline void workload2(void)
+{
+ asm volatile(
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "mov $0x0, %%eax\n"
+ "cmp $0x0, %%eax\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "jne label3\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "mov $0xa, %%eax\n"
+ "cpuid\n"
+ "label3:\n"
+ :
+ :
+ : "eax", "ebx", "ecx", "edx");
+}
+
+static void alloc_buffers(void)
+{
+ ds_bufer = alloc_page();
+ force_4k_page(ds_bufer);
+ memset(ds_bufer, 0x0, PAGE_SIZE);
+
+ pebs_buffer = alloc_page();
+ force_4k_page(pebs_buffer);
+ memset(pebs_buffer, 0x0, PAGE_SIZE);
+}
+
+static void free_buffers(void)
+{
+ if (ds_bufer)
+ free_page(ds_bufer);
+
+ if (pebs_buffer)
+ free_page(pebs_buffer);
+}
+
+static void pebs_enable(u64 bitmask, u64 pebs_data_cfg)
+{
+ static struct debug_store *ds;
+ u64 baseline_extra_ctrl, fixed_ctr_ctrl = 0;
+ unsigned int idx;
+
+ if (perf.pebs_baseline)
+ wrmsr(MSR_PEBS_DATA_CFG, pebs_data_cfg);
+
+ ds = (struct debug_store *)ds_bufer;
+ ds->pebs_index = ds->pebs_buffer_base = (unsigned long)pebs_buffer;
+ ds->pebs_absolute_maximum = (unsigned long)pebs_buffer + PAGE_SIZE;
+ ds->pebs_interrupt_threshold = ds->pebs_buffer_base +
+ get_adaptive_pebs_record_size(pebs_data_cfg);
+
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++) {
+ if (!(BIT_ULL(INTEL_PMC_IDX_FIXED + idx) & bitmask))
+ continue;
+ baseline_extra_ctrl = perf.pebs_baseline ?
+ (1ULL << (INTEL_PMC_IDX_FIXED + idx * 4)) : 0;
+ wrmsr(MSR_CORE_PERF_FIXED_CTR0 + idx, ctr_start_val);
+ fixed_ctr_ctrl |= (0xbULL << (idx * 4) | baseline_extra_ctrl);
+ }
+ if (fixed_ctr_ctrl)
+ wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, fixed_ctr_ctrl);
+
+ for (idx = 0; idx < max_nr_gp_events; idx++) {
+ if (!(BIT_ULL(idx) & bitmask))
+ continue;
+ baseline_extra_ctrl = perf.pebs_baseline ?
+ ICL_EVENTSEL_ADAPTIVE : 0;
+ wrmsr(MSR_P6_EVNTSEL0 + idx,
+ EVNTSEL_EN | EVNTSEL_OS | EVNTSEL_USR |
+ intel_arch_events[idx] | baseline_extra_ctrl);
+ wrmsr(gp_counter_base + idx, ctr_start_val);
+ }
+
+ wrmsr(MSR_IA32_DS_AREA, (unsigned long)ds_bufer);
+ wrmsr(MSR_IA32_PEBS_ENABLE, bitmask);
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, bitmask);
+}
+
+static void pmu_env_cleanup(void)
+{
+ unsigned int idx;
+
+ memset(ds_bufer, 0x0, PAGE_SIZE);
+ memset(pebs_buffer, 0x0, PAGE_SIZE);
+ wrmsr(MSR_IA32_PEBS_ENABLE, 0);
+ wrmsr(MSR_IA32_DS_AREA, 0);
+ if (perf.pebs_baseline)
+ wrmsr(MSR_PEBS_DATA_CFG, 0);
+
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+
+ wrmsr(MSR_CORE_PERF_FIXED_CTR_CTRL, 0);
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++) {
+ wrmsr(MSR_CORE_PERF_FIXED_CTR0 + idx, 0);
+ }
+
+ for (idx = 0; idx < pmu_eax.split.num_counters; idx++) {
+ wrmsr(MSR_P6_EVNTSEL0 + idx, 0);
+ wrmsr(MSR_IA32_PERFCTR0 + idx, 0);
+ }
+
+ wrmsr(MSR_CORE_PERF_GLOBAL_OVF_CTRL, rdmsr(MSR_CORE_PERF_GLOBAL_STATUS));
+}
+
+static inline void pebs_disable_1(void)
+{
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+}
+
+static inline void pebs_disable_2(void)
+{
+ wrmsr(MSR_IA32_PEBS_ENABLE, 0);
+ wrmsr(MSR_CORE_PERF_GLOBAL_CTRL, 0);
+}
+
+static void pebs_disable(unsigned int idx)
+{
+ if (idx % 2) {
+ pebs_disable_1();
+ } else {
+ pebs_disable_2();
+ }
+}
+
+static void check_pebs_records(u64 bitmask, u64 pebs_data_cfg)
+{
+ struct pebs_basic *pebs_rec = (struct pebs_basic *)pebs_buffer;
+ struct debug_store *ds = (struct debug_store *)ds_bufer;
+ unsigned int pebs_record_size = get_adaptive_pebs_record_size(pebs_data_cfg);
+ unsigned int count = 0;
+ bool expected, pebs_idx_match, pebs_size_match, data_cfg_match;
+ void *vernier;
+
+ expected = (ds->pebs_index == ds->pebs_buffer_base) && !pebs_rec->format_size;
+ if (!(rdmsr(MSR_CORE_PERF_GLOBAL_STATUS) & GLOBAL_STATUS_BUFFER_OVF)) {
+ report(expected, "No OVF irq, none PEBS records.");
+ return;
+ }
+
+ if (expected) {
+ report(!expected, "A OVF irq, but none PEBS records.");
+ return;
+ }
+
+ expected = ds->pebs_index >= ds->pebs_interrupt_threshold;
+ vernier = (void *)pebs_buffer;
+ do {
+ pebs_rec = (struct pebs_basic *)vernier;
+ pebs_record_size = pebs_rec->format_size >> 48;
+ pebs_idx_match =
+ pebs_rec->applicable_counters & bitmask;
+ pebs_size_match =
+ pebs_record_size == get_adaptive_pebs_record_size(pebs_data_cfg);
+ data_cfg_match =
+ (pebs_rec->format_size & 0xffffffffffff) == pebs_data_cfg;
+ expected = pebs_idx_match && pebs_size_match && data_cfg_match;
+ report(expected,
+ "PEBS record (written seq %d) is verified (inclduing size, counters and cfg).", count);
+ vernier = vernier + pebs_record_size;
+ count++;
+ } while (expected && (void *)vernier < (void *)ds->pebs_index);
+
+ if (!expected) {
+ if (!pebs_idx_match)
+ printf("FAIL: The applicable_counters (0x%lx) doesn't match with pmc_bitmask (0x%lx).\n",
+ pebs_rec->applicable_counters, bitmask);
+ if (!pebs_size_match)
+ printf("FAIL: The pebs_record_size (%d) doesn't match with MSR_PEBS_DATA_CFG (%d).\n",
+ pebs_record_size, get_adaptive_pebs_record_size(pebs_data_cfg));
+ if (!data_cfg_match)
+ printf("FAIL: The pebs_data_cfg (0x%lx) doesn't match with MSR_PEBS_DATA_CFG (0x%lx).\n",
+ pebs_rec->format_size & 0xffffffffffff, pebs_data_cfg);
+ }
+}
+
+static void check_one_counter(enum pmc_type type,
+ unsigned int idx, u64 pebs_data_cfg)
+{
+ report_prefix_pushf("%s counter %d (0x%lx)",
+ type == FIXED ? "Extended Fixed" : "GP", idx, ctr_start_val);
+ pmu_env_cleanup();
+ pebs_enable(BIT_ULL(type == FIXED ? INTEL_PMC_IDX_FIXED + idx : idx), pebs_data_cfg);
+ workload();
+ pebs_disable(idx);
+ check_pebs_records(BIT_ULL(type == FIXED ? INTEL_PMC_IDX_FIXED + idx : idx), pebs_data_cfg);
+ report_prefix_pop();
+}
+
+static void check_multiple_counters(u64 bitmask, u64 pebs_data_cfg)
+{
+ pmu_env_cleanup();
+ pebs_enable(bitmask, pebs_data_cfg);
+ workload2();
+ pebs_disable(0);
+ check_pebs_records(bitmask, pebs_data_cfg);
+}
+
+static void check_pebs_counters(u64 pebs_data_cfg)
+{
+ unsigned int idx;
+ u64 bitmask = 0;
+
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++)
+ check_one_counter(FIXED, idx, pebs_data_cfg);
+
+ for (idx = 0; idx < max_nr_gp_events; idx++)
+ check_one_counter(GP, idx, pebs_data_cfg);
+
+ for (idx = 0; idx < pmu_edx.split.num_counters_fixed; idx++)
+ bitmask |= BIT_ULL(INTEL_PMC_IDX_FIXED + idx);
+ for (idx = 0; idx < max_nr_gp_events; idx += 2)
+ bitmask |= BIT_ULL(idx);
+ report_prefix_pushf("Multiple (0x%lx)", bitmask);
+ check_multiple_counters(bitmask, pebs_data_cfg);
+ report_prefix_pop();
+}
+
+int main(int ac, char **av)
+{
+ struct cpuid id;
+ unsigned int i, j;
+
+ setup_vm();
+ id = cpuid(10);
+
+ pmu_eax.full = id.a;
+ pmu_edx.full = id.d;
+ max_nr_gp_events = MIN(pmu_eax.split.num_counters, ARRAY_SIZE(intel_arch_events));
+
+ printf("PMU version: %d\n", pmu_eax.split.version_id);
+ if (this_cpu_has(X86_FEATURE_PDCM))
+ perf.capabilities = rdmsr(MSR_IA32_PERF_CAPABILITIES);
+
+ if (perf.capabilities & PMU_CAP_FW_WRITES)
+ gp_counter_base = MSR_IA32_PMC0;
+
+ if (!is_intel() || (pmu_eax.split.version_id < 2) ||
+ !(perf.capabilities & PERF_CAP_PEBS_FORMAT) ||
+ (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL)) {
+ report_skip("This platform doesn't support guest PEBS.");
+ return 0;
+ }
+
+ printf("PEBS format: %d\n", perf.pebs_format);
+ printf("PEBS GP counters: %d\n", pmu_eax.split.num_counters);
+ printf("PEBS Fixed counters: %d\n", pmu_edx.split.num_counters_fixed);
+ printf("PEBS baseline (Adaptive PEBS): %d\n", perf.pebs_baseline);
+
+ printf("Known reasons for none PEBS records:\n");
+ printf("1. The selected event does not support PEBS;\n");
+ printf("2. From a core pmu perspective, the vCPU and pCPU models are not same;\n");
+ printf("3. Guest counter has not yet overflowed or been cross-mapped by the host;\n");
+
+ handle_irq(PC_VECTOR, cnt_overflow);
+ alloc_buffers();
+
+ for (i = 0; i < ARRAY_SIZE(counter_start_values); i++) {
+ ctr_start_val = counter_start_values[i];
+ check_pebs_counters(0);
+ if (!perf.pebs_baseline)
+ continue;
+
+ for (j = 0; j < ARRAY_SIZE(pebs_data_cfgs); j++) {
+ report_prefix_pushf("Adaptive (0x%lx)", pebs_data_cfgs[j]);
+ check_pebs_counters(pebs_data_cfgs[j]);
+ report_prefix_pop();
+ }
+ }
+
+ free_buffers();
+
+ return report_summary();
+}
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index d6dc19f..5731454 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -198,6 +198,13 @@ check = /sys/module/kvm/parameters/ignore_msrs=N
check = /proc/sys/kernel/nmi_watchdog=0
accel = kvm
+[pmu_pebs]
+arch = x86_64
+file = pmu_pebs.flat
+extra_params = -cpu host,migratable=no
+check = /proc/sys/kernel/nmi_watchdog=0
+accel = kvm
+
[pmu_emulation]
file = pmu.flat
arch = x86_64
--
2.37.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS)
2022-07-21 10:35 ` [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS) Like Xu
@ 2022-07-27 22:42 ` Sean Christopherson
2022-07-28 11:31 ` Like Xu
0 siblings, 1 reply; 17+ messages in thread
From: Sean Christopherson @ 2022-07-27 22:42 UTC (permalink / raw)
To: Like Xu; +Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm
On Thu, Jul 21, 2022, Like Xu wrote:
> +union perf_capabilities {
> + struct {
> + u64 lbr_format:6;
> + u64 pebs_trap:1;
> + u64 pebs_arch_reg:1;
> + u64 pebs_format:4;
> + u64 smm_freeze:1;
> + /*
> + * PMU supports separate counter range for writing
> + * values > 32bit.
> + */
> + u64 full_width_write:1;
> + u64 pebs_baseline:1;
> + u64 perf_metrics:1;
> + u64 pebs_output_pt_available:1;
> + u64 anythread_deprecated:1;
> + };
> + u64 capabilities;
> +};
> +
> +union cpuid10_eax {
> + struct {
> + unsigned int version_id:8;
> + unsigned int num_counters:8;
> + unsigned int bit_width:8;
> + unsigned int mask_length:8;
> + } split;
> + unsigned int full;
> +} pmu_eax;
> +
> +union cpuid10_edx {
> + struct {
> + unsigned int num_counters_fixed:5;
> + unsigned int bit_width_fixed:8;
> + unsigned int reserved:19;
> + } split;
> + unsigned int full;
> +} pmu_edx;
The generic unions are hopefully unnecessary now that helpers are provided by
lib/x86/processor.h, e.g. for pmu_version().
I would prefer to have similar helpers instead of "union perf_capabilities",
but it's not a sticking point if helpers a signifiantly more painful to use.
> + if (!is_intel() || (pmu_eax.split.version_id < 2) ||
> + !(perf.capabilities & PERF_CAP_PEBS_FORMAT) ||
> + (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL)) {
Split these up, it's really, really annoying to have to guess which one of the
four checks failed.
> + report_skip("This platform doesn't support guest PEBS.");
> + return 0;
This needs be be "return report_summary()", otherwise the test says pass when it
didn't do anyting:
TESTNAME=pmu_pebs TIMEOUT=90s ACCEL=kvm ./x86/run x86/pmu_pebs.flat -smp 1 -cpu host,migratable=no
PASS pmu_pebs
wait a second...
SKIP: This platform doesn't support guest PEBS.
E.g. (though if KUT can provide more information on why PERF_CAP_PEBS_FORMAT
may not be advertised, e.g. requires ICX+?, that would be nice to have)
if (!is_intel()) {
report_skip("PEBS is only supported on Intel CPUs");
return report_summary();
}
if (pmu_version() < 2) {
report_skip("Architectural PMU not available");
return report_summary();
}
if (!(perf.capabilities & PERF_CAP_PEBS_FORMAT)) {
report_skip("PEBS not enumerated in PERF_CAPABILITIES");
return report_summary();
}
if (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL) {
report_skip("PEBS unavailable according to MISC_ENABLE");
return report_summary();
}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS)
2022-07-27 22:42 ` Sean Christopherson
@ 2022-07-28 11:31 ` Like Xu
0 siblings, 0 replies; 17+ messages in thread
From: Like Xu @ 2022-07-28 11:31 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, Jim Mattson, linux-kernel, kvm
On 28/7/2022 6:42 am, Sean Christopherson wrote:
> On Thu, Jul 21, 2022, Like Xu wrote:
>> +union perf_capabilities {
>> + struct {
>> + u64 lbr_format:6;
>> + u64 pebs_trap:1;
>> + u64 pebs_arch_reg:1;
>> + u64 pebs_format:4;
>> + u64 smm_freeze:1;
>> + /*
>> + * PMU supports separate counter range for writing
>> + * values > 32bit.
>> + */
>> + u64 full_width_write:1;
>> + u64 pebs_baseline:1;
>> + u64 perf_metrics:1;
>> + u64 pebs_output_pt_available:1;
>> + u64 anythread_deprecated:1;
>> + };
>> + u64 capabilities;
>> +};
>> +
>> +union cpuid10_eax {
>> + struct {
>> + unsigned int version_id:8;
>> + unsigned int num_counters:8;
>> + unsigned int bit_width:8;
>> + unsigned int mask_length:8;
>> + } split;
>> + unsigned int full;
>> +} pmu_eax;
>> +
>> +union cpuid10_edx {
>> + struct {
>> + unsigned int num_counters_fixed:5;
>> + unsigned int bit_width_fixed:8;
>> + unsigned int reserved:19;
>> + } split;
>> + unsigned int full;
>> +} pmu_edx;
>
> The generic unions are hopefully unnecessary now that helpers are provided by
> lib/x86/processor.h, e.g. for pmu_version().
>
> I would prefer to have similar helpers instead of "union perf_capabilities",
> but it's not a sticking point if helpers a signifiantly more painful to use.
>
>> + if (!is_intel() || (pmu_eax.split.version_id < 2) ||
>> + !(perf.capabilities & PERF_CAP_PEBS_FORMAT) ||
>> + (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL)) {
>
> Split these up, it's really, really annoying to have to guess which one of the
> four checks failed.
>
>> + report_skip("This platform doesn't support guest PEBS.");
>> + return 0;
>
> This needs be be "return report_summary()", otherwise the test says pass when it
> didn't do anyting:
>
> TESTNAME=pmu_pebs TIMEOUT=90s ACCEL=kvm ./x86/run x86/pmu_pebs.flat -smp 1 -cpu host,migratable=no
> PASS pmu_pebs
>
> wait a second...
>
> SKIP: This platform doesn't support guest PEBS.
>
> E.g. (though if KUT can provide more information on why PERF_CAP_PEBS_FORMAT
> may not be advertised, e.g. requires ICX+?, that would be nice to have)
>
> if (!is_intel()) {
> report_skip("PEBS is only supported on Intel CPUs");
> return report_summary();
> }
> if (pmu_version() < 2) {
> report_skip("Architectural PMU not available");
> return report_summary();
> }
> if (!(perf.capabilities & PERF_CAP_PEBS_FORMAT)) {
> report_skip("PEBS not enumerated in PERF_CAPABILITIES");
> return report_summary();
> }
> if (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL) {
> report_skip("PEBS unavailable according to MISC_ENABLE");
> return report_summary();
> }
Thanks, all applied. Please review this new version as a separate thread.
https://lore.kernel.org/kvm/20220728112119.58173-1-likexu@tencent.com/
^ permalink raw reply [flat|nested] 17+ messages in thread