From: Sean Christopherson <seanjc@google.com>
To: Like Xu <like.xu.linux@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 08/12] KVM: x86/pmu: Disable vPMU if the minimum num of counters isn't met
Date: Thu, 6 Apr 2023 17:06:55 -0700 [thread overview]
Message-ID: <ZC9enygbWzJ59Ssb@google.com> (raw)
In-Reply-To: <20230214050757.9623-9-likexu@tencent.com>
On Tue, Feb 14, 2023, Like Xu wrote:
> From: Like Xu <likexu@tencent.com>
>
> For compatibility with old software, KVM/AMD should never report less
> than four counters if vPMU is supported.
Explain _why_. Anchor what "should" be done in hardware specifications and
architecture.
> Thus KVM should sanity check the number of counters enumerated by perf and
> explicitly disable vPMU support if the min isn't met. E.g. if KVM needs 4
> counters and perf says there are 3, then something is wrong and enumerating 4
> to the guest is only going to cause more troubles.
Again, state what the patch actually does, not what KVM "should do". E.g.
Disable PMU support when running on AMD and perf reports fewer than four
general purpose counters. All AMD PMUs must define at least four counters
due to AMD's legacy architecture hardcoding the number of counters
without providing a way to enumerate the number of counters to software,
e.g. from AMD's APM.
The legacy architecture defines four performance counters
Virtualizing fewer than four counters can lead to guest instability as
software expects four counters to be available.
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Like Xu <likexu@tencent.com>
> ---
> arch/x86/kvm/pmu.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index d1cc02c8da88..46db5404894e 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
> @@ -170,6 +170,12 @@ static inline void kvm_init_pmu_capability(const struct kvm_pmu_ops *pmu_ops)
> if ((is_intel && !kvm_pmu_cap.version) || !kvm_pmu_cap.num_counters_gp)
> enable_pmu = false;
>
> + /*
> + * For AMD, disable vPMU if the minimum number of counters isn't met.
> + */
Doesn't need to be a multiple line comment. This comment is also useless. It's
quite clear from the code that PMU support is being disabled when there aren't
enough counters, what's missing is _why_.
> + if (!is_intel && kvm_pmu_cap.num_counters_gp < AMD64_NUM_COUNTERS)
> + enable_pmu = false;
> +
> if (!enable_pmu) {
> memset(&kvm_pmu_cap, 0, sizeof(kvm_pmu_cap));
> return;
> --
> 2.39.1
>
next prev parent reply other threads:[~2023-04-07 0:07 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-14 5:07 [PATCH v4 00/12] KVM: x86: Add AMD Guest PerfMonV2 PMU support Like Xu
2023-02-14 5:07 ` [PATCH v4 01/12] KVM: x86/pmu: Rename pmc_is_enabled() to pmc_is_globally_enabled() Like Xu
2023-02-14 5:07 ` [PATCH v4 02/12] KVM: VMX: Refactor intel_pmu_set_msr() to align with other set_msr() helpers Like Xu
2023-02-16 21:13 ` Sean Christopherson
2023-02-21 8:44 ` Like Xu
2023-03-23 7:43 ` Like Xu
2023-03-23 14:28 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 03/12] KVM: x86/pmu: Rewrite reprogram_counters() to improve performance Like Xu
2023-02-14 5:07 ` [PATCH v4 04/12] KVM: x86/pmu: Expose reprogram_counters() in pmu.h Like Xu
2023-02-14 5:07 ` [PATCH v4 05/12] KVM: x86/pmu: Error when user sets the GLOBAL_STATUS reserved bits Like Xu
2023-04-06 23:45 ` Sean Christopherson
2023-04-07 5:08 ` Like Xu
2023-04-07 15:43 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 06/12] KVM: x86/pmu: Make part of the Intel v2 PMU MSRs handling x86 generic Like Xu
2023-04-06 23:57 ` Sean Christopherson
2023-04-07 1:39 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 07/12] KVM: x86/cpuid: Use fast return for cpuid "0xa" leaf when !enable_pmu Like Xu
2023-04-06 23:59 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 08/12] KVM: x86/pmu: Disable vPMU if the minimum num of counters isn't met Like Xu
2023-04-07 0:06 ` Sean Christopherson [this message]
2023-04-07 0:23 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 09/12] KVM: x86/pmu: Forget PERFCTR_CORE if the min " Like Xu
2023-04-07 0:32 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 10/12] KVM: x86/cpuid: Add X86_FEATURE_PERFMON_V2 as a scattered flag Like Xu
2023-04-07 0:41 ` Sean Christopherson
2023-02-14 5:07 ` [PATCH v4 11/12] KVM: x86/svm/pmu: Add AMD PerfMonV2 support Like Xu
2023-04-07 1:35 ` Sean Christopherson
2023-04-07 7:08 ` Like Xu
2023-04-07 14:44 ` Sean Christopherson
2023-04-10 11:34 ` Like Xu
2023-02-14 5:07 ` [PATCH v4 12/12] KVM: x86/cpuid: Add AMD CPUID ExtPerfMonAndDbg leaf 0x80000022 Like Xu
2023-04-07 1:50 ` Sean Christopherson
2023-04-07 7:19 ` Like Xu
2023-04-07 2:02 ` [PATCH v4 00/12] KVM: x86: Add AMD Guest PerfMonV2 PMU support Sean Christopherson
2023-04-07 7:28 ` Like Xu
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=ZC9enygbWzJ59Ssb@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=like.xu.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.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;
as well as URLs for NNTP newsgroup(s).