Messages in this thread | | | Subject | Re: [PATCH V2 1/3] perf/x86: Disable non generic regs for software/probe events | From | "Liang, Kan" <> | Date | Tue, 28 May 2019 09:33:40 -0400 |
| |
On 5/28/2019 4:56 AM, Peter Zijlstra wrote: > On Mon, May 27, 2019 at 12:07:55PM -0700, kan.liang@linux.intel.com wrote: >> diff --git a/arch/x86/include/uapi/asm/perf_regs.h b/arch/x86/include/uapi/asm/perf_regs.h >> index ac67bbe..3a96971 100644 >> --- a/arch/x86/include/uapi/asm/perf_regs.h >> +++ b/arch/x86/include/uapi/asm/perf_regs.h >> @@ -52,4 +52,7 @@ enum perf_event_x86_regs { >> /* These include both GPRs and XMMX registers */ >> PERF_REG_X86_XMM_MAX = PERF_REG_X86_XMM15 + 2, >> }; >> + >> +#define PERF_REG_NON_GENERIC_MASK (~((1ULL << PERF_REG_X86_XMM0) - 1)) >> + >> #endif /* _ASM_X86_PERF_REGS_H */ >> diff --git a/include/linux/perf_regs.h b/include/linux/perf_regs.h >> index 4767474..1d794355 100644 >> --- a/include/linux/perf_regs.h >> +++ b/include/linux/perf_regs.h >> @@ -11,6 +11,11 @@ struct perf_regs { >> >> #ifdef CONFIG_HAVE_PERF_REGS >> #include <asm/perf_regs.h> >> + >> +#ifndef PERF_REG_NON_GENERIC_MASK >> +#define PERF_REG_NON_GENERIC_MASK 0 >> +#endif >> + >> u64 perf_reg_value(struct pt_regs *regs, int idx); >> int perf_reg_validate(u64 mask); >> u64 perf_reg_abi(struct task_struct *task); >> @@ -18,6 +23,9 @@ void perf_get_regs_user(struct perf_regs *regs_user, >> struct pt_regs *regs, >> struct pt_regs *regs_user_copy); >> #else >> + >> +#define PERF_REG_NON_GENERIC_MASK 0 >> + >> static inline u64 perf_reg_value(struct pt_regs *regs, int idx) >> { >> return 0; > > Much better than the last version; however.. > >> diff --git a/kernel/events/core.c b/kernel/events/core.c >> index abbd4b3..4865bdf 100644 >> --- a/kernel/events/core.c >> +++ b/kernel/events/core.c >> @@ -8457,6 +8457,12 @@ static void sw_perf_event_destroy(struct perf_event *event) >> swevent_hlist_put(); >> } >> >> +static inline bool has_non_generic_regs(struct perf_event *event) >> +{ >> + return (event->attr.sample_regs_user & PERF_REG_NON_GENERIC_MASK) || >> + (event->attr.sample_regs_intr & PERF_REG_NON_GENERIC_MASK); >> +} >> + >> static int perf_swevent_init(struct perf_event *event) >> { >> u64 event_id = event->attr.config; >> @@ -8470,6 +8476,10 @@ static int perf_swevent_init(struct perf_event *event) >> if (has_branch_stack(event)) >> return -EOPNOTSUPP; >> >> + /* Only support generic registers */ >> + if (has_non_generic_regs(event)) >> + return -EOPNOTSUPP; >> + >> switch (event_id) { >> case PERF_COUNT_SW_CPU_CLOCK: >> case PERF_COUNT_SW_TASK_CLOCK: >> @@ -8633,6 +8643,10 @@ static int perf_tp_event_init(struct perf_event *event) >> if (has_branch_stack(event)) >> return -EOPNOTSUPP; >> >> + /* Only support generic registers */ >> + if (has_non_generic_regs(event)) >> + return -EOPNOTSUPP; >> + >> err = perf_trace_init(event); >> if (err) >> return err; >> @@ -8722,6 +8736,10 @@ static int perf_kprobe_event_init(struct perf_event *event) >> if (has_branch_stack(event)) >> return -EOPNOTSUPP; >> >> + /* Only support generic registers */ >> + if (has_non_generic_regs(event)) >> + return -EOPNOTSUPP; >> + >> is_retprobe = event->attr.config & PERF_PROBE_CONFIG_IS_RETPROBE; >> err = perf_kprobe_init(event, is_retprobe); >> if (err) >> @@ -8782,6 +8800,10 @@ static int perf_uprobe_event_init(struct perf_event *event) >> if (has_branch_stack(event)) >> return -EOPNOTSUPP; >> >> + /* Only support generic registers */ >> + if (has_non_generic_regs(event)) >> + return -EOPNOTSUPP; >> + >> is_retprobe = event->attr.config & PERF_PROBE_CONFIG_IS_RETPROBE; >> ref_ctr_offset = event->attr.config >> PERF_UPROBE_REF_CTR_OFFSET_SHIFT; >> err = perf_uprobe_init(event, ref_ctr_offset, is_retprobe); >> @@ -9562,6 +9584,10 @@ static int cpu_clock_event_init(struct perf_event *event) >> if (has_branch_stack(event)) >> return -EOPNOTSUPP; >> >> + /* Only support generic registers */ >> + if (has_non_generic_regs(event)) >> + return -EOPNOTSUPP; >> + >> perf_swevent_init_hrtimer(event); >> >> return 0; >> @@ -9643,6 +9669,10 @@ static int task_clock_event_init(struct perf_event *event) >> if (has_branch_stack(event)) >> return -EOPNOTSUPP; >> >> + /* Only support generic registers */ >> + if (has_non_generic_regs(event)) >> + return -EOPNOTSUPP; >> + >> perf_swevent_init_hrtimer(event); >> >> return 0; > > I don't think this is anywhere near sufficient. What happens if we > request XMM regs for an uncore PMU ? >
Uncore PMU doesn't support sampling. It will return -EINVAL. There is no regs support for counting. The request will be ignored.
I think current check for uncore is good enough.
Thanks, Kan
> I'm thinking you want something along these lines... > > --- > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > index a5436cee20b1..3ef1c2e0b177 100644 > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -3281,7 +3281,13 @@ static int intel_pmu_hw_config(struct perf_event *event) > > if (event->attr.sample_type & PERF_SAMPLE_CALLCHAIN) > event->attr.sample_type |= __PERF_SAMPLE_CALLCHAIN_EARLY; > - } > + > + /* we only support extended (XMM) registers for sample_regs_intr */ > + if (event->attr.sample_regs_user & PERF_REGS_EXTENDED_MASK) > + return -EOPNOTSUPP; > + > + } else if (has_extended_regs(event)) > + return -EOPNOTSUPP; > > if (needs_branch_stack(event)) { > ret = intel_pmu_setup_lbr_filter(event); > diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c > index 5e9bb246b3a6..4fae37f8c7c2 100644 > --- a/arch/x86/events/intel/ds.c > +++ b/arch/x86/events/intel/ds.c > @@ -2020,6 +2020,7 @@ void __init intel_ds_init(void) > PERF_SAMPLE_TIME; > x86_pmu.flags |= PMU_FL_PEBS_ALL; > pebs_qual = "-baseline"; > + x86_get_pmu()->capabilities |= PERF_PMU_CAP_EXTENDED_REGS; > } else { > /* Only basic record supported */ > x86_pmu.pebs_no_xmm_regs = 1; > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h > index 0ab99c7b652d..2bca72f3028b 100644 > --- a/include/linux/perf_event.h > +++ b/include/linux/perf_event.h > @@ -241,6 +241,7 @@ struct perf_event; > #define PERF_PMU_CAP_NO_INTERRUPT 0x01 > #define PERF_PMU_CAP_NO_NMI 0x02 > #define PERF_PMU_CAP_AUX_NO_SG 0x04 > +#define PERF_PMU_CAP_EXTENDED_REGS 0x08 > #define PERF_PMU_CAP_EXCLUSIVE 0x10 > #define PERF_PMU_CAP_ITRACE 0x20 > #define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x40 > diff --git a/kernel/events/core.c b/kernel/events/core.c > index abbd4b3b96c2..0c4872426b70 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -10064,12 +10064,16 @@ static int perf_try_init_event(struct pmu *pmu, struct perf_event *event) > perf_event_ctx_unlock(event->group_leader, ctx); > > if (!ret) { > + if (!(pmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS) && > + has_extended_regs(event)) > + ret = -EOPNOTSUPP; > + > if (pmu->capabilities & PERF_PMU_CAP_NO_EXCLUDE && > - event_has_any_exclude_flag(event)) { > - if (event->destroy) > - event->destroy(event); > + event_has_any_exclude_flag(event)) > ret = -EINVAL; > - } > + > + if (ret && event->destroy) > + event->destroy(event); > } > > if (ret) >
| |