Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Jun 2023 17:50:41 -0700
In-Reply-To: <20230530060423.32361-6-likexu@tencent.com>
Mime-Version: 1.0
References: <20230530060423.32361-1-likexu@tencent.com> <20230530060423.32361-6-likexu@tencent.com>
Message-ID: <ZHk84Ts9txpz5djC@google.com>
Subject: Re: [PATCH v6 05/10] KVM: x86/pmu: Disable vPMU if the minimum num of
 counters isn't met
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, Jim Mattson <jmattson@google.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Tue, May 30, 2023, Like Xu wrote:
> According to Intel SDM, "Intel Core Solo and Intel Core Duo processors
> support base level functionality identified by version ID of 1. Processors
> based on Intel Core microarchitecture support, at a minimum, the base
> level functionality of architectural performance monitoring." Those
> antique processors mentioned above all had at least two GP counters,
> subsequent processors had more and more GP counters, and given KVM's
> quirky handling of MSR_P6_PERFCTR0/1, the value of MIN_NR_GP_COUNTERS
> for the Intel Arch PMU can safely be 2.

Not sure what you mean by "safely", but I don't think this is correct.  KVM can,
and more importantly has up until this point, supported a vPMU so long as the CPU
has at least one counter.  Perf's support for P6/Core CPUs does appear to expect
and require 2 counters, but unless I'm misreading arch/x86/events/intel/core.c,
perf will happily chug along with a single counter when running on a modern CPU.
I doubt such a CPU exists in real silicon, but I can certainly imagine a virtual
CPU being configured with a single counter, and this change would break such a
setup.

And *if* we really want to raise the minimum to '2', that should be done in a
separate commit.  But I don't see any reason to force the issue.

No need to send v7 just for this, I can fixup when applying (planning on reviewing
the series tomorrow).