On Thu, Jul 22, 2021 at 01:41:41PM +0800, Zhu Lingshan wrote: > The guest Precise Event Based Sampling (PEBS) feature can provide an > architectural state of the instruction executed after the guest instruction > that exactly caused the event. It needs new hardware facility only available > on Intel Ice Lake Server platforms. This patch set enables the basic PEBS > feature for KVM guests on ICX. > > We can use PEBS feature on the Linux guest like native: > > # echo 0 > /proc/sys/kernel/watchdog (on the host) > # perf record -e instructions:ppp ./br_instr a > # perf record -c 100000 -e instructions:pp ./br_instr a Why does the host need to disable the watchdog? IIRC ICL has multiple PEBS capable counters. Also, I think the watchdog ends up on a fixed counter by default anyway. > Like Xu (17): > perf/core: Use static_call to optimize perf_guest_info_callbacks > perf/x86/intel: Add EPT-Friendly PEBS for Ice Lake Server > perf/x86/intel: Handle guest PEBS overflow PMI for KVM guest > perf/x86/core: Pass "struct kvm_pmu *" to determine the guest values > KVM: x86/pmu: Set MSR_IA32_MISC_ENABLE_EMON bit when vPMU is enabled > KVM: x86/pmu: Introduce the ctrl_mask value for fixed counter > KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS > KVM: x86/pmu: Reprogram PEBS event to emulate guest PEBS counter > KVM: x86/pmu: Adjust precise_ip to emulate Ice Lake guest PDIR counter > KVM: x86/pmu: Add IA32_DS_AREA MSR emulation to support guest DS > KVM: x86/pmu: Add PEBS_DATA_CFG MSR emulation to support adaptive PEBS > KVM: x86: Set PEBS_UNAVAIL in IA32_MISC_ENABLE when PEBS is enabled > KVM: x86/pmu: Move pmc_speculative_in_use() to arch/x86/kvm/pmu.h > KVM: x86/pmu: Disable guest PEBS temporarily in two rare situations > KVM: x86/pmu: Add kvm_pmu_cap to optimize perf_get_x86_pmu_capability > KVM: x86/cpuid: Refactor host/guest CPU model consistency check > KVM: x86/pmu: Expose CPUIDs feature bits PDCM, DS, DTES64 > > Peter Zijlstra (Intel) (1): > x86/perf/core: Add pebs_capable to store valid PEBS_COUNTER_MASK value Looks good: Acked-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> How do we want to route this, all through the KVM tree? One little nit I had; would something like the below (on top perhaps) make the code easier to read? --- --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -3921,9 +3921,12 @@ static struct perf_guest_switch_msr *int struct kvm_pmu *kvm_pmu = (struct kvm_pmu *)data; u64 intel_ctrl = hybrid(cpuc->pmu, intel_ctrl); u64 pebs_mask = cpuc->pebs_enabled & x86_pmu.pebs_capable; + int global_ctrl, pebs_enable; *nr = 0; - arr[(*nr)++] = (struct perf_guest_switch_msr){ + + global_ctrl = (*nr)++; + arr[global_ctrl] = (struct perf_guest_switch_msr){ .msr = MSR_CORE_PERF_GLOBAL_CTRL, .host = intel_ctrl & ~cpuc->intel_ctrl_guest_mask, .guest = intel_ctrl & (~cpuc->intel_ctrl_host_mask | ~pebs_mask), @@ -3966,23 +3969,23 @@ static struct perf_guest_switch_msr *int }; } - arr[*nr] = (struct perf_guest_switch_msr){ + pebs_enable = (*nr)++; + arr[pebs_enable] = (struct perf_guest_switch_msr){ .msr = MSR_IA32_PEBS_ENABLE, .host = cpuc->pebs_enabled & ~cpuc->intel_ctrl_guest_mask, .guest = pebs_mask & ~cpuc->intel_ctrl_host_mask, }; - if (arr[*nr].host) { + if (arr[pebs_enable].host) { /* Disable guest PEBS if host PEBS is enabled. */ - arr[*nr].guest = 0; + arr[pebs_enable].guest = 0; } else { /* Disable guest PEBS for cross-mapped PEBS counters. */ - arr[*nr].guest &= ~kvm_pmu->host_cross_mapped_mask; + arr[pebs_enable].guest &= ~kvm_pmu->host_cross_mapped_mask; /* Set hw GLOBAL_CTRL bits for PEBS counter when it runs for guest */ - arr[0].guest |= arr[*nr].guest; + arr[global_ctrl].guest |= arr[pebs_enable].guest; } - ++(*nr); return arr; }