RE: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI

[Liang, Kan]

Hi tglx,

Are you OK with patch?
Could I get your "acked-by"?

Thanks,
Kan
> 
> 
> Ping.
> Any comments for the patch?
> 
> Thanks,
> Kan
> 
> > Subject: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI
> >
> > From: Kan Liang <Kan.liang@xxxxxxxxx>
> >
> > Currently, the SMIs are visible to all performance counters. Because
> > many users want to measure everything including SMIs. But in some
> > cases, the SMI cycles should not be count. For example, to calculate
> > the cost of SMI itself. So a knob is needed.
> >
> > When setting FREEZE_WHILE_SMM bit in IA32_DEBUGCTL, all performance
> > counters will be effected. There is no way to do per-counter freeze on SMI.
> > So it should not use the per-event interface (e.g. ioctl or event
> > attribute) to set FREEZE_WHILE_SMM bit.
> >
> > Adds sysfs entry /sys/device/cpu/freeze_on_smi to set
> FREEZE_WHILE_SMM
> > bit in IA32_DEBUGCTL. When set, freezes perfmon and trace messages
> > while in SMM.
> > Value has to be 0 or 1. It will be applied to all processors.
> > Also serialize the entire setting so we don't get multiple concurrent
> > threads trying to update to different values.
> >
> > Signed-off-by: Kan Liang <Kan.liang@xxxxxxxxx>
> > ---
> >
> > Changes since V4:
> >  - drop msr_flip_bit function
> >  - Use on_each_cpu() which already include all the needed protection
> >  - Some small changes according to the comments
> >
> >  arch/x86/events/core.c           | 10 +++++++
> >  arch/x86/events/intel/core.c     | 63
> > ++++++++++++++++++++++++++++++++++++++++
> >  arch/x86/events/perf_event.h     |  3 ++
> >  arch/x86/include/asm/msr-index.h |  2 ++
> >  4 files changed, 78 insertions(+)
> >
> > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index
> > 349d4d1..c16fb50 100644
> > --- a/arch/x86/events/core.c
> > +++ b/arch/x86/events/core.c
> > @@ -1750,6 +1750,8 @@ ssize_t x86_event_sysfs_show(char *page, u64
> > config, u64 event)
> >  	return ret;
> >  }
> >
> > +static struct attribute_group x86_pmu_attr_group;
> > +
> >  static int __init init_hw_perf_events(void)  {
> >  	struct x86_pmu_quirk *quirk;
> > @@ -1813,6 +1815,14 @@ static int __init init_hw_perf_events(void)
> >  			x86_pmu_events_group.attrs = tmp;
> >  	}
> >
> > +	if (x86_pmu.attrs) {
> > +		struct attribute **tmp;
> > +
> > +		tmp = merge_attr(x86_pmu_attr_group.attrs, x86_pmu.attrs);
> > +		if (!WARN_ON(!tmp))
> > +			x86_pmu_attr_group.attrs = tmp;
> > +	}
> > +
> >  	pr_info("... version:                %d\n",     x86_pmu.version);
> >  	pr_info("... bit width:              %d\n",     x86_pmu.cntval_bits);
> >  	pr_info("... generic registers:      %d\n",     x86_pmu.num_counters);
> > diff --git a/arch/x86/events/intel/core.c
> > b/arch/x86/events/intel/core.c index
> > 4244bed..a5bc4e4 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -3160,6 +3160,19 @@ static int intel_pmu_cpu_prepare(int cpu)
> >  	return -ENOMEM;
> >  }
> >
> > +static void flip_smm_bit(void *data)
> > +{
> > +	bool set = *(int *)data;
> > +
> > +	if (set) {
> > +		msr_set_bit(MSR_IA32_DEBUGCTLMSR,
> > +			    DEBUGCTLMSR_FREEZE_IN_SMM_BIT);
> > +	} else {
> > +		msr_clear_bit(MSR_IA32_DEBUGCTLMSR,
> > +			      DEBUGCTLMSR_FREEZE_IN_SMM_BIT);
> > +	}
> > +}
> > +
> >  static void intel_pmu_cpu_starting(int cpu)  {
> >  	struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu); @@ -
> > 3174,6 +3187,8 @@ static void intel_pmu_cpu_starting(int cpu)
> >
> >  	cpuc->lbr_sel = NULL;
> >
> > +	flip_smm_bit(&x86_pmu.attr_freeze_on_smi);
> > +
> >  	if (!cpuc->shared_regs)
> >  		return;
> >
> > @@ -3595,6 +3610,52 @@ static struct attribute *hsw_events_attrs[] = {
> >  	NULL
> >  };
> >
> > +static ssize_t freeze_on_smi_show(struct device *cdev,
> > +				  struct device_attribute *attr,
> > +				  char *buf)
> > +{
> > +	return sprintf(buf, "%d\n", x86_pmu.attr_freeze_on_smi); }
> > +
> > +static DEFINE_MUTEX(freeze_on_smi_mutex);
> > +
> > +static ssize_t freeze_on_smi_store(struct device *cdev,
> > +				   struct device_attribute *attr,
> > +				   const char *buf, size_t count) {
> > +	unsigned long val;
> > +	ssize_t ret;
> > +
> > +	ret = kstrtoul(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (val > 1)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&freeze_on_smi_mutex);
> > +
> > +	if (x86_pmu.attr_freeze_on_smi == val)
> > +		goto done;
> > +
> > +	x86_pmu.attr_freeze_on_smi = val;
> > +
> > +	get_online_cpus();
> > +	on_each_cpu(flip_smm_bit, &val, 1);
> > +	put_online_cpus();
> > +done:
> > +	mutex_unlock(&freeze_on_smi_mutex);
> > +
> > +	return count;
> > +}
> > +
> > +static DEVICE_ATTR_RW(freeze_on_smi);
> > +
> > +static struct attribute *intel_pmu_attrs[] = {
> > +	&dev_attr_freeze_on_smi.attr,
> > +	NULL,
> > +};
> > +
> >  __init int intel_pmu_init(void)
> >  {
> >  	union cpuid10_edx edx;
> > @@ -3641,6 +3702,8 @@ __init int intel_pmu_init(void)
> >
> >  	x86_pmu.max_pebs_events		= min_t(unsigned,
> > MAX_PEBS_EVENTS, x86_pmu.num_counters);
> >
> > +
> > +	x86_pmu.attrs			= intel_pmu_attrs;
> >  	/*
> >  	 * Quirk: v2 perfmon does not report fixed-purpose events, so
> >  	 * assume at least 3 events, when not running in a hypervisor:
> > diff --git a/arch/x86/events/perf_event.h
> > b/arch/x86/events/perf_event.h index bcbb1d2..110cb9b0 100644
> > --- a/arch/x86/events/perf_event.h
> > +++ b/arch/x86/events/perf_event.h
> > @@ -561,6 +561,9 @@ struct x86_pmu {
> >  	ssize_t		(*events_sysfs_show)(char *page, u64 config);
> >  	struct attribute **cpu_events;
> >
> > +	int		attr_freeze_on_smi;
> > +	struct attribute **attrs;
> > +
> >  	/*
> >  	 * CPU Hotplug hooks
> >  	 */
> > diff --git a/arch/x86/include/asm/msr-index.h
> > b/arch/x86/include/asm/msr- index.h index d8b5f8a..0572f91 100644
> > --- a/arch/x86/include/asm/msr-index.h
> > +++ b/arch/x86/include/asm/msr-index.h
> > @@ -134,6 +134,8 @@
> >  #define DEBUGCTLMSR_BTS_OFF_OS		(1UL <<  9)
> >  #define DEBUGCTLMSR_BTS_OFF_USR		(1UL << 10)
> >  #define DEBUGCTLMSR_FREEZE_LBRS_ON_PMI	(1UL << 11)
> > +#define DEBUGCTLMSR_FREEZE_IN_SMM_BIT	14
> > +#define DEBUGCTLMSR_FREEZE_IN_SMM	(1UL <<
> > DEBUGCTLMSR_FREEZE_IN_SMM_BIT)
> >
> >  #define MSR_PEBS_FRONTEND		0x000003f7
> >
> > --
> > 2.7.4