Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1610070 > unrolled thread

[PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on SMI

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on SMI kan.liang@intel.com - 2017-03-27 20:50 +0200
    Re: [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on  SMI Thomas Gleixner <tglx@linutronix.de> - 2017-03-28 10:50 +0200
      RE: [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on  SMI "Liang, Kan" <kan.liang@intel.com> - 2017-03-28 15:30 +0200
        RE: [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on  SMI Thomas Gleixner <tglx@linutronix.de> - 2017-03-28 19:10 +0200

#1610070 — [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on SMI

From: Kan Liang <Kan.liang@intel.com>

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 possible cpus.

Signed-off-by: Kan Liang <Kan.liang@intel.com>
---
 arch/x86/events/core.c           | 10 +++++++++
 arch/x86/events/intel/core.c     | 48 ++++++++++++++++++++++++++++++++++++++++
 arch/x86/events/perf_event.h     |  3 +++
 arch/x86/include/asm/msr-index.h |  2 ++
 4 files changed, 63 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..ecb321e 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3174,6 +3174,11 @@ static void intel_pmu_cpu_starting(int cpu)
 
 	cpuc->lbr_sel = NULL;
 
+	if (x86_pmu.attr_freeze_on_smi)
+		msr_set_bit_on_cpu(cpu, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
+	else
+		msr_clear_bit_on_cpu(cpu, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
+
 	if (!cpuc->shared_regs)
 		return;
 
@@ -3595,6 +3600,47 @@ 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 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;
+
+	if (x86_pmu.attr_freeze_on_smi == val)
+		return count;
+
+	if (val)
+		msr_set_bit_on_cpus(cpu_possible_mask, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
+	else
+		msr_clear_bit_on_cpus(cpu_possible_mask, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
+
+	x86_pmu.attr_freeze_on_smi = val;
+
+	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 +3687,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..bdb00fa 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_WHILE_SMM_BIT	14
+#define DEBUGCTLMSR_FREEZE_WHILE_SMM	(1UL << DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT)
 
 #define MSR_PEBS_FRONTEND		0x000003f7
 
-- 
2.7.4

[toc] | [next] | [standalone]

#1610421 — Re: [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on SMI

On Mon, 27 Mar 2017, kan.liang@intel.com wrote:
> +
> +	if (val)
> +		msr_set_bit_on_cpus(cpu_possible_mask, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
> +	else
> +		msr_clear_bit_on_cpus(cpu_possible_mask, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);

This is still not protected against CPU hotplug. What's so hard about:

	get_online_cpus();

	if (val) {
		msr_set_bit_on_cpus(cpu_online_mask, MSR_IA32_DEBUGCTLMSR,
				    DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
	} else {
		msr_clear_bit_on_cpus(cpu_online_mask, MSR_IA32_DEBUGCTLMSR,
				      DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
	}

	put_online_cpus();

Aside of that, when this is set to SMI freeze, what causes a CPU which
comes online after that point to set the bit as well? Nothing AFAICT.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1610872 — RE: [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on SMI

> 
> On Mon, 27 Mar 2017, kan.liang@intel.com wrote:
> > +
> > +	if (val)
> > +		msr_set_bit_on_cpus(cpu_possible_mask,
> MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
> > +	else
> > +		msr_clear_bit_on_cpus(cpu_possible_mask,
> MSR_IA32_DEBUGCTLMSR,
> > +DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
> 
> This is still not protected against CPU hotplug. What's so hard about:
> 
> 	get_online_cpus();
> 
> 	if (val) {
> 		msr_set_bit_on_cpus(cpu_online_mask,
> MSR_IA32_DEBUGCTLMSR,
> 				    DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
> 	} else {
> 		msr_clear_bit_on_cpus(cpu_online_mask,
> MSR_IA32_DEBUGCTLMSR,
> 
> DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
> 	}
> 
> 	put_online_cpus();
> 
> Aside of that, when this is set to SMI freeze, what causes a CPU which
> comes online after that point to set the bit as well? Nothing AFAICT.
> 
> 

I've patched the intel_pmu_cpu_starting.
I think it guarantees that the new online CPU is set.

@@ -3174,6 +3174,11 @@ static void intel_pmu_cpu_starting(int cpu)
 
 	cpuc->lbr_sel = NULL;
 
+	if (x86_pmu.attr_freeze_on_smi)
+		msr_set_bit_on_cpu(cpu, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
+	else
+		msr_clear_bit_on_cpu(cpu, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);
+
 	if (!cpuc->shared_regs)
 		return;

Thanks,
Kan

[toc] | [prev] | [next] | [standalone]

#1611227 — RE: [PATCH V3 2/2] perf/x86: add sysfs entry to freeze counter on SMI

On Tue, 28 Mar 2017, Liang, Kan wrote:
> > On Mon, 27 Mar 2017, kan.liang@intel.com wrote:
> > 	put_online_cpus();
> > 
> > Aside of that, when this is set to SMI freeze, what causes a CPU which
> > comes online after that point to set the bit as well? Nothing AFAICT.
> 
> I've patched the intel_pmu_cpu_starting.
> I think it guarantees that the new online CPU is set.

Only when the hotplug protection of the write is in place.....

> @@ -3174,6 +3174,11 @@ static void intel_pmu_cpu_starting(int cpu)
>  
>  	cpuc->lbr_sel = NULL;
>  
> +	if (x86_pmu.attr_freeze_on_smi)
> +		msr_set_bit_on_cpu(cpu, MSR_IA32_DEBUGCTLMSR, DEBUGCTLMSR_FREEZE_WHILE_SMM_BIT);

Can you please use brackets and line breaks?

Thanks,

	tglx

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web