linux-kernel - Re: [PATCH v10 16/29] KVM: selftests: Test Intel PMU architectural events on gp 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: <cce0483f-539b-4be3-838d-af0ec91db8f0@linux.intel.com>
Date: Mon, 15 Jan 2024 10:03:04 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Sean Christopherson <seanjc@...gle.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 v10 16/29] KVM: selftests: Test Intel PMU architectural
 events on gp counters


On 1/13/2024 5:37 AM, Sean Christopherson wrote:
> On Fri, Jan 12, 2024, Dapeng Mi wrote:
>> On 1/10/2024 7:02 AM, Sean Christopherson wrote:
>>> +/*
>>> + * If an architectural event is supported and guaranteed to generate at least
>>> + * one "hit, assert that its count is non-zero.  If an event isn't supported or
>>> + * the test can't guarantee the associated action will occur, then all bets are
>>> + * off regarding the count, i.e. no checks can be done.
>>> + *
>>> + * Sanity check that in all cases, the event doesn't count when it's disabled,
>>> + * and that KVM correctly emulates the write of an arbitrary value.
>>> + */
>>> +static void guest_assert_event_count(uint8_t idx,
>>> +				     struct kvm_x86_pmu_feature event,
>>> +				     uint32_t pmc, uint32_t pmc_msr)
>>> +{
>>> +	uint64_t count;
>>> +
>>> +	count = _rdpmc(pmc);
>>> +	if (!this_pmu_has(event))
>>> +		goto sanity_checks;
>>> +
>>> +	switch (idx) {
>>> +	case INTEL_ARCH_INSTRUCTIONS_RETIRED_INDEX:
>>> +		GUEST_ASSERT_EQ(count, NUM_INSNS_RETIRED);
>>> +		break;
>>> +	case INTEL_ARCH_BRANCHES_RETIRED_INDEX:
>>> +		GUEST_ASSERT_EQ(count, NUM_BRANCHES);
>>> +		break;
>>> +	case INTEL_ARCH_CPU_CYCLES_INDEX:
>>> +	case INTEL_ARCH_REFERENCE_CYCLES_INDEX:
>> Since we already support slots event in below guest_test_arch_event(), we
>> can add check for INTEL_ARCH_TOPDOWN_SLOTS_INDEX here.
> Can that actually be tested at this point, since KVM doesn't support
> X86_PMU_FEATURE_TOPDOWN_SLOTS, i.e. this_pmu_has() above should always fail, no?

I suppose X86_PMU_FEATURE_TOPDOWN_SLOTS has been supported in KVM.  The 
following output comes from a guest with latest kvm-x86 code on the 
Sapphire Rapids platform.

sudo cpuid -l 0xa
CPU 0:
    Architecture Performance Monitoring Features (0xa):
       version ID                               = 0x2 (2)
       number of counters per logical processor = 0x8 (8)
       bit width of counter                     = 0x30 (48)
       length of EBX bit vector                 = 0x8 (8)
       core cycle event                         = available
       instruction retired event                = available
       reference cycles event                   = available
       last-level cache ref event               = available
       last-level cache miss event              = available
       branch inst retired event                = available
       branch mispred retired event             = available
       top-down slots event                     = available

Current KVM doesn't support fixed counter 3 and pseudo slots event yet, 
but the architectural slots event is supported and can be programed on a 
GP counter. Current test code can cover this case, so I think we'd 
better add the check for the slots count.


>
> I'm hesitant to add an assertion of any king without the ability to actually test
> the code.