linux-kernel - Re: [PATCH v7 12/19] KVM: selftests: Test consistency of CPUID with num of fixed counters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZUz4ftgXQnOL_sHJ@google.com>
Date:   Thu, 9 Nov 2023 07:19:26 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Dapeng Mi <dapeng1.mi@...ux.intel.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Kan Liang <kan.liang@...ux.intel.com>,
        Jim Mattson <jmattson@...gle.com>,
        Jinrong Liang <cloudliang@...cent.com>,
        Aaron Lewis <aaronlewis@...gle.com>,
        Like Xu <likexu@...cent.com>
Subject: Re: [PATCH v7 12/19] KVM: selftests: Test consistency of CPUID with
 num of fixed counters

On Thu, Nov 09, 2023, Dapeng Mi wrote:
> 
> On 11/8/2023 8:31 AM, Sean Christopherson wrote:
> > From: Jinrong Liang <cloudliang@...cent.com>
> > 
> > Extend the PMU counters test to verify KVM emulation of fixed counters in
> > addition to general purpose counters.  Fixed counters add an extra wrinkle
> > in the form of an extra supported bitmask.  Thus quoth the SDM:
> > 
> >    fixed-function performance counter 'i' is supported if ECX[i] || (EDX[4:0] > i)
> > 
> > Test that KVM handles a counter being available through either method.
> > 
> > Co-developed-by: Like Xu <likexu@...cent.com>
> > Signed-off-by: Like Xu <likexu@...cent.com>
> > Signed-off-by: Jinrong Liang <cloudliang@...cent.com>
> > Co-developed-by: Sean Christopherson <seanjc@...gle.com>
> > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> > ---
> >   .../selftests/kvm/x86_64/pmu_counters_test.c  | 60 ++++++++++++++++++-
> >   1 file changed, 57 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> > index 6f2d3a64a118..8c934e261f2d 100644
> > --- a/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> > +++ b/tools/testing/selftests/kvm/x86_64/pmu_counters_test.c
> > @@ -285,13 +285,19 @@ __GUEST_ASSERT(expect_gp ? vector == GP_VECTOR : !vector,			\
> >   	       expect_gp ? "#GP" : "no fault", msr, vector)			\
> >   static void guest_rd_wr_counters(uint32_t base_msr, uint8_t nr_possible_counters,
> > -				 uint8_t nr_counters)
> > +				 uint8_t nr_counters, uint32_t or_mask)
> 
> 
> 'or_mask' doesn't show a clear meaning, "counters_bitmap" may be a better
> name.

I don't love "or_mask" either, but I like "counters_bitmap" far less, as it doesn't
provide any hint as to the polarity or behavior.  Readers that aren't familiar with
the kludgy enumeration of PMCs in CPUID won't already know that it's a mask that's
OR-d in, e.g. counters_bitmap could be a replacement, it could be an AND-mask, it
could be something entirely unrelated.  I opted for a name that describe the behavior
because I don't see a way to succintly capture the (IMO) weird enumeration.