On Tue, May 18, 2021 at 03:38:52PM +0800, Xu, Like wrote: > > I'm thinking you have your conditions in the wrong order; would it not > > be much cheaper to first check: '!x86_pmu.pebs_active || !guest_pebs_idx' > > than to do that horrible indirect ->is_in_guest() call? > > > > After all, if the guest doesn't have PEBS enabled, who cares if we're > > currently in a guest or not. > > Yes, it makes sense. How about: > > @@ -2833,6 +2867,10 @@ static int handle_pmi_common(struct pt_regs *regs, > u64 status) > u64 pebs_enabled = cpuc->pebs_enabled; > > handled++; > + if (x86_pmu.pebs_vmx && x86_pmu.pebs_active && > + (cpuc->pebs_enabled & ~cpuc->intel_ctrl_host_mask) && > + (static_call(x86_guest_state)() & PERF_GUEST_ACTIVE)) > + x86_pmu_handle_guest_pebs(regs, &data); This is terruble, just call x86_pmu_handle_guest_pebs() unconditionally and put all the ugly inside it. > x86_pmu.drain_pebs(regs, &data); > status &= intel_ctrl | GLOBAL_STATUS_TRACE_TOPAPMI; > > > > > Also, something like the below perhaps (arm64 and xen need fixing up at > > the very least) could make all that perf_guest_cbs stuff suck less. > > How about the commit message for your below patch: > > From: "Peter Zijlstra (Intel)" <peterz@xxxxxxxxxxxxx> > > x86/core: Use static_call to rewrite perf_guest_info_callbacks > > The two fields named "is_in_guest" and "is_user_mode" in > perf_guest_info_callbacks are replaced with a new multiplexed member > named "state", and the "get_guest_ip" field will be renamed to "get_ip". > > The application of DEFINE_STATIC_CALL_RET0 (arm64 and xen need fixing > up at the very least) could make all that perf_guest_cbs stuff suck less. > For KVM, these callbacks will be updated in the kvm_arch_init(). > > Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> Well, you *do* need to fix up arm64 and xen, we can't very well break their builds can we now. > ---- > > I'm not sue if you have a strong reason to violate the check-patch rule: > > ERROR: Using weak declarations can have unintended link defects > #238: FILE: include/linux/perf_event.h:1242: > +extern void __weak arch_perf_update_guest_cbs(void); Copy/paste fail I think. I didn't really put much effort into the patch, only made sure defconfig+kvm_guest.config compiled.
