On Mon, May 27, 2019 at 12:07:55PM -0700, [email protected] 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 ?

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)

Reply via email to