Messages in this thread | | | Date | Thu, 28 Jul 2022 19:31:49 +0800 | Subject | Re: [kvm-unit-tests PATCH] x86: Add tests for Guest Processor Event Based Sampling (PEBS) | From | Like Xu <> |
| |
On 28/7/2022 6:42 am, Sean Christopherson wrote: > On Thu, Jul 21, 2022, Like Xu wrote: >> +union perf_capabilities { >> + struct { >> + u64 lbr_format:6; >> + u64 pebs_trap:1; >> + u64 pebs_arch_reg:1; >> + u64 pebs_format:4; >> + u64 smm_freeze:1; >> + /* >> + * PMU supports separate counter range for writing >> + * values > 32bit. >> + */ >> + u64 full_width_write:1; >> + u64 pebs_baseline:1; >> + u64 perf_metrics:1; >> + u64 pebs_output_pt_available:1; >> + u64 anythread_deprecated:1; >> + }; >> + u64 capabilities; >> +}; >> + >> +union cpuid10_eax { >> + struct { >> + unsigned int version_id:8; >> + unsigned int num_counters:8; >> + unsigned int bit_width:8; >> + unsigned int mask_length:8; >> + } split; >> + unsigned int full; >> +} pmu_eax; >> + >> +union cpuid10_edx { >> + struct { >> + unsigned int num_counters_fixed:5; >> + unsigned int bit_width_fixed:8; >> + unsigned int reserved:19; >> + } split; >> + unsigned int full; >> +} pmu_edx; > > The generic unions are hopefully unnecessary now that helpers are provided by > lib/x86/processor.h, e.g. for pmu_version(). > > I would prefer to have similar helpers instead of "union perf_capabilities", > but it's not a sticking point if helpers a signifiantly more painful to use. > >> + if (!is_intel() || (pmu_eax.split.version_id < 2) || >> + !(perf.capabilities & PERF_CAP_PEBS_FORMAT) || >> + (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL)) { > > Split these up, it's really, really annoying to have to guess which one of the > four checks failed. > >> + report_skip("This platform doesn't support guest PEBS."); >> + return 0; > > This needs be be "return report_summary()", otherwise the test says pass when it > didn't do anyting: > > TESTNAME=pmu_pebs TIMEOUT=90s ACCEL=kvm ./x86/run x86/pmu_pebs.flat -smp 1 -cpu host,migratable=no > PASS pmu_pebs > > wait a second... > > SKIP: This platform doesn't support guest PEBS. > > E.g. (though if KUT can provide more information on why PERF_CAP_PEBS_FORMAT > may not be advertised, e.g. requires ICX+?, that would be nice to have) > > if (!is_intel()) { > report_skip("PEBS is only supported on Intel CPUs"); > return report_summary(); > } > if (pmu_version() < 2) { > report_skip("Architectural PMU not available"); > return report_summary(); > } > if (!(perf.capabilities & PERF_CAP_PEBS_FORMAT)) { > report_skip("PEBS not enumerated in PERF_CAPABILITIES"); > return report_summary(); > } > if (rdmsr(MSR_IA32_MISC_ENABLE) & MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL) { > report_skip("PEBS unavailable according to MISC_ENABLE"); > return report_summary(); > }
Thanks, all applied. Please review this new version as a separate thread. https://lore.kernel.org/kvm/20220728112119.58173-1-likexu@tencent.com/
| |