Messages in this thread | | | Date | Wed, 31 Jan 2024 10:11:20 +0800 | Subject | Re: [PATCH v10 16/29] KVM: selftests: Test Intel PMU architectural events on gp counters | From | "Mi, Dapeng" <> |
| |
On 1/31/2024 7:27 AM, Sean Christopherson wrote: > On Mon, Jan 15, 2024, Dapeng Mi wrote: >> 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. > Can you submit a patch on top, with a changelog that includes justification that > that explains exactly what assertions can be made on the top-down slots event > given the "workload" being measured? I'm definitely not opposed to adding coverage > for top-down slots, but at this point, I don't want to respin this series, nor do > I want to make that change when applying on the fly.
Yeah, I'm glad to submit a patch for this. :)
BTW, I have a patch series to do the bug fixes and improvements for kvm-unit-tests/pmu test. (some improvement ideas come from this patchset.)
https://lore.kernel.org/kvm/20240103031409.2504051-1-dapeng1.mi@linux.intel.com/
Could you please kindly review them? Thanks.
>
| |