[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZN6qvKnltoyzbzDW@google.com>
Date: Thu, 17 Aug 2023 16:18:20 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Jinrong Liang <ljr.kernel@...il.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, Like Xu <likexu@...cent.com>,
David Matlack <dmatlack@...gle.com>,
Aaron Lewis <aaronlewis@...gle.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Wanpeng Li <wanpengli@...cent.com>,
Jinrong Liang <cloudliang@...cent.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 05/11] KVM: selftests: Test consistency of CPUID with
num of gp counters
On Thu, Aug 17, 2023, Sean Christopherson wrote:
> On Mon, Aug 14, 2023, Jinrong Liang wrote:
> > From: Jinrong Liang <cloudliang@...cent.com>
> >
> > Add test to check if non-existent counters can be accessed in guest after
> > determining the number of Intel generic performance counters by CPUID.
> > When the num of counters is less than 3, KVM does not emulate #GP if
> > a counter isn't present due to compatibility MSR_P6_PERFCTRx handling.
> > Nor will the KVM emulate more counters than it can support.
> >
> > Co-developed-by: Like Xu <likexu@...cent.com>
> > Signed-off-by: Like Xu <likexu@...cent.com>
> > Signed-off-by: Jinrong Liang <cloudliang@...cent.com>
> > ---
> > .../kvm/x86_64/pmu_basic_functionality_test.c | 78 +++++++++++++++++++
> > 1 file changed, 78 insertions(+)
> >
> > diff --git a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > index daa45aa285bb..b86033e51d5c 100644
> > --- a/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/pmu_basic_functionality_test.c
> > @@ -16,6 +16,11 @@
> > /* Guest payload for any performance counter counting */
> > #define NUM_BRANCHES 10
> >
> > +static const uint64_t perf_caps[] = {
> > + 0,
> > + PMU_CAP_FW_WRITES,
> > +};
> > +
> > static struct kvm_vm *pmu_vm_create_with_one_vcpu(struct kvm_vcpu **vcpu,
> > void *guest_code)
> > {
> > @@ -164,6 +169,78 @@ static void intel_test_arch_events(void)
> > }
> > }
> >
> > +static void guest_wr_and_rd_msrs(uint32_t base, uint8_t begin, uint8_t offset)
> > +{
> > + uint8_t wr_vector, rd_vector;
> > + uint64_t msr_val;
> > + unsigned int i;
> > +
> > + for (i = begin; i < begin + offset; i++) {
> > + wr_vector = wrmsr_safe(base + i, 0xffff);
> > + rd_vector = rdmsr_safe(base + i, &msr_val);
Unless I'm missing something, there is zero reason to pass "base" and "being"
separately, just do the math in the host. A "base" that isn't actually the base
when viewed without the full context is super confusing.
> > + if (wr_vector == GP_VECTOR || rd_vector == GP_VECTOR)
> > + GUEST_SYNC(GP_VECTOR);
>
> Rather than pass around the "expected" vector, and shuffle #GP vs. the msr_val
> up (which can get false negatives if msr_val == 13), just read
> MSR_IA32_PERF_CAPABILITIES from within the guest and GUEST_ASSERT accordingly.
Ah, you did that so that the fixed counter test can reuse the guest code. Just
use separate trampolines in the guest, e.g.
static void __guest_wrmsr_rdmsr(uint32_t base, uint8_t nr_msrs, bool expect_gp)
{
uint64_t msr_val;
uint8_t vector;
uint32_t i;
for (i = base; i < base + nr_msrs; i++) {
vector = wrmsr_safe(i, 0xffff);
GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,
"...");
vector = rdmsr_safe(i, &msr_val);
GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,
"...");
if (!expect_gp)
GUEST_ASSERT_EQ(msr_val, 0);
}
GUEST_DONE();
}
static void guest_rd_wr_fixed_counter(uint32_t base, uint8_t nr_msrs)
{
__guest_wrmsr_rdmsr(base, nr_msrs, true);
}
static void guest_rd_wr_gp_counter(uint32_t base, uint8_t nr_msrs)
{
uint64_t perf_capabilities = rdmsr();
__guest_wrmsr_rdmsr(base, nr_msrs, !!perf_capabilities);
}
Powered by blists - more mailing lists