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@xxxxxxxxxxx/
