Message ID | 20231202000417.922113-11-seanjc@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp1492903vqy; Fri, 1 Dec 2023 16:05:22 -0800 (PST) X-Google-Smtp-Source: AGHT+IF2tjWJ+OgUTfFUh2Gh1r9fbEyspMNq3+Sc7XrK4kRU5U63Y6M7AvZ4B0KG9mhHEQpngGxe X-Received: by 2002:a17:903:32d1:b0:1cf:6656:69cf with SMTP id i17-20020a17090332d100b001cf665669cfmr396759plr.21.1701475522423; Fri, 01 Dec 2023 16:05:22 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701475522; cv=none; d=google.com; s=arc-20160816; b=ELzMOY568P+bEGEs0sYxkLFGoP+E7afdj56CZZ0S7JjkOtuO+BbRLVlB1Fg9jD8i/9 SlpbyUifT5qxaRarfUWQ4dEzstJvLNGvvrS/ZbkcHo8ngYXK6hZ2doaH/rVa6lDxBCUN FuIFMIHqW7oNPrjbK5vWF/AH6Z2CMt+Jv9bFJ27hlGgcWQ/mA9VB3uWrEtQ/bFghG+h4 jgDTlOBbic83aSJiU/UT+6t12qc08vEWf4J7szzw873dINl4x+/uyfxr0wT3cAZ00vuJ 3RcXLRNascs4zEyu0JYkTHPqAvKYjCqkMpY91+aCq2ESE7IdSgwjCuSzeOr/xbX3v7j9 hOJg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:reply-to:dkim-signature; bh=kARN2t1cDT1nPhBBpb3LTJ5yo8qfPIAw+yo3QqNarOE=; fh=gm96CM+cG0KJxn7vnhn0c1c98ILNP1HHUrKfrGecw3M=; b=GIIfIhL5fMdtkQ9ZHTbBbr+vyyCB4HbaI3it8wSZzuMUHCIn3a4vS8rXFb9pqjpDyV KkoSjn49jE17IEg1UhuEPVI4zZDV6jVb/fYvGqfZGJRcTJKaNuOJRceoqHEN3paK+6fO 4SVtKbnIx9G72HemfQLasVBxb+3e/DN+MH/KD1cEmRMmzm7I7NUdWMIZ3/CbB2abx8wT 1ULNrP/U2CMtK5uSCgFjkMVF0a4fTTs7un6qma5zXzjjMw6vGxTUCLTMtYE1Y60k6W1N MjPQZZUsohu5jaEcGGWkQrcSvXx4oGQtdI46jbnl8KG0GFMgnDnMVZxuG/nyRrFZe2a5 0TXA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=AAY+Tv3H; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id jf20-20020a170903269400b001c3886b091asi2651612plb.127.2023.12.01.16.05.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Dec 2023 16:05:22 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=AAY+Tv3H; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 28F438184E0E; Fri, 1 Dec 2023 16:05:21 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1441953AbjLBAFJ (ORCPT <rfc822;pwkd43@gmail.com> + 99 others); Fri, 1 Dec 2023 19:05:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42576 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1441990AbjLBAEu (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 1 Dec 2023 19:04:50 -0500 Received: from mail-pl1-x64a.google.com (mail-pl1-x64a.google.com [IPv6:2607:f8b0:4864:20::64a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8B6A31BCD for <linux-kernel@vger.kernel.org>; Fri, 1 Dec 2023 16:04:40 -0800 (PST) Received: by mail-pl1-x64a.google.com with SMTP id d9443c01a7336-1cfd4ef9f06so12115215ad.0 for <linux-kernel@vger.kernel.org>; Fri, 01 Dec 2023 16:04:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701475479; x=1702080279; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=kARN2t1cDT1nPhBBpb3LTJ5yo8qfPIAw+yo3QqNarOE=; b=AAY+Tv3HRsRRO7etlXqC0lCOCShST/GGftsFzjIet2J7cna2R5K4TW+Y61HpbcKnLf sSm13SLogQ3Wex8o9pMrXgfes8g8h3edtoWHygcq2Nw89h/Esp3vVCX9u6StgV8ZbOr/ xp+dMlXlN3M8BeyHDnB8RWxkCPQuoTXhnXTMRCKeJi9/ypGqDa8hyDlGjhdfoLel7NFg rp5Ph59GGvzjmxvsfB+liF6LhCgqcC8c6VNCZxXV2svH8cHUTcplApVgnI+LW8RfTZY3 cMyq/7FpSrVhNmePc9xljNXA/h2WYyj7sjS4BthD5+lum2rwrNFPqYl6AfXNAiZpvxP8 03mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701475479; x=1702080279; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=kARN2t1cDT1nPhBBpb3LTJ5yo8qfPIAw+yo3QqNarOE=; b=he9lrv1iDY4dZ8NEpwe3qS+QAmdp70PF/sn1OkzFaHatdX7a93r/a062rSYPBjYR5x +HnS6JdUZTOX87gXfqzRQRIeYnXDfBON5FtcWDcYYhYwueDglUzEbLWrnE5ARjfHS+w4 sw38cZj6UuPY2IPjZrvX0kTydHtRrWE1ZobyJaVoXXmF4quOo9ls1fIfIPxs2DrDwVEs XKq/hJEAYyLilWJ0LAd7GPBJOUNS+N/PapBu4e55PhzmqyeeP44cccEOBUOZtQ22DuIS 44aRMJBgkUK5NEipBIQUrz8eUQT5m1qCxxK/46Kto5+8BTjFDj5rogpedndbtyTFnXTt F+GA== X-Gm-Message-State: AOJu0Yx9Y54kG817Gn5s/PSO2gUm3sdESTigXoXoVXzOYrwlGdMWwn82 keubTc/5LCOg9cwP76+Bv3RiOkB8YL8= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:903:3291:b0:1cf:5cd7:8416 with SMTP id jh17-20020a170903329100b001cf5cd78416mr5059080plb.13.1701475479406; Fri, 01 Dec 2023 16:04:39 -0800 (PST) Reply-To: Sean Christopherson <seanjc@google.com> Date: Fri, 1 Dec 2023 16:03:59 -0800 In-Reply-To: <20231202000417.922113-1-seanjc@google.com> Mime-Version: 1.0 References: <20231202000417.922113-1-seanjc@google.com> X-Mailer: git-send-email 2.43.0.rc2.451.g8631bc7472-goog Message-ID: <20231202000417.922113-11-seanjc@google.com> Subject: [PATCH v9 10/28] KVM: x86/pmu: Explicitly check for RDPMC of unsupported Intel PMC types From: Sean Christopherson <seanjc@google.com> To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Kan Liang <kan.liang@linux.intel.com>, Dapeng Mi <dapeng1.mi@linux.intel.com>, Jim Mattson <jmattson@google.com>, Jinrong Liang <cloudliang@tencent.com>, Aaron Lewis <aaronlewis@google.com>, Like Xu <likexu@tencent.com> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Fri, 01 Dec 2023 16:05:21 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784126397524528897 X-GMAIL-MSGID: 1784126397524528897 |
Series |
KVM: x86/pmu: selftests: Fixes and new tests
|
|
Commit Message
Sean Christopherson
Dec. 2, 2023, 12:03 a.m. UTC
Explicitly check for attempts to read unsupported PMC types instead of
letting the bounds check fail. Functionally, letting the check fail is
ok, but it's unnecessarily subtle and does a poor job of documenting the
architectural behavior that KVM is emulating.
Opportunistically add macros for the type vs. index to further document
what is going on.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/vmx/pmu_intel.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
Comments
On 12/2/2023 8:03 AM, Sean Christopherson wrote: > Explicitly check for attempts to read unsupported PMC types instead of > letting the bounds check fail. Functionally, letting the check fail is > ok, but it's unnecessarily subtle and does a poor job of documenting the > architectural behavior that KVM is emulating. > > Opportunistically add macros for the type vs. index to further document > what is going on. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/vmx/pmu_intel.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > index 644de27bd48a..bd4f4bdf5419 100644 > --- a/arch/x86/kvm/vmx/pmu_intel.c > +++ b/arch/x86/kvm/vmx/pmu_intel.c > @@ -23,6 +23,9 @@ > /* Perf's "BASE" is wildly misleading, this is a single-bit flag, not a base. */ > #define INTEL_RDPMC_FIXED INTEL_PMC_FIXED_RDPMC_BASE > > +#define INTEL_RDPMC_TYPE_MASK GENMASK(31, 16) > +#define INTEL_RDPMC_INDEX_MASK GENMASK(15, 0) > + > #define MSR_PMC_FULL_WIDTH_BIT (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0) > > static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data) > @@ -82,9 +85,13 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu, > /* > * Fixed PMCs are supported on all architectural PMUs. Note, KVM only > * emulates fixed PMCs for PMU v2+, but the flag itself is still valid, > - * i.e. let RDPMC fail due to accessing a non-existent counter. > + * i.e. let RDPMC fail due to accessing a non-existent counter. Reject > + * attempts to read all other types, which are unknown/unsupported. > */ > - idx &= ~INTEL_RDPMC_FIXED; > + if (idx & INTEL_RDPMC_TYPE_MASK & ~INTEL_RDPMC_FIXED) > + return NULL; > + > + idx &= INTEL_RDPMC_INDEX_MASK; > if (fixed) { > counters = pmu->fixed_counters; > num_counters = pmu->nr_arch_fixed_counters; Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
On Sun, Dec 10, 2023 at 10:26 PM Mi, Dapeng <dapeng1.mi@linux.intel.com> wrote: > > > On 12/2/2023 8:03 AM, Sean Christopherson wrote: > > Explicitly check for attempts to read unsupported PMC types instead of > > letting the bounds check fail. Functionally, letting the check fail is > > ok, but it's unnecessarily subtle and does a poor job of documenting the > > architectural behavior that KVM is emulating. > > > > Opportunistically add macros for the type vs. index to further document > > what is going on. > > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/kvm/vmx/pmu_intel.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > > index 644de27bd48a..bd4f4bdf5419 100644 > > --- a/arch/x86/kvm/vmx/pmu_intel.c > > +++ b/arch/x86/kvm/vmx/pmu_intel.c > > @@ -23,6 +23,9 @@ > > /* Perf's "BASE" is wildly misleading, this is a single-bit flag, not a base. */ > > #define INTEL_RDPMC_FIXED INTEL_PMC_FIXED_RDPMC_BASE > > > > +#define INTEL_RDPMC_TYPE_MASK GENMASK(31, 16) > > +#define INTEL_RDPMC_INDEX_MASK GENMASK(15, 0) > > + > > #define MSR_PMC_FULL_WIDTH_BIT (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0) > > > > static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data) > > @@ -82,9 +85,13 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu, > > /* > > * Fixed PMCs are supported on all architectural PMUs. Note, KVM only > > * emulates fixed PMCs for PMU v2+, but the flag itself is still valid, > > - * i.e. let RDPMC fail due to accessing a non-existent counter. > > + * i.e. let RDPMC fail due to accessing a non-existent counter. Reject > > + * attempts to read all other types, which are unknown/unsupported. > > */ > > - idx &= ~INTEL_RDPMC_FIXED; > > + if (idx & INTEL_RDPMC_TYPE_MASK & ~INTEL_RDPMC_FIXED) You know how I hate to be pedantic (ROFL), but the SDM only says: If the processor does support architectural performance monitoring (CPUID.0AH:EAX[7:0] ≠ 0), ECX[31:16] specifies type of PMC while ECX[15:0] specifies the index of the PMC to be read within that type. It does not say that the types are bitwise-exclusive. Yes, the types defined thus far are bitwise-exclusive, but who knows what tomorrow may bring? > > + return NULL; > > + > > + idx &= INTEL_RDPMC_INDEX_MASK; > > if (fixed) { > > counters = pmu->fixed_counters; > > num_counters = pmu->nr_arch_fixed_counters; > Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
On Mon, Dec 11, 2023, Jim Mattson wrote: > On Sun, Dec 10, 2023 at 10:26 PM Mi, Dapeng <dapeng1.mi@linux.intel.com> wrote: > > > > > > On 12/2/2023 8:03 AM, Sean Christopherson wrote: > > > Explicitly check for attempts to read unsupported PMC types instead of > > > letting the bounds check fail. Functionally, letting the check fail is > > > ok, but it's unnecessarily subtle and does a poor job of documenting the > > > architectural behavior that KVM is emulating. > > > > > > Opportunistically add macros for the type vs. index to further document > > > what is going on. > > > > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > --- > > > arch/x86/kvm/vmx/pmu_intel.c | 11 +++++++++-- > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > > > index 644de27bd48a..bd4f4bdf5419 100644 > > > --- a/arch/x86/kvm/vmx/pmu_intel.c > > > +++ b/arch/x86/kvm/vmx/pmu_intel.c > > > @@ -23,6 +23,9 @@ > > > /* Perf's "BASE" is wildly misleading, this is a single-bit flag, not a base. */ > > > #define INTEL_RDPMC_FIXED INTEL_PMC_FIXED_RDPMC_BASE > > > > > > +#define INTEL_RDPMC_TYPE_MASK GENMASK(31, 16) > > > +#define INTEL_RDPMC_INDEX_MASK GENMASK(15, 0) > > > + > > > #define MSR_PMC_FULL_WIDTH_BIT (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0) > > > > > > static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data) > > > @@ -82,9 +85,13 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu, > > > /* > > > * Fixed PMCs are supported on all architectural PMUs. Note, KVM only > > > * emulates fixed PMCs for PMU v2+, but the flag itself is still valid, > > > - * i.e. let RDPMC fail due to accessing a non-existent counter. > > > + * i.e. let RDPMC fail due to accessing a non-existent counter. Reject > > > + * attempts to read all other types, which are unknown/unsupported. > > > */ > > > - idx &= ~INTEL_RDPMC_FIXED; > > > + if (idx & INTEL_RDPMC_TYPE_MASK & ~INTEL_RDPMC_FIXED) > > You know how I hate to be pedantic (ROFL), but the SDM only says: > > If the processor does support architectural performance monitoring > (CPUID.0AH:EAX[7:0] ≠ 0), ECX[31:16] specifies type of PMC while > ECX[15:0] specifies the index of the PMC to be read within that type. > > It does not say that the types are bitwise-exclusive. > > Yes, the types defined thus far are bitwise-exclusive, but who knows > what tomorrow may bring? The goal isn't to make the types exclusive, the goal is to reject types that aren't supported by KVM. The above accomplishes that, no? I don't see how KVM could get a false negative or false positive, the above allows exactly FIXED and "none" types. Or are you objecting to the comment?
On Mon, Dec 11, 2023 at 3:43 PM Sean Christopherson <seanjc@google.com> wrote: > > On Mon, Dec 11, 2023, Jim Mattson wrote: > > On Sun, Dec 10, 2023 at 10:26 PM Mi, Dapeng <dapeng1.mi@linux.intel.com> wrote: > > > > > > > > > On 12/2/2023 8:03 AM, Sean Christopherson wrote: > > > > Explicitly check for attempts to read unsupported PMC types instead of > > > > letting the bounds check fail. Functionally, letting the check fail is > > > > ok, but it's unnecessarily subtle and does a poor job of documenting the > > > > architectural behavior that KVM is emulating. > > > > > > > > Opportunistically add macros for the type vs. index to further document > > > > what is going on. > > > > > > > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > > > --- > > > > arch/x86/kvm/vmx/pmu_intel.c | 11 +++++++++-- > > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > > > > index 644de27bd48a..bd4f4bdf5419 100644 > > > > --- a/arch/x86/kvm/vmx/pmu_intel.c > > > > +++ b/arch/x86/kvm/vmx/pmu_intel.c > > > > @@ -23,6 +23,9 @@ > > > > /* Perf's "BASE" is wildly misleading, this is a single-bit flag, not a base. */ > > > > #define INTEL_RDPMC_FIXED INTEL_PMC_FIXED_RDPMC_BASE > > > > > > > > +#define INTEL_RDPMC_TYPE_MASK GENMASK(31, 16) > > > > +#define INTEL_RDPMC_INDEX_MASK GENMASK(15, 0) > > > > + > > > > #define MSR_PMC_FULL_WIDTH_BIT (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0) > > > > > > > > static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data) > > > > @@ -82,9 +85,13 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu, > > > > /* > > > > * Fixed PMCs are supported on all architectural PMUs. Note, KVM only > > > > * emulates fixed PMCs for PMU v2+, but the flag itself is still valid, > > > > - * i.e. let RDPMC fail due to accessing a non-existent counter. > > > > + * i.e. let RDPMC fail due to accessing a non-existent counter. Reject > > > > + * attempts to read all other types, which are unknown/unsupported. > > > > */ > > > > - idx &= ~INTEL_RDPMC_FIXED; > > > > + if (idx & INTEL_RDPMC_TYPE_MASK & ~INTEL_RDPMC_FIXED) > > > > You know how I hate to be pedantic (ROFL), but the SDM only says: > > > > If the processor does support architectural performance monitoring > > (CPUID.0AH:EAX[7:0] ≠ 0), ECX[31:16] specifies type of PMC while > > ECX[15:0] specifies the index of the PMC to be read within that type. > > > > It does not say that the types are bitwise-exclusive. > > > > Yes, the types defined thus far are bitwise-exclusive, but who knows > > what tomorrow may bring? > > The goal isn't to make the types exclusive, the goal is to reject types that > aren't supported by KVM. The above accomplishes that, no? I don't see how KVM > could get a false negative or false positive, the above allows exactly FIXED and > "none" types. Or are you objecting to the comment? You're right. The code is fine. My brain is not. But what's wrong with something like: type = idx & INTEL_RDPMC_TYPE_MASK; if (type != INTEL_RDPMC_GP && type != INTEL_RDPMC_FIXED) ... This makes it more clear what kvm accepts and what it doesn't accept, regardless of the actual values of the macros.
On Mon, Dec 11, 2023, Jim Mattson wrote: > On Mon, Dec 11, 2023 at 3:43 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > @@ -82,9 +85,13 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu, > > > > > /* > > > > > * Fixed PMCs are supported on all architectural PMUs. Note, KVM only > > > > > * emulates fixed PMCs for PMU v2+, but the flag itself is still valid, > > > > > - * i.e. let RDPMC fail due to accessing a non-existent counter. > > > > > + * i.e. let RDPMC fail due to accessing a non-existent counter. Reject > > > > > + * attempts to read all other types, which are unknown/unsupported. > > > > > */ > > > > > - idx &= ~INTEL_RDPMC_FIXED; > > > > > + if (idx & INTEL_RDPMC_TYPE_MASK & ~INTEL_RDPMC_FIXED) > > > > > > You know how I hate to be pedantic (ROFL), but the SDM only says: > > > > > > If the processor does support architectural performance monitoring > > > (CPUID.0AH:EAX[7:0] ≠ 0), ECX[31:16] specifies type of PMC while > > > ECX[15:0] specifies the index of the PMC to be read within that type. > > > > > > It does not say that the types are bitwise-exclusive. > > > > > > Yes, the types defined thus far are bitwise-exclusive, but who knows > > > what tomorrow may bring? > > > > The goal isn't to make the types exclusive, the goal is to reject types that > > aren't supported by KVM. The above accomplishes that, no? I don't see how KVM > > could get a false negative or false positive, the above allows exactly FIXED and > > "none" types. Or are you objecting to the comment? > > You're right. The code is fine. My brain is not. > > But what's wrong with something like: > > type = idx & INTEL_RDPMC_TYPE_MASK; > if (type != INTEL_RDPMC_GP && type != INTEL_RDPMC_FIXED) ... > > This makes it more clear what kvm accepts and what it doesn't accept, > regardless of the actual values of the macros. Because when I read the SDM, my reading was heavily colored by KVM's existing implementation. And the SDM using 4000H and 2000H for the non-zero types doesn't help (those scream "flags" to me). But rereading things, the SDM clearly states they are explicit, distinct types. I'll massage this to have KVM treat them as such.
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index 644de27bd48a..bd4f4bdf5419 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -23,6 +23,9 @@ /* Perf's "BASE" is wildly misleading, this is a single-bit flag, not a base. */ #define INTEL_RDPMC_FIXED INTEL_PMC_FIXED_RDPMC_BASE +#define INTEL_RDPMC_TYPE_MASK GENMASK(31, 16) +#define INTEL_RDPMC_INDEX_MASK GENMASK(15, 0) + #define MSR_PMC_FULL_WIDTH_BIT (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0) static void reprogram_fixed_counters(struct kvm_pmu *pmu, u64 data) @@ -82,9 +85,13 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu, /* * Fixed PMCs are supported on all architectural PMUs. Note, KVM only * emulates fixed PMCs for PMU v2+, but the flag itself is still valid, - * i.e. let RDPMC fail due to accessing a non-existent counter. + * i.e. let RDPMC fail due to accessing a non-existent counter. Reject + * attempts to read all other types, which are unknown/unsupported. */ - idx &= ~INTEL_RDPMC_FIXED; + if (idx & INTEL_RDPMC_TYPE_MASK & ~INTEL_RDPMC_FIXED) + return NULL; + + idx &= INTEL_RDPMC_INDEX_MASK; if (fixed) { counters = pmu->fixed_counters; num_counters = pmu->nr_arch_fixed_counters;