On Thursday, September 15, 2022 4:26 AM, Liang, Kan wrote: > The perf_event_disable() eventually invokes the intel_pt_stop(). > We already expose the intel_pt_stop()/cpu_emergency_stop_pt() to other > modules. I don't think we have to use the perf_event_disable(). Also, the > perf_event_disable() requires extra codes. > > I went through the discussions. I agree with Sean's suggestion. > We should only put the logic in the KVM but all the MSR access details into the PT > driver. Even the driver itself doesn’t drive the save/restore of the MSRs, it is drived by perf. 1. If we make KVM a user of perf, we should do this via perf_event_disable/enable_*. 2. If we make KVM an alternative to perf (i.e. have direct control over PMU HW), we can do this via driver interfaces like perf. Per my experience, we should go for 1. Probably need Peter's opinions on this. > But I prefer a more generic and straightforward function name, e.g., > intel_pt_stop_save()/intel_pt_start_load(), in case other modules may want to > save/restore the PT information in their context switch later. > > Thanks, > Kan > > > > >> It seems perf_event_disable() is not used widely by other kernel > >> component. Because there are not lots of kernel users. You can check another user, watchdog_hld.c, perf_event_enable/disable are used there.