mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>
To: "Chen, Zide" <zide.chen@intel.com>,
	Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jim Mattson <jmattson@google.com>,
	Mingwei Zhang <mizhang@google.com>,
	Das Sandipan <Sandipan.Das@amd.com>,
	Shukla Manali <Manali.Shukla@amd.com>,
	Falcon Thomas <thomas.falcon@intel.com>,
	Xudong Hao <xudong.hao@intel.com>
Subject: Re: [PATCH V5 6/7] KVM: x86/pmu: Emulate RDPMC on performance metrics
Date: Fri, 26 Jun 2026 08:28:11 +0800	[thread overview]
Message-ID: <3ed61c5c-cf91-4af3-90ad-f416f14ac644@linux.intel.com> (raw)
In-Reply-To: <1f1c2f34-1f89-4886-a115-41666532f032@intel.com>


On 6/26/2026 12:30 AM, Chen, Zide wrote:
>
> On 6/25/2026 4:17 AM, Mi, Dapeng wrote:
>> On 6/25/2026 11:45 AM, Zide Chen wrote:
>>> If the host has the PERF_METRICS capability but it's not present on
>>> the guest, RDPMC interception must be enabled and KVM should inject
>>> an #GP when the guest attempts to read it.
>>>
>>> If the guest has PERF_METRICS, but RDPMC interception is enabled for
>>> other reasons, KVM needs to emulate RDPMC with type 2000H.
>>>
>>> For simplicity, Metrics Clear Mode is not supported.
>>>
>>> Signed-off-by: Zide Chen <zide.chen@intel.com>
>>> ---
>>> v5: new patch.
>>> ---
>>>  arch/x86/kvm/pmu.c | 30 ++++++++++++++++++++++++++----
>>>  1 file changed, 26 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
>>> index 7aafc5db1346..af5b14f44e4b 100644
>>> --- a/arch/x86/kvm/pmu.c
>>> +++ b/arch/x86/kvm/pmu.c
>>> @@ -755,6 +755,16 @@ static int kvm_pmu_rdpmc_vmware(struct kvm_vcpu *vcpu, unsigned idx, u64 *data)
>>>  	return 0;
>>>  }
>>>  
>>> +static int kvm_pmu_rdpmc_metrics(struct kvm_vcpu *vcpu,
>>> +				 unsigned idx, u64 *data)
>>> +{
>>> +	if (!kvm_vcpu_has_perf_metrics(vcpu))
>>> +		return 1;
>>> +
>>> +	*data = vcpu_to_pmu(vcpu)->perf_metrics;
>>> +	return 0;
>>> +}
>>> +
>>>  int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data)
>>>  {
>>>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>> @@ -767,15 +777,18 @@ int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data)
>>>  	if (is_vmware_backdoor_pmc(idx))
>>>  		return kvm_pmu_rdpmc_vmware(vcpu, idx, data);
>>>  
>>> -	pmc = kvm_pmu_call(rdpmc_ecx_to_pmc)(vcpu, idx, &mask);
>>> -	if (!pmc)
>>> -		return 1;
>>> -
>>>  	if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_PCE) &&
>>>  	    (kvm_x86_call(get_cpl)(vcpu) != 0) &&
>>>  	    kvm_is_cr0_bit_set(vcpu, X86_CR0_PE))
>>>  		return 1;
>>>  
>>> +	if (idx & INTEL_PMC_FIXED_RDPMC_METRICS)
>> I'm not quite sure if this check is good enough. Although the SDM vol.2
>> says "Performance metrics use type 2000H. This type can be used only if
>> IA32_PERF_CAPABILITIES.PERF_METRICS_AVAILABLE[bit 15]=1. For this type, the
>> index in ECX[15:0] is implementation specific." It doesn't say the PMC
>> index must be 0, but we always set the PMC index to 0 when reading topdown
>> metrics.
> Simply AND the idx is problematic for two reasons: 1) it doesn't catch
> invalid type. for example 6000H; 2) fundamentally,
> INTEL_PMC_FIXED_RDPMC_METRICS is Intel specific and it can't be compared
> in common code.
>
> The proposal is to change the callback rdpmc_ecx_to_pmc() to
> emu_rdpmc(), and mode the RDPMC metrics code into vendor. Part of the
> proposed code:
>
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 7aafc5db1346..af6fc8c96037 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -758,8 +758,6 @@ static int kvm_pmu_rdpmc_vmware(struct kvm_vcpu
> *vcpu, unsigned idx, u64 *data)
>  int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data)
>  {
>         struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> -       struct kvm_pmc *pmc;
> -       u64 mask = ~0ull;
>
>         if (!pmu->version)
>                 return 1;
> @@ -767,17 +765,12 @@ int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned
> idx, u64 *data)
>         if (is_vmware_backdoor_pmc(idx))
>                 return kvm_pmu_rdpmc_vmware(vcpu, idx, data);
>
> -       pmc = kvm_pmu_call(rdpmc_ecx_to_pmc)(vcpu, idx, &mask);
> -       if (!pmc)
> -               return 1;
> -
>         if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_PCE) &&
>             (kvm_x86_call(get_cpl)(vcpu) != 0) &&
>             kvm_is_cr0_bit_set(vcpu, X86_CR0_PE))
>                 return 1;
>
> -       *data = pmc_read_counter(pmc) & mask;
> -       return 0;
> +       return kvm_pmu_call(emu_rdpmc)(vcpu, idx, data);
>  }
>
>  static bool kvm_need_any_pmc_intercept(struct kvm_vcpu *vcpu)
> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index 3066cade5790..24634aa92875 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
> @@ -24,8 +24,7 @@
>  #define KVM_FIXED_PMC_BASE_IDX INTEL_PMC_IDX_FIXED
>
>  struct kvm_pmu_ops {
> -       struct kvm_pmc *(*rdpmc_ecx_to_pmc)(struct kvm_vcpu *vcpu,
> -               unsigned int idx, u64 *mask);
> +       int (*emu_rdpmc)(struct kvm_vcpu *vcpu, unsigned idx, u64 *data);
>         struct kvm_pmc *(*msr_idx_to_pmc)(struct kvm_vcpu *vcpu, u32 msr);
>         int (*check_rdpmc_early)(struct kvm_vcpu *vcpu, unsigned int idx);
>         bool (*is_valid_msr)(struct kvm_vcpu *vcpu, u32 msr);
> @@ -84,14 +85,12 @@ static void reprogram_fixed_counters(struct kvm_pmu
> *pmu, u64 data)
>         }
>  }
>
> -static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu,
> -                                           unsigned int idx, u64 *mask)
> +static int intel_emu_rdpmc(struct kvm_vcpu *vcpu, unsigned idx, u64 *data)

What does "emu" mean? Is it a typo?


>  {
>         unsigned int type = idx & INTEL_RDPMC_TYPE_MASK;
>         struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> -       struct kvm_pmc *counters;
> +       struct kvm_pmc *counters, *pmc;
>         unsigned int num_counters;
> -       u64 bitmask;
>
>         /*
>          * The encoding of ECX for RDPMC is different for architectural
> versus
> @@ -104,7 +103,7 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct
> kvm_vcpu *vcpu,
>          * as KVM doesn't support such PMUs.
>          */
>         if (WARN_ON_ONCE(!pmu->version))
> -               return NULL;
> +               return 1;
>
>         /*
>          * General Purpose (GP) PMCs are supported on all PMUs, and
> fixed PMCs
> @@ -118,23 +117,28 @@ static struct kvm_pmc
> *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu,
>         case INTEL_RDPMC_FIXED:
>                 counters = pmu->fixed_counters;
>                 num_counters = pmu->nr_arch_fixed_counters;
> -               bitmask = pmu->counter_bitmask[KVM_PMC_FIXED];
>                 break;
>         case INTEL_RDPMC_GP:
>                 counters = pmu->gp_counters;
>                 num_counters = pmu->nr_arch_gp_counters;
> -               bitmask = pmu->counter_bitmask[KVM_PMC_GP];
>                 break;
> +       case INTEL_RDPMC_METRICS:
> +               if (!kvm_vcpu_has_perf_metrics(vcpu))
> +                       return 1;
> +
> +               *data = vcpu_to_pmu(vcpu)->perf_metrics;
> +               return 0;
>         default:
> -               return NULL;
> +               return 1;
>         }
>
>         idx &= INTEL_RDPMC_INDEX_MASK;
>         if (idx >= num_counters)
> -               return NULL;
> +               return 1;
>
> -       *mask &= bitmask;
> -       return &counters[array_index_nospec(idx, num_counters)];
> +       pmc = &counters[array_index_nospec(idx, num_counters)];
> +       *data = pmc_read_counter(pmc);
> +       return 0;
>  }
>
>  static inline struct kvm_pmc *get_fw_gp_pmc(struct kvm_pmu *pmu, u32 msr)
>
>
>> Could you please test to run the rdpmc instruction to read topdown metrics
>> with valid and invalid PMC index on real HW and look what would happen? Thanks.
> RDPMC with invalid type, for example 0x6000 got an #GP. non-zero index
> (ECX[15:0]) with type 2000H got an #GP on SPR/NVL/GNR as well.
>
> I haven't found any documentation talking about non-zero index on type
> 2000H. Since it's implementation specific, for now seems it's OK to
> reject (#GP) non-zero index.

Yeah, this is consistent with what we ever know. Let us follow the HW's
behavior here. Thanks.


>
>
>>> +		return kvm_pmu_rdpmc_metrics(vcpu, idx, data);
>>> +
>>> +	pmc = kvm_pmu_call(rdpmc_ecx_to_pmc)(vcpu, idx, &mask);
>>> +	if (!pmc)
>>> +		return 1;
>>> +
>>>  	*data = pmc_read_counter(pmc) & mask;
>>>  	return 0;
>>>  }
>>> @@ -803,6 +816,14 @@ bool kvm_need_perf_global_ctrl_intercept(struct kvm_vcpu *vcpu)
>>>  }
>>>  EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_need_perf_global_ctrl_intercept);
>>>  
>>> +static bool kvm_need_perf_metrics_intercept(struct kvm_vcpu *vcpu)
>>> +{
>>> +	if (!(kvm_host.perf_capabilities & PERF_CAP_PERF_METRICS))
>>> +		return false;
>>> +
>>> +	return !kvm_vcpu_has_perf_metrics(vcpu);
>>> +}
>>> +
>>>  bool kvm_need_rdpmc_intercept(struct kvm_vcpu *vcpu)
>>>  {
>>>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>> @@ -815,6 +836,7 @@ bool kvm_need_rdpmc_intercept(struct kvm_vcpu *vcpu)
>>>  		return true;
>>>  
>>>  	return kvm_need_any_pmc_intercept(vcpu) ||
>>> +	       kvm_need_perf_metrics_intercept(vcpu) ||
>>>  	       pmu->counter_bitmask[KVM_PMC_GP] != (BIT_ULL(kvm_host_pmu.bit_width_gp) - 1) ||
>>>  	       pmu->counter_bitmask[KVM_PMC_FIXED] != (BIT_ULL(kvm_host_pmu.bit_width_fixed) - 1);
>>>  }

  reply	other threads:[~2026-06-26  0:28 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-25  3:45 [PATCH V5 0/7] KVM: x86/pmu: Add hardware Topdown metrics support Zide Chen
2026-06-25  3:45 ` [PATCH V5 1/7] KVM: x86/pmu: Do not map fixed counters >= 3 to generic perf events Zide Chen
2026-06-25  3:45 ` [PATCH V5 2/7] KVM: x86/pmu: Support Intel fixed counter 3 on mediated vPMU Zide Chen
2026-06-25  3:45 ` [PATCH V5 3/7] KVM: x86/pmu: Rename and move vcpu_get_perf_capabilities() to pmu.h Zide Chen
2026-06-25  3:45 ` [PATCH V5 4/7] KVM: x86/pmu: Support PERF_METRICS MSR in mediated vPMU Zide Chen
2026-06-25  3:45 ` [PATCH V5 5/7] KVM: x86/pmu: Snapshot host IA32_PERF_CAPABILITIES in kvm_host Zide Chen
2026-06-25  3:45 ` [PATCH V5 6/7] KVM: x86/pmu: Emulate RDPMC on performance metrics Zide Chen
2026-06-25  9:17   ` Mi, Dapeng
2026-06-25 16:30     ` Chen, Zide
2026-06-26  0:28       ` Mi, Dapeng [this message]
2026-06-26  1:36         ` Chen, Zide
2026-06-25  3:45 ` [PATCH V5 7/7] KVM: selftests: Add perf_metrics and fixed counter 3 tests Zide Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ed61c5c-cf91-4af3-90ad-f416f14ac644@linux.intel.com \
    --to=dapeng1.mi@linux.intel.com \
    --cc=Manali.Shukla@amd.com \
    --cc=Sandipan.Das@amd.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=thomas.falcon@intel.com \
    --cc=xudong.hao@intel.com \
    --cc=zide.chen@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome