linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management
@ 2012-08-20 16:22 Stephane Eranian
  2012-08-21  8:00 ` Yan, Zheng
  2012-08-21 10:46 ` Peter Zijlstra
  0 siblings, 2 replies; 6+ messages in thread
From: Stephane Eranian @ 2012-08-20 16:22 UTC (permalink / raw)
  To: linux-kernel; +Cc: peterz, mingo, andi, zheng.z.yan

The existing code had a bug whereby it would refuse to
measure two events in a group for either CBO or PCU PMUs,
if one of the events was using a filter. This was due to
the fact that the kernel assumed all CBO and PCU events
were using filters, and thus would detect false positive
conflicts between attr->config1 values.
    
This patch fixes the problem by introducing the extra_reg
event mapping tables for both CBO and PCU PMUs. Those
tables associate an event code+umask with extra (filter)
register. We used the same approach for the offcore_response
core PMU event.
    
With this patch applied, it is possible to measure, for
instance, CBO unc_c_tor_inserts:opcode:pcirdcur with
unc_c_clockticks in the same group.

The filter for both CBO and PCU are more complex than what the
current code support. Each filter is sub-divided into event
specific filters. For now, we consider the filter as one single
MSR value. We lose a bit of flexibility and force multiplexing
when this is not really necessary in HW. We could later create
the notion of virtual filters that would be aggregated at the
last step (wrmsrl). But I think this is good enough for now.

Signed-off-by: Stephane Eranian <eranian@google.com>
---

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 0a55710..cd2c930 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -224,29 +224,86 @@ static void snbep_uncore_msr_init_box(struct intel_uncore_box *box)
 		wrmsrl(msr, SNBEP_PMON_BOX_CTL_INT);
 }
 
-static int snbep_uncore_hw_config(struct intel_uncore_box *box, struct perf_event *event)
+static int uncore_pmu_extra_regs(struct intel_uncore_box *box,
+				 struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg;
+	struct extra_reg *er = box->pmu->type->extra_regs;
+
+	if (!er)
+		return 0;
+
+	reg = &event->hw.extra_reg;
+
+	for (; er->msr; er++) {
+		if (er->event != (event->attr.config & er->config_mask))
+			continue;
+
+		if (event->attr.config1 & ~er->valid_mask)
+			return -EINVAL;
+
+		reg->idx = er->idx;
+		reg->config = event->attr.config1;
+		reg->reg = er->msr;
+		break;
+	}
+	return 0;
+}
+
+
+static int snbep_cbo_extra_regs(struct intel_uncore_box *box,
+				struct perf_event *event)
 {
+#define SNBEP_CBO_TIDEN_MASK		(1ULL<< 19)
+#define SNBEP_CBO_FILT_CIDTID_MASK	0xfULL
+
 	struct hw_perf_event *hwc = &event->hw;
-	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
+	struct hw_perf_event_extra *reg = &hwc->extra_reg;
+	u64 config1 = event->attr.config1;
 
-	if (box->pmu->type == &snbep_uncore_cbox) {
-		reg1->reg = SNBEP_C0_MSR_PMON_BOX_FILTER +
-			SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
-		reg1->config = event->attr.config1 &
-			SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK;
-	} else {
-		if (box->pmu->type == &snbep_uncore_pcu) {
-			reg1->reg = SNBEP_PCU_MSR_PMON_BOX_FILTER;
-			reg1->config = event->attr.config1 & SNBEP_PCU_MSR_PMON_BOX_FILTER_MASK;
-		} else {
-			return 0;
-		}
+	/*
+	 * if extra_reg not set via the snbep_uncore_cbo_extra_regs[]
+	 * then we need to check whether or not the tid_en bit is set
+	 * in the config. If so, then we do need to enable the cbo filter
+	 */
+	if (reg->idx == EXTRA_REG_NONE
+	    && (event->attr.config & SNBEP_CBO_TIDEN_MASK)) {
+		/*
+		 * validate that only the core-id/thread-id
+		 * bits are set, otherwise we must go through
+		 * the snbep_uncore_cbo_extra_regs[] table
+		 */
+		if (config1 & ~SNBEP_CBO_FILT_CIDTID_MASK)
+			return -EINVAL;
+
+		reg->idx = 0;
+		reg->config = config1 & SNBEP_CBO_FILT_CIDTID_MASK;
+		reg->reg = SNBEP_C0_MSR_PMON_BOX_FILTER;
 	}
-	reg1->idx = 0;
+	/*
+	 * adjust the cbo index
+	 */
+	if (reg->idx != EXTRA_REG_NONE)
+		reg->reg += SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
 
 	return 0;
 }
 
+static int snbep_uncore_hw_config(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct intel_uncore_type *type = box->pmu->type;
+	int ret;
+
+	ret = uncore_pmu_extra_regs(box, event);
+	if (ret)
+		return ret;
+
+	if (type == &snbep_uncore_cbox)
+		ret = snbep_cbo_extra_regs(box, event);
+
+	return ret;
+}
+
 static struct attribute *snbep_uncore_formats_attr[] = {
 	&format_attr_event.attr,
 	&format_attr_umask.attr,
@@ -444,6 +501,41 @@ static struct intel_uncore_type snbep_uncore_ubox = {
 	.format_group	= &snbep_uncore_ubox_format_group,
 };
 
+#define CBO_EVENT_EXTRA_REG(a) \
+	UNC_EXTRA_REG(a, \
+		      SNBEP_C0_MSR_PMON_BOX_FILTER, \
+		      ARCH_PERFMON_EVENTSEL_EVENT, \
+		      SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK, \
+		      0)
+
+#define CBO_EVTUM_EXTRA_REG(a) \
+	UNC_EXTRA_REG(a, \
+		      SNBEP_C0_MSR_PMON_BOX_FILTER, \
+		      INTEL_ARCH_EVENT_MASK, \
+		      SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK, \
+		      0)
+
+static struct extra_reg snbep_uncore_cbo_extra_regs[] __read_mostly =
+{
+	CBO_EVENT_EXTRA_REG(0x0034), /* LLC_LOOKUP */
+	CBO_EVTUM_EXTRA_REG(0x0135), /* TOR_INSERTS.OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x0335), /* TOR_INSERTS.MISS_OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x4135), /* TOR_INSERTS.NID_OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x4335), /* TOR_INSERTS.NID_MISS_OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x4435), /* TOR_INSERTS.NID_EVICTION */
+	CBO_EVTUM_EXTRA_REG(0x4835), /* TOR_INSERTS.NID_ALL */
+	CBO_EVTUM_EXTRA_REG(0x4a35), /* TOR_INSERTS.NID_MISS_ALL */
+	CBO_EVTUM_EXTRA_REG(0x5035), /* TOR_INSERTS.NID_WB */
+	CBO_EVTUM_EXTRA_REG(0x0136), /* TOR_OCCUPANCY.OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x0336), /* TOR_OCCUPANCY.MISS_OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x4136), /* TOR_OCCUPANCY.NID_OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x4336), /* TOR_OCCUPANCY.NID_MISS_OPCODE */
+	CBO_EVTUM_EXTRA_REG(0x4436), /* TOR_OCCUPANCY.NID_EVICTION */
+	CBO_EVTUM_EXTRA_REG(0x4836), /* TOR_OCCUPANCY.NID_ALL */
+	CBO_EVTUM_EXTRA_REG(0x4a36), /* TOR_OCCUPANCY.NID_MISS_ALL */
+	EVENT_EXTRA_END
+};
+
 static struct intel_uncore_type snbep_uncore_cbox = {
 	.name			= "cbox",
 	.num_counters		= 4,
@@ -458,6 +550,23 @@ static struct intel_uncore_type snbep_uncore_cbox = {
 	.constraints		= snbep_uncore_cbox_constraints,
 	.ops			= &snbep_uncore_msr_ops,
 	.format_group		= &snbep_uncore_cbox_format_group,
+	.extra_regs		= snbep_uncore_cbo_extra_regs,
+};
+
+#define PCU_EVENT_EXTRA_REG(a) \
+	UNC_EXTRA_REG(a, \
+		      SNBEP_PCU_MSR_PMON_BOX_FILTER, \
+		      ARCH_PERFMON_EVENTSEL_EVENT, \
+		      0xffffffff, \
+		      0)
+
+static struct extra_reg snbep_uncore_pcu_extra_regs[] __read_mostly =
+{
+	PCU_EVENT_EXTRA_REG(0xb), /* FREQ_BAND0_CYCLES */
+	PCU_EVENT_EXTRA_REG(0xc), /* FREQ_BAND1_CYCLES */
+	PCU_EVENT_EXTRA_REG(0xd), /* FREQ_BAND2_CYCLES */
+	PCU_EVENT_EXTRA_REG(0xe), /* FREQ_BAND3_CYCLES */
+	EVENT_EXTRA_END
 };
 
 static struct intel_uncore_type snbep_uncore_pcu = {
@@ -472,6 +581,7 @@ static struct intel_uncore_type snbep_uncore_pcu = {
 	.num_shared_regs	= 1,
 	.ops			= &snbep_uncore_msr_ops,
 	.format_group		= &snbep_uncore_pcu_format_group,
+	.extra_regs		= snbep_uncore_pcu_extra_regs,
 };
 
 static struct intel_uncore_type *snbep_msr_uncores[] = {
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
index 5b81c18..37d8732 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -369,6 +369,7 @@ struct intel_uncore_type {
 	struct intel_uncore_pmu *pmus;
 	struct intel_uncore_ops *ops;
 	struct uncore_event_desc *event_descs;
+	struct extra_reg *extra_regs;
 	const struct attribute_group *attr_groups[3];
 };
 
@@ -428,6 +429,14 @@ struct uncore_event_desc {
 	const char *config;
 };
 
+#define UNC_EXTRA_REG(e, ms, m, vm, i) {\
+	.event = (e),		\
+	.msr = (ms),		\
+	.config_mask = (m),	\
+	.valid_mask = (vm),	\
+	.idx = i\
+	}
+
 #define INTEL_UNCORE_EVENT_DESC(_name, _config)			\
 {								\
 	.attr	= __ATTR(_name, 0444, uncore_event_show, NULL),	\

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management
  2012-08-20 16:22 [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management Stephane Eranian
@ 2012-08-21  8:00 ` Yan, Zheng
  2012-08-21 10:46 ` Peter Zijlstra
  1 sibling, 0 replies; 6+ messages in thread
From: Yan, Zheng @ 2012-08-21  8:00 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: linux-kernel, peterz, mingo, andi

On 08/21/2012 12:22 AM, Stephane Eranian wrote:
> The existing code had a bug whereby it would refuse to
> measure two events in a group for either CBO or PCU PMUs,
> if one of the events was using a filter. This was due to
> the fact that the kernel assumed all CBO and PCU events
> were using filters, and thus would detect false positive
> conflicts between attr->config1 values.
>     
> This patch fixes the problem by introducing the extra_reg
> event mapping tables for both CBO and PCU PMUs. Those
> tables associate an event code+umask with extra (filter)
> register. We used the same approach for the offcore_response
> core PMU event.
>     
> With this patch applied, it is possible to measure, for
> instance, CBO unc_c_tor_inserts:opcode:pcirdcur with
> unc_c_clockticks in the same group.

Thank you for fixing this.

> 
> The filter for both CBO and PCU are more complex than what the
> current code support. Each filter is sub-divided into event
> specific filters. For now, we consider the filter as one single
> MSR value. We lose a bit of flexibility and force multiplexing
> when this is not really necessary in HW. We could later create
> the notion of virtual filters that would be aggregated at the
> last step (wrmsrl). But I think this is good enough for now.
> 

Actually there are codes that handle similar case properly. See how
the ZDP_CTL_FVC register is handled in nhmex_mbox_get/put_shared_reg().
Do you like to update this patch or I will update it later.

Acked-by: Yan, Zheng <zheng.z.yan@intel.com>

Regards
Yan, Zheng

> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
> 
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> index 0a55710..cd2c930 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
> @@ -224,29 +224,86 @@ static void snbep_uncore_msr_init_box(struct intel_uncore_box *box)
>  		wrmsrl(msr, SNBEP_PMON_BOX_CTL_INT);
>  }
>  
> -static int snbep_uncore_hw_config(struct intel_uncore_box *box, struct perf_event *event)
> +static int uncore_pmu_extra_regs(struct intel_uncore_box *box,
> +				 struct perf_event *event)
> +{
> +	struct hw_perf_event_extra *reg;
> +	struct extra_reg *er = box->pmu->type->extra_regs;
> +
> +	if (!er)
> +		return 0;
> +
> +	reg = &event->hw.extra_reg;
> +
> +	for (; er->msr; er++) {
> +		if (er->event != (event->attr.config & er->config_mask))
> +			continue;
> +
> +		if (event->attr.config1 & ~er->valid_mask)
> +			return -EINVAL;
> +
> +		reg->idx = er->idx;
> +		reg->config = event->attr.config1;
> +		reg->reg = er->msr;
> +		break;
> +	}
> +	return 0;
> +}
> +
> +
> +static int snbep_cbo_extra_regs(struct intel_uncore_box *box,
> +				struct perf_event *event)
>  {
> +#define SNBEP_CBO_TIDEN_MASK		(1ULL<< 19)
> +#define SNBEP_CBO_FILT_CIDTID_MASK	0xfULL
> +
>  	struct hw_perf_event *hwc = &event->hw;
> -	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
> +	struct hw_perf_event_extra *reg = &hwc->extra_reg;
> +	u64 config1 = event->attr.config1;
>  
> -	if (box->pmu->type == &snbep_uncore_cbox) {
> -		reg1->reg = SNBEP_C0_MSR_PMON_BOX_FILTER +
> -			SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
> -		reg1->config = event->attr.config1 &
> -			SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK;
> -	} else {
> -		if (box->pmu->type == &snbep_uncore_pcu) {
> -			reg1->reg = SNBEP_PCU_MSR_PMON_BOX_FILTER;
> -			reg1->config = event->attr.config1 & SNBEP_PCU_MSR_PMON_BOX_FILTER_MASK;
> -		} else {
> -			return 0;
> -		}
> +	/*
> +	 * if extra_reg not set via the snbep_uncore_cbo_extra_regs[]
> +	 * then we need to check whether or not the tid_en bit is set
> +	 * in the config. If so, then we do need to enable the cbo filter
> +	 */
> +	if (reg->idx == EXTRA_REG_NONE
> +	    && (event->attr.config & SNBEP_CBO_TIDEN_MASK)) {
> +		/*
> +		 * validate that only the core-id/thread-id
> +		 * bits are set, otherwise we must go through
> +		 * the snbep_uncore_cbo_extra_regs[] table
> +		 */
> +		if (config1 & ~SNBEP_CBO_FILT_CIDTID_MASK)
> +			return -EINVAL;
> +
> +		reg->idx = 0;
> +		reg->config = config1 & SNBEP_CBO_FILT_CIDTID_MASK;
> +		reg->reg = SNBEP_C0_MSR_PMON_BOX_FILTER;
>  	}
> -	reg1->idx = 0;
> +	/*
> +	 * adjust the cbo index
> +	 */
> +	if (reg->idx != EXTRA_REG_NONE)
> +		reg->reg += SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
>  
>  	return 0;
>  }
>  
> +static int snbep_uncore_hw_config(struct intel_uncore_box *box, struct perf_event *event)
> +{
> +	struct intel_uncore_type *type = box->pmu->type;
> +	int ret;
> +
> +	ret = uncore_pmu_extra_regs(box, event);
> +	if (ret)
> +		return ret;
> +
> +	if (type == &snbep_uncore_cbox)
> +		ret = snbep_cbo_extra_regs(box, event);
> +
> +	return ret;
> +}
> +
>  static struct attribute *snbep_uncore_formats_attr[] = {
>  	&format_attr_event.attr,
>  	&format_attr_umask.attr,
> @@ -444,6 +501,41 @@ static struct intel_uncore_type snbep_uncore_ubox = {
>  	.format_group	= &snbep_uncore_ubox_format_group,
>  };
>  
> +#define CBO_EVENT_EXTRA_REG(a) \
> +	UNC_EXTRA_REG(a, \
> +		      SNBEP_C0_MSR_PMON_BOX_FILTER, \
> +		      ARCH_PERFMON_EVENTSEL_EVENT, \
> +		      SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK, \
> +		      0)
> +
> +#define CBO_EVTUM_EXTRA_REG(a) \
> +	UNC_EXTRA_REG(a, \
> +		      SNBEP_C0_MSR_PMON_BOX_FILTER, \
> +		      INTEL_ARCH_EVENT_MASK, \
> +		      SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK, \
> +		      0)
> +
> +static struct extra_reg snbep_uncore_cbo_extra_regs[] __read_mostly =
> +{
> +	CBO_EVENT_EXTRA_REG(0x0034), /* LLC_LOOKUP */
> +	CBO_EVTUM_EXTRA_REG(0x0135), /* TOR_INSERTS.OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x0335), /* TOR_INSERTS.MISS_OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x4135), /* TOR_INSERTS.NID_OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x4335), /* TOR_INSERTS.NID_MISS_OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x4435), /* TOR_INSERTS.NID_EVICTION */
> +	CBO_EVTUM_EXTRA_REG(0x4835), /* TOR_INSERTS.NID_ALL */
> +	CBO_EVTUM_EXTRA_REG(0x4a35), /* TOR_INSERTS.NID_MISS_ALL */
> +	CBO_EVTUM_EXTRA_REG(0x5035), /* TOR_INSERTS.NID_WB */
> +	CBO_EVTUM_EXTRA_REG(0x0136), /* TOR_OCCUPANCY.OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x0336), /* TOR_OCCUPANCY.MISS_OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x4136), /* TOR_OCCUPANCY.NID_OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x4336), /* TOR_OCCUPANCY.NID_MISS_OPCODE */
> +	CBO_EVTUM_EXTRA_REG(0x4436), /* TOR_OCCUPANCY.NID_EVICTION */
> +	CBO_EVTUM_EXTRA_REG(0x4836), /* TOR_OCCUPANCY.NID_ALL */
> +	CBO_EVTUM_EXTRA_REG(0x4a36), /* TOR_OCCUPANCY.NID_MISS_ALL */
> +	EVENT_EXTRA_END
> +};
> +
>  static struct intel_uncore_type snbep_uncore_cbox = {
>  	.name			= "cbox",
>  	.num_counters		= 4,
> @@ -458,6 +550,23 @@ static struct intel_uncore_type snbep_uncore_cbox = {
>  	.constraints		= snbep_uncore_cbox_constraints,
>  	.ops			= &snbep_uncore_msr_ops,
>  	.format_group		= &snbep_uncore_cbox_format_group,
> +	.extra_regs		= snbep_uncore_cbo_extra_regs,
> +};
> +
> +#define PCU_EVENT_EXTRA_REG(a) \
> +	UNC_EXTRA_REG(a, \
> +		      SNBEP_PCU_MSR_PMON_BOX_FILTER, \
> +		      ARCH_PERFMON_EVENTSEL_EVENT, \
> +		      0xffffffff, \
> +		      0)
> +
> +static struct extra_reg snbep_uncore_pcu_extra_regs[] __read_mostly =
> +{
> +	PCU_EVENT_EXTRA_REG(0xb), /* FREQ_BAND0_CYCLES */
> +	PCU_EVENT_EXTRA_REG(0xc), /* FREQ_BAND1_CYCLES */
> +	PCU_EVENT_EXTRA_REG(0xd), /* FREQ_BAND2_CYCLES */
> +	PCU_EVENT_EXTRA_REG(0xe), /* FREQ_BAND3_CYCLES */
> +	EVENT_EXTRA_END
>  };
>  
>  static struct intel_uncore_type snbep_uncore_pcu = {
> @@ -472,6 +581,7 @@ static struct intel_uncore_type snbep_uncore_pcu = {
>  	.num_shared_regs	= 1,
>  	.ops			= &snbep_uncore_msr_ops,
>  	.format_group		= &snbep_uncore_pcu_format_group,
> +	.extra_regs		= snbep_uncore_pcu_extra_regs,
>  };
>  
>  static struct intel_uncore_type *snbep_msr_uncores[] = {
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> index 5b81c18..37d8732 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> +++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
> @@ -369,6 +369,7 @@ struct intel_uncore_type {
>  	struct intel_uncore_pmu *pmus;
>  	struct intel_uncore_ops *ops;
>  	struct uncore_event_desc *event_descs;
> +	struct extra_reg *extra_regs;
>  	const struct attribute_group *attr_groups[3];
>  };
>  
> @@ -428,6 +429,14 @@ struct uncore_event_desc {
>  	const char *config;
>  };
>  
> +#define UNC_EXTRA_REG(e, ms, m, vm, i) {\
> +	.event = (e),		\
> +	.msr = (ms),		\
> +	.config_mask = (m),	\
> +	.valid_mask = (vm),	\
> +	.idx = i\
> +	}
> +
>  #define INTEL_UNCORE_EVENT_DESC(_name, _config)			\
>  {								\
>  	.attr	= __ATTR(_name, 0444, uncore_event_show, NULL),	\
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management
  2012-08-20 16:22 [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management Stephane Eranian
  2012-08-21  8:00 ` Yan, Zheng
@ 2012-08-21 10:46 ` Peter Zijlstra
  2012-08-21 11:45   ` Stephane Eranian
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2012-08-21 10:46 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: linux-kernel, mingo, andi, zheng.z.yan

On Mon, 2012-08-20 at 18:22 +0200, Stephane Eranian wrote:
>         .constraints            = snbep_uncore_cbox_constraints,
>         .ops                    = &snbep_uncore_msr_ops,
>         .format_group           = &snbep_uncore_cbox_format_group,
> +       .extra_regs             = snbep_uncore_cbo_extra_regs, 

The whole cbo vs cbox thing is a bit unfortunate, I know Intel tends to
forget to type the last letter, but could we be consistent?

Ideally Intel would have called the thing a Cache Coherence Unit or
somesuch to match the Power Control Unit, But seeing its called C-Box, I
don't see the point of saving the 1 character and obfuscate the name.
I've even seen CBO Box used, which is of course completely ridiculous.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management
  2012-08-21 10:46 ` Peter Zijlstra
@ 2012-08-21 11:45   ` Stephane Eranian
  0 siblings, 0 replies; 6+ messages in thread
From: Stephane Eranian @ 2012-08-21 11:45 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, mingo, andi, zheng.z.yan

On Tue, Aug 21, 2012 at 12:46 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, 2012-08-20 at 18:22 +0200, Stephane Eranian wrote:
>>         .constraints            = snbep_uncore_cbox_constraints,
>>         .ops                    = &snbep_uncore_msr_ops,
>>         .format_group           = &snbep_uncore_cbox_format_group,
>> +       .extra_regs             = snbep_uncore_cbo_extra_regs,
>
> The whole cbo vs cbox thing is a bit unfortunate, I know Intel tends to
> forget to type the last letter, but could we be consistent?
>
> Ideally Intel would have called the thing a Cache Coherence Unit or
> somesuch to match the Power Control Unit, But seeing its called C-Box, I
> don't see the point of saving the 1 character and obfuscate the name.
> I've even seen CBO Box used, which is of course completely ridiculous.

Ok, I can resubmit with the word cbox instead.

Still missing is the HA box opcode match/mask support.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management
@ 2012-10-22  6:14 Yan, Zheng
  0 siblings, 0 replies; 6+ messages in thread
From: Yan, Zheng @ 2012-10-22  6:14 UTC (permalink / raw)
  To: linux-kernel, a.p.zijlstra; +Cc: eranian, ak, Yan, Zheng

From: "Yan, Zheng" <zheng.z.yan@intel.com>

The existing code assumes all Cbox and PCU events are using
filter, but actually the filter is event specific. Furthermore
the filter is sub-divided into multiple fields which are used
by different events.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reported-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 319 +++++++++++++++++++++-----
 arch/x86/kernel/cpu/perf_event_intel_uncore.h |  13 +-
 2 files changed, 273 insertions(+), 59 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index d4afbeb..1f6a440 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -17,6 +17,9 @@ static struct event_constraint constraint_fixed =
 static struct event_constraint constraint_empty =
 	EVENT_CONSTRAINT(0, 0, 0);
 
+#define __BITS_VALUE(x, i, n)  ((typeof(x))(((x) >> ((i) * (n))) & \
+				((1ULL << (n)) - 1)))
+
 DEFINE_UNCORE_FORMAT_ATTR(event, event, "config:0-7");
 DEFINE_UNCORE_FORMAT_ATTR(event_ext, event, "config:0-7,21");
 DEFINE_UNCORE_FORMAT_ATTR(umask, umask, "config:8-15");
@@ -110,6 +113,21 @@ static void uncore_put_constraint(struct intel_uncore_box *box, struct perf_even
 	reg1->alloc = 0;
 }
 
+static u64 uncore_shared_reg_config(struct intel_uncore_box *box, int idx)
+{
+	struct intel_uncore_extra_reg *er;
+	unsigned long flags;
+	u64 config;
+
+	er = &box->shared_regs[idx];
+
+	raw_spin_lock_irqsave(&er->lock, flags);
+	config = er->config;
+	raw_spin_unlock_irqrestore(&er->lock, flags);
+
+	return config;
+}
+
 /* Sandy Bridge-EP uncore support */
 static struct intel_uncore_type snbep_uncore_cbox;
 static struct intel_uncore_type snbep_uncore_pcu;
@@ -203,7 +221,7 @@ static void snbep_uncore_msr_enable_event(struct intel_uncore_box *box, struct p
 	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
 
 	if (reg1->idx != EXTRA_REG_NONE)
-		wrmsrl(reg1->reg, reg1->config);
+		wrmsrl(reg1->reg, uncore_shared_reg_config(box, 0));
 
 	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
@@ -224,29 +242,6 @@ static void snbep_uncore_msr_init_box(struct intel_uncore_box *box)
 		wrmsrl(msr, SNBEP_PMON_BOX_CTL_INT);
 }
 
-static int snbep_uncore_hw_config(struct intel_uncore_box *box, struct perf_event *event)
-{
-	struct hw_perf_event *hwc = &event->hw;
-	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
-
-	if (box->pmu->type == &snbep_uncore_cbox) {
-		reg1->reg = SNBEP_C0_MSR_PMON_BOX_FILTER +
-			SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
-		reg1->config = event->attr.config1 &
-			SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK;
-	} else {
-		if (box->pmu->type == &snbep_uncore_pcu) {
-			reg1->reg = SNBEP_PCU_MSR_PMON_BOX_FILTER;
-			reg1->config = event->attr.config1 & SNBEP_PCU_MSR_PMON_BOX_FILTER_MASK;
-		} else {
-			return 0;
-		}
-	}
-	reg1->idx = 0;
-
-	return 0;
-}
-
 static struct attribute *snbep_uncore_formats_attr[] = {
 	&format_attr_event.attr,
 	&format_attr_umask.attr,
@@ -343,16 +338,16 @@ static struct attribute_group snbep_uncore_qpi_format_group = {
 	.attrs = snbep_uncore_qpi_formats_attr,
 };
 
+#define SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
+	.init_box	= snbep_uncore_msr_init_box,		\
+	.disable_box	= snbep_uncore_msr_disable_box,		\
+	.enable_box	= snbep_uncore_msr_enable_box,		\
+	.disable_event	= snbep_uncore_msr_disable_event,	\
+	.enable_event	= snbep_uncore_msr_enable_event,	\
+	.read_counter	= uncore_msr_read_counter
+
 static struct intel_uncore_ops snbep_uncore_msr_ops = {
-	.init_box	= snbep_uncore_msr_init_box,
-	.disable_box	= snbep_uncore_msr_disable_box,
-	.enable_box	= snbep_uncore_msr_enable_box,
-	.disable_event	= snbep_uncore_msr_disable_event,
-	.enable_event	= snbep_uncore_msr_enable_event,
-	.read_counter	= uncore_msr_read_counter,
-	.get_constraint = uncore_get_constraint,
-	.put_constraint = uncore_put_constraint,
-	.hw_config	= snbep_uncore_hw_config,
+	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 };
 
 static struct intel_uncore_ops snbep_uncore_pci_ops = {
@@ -444,6 +439,138 @@ static struct intel_uncore_type snbep_uncore_ubox = {
 	.format_group	= &snbep_uncore_ubox_format_group,
 };
 
+static struct extra_reg snbep_uncore_cbox_extra_regs[] = {
+	CBO_EVENT_EXTRA_REG(SNBEP_CBO_PMON_CTL_TID_EN,
+			    SNBEP_CBO_PMON_CTL_TID_EN, 0x1),
+	CBO_EVENT_EXTRA_REG(0x0334, 0xffff, 0x4),
+	CBO_EVENT_EXTRA_REG(0x0534, 0xffff, 0x4),
+	CBO_EVENT_EXTRA_REG(0x0934, 0xffff, 0x4),
+	CBO_EVENT_EXTRA_REG(0x4134, 0xffff, 0x6),
+	CBO_EVENT_EXTRA_REG(0x0135, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x0335, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x4135, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4335, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4435, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4835, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4a35, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x5035, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x0136, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x0336, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x4136, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4336, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4435, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4835, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4a35, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4037, 0x40ff, 0x2),
+	EVENT_EXTRA_END
+};
+
+static u64 snbep_cbox_filter_mask(int fields)
+{
+	u64 mask = 0;
+
+	if (fields & 0x1)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_TID;
+	if (fields & 0x2)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_NID;
+	if (fields & 0x4)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_STATE;
+	if (fields & 0x8)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_OPC;
+
+	return mask;
+}
+
+static struct event_constraint *
+snbep_cbox_get_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+	int i, alloc = 0;
+	unsigned long flags;
+	u64 mask;
+
+	if (reg1->idx == EXTRA_REG_NONE)
+		return NULL;
+
+	raw_spin_lock_irqsave(&er->lock, flags);
+	for (i = 0; i < 4; i++) {
+		if (!(reg1->idx & (0x1 << i)))
+			continue;
+		if (!uncore_box_is_fake(box) && (reg1->alloc & (0x1 << i)))
+			continue;
+
+		mask = snbep_cbox_filter_mask(0x1 << i);
+		if (!__BITS_VALUE(atomic_read(&er->ref), i, 8) ||
+		    !((reg1->config ^ er->config) & mask)) {
+			atomic_add(1 << (i * 8), &er->ref);
+			er->config &= ~mask;
+			er->config |= reg1->config & mask;
+			alloc |= (0x1 << i);
+		} else {
+			break;
+		}
+	}
+	raw_spin_unlock_irqrestore(&er->lock, flags);
+	if (i < 4)
+		goto fail;
+
+	if (!uncore_box_is_fake(box))
+		reg1->alloc |= alloc;
+
+	return 0;
+fail:
+	for (; i >= 0; i--) {
+		if (alloc & (0x1 << i))
+			atomic_sub(1 << (i * 8), &er->ref);
+	}
+	return &constraint_empty;
+}
+
+static void snbep_cbox_put_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+	int i;
+
+	if (uncore_box_is_fake(box))
+		return;
+
+	for (i = 0; i < 4; i++) {
+		if (reg1->alloc & (0x1 << i))
+			atomic_sub(1 << (i * 8), &er->ref);
+	}
+	reg1->alloc = 0;
+}
+
+static int snbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct extra_reg *er;
+	int idx = 0;
+
+	for (er = snbep_uncore_cbox_extra_regs; er->msr; er++) {
+		if (er->event != (event->hw.config & er->config_mask))
+			continue;
+		idx |= er->idx;
+	}
+
+	if (idx) {
+		reg1->reg = SNBEP_C0_MSR_PMON_BOX_FILTER +
+			SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
+		reg1->config = event->attr.config1 & snbep_cbox_filter_mask(idx);
+		reg1->idx = idx;
+	}
+	return 0;
+}
+
+static struct intel_uncore_ops snbep_uncore_cbox_ops = {
+	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
+	.hw_config		= snbep_cbox_hw_config,
+	.get_constraint		= snbep_cbox_get_constraint,
+	.put_constraint		= snbep_cbox_put_constraint,
+};
+
 static struct intel_uncore_type snbep_uncore_cbox = {
 	.name			= "cbox",
 	.num_counters		= 4,
@@ -456,10 +583,104 @@ static struct intel_uncore_type snbep_uncore_cbox = {
 	.msr_offset		= SNBEP_CBO_MSR_OFFSET,
 	.num_shared_regs	= 1,
 	.constraints		= snbep_uncore_cbox_constraints,
-	.ops			= &snbep_uncore_msr_ops,
+	.ops			= &snbep_uncore_cbox_ops,
 	.format_group		= &snbep_uncore_cbox_format_group,
 };
 
+static u64 snbep_pcu_alter_er(struct perf_event *event, int new_idx, bool modify)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
+	u64 config = reg1->config;
+
+	if (new_idx > reg1->idx)
+		config <<= 8 * (new_idx - reg1->idx);
+	else
+		config >>= 8 * (reg1->idx - new_idx);
+
+	if (modify) {
+		hwc->config += new_idx - reg1->idx;
+		reg1->config = config;
+		reg1->idx = new_idx;
+	}
+	return config;
+}
+
+static struct event_constraint *
+snbep_pcu_get_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+	unsigned long flags;
+	int idx = reg1->idx;
+	u64 mask, config1 = reg1->config;
+	bool ok = false;
+
+	if (reg1->idx == EXTRA_REG_NONE ||
+	    (!uncore_box_is_fake(box) && reg1->alloc))
+		return NULL;
+again:
+	mask = 0xff << (idx * 8);
+	raw_spin_lock_irqsave(&er->lock, flags);
+	if (!__BITS_VALUE(atomic_read(&er->ref), idx, 8) ||
+	    !((config1 ^ er->config) & mask)) {
+		atomic_add(1 << (idx * 8), &er->ref);
+		er->config &= ~mask;
+		er->config |= config1 & mask;
+		ok = true;
+	}
+	raw_spin_unlock_irqrestore(&er->lock, flags);
+
+	if (!ok) {
+		idx = (idx + 1) % 4;
+		if (idx != reg1->idx) {
+			config1 = snbep_pcu_alter_er(event, idx, false);
+			goto again;
+		}
+		return &constraint_empty;
+	}
+
+	if (!uncore_box_is_fake(box)) {
+		if (idx != reg1->idx)
+			snbep_pcu_alter_er(event, idx, true);
+		reg1->alloc = 1;
+	}
+	return NULL;
+}
+
+static void snbep_pcu_put_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+
+	if (uncore_box_is_fake(box) || !reg1->alloc)
+		return;
+
+	atomic_sub(1 << (reg1->idx * 8), &er->ref);
+	reg1->alloc = 0;
+}
+
+static int snbep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
+	int ev_sel = hwc->config & SNBEP_PMON_CTL_EV_SEL_MASK;
+
+	if (ev_sel >= 0xb && ev_sel <= 0xe) {
+		reg1->reg = SNBEP_PCU_MSR_PMON_BOX_FILTER;
+		reg1->idx = ev_sel - 0xb;
+		reg1->config = event->attr.config1 & (0xff << reg1->idx);
+	}
+	return 0;
+}
+
+static struct intel_uncore_ops snbep_uncore_pcu_ops = {
+	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
+	.hw_config		= snbep_pcu_hw_config,
+	.get_constraint		= snbep_pcu_get_constraint,
+	.put_constraint		= snbep_pcu_put_constraint,
+};
+
 static struct intel_uncore_type snbep_uncore_pcu = {
 	.name			= "pcu",
 	.num_counters		= 4,
@@ -470,7 +691,7 @@ static struct intel_uncore_type snbep_uncore_pcu = {
 	.event_mask		= SNBEP_PCU_MSR_PMON_RAW_EVENT_MASK,
 	.box_ctl		= SNBEP_PCU_MSR_PMON_BOX_CTL,
 	.num_shared_regs	= 1,
-	.ops			= &snbep_uncore_msr_ops,
+	.ops			= &snbep_uncore_pcu_ops,
 	.format_group		= &snbep_uncore_pcu_format_group,
 };
 
@@ -797,9 +1018,6 @@ static struct intel_uncore_type *nhm_msr_uncores[] = {
 /* end of Nehalem uncore support */
 
 /* Nehalem-EX uncore support */
-#define __BITS_VALUE(x, i, n)  ((typeof(x))(((x) >> ((i) * (n))) & \
-				((1ULL << (n)) - 1)))
-
 DEFINE_UNCORE_FORMAT_ATTR(event5, event, "config:1-5");
 DEFINE_UNCORE_FORMAT_ATTR(counter, counter, "config:6-7");
 DEFINE_UNCORE_FORMAT_ATTR(match, match, "config1:0-63");
@@ -1150,7 +1368,7 @@ static struct extra_reg nhmex_uncore_mbox_extra_regs[] = {
 };
 
 /* Nehalem-EX or Westmere-EX ? */
-bool uncore_nhmex;
+static bool uncore_nhmex;
 
 static bool nhmex_mbox_get_shared_reg(struct intel_uncore_box *box, int idx, u64 config)
 {
@@ -1228,7 +1446,7 @@ static void nhmex_mbox_put_shared_reg(struct intel_uncore_box *box, int idx)
 	atomic_sub(1 << (idx * 8), &er->ref);
 }
 
-u64 nhmex_mbox_alter_er(struct perf_event *event, int new_idx, bool modify)
+static u64 nhmex_mbox_alter_er(struct perf_event *event, int new_idx, bool modify)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
@@ -1543,7 +1761,7 @@ static struct intel_uncore_type nhmex_uncore_mbox = {
 	.format_group		= &nhmex_uncore_mbox_format_group,
 };
 
-void nhmex_rbox_alter_er(struct intel_uncore_box *box, struct perf_event *event)
+static void nhmex_rbox_alter_er(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
@@ -1715,21 +1933,6 @@ static int nhmex_rbox_hw_config(struct intel_uncore_box *box, struct perf_event
 	return 0;
 }
 
-static u64 nhmex_rbox_shared_reg_config(struct intel_uncore_box *box, int idx)
-{
-	struct intel_uncore_extra_reg *er;
-	unsigned long flags;
-	u64 config;
-
-	er = &box->shared_regs[idx];
-
-	raw_spin_lock_irqsave(&er->lock, flags);
-	config = er->config;
-	raw_spin_unlock_irqrestore(&er->lock, flags);
-
-	return config;
-}
-
 static void nhmex_rbox_msr_enable_event(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
@@ -1750,7 +1953,7 @@ static void nhmex_rbox_msr_enable_event(struct intel_uncore_box *box, struct per
 	case 2:
 	case 3:
 		wrmsrl(NHMEX_R_MSR_PORTN_QLX_CFG(port),
-			nhmex_rbox_shared_reg_config(box, 2 + (idx / 6) * 5));
+			uncore_shared_reg_config(box, 2 + (idx / 6) * 5));
 		break;
 	case 4:
 		wrmsrl(NHMEX_R_MSR_PORTN_XBR_SET1_MM_CFG(port),
@@ -2276,7 +2479,7 @@ out:
 	return ret;
 }
 
-int uncore_pmu_event_init(struct perf_event *event)
+static int uncore_pmu_event_init(struct perf_event *event)
 {
 	struct intel_uncore_pmu *pmu;
 	struct intel_uncore_box *box;
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
index e68a455..3e382f1 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -148,9 +148,20 @@
 #define SNBEP_C0_MSR_PMON_CTL0			0xd10
 #define SNBEP_C0_MSR_PMON_BOX_CTL		0xd04
 #define SNBEP_C0_MSR_PMON_BOX_FILTER		0xd14
-#define SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK	0xfffffc1f
 #define SNBEP_CBO_MSR_OFFSET			0x20
 
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_TID	0x1f
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_NID	0x3fc00
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_STATE	0x7c0000
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_OPC	0xff800000
+
+#define CBO_EVENT_EXTRA_REG(e, m, f) {	\
+	.event = (e),				\
+	.msr = SNBEP_C0_MSR_PMON_BOX_FILTER,	\
+	.config_mask = (m),			\
+	.idx = f				\
+}
+
 /* SNB-EP PCU register */
 #define SNBEP_PCU_MSR_PMON_CTR0			0xc36
 #define SNBEP_PCU_MSR_PMON_CTL0			0xc30
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management
@ 2012-10-22  6:02 Yan, Zheng
  0 siblings, 0 replies; 6+ messages in thread
From: Yan, Zheng @ 2012-10-22  6:02 UTC (permalink / raw)
  To: linux-kernel, a.p.zijlstra; +Cc: eranian, ak, Yan, Zheng

From: "Yan, Zheng" <zheng.z.yan@intel.com>

The existing code assumes all Cbox and PCU events are using
filter, but actually the filter is event specific. Furthermore
the filter is sub-divided into multiple fields which are used
by different events.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
Reported-by: Stephane Eranian <eranian@google.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 319 +++++++++++++++++++++-----
 arch/x86/kernel/cpu/perf_event_intel_uncore.h |  13 +-
 2 files changed, 273 insertions(+), 59 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index d4afbeb..1f6a440 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -17,6 +17,9 @@ static struct event_constraint constraint_fixed =
 static struct event_constraint constraint_empty =
 	EVENT_CONSTRAINT(0, 0, 0);
 
+#define __BITS_VALUE(x, i, n)  ((typeof(x))(((x) >> ((i) * (n))) & \
+				((1ULL << (n)) - 1)))
+
 DEFINE_UNCORE_FORMAT_ATTR(event, event, "config:0-7");
 DEFINE_UNCORE_FORMAT_ATTR(event_ext, event, "config:0-7,21");
 DEFINE_UNCORE_FORMAT_ATTR(umask, umask, "config:8-15");
@@ -110,6 +113,21 @@ static void uncore_put_constraint(struct intel_uncore_box *box, struct perf_even
 	reg1->alloc = 0;
 }
 
+static u64 uncore_shared_reg_config(struct intel_uncore_box *box, int idx)
+{
+	struct intel_uncore_extra_reg *er;
+	unsigned long flags;
+	u64 config;
+
+	er = &box->shared_regs[idx];
+
+	raw_spin_lock_irqsave(&er->lock, flags);
+	config = er->config;
+	raw_spin_unlock_irqrestore(&er->lock, flags);
+
+	return config;
+}
+
 /* Sandy Bridge-EP uncore support */
 static struct intel_uncore_type snbep_uncore_cbox;
 static struct intel_uncore_type snbep_uncore_pcu;
@@ -203,7 +221,7 @@ static void snbep_uncore_msr_enable_event(struct intel_uncore_box *box, struct p
 	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
 
 	if (reg1->idx != EXTRA_REG_NONE)
-		wrmsrl(reg1->reg, reg1->config);
+		wrmsrl(reg1->reg, uncore_shared_reg_config(box, 0));
 
 	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
@@ -224,29 +242,6 @@ static void snbep_uncore_msr_init_box(struct intel_uncore_box *box)
 		wrmsrl(msr, SNBEP_PMON_BOX_CTL_INT);
 }
 
-static int snbep_uncore_hw_config(struct intel_uncore_box *box, struct perf_event *event)
-{
-	struct hw_perf_event *hwc = &event->hw;
-	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
-
-	if (box->pmu->type == &snbep_uncore_cbox) {
-		reg1->reg = SNBEP_C0_MSR_PMON_BOX_FILTER +
-			SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
-		reg1->config = event->attr.config1 &
-			SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK;
-	} else {
-		if (box->pmu->type == &snbep_uncore_pcu) {
-			reg1->reg = SNBEP_PCU_MSR_PMON_BOX_FILTER;
-			reg1->config = event->attr.config1 & SNBEP_PCU_MSR_PMON_BOX_FILTER_MASK;
-		} else {
-			return 0;
-		}
-	}
-	reg1->idx = 0;
-
-	return 0;
-}
-
 static struct attribute *snbep_uncore_formats_attr[] = {
 	&format_attr_event.attr,
 	&format_attr_umask.attr,
@@ -343,16 +338,16 @@ static struct attribute_group snbep_uncore_qpi_format_group = {
 	.attrs = snbep_uncore_qpi_formats_attr,
 };
 
+#define SNBEP_UNCORE_MSR_OPS_COMMON_INIT()			\
+	.init_box	= snbep_uncore_msr_init_box,		\
+	.disable_box	= snbep_uncore_msr_disable_box,		\
+	.enable_box	= snbep_uncore_msr_enable_box,		\
+	.disable_event	= snbep_uncore_msr_disable_event,	\
+	.enable_event	= snbep_uncore_msr_enable_event,	\
+	.read_counter	= uncore_msr_read_counter
+
 static struct intel_uncore_ops snbep_uncore_msr_ops = {
-	.init_box	= snbep_uncore_msr_init_box,
-	.disable_box	= snbep_uncore_msr_disable_box,
-	.enable_box	= snbep_uncore_msr_enable_box,
-	.disable_event	= snbep_uncore_msr_disable_event,
-	.enable_event	= snbep_uncore_msr_enable_event,
-	.read_counter	= uncore_msr_read_counter,
-	.get_constraint = uncore_get_constraint,
-	.put_constraint = uncore_put_constraint,
-	.hw_config	= snbep_uncore_hw_config,
+	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 };
 
 static struct intel_uncore_ops snbep_uncore_pci_ops = {
@@ -444,6 +439,138 @@ static struct intel_uncore_type snbep_uncore_ubox = {
 	.format_group	= &snbep_uncore_ubox_format_group,
 };
 
+static struct extra_reg snbep_uncore_cbox_extra_regs[] = {
+	CBO_EVENT_EXTRA_REG(SNBEP_CBO_PMON_CTL_TID_EN,
+			    SNBEP_CBO_PMON_CTL_TID_EN, 0x1),
+	CBO_EVENT_EXTRA_REG(0x0334, 0xffff, 0x4),
+	CBO_EVENT_EXTRA_REG(0x0534, 0xffff, 0x4),
+	CBO_EVENT_EXTRA_REG(0x0934, 0xffff, 0x4),
+	CBO_EVENT_EXTRA_REG(0x4134, 0xffff, 0x6),
+	CBO_EVENT_EXTRA_REG(0x0135, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x0335, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x4135, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4335, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4435, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4835, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4a35, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x5035, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x0136, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x0336, 0xffff, 0x8),
+	CBO_EVENT_EXTRA_REG(0x4136, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4336, 0xffff, 0xc),
+	CBO_EVENT_EXTRA_REG(0x4435, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4835, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4a35, 0xffff, 0x2),
+	CBO_EVENT_EXTRA_REG(0x4037, 0x40ff, 0x2),
+	EVENT_EXTRA_END
+};
+
+static u64 snbep_cbox_filter_mask(int fields)
+{
+	u64 mask = 0;
+
+	if (fields & 0x1)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_TID;
+	if (fields & 0x2)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_NID;
+	if (fields & 0x4)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_STATE;
+	if (fields & 0x8)
+		mask |= SNBEP_CB0_MSR_PMON_BOX_FILTER_OPC;
+
+	return mask;
+}
+
+static struct event_constraint *
+snbep_cbox_get_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+	int i, alloc = 0;
+	unsigned long flags;
+	u64 mask;
+
+	if (reg1->idx == EXTRA_REG_NONE)
+		return NULL;
+
+	raw_spin_lock_irqsave(&er->lock, flags);
+	for (i = 0; i < 4; i++) {
+		if (!(reg1->idx & (0x1 << i)))
+			continue;
+		if (!uncore_box_is_fake(box) && (reg1->alloc & (0x1 << i)))
+			continue;
+
+		mask = snbep_cbox_filter_mask(0x1 << i);
+		if (!__BITS_VALUE(atomic_read(&er->ref), i, 8) ||
+		    !((reg1->config ^ er->config) & mask)) {
+			atomic_add(1 << (i * 8), &er->ref);
+			er->config &= ~mask;
+			er->config |= reg1->config & mask;
+			alloc |= (0x1 << i);
+		} else {
+			break;
+		}
+	}
+	raw_spin_unlock_irqrestore(&er->lock, flags);
+	if (i < 4)
+		goto fail;
+
+	if (!uncore_box_is_fake(box))
+		reg1->alloc |= alloc;
+
+	return 0;
+fail:
+	for (; i >= 0; i--) {
+		if (alloc & (0x1 << i))
+			atomic_sub(1 << (i * 8), &er->ref);
+	}
+	return &constraint_empty;
+}
+
+static void snbep_cbox_put_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+	int i;
+
+	if (uncore_box_is_fake(box))
+		return;
+
+	for (i = 0; i < 4; i++) {
+		if (reg1->alloc & (0x1 << i))
+			atomic_sub(1 << (i * 8), &er->ref);
+	}
+	reg1->alloc = 0;
+}
+
+static int snbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct extra_reg *er;
+	int idx = 0;
+
+	for (er = snbep_uncore_cbox_extra_regs; er->msr; er++) {
+		if (er->event != (event->hw.config & er->config_mask))
+			continue;
+		idx |= er->idx;
+	}
+
+	if (idx) {
+		reg1->reg = SNBEP_C0_MSR_PMON_BOX_FILTER +
+			SNBEP_CBO_MSR_OFFSET * box->pmu->pmu_idx;
+		reg1->config = event->attr.config1 & snbep_cbox_filter_mask(idx);
+		reg1->idx = idx;
+	}
+	return 0;
+}
+
+static struct intel_uncore_ops snbep_uncore_cbox_ops = {
+	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
+	.hw_config		= snbep_cbox_hw_config,
+	.get_constraint		= snbep_cbox_get_constraint,
+	.put_constraint		= snbep_cbox_put_constraint,
+};
+
 static struct intel_uncore_type snbep_uncore_cbox = {
 	.name			= "cbox",
 	.num_counters		= 4,
@@ -456,10 +583,104 @@ static struct intel_uncore_type snbep_uncore_cbox = {
 	.msr_offset		= SNBEP_CBO_MSR_OFFSET,
 	.num_shared_regs	= 1,
 	.constraints		= snbep_uncore_cbox_constraints,
-	.ops			= &snbep_uncore_msr_ops,
+	.ops			= &snbep_uncore_cbox_ops,
 	.format_group		= &snbep_uncore_cbox_format_group,
 };
 
+static u64 snbep_pcu_alter_er(struct perf_event *event, int new_idx, bool modify)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
+	u64 config = reg1->config;
+
+	if (new_idx > reg1->idx)
+		config <<= 8 * (new_idx - reg1->idx);
+	else
+		config >>= 8 * (reg1->idx - new_idx);
+
+	if (modify) {
+		hwc->config += new_idx - reg1->idx;
+		reg1->config = config;
+		reg1->idx = new_idx;
+	}
+	return config;
+}
+
+static struct event_constraint *
+snbep_pcu_get_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+	unsigned long flags;
+	int idx = reg1->idx;
+	u64 mask, config1 = reg1->config;
+	bool ok = false;
+
+	if (reg1->idx == EXTRA_REG_NONE ||
+	    (!uncore_box_is_fake(box) && reg1->alloc))
+		return NULL;
+again:
+	mask = 0xff << (idx * 8);
+	raw_spin_lock_irqsave(&er->lock, flags);
+	if (!__BITS_VALUE(atomic_read(&er->ref), idx, 8) ||
+	    !((config1 ^ er->config) & mask)) {
+		atomic_add(1 << (idx * 8), &er->ref);
+		er->config &= ~mask;
+		er->config |= config1 & mask;
+		ok = true;
+	}
+	raw_spin_unlock_irqrestore(&er->lock, flags);
+
+	if (!ok) {
+		idx = (idx + 1) % 4;
+		if (idx != reg1->idx) {
+			config1 = snbep_pcu_alter_er(event, idx, false);
+			goto again;
+		}
+		return &constraint_empty;
+	}
+
+	if (!uncore_box_is_fake(box)) {
+		if (idx != reg1->idx)
+			snbep_pcu_alter_er(event, idx, true);
+		reg1->alloc = 1;
+	}
+	return NULL;
+}
+
+static void snbep_pcu_put_constraint(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
+	struct intel_uncore_extra_reg *er = &box->shared_regs[0];
+
+	if (uncore_box_is_fake(box) || !reg1->alloc)
+		return;
+
+	atomic_sub(1 << (reg1->idx * 8), &er->ref);
+	reg1->alloc = 0;
+}
+
+static int snbep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
+	int ev_sel = hwc->config & SNBEP_PMON_CTL_EV_SEL_MASK;
+
+	if (ev_sel >= 0xb && ev_sel <= 0xe) {
+		reg1->reg = SNBEP_PCU_MSR_PMON_BOX_FILTER;
+		reg1->idx = ev_sel - 0xb;
+		reg1->config = event->attr.config1 & (0xff << reg1->idx);
+	}
+	return 0;
+}
+
+static struct intel_uncore_ops snbep_uncore_pcu_ops = {
+	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
+	.hw_config		= snbep_pcu_hw_config,
+	.get_constraint		= snbep_pcu_get_constraint,
+	.put_constraint		= snbep_pcu_put_constraint,
+};
+
 static struct intel_uncore_type snbep_uncore_pcu = {
 	.name			= "pcu",
 	.num_counters		= 4,
@@ -470,7 +691,7 @@ static struct intel_uncore_type snbep_uncore_pcu = {
 	.event_mask		= SNBEP_PCU_MSR_PMON_RAW_EVENT_MASK,
 	.box_ctl		= SNBEP_PCU_MSR_PMON_BOX_CTL,
 	.num_shared_regs	= 1,
-	.ops			= &snbep_uncore_msr_ops,
+	.ops			= &snbep_uncore_pcu_ops,
 	.format_group		= &snbep_uncore_pcu_format_group,
 };
 
@@ -797,9 +1018,6 @@ static struct intel_uncore_type *nhm_msr_uncores[] = {
 /* end of Nehalem uncore support */
 
 /* Nehalem-EX uncore support */
-#define __BITS_VALUE(x, i, n)  ((typeof(x))(((x) >> ((i) * (n))) & \
-				((1ULL << (n)) - 1)))
-
 DEFINE_UNCORE_FORMAT_ATTR(event5, event, "config:1-5");
 DEFINE_UNCORE_FORMAT_ATTR(counter, counter, "config:6-7");
 DEFINE_UNCORE_FORMAT_ATTR(match, match, "config1:0-63");
@@ -1150,7 +1368,7 @@ static struct extra_reg nhmex_uncore_mbox_extra_regs[] = {
 };
 
 /* Nehalem-EX or Westmere-EX ? */
-bool uncore_nhmex;
+static bool uncore_nhmex;
 
 static bool nhmex_mbox_get_shared_reg(struct intel_uncore_box *box, int idx, u64 config)
 {
@@ -1228,7 +1446,7 @@ static void nhmex_mbox_put_shared_reg(struct intel_uncore_box *box, int idx)
 	atomic_sub(1 << (idx * 8), &er->ref);
 }
 
-u64 nhmex_mbox_alter_er(struct perf_event *event, int new_idx, bool modify)
+static u64 nhmex_mbox_alter_er(struct perf_event *event, int new_idx, bool modify)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
@@ -1543,7 +1761,7 @@ static struct intel_uncore_type nhmex_uncore_mbox = {
 	.format_group		= &nhmex_uncore_mbox_format_group,
 };
 
-void nhmex_rbox_alter_er(struct intel_uncore_box *box, struct perf_event *event)
+static void nhmex_rbox_alter_er(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
 	struct hw_perf_event_extra *reg1 = &hwc->extra_reg;
@@ -1715,21 +1933,6 @@ static int nhmex_rbox_hw_config(struct intel_uncore_box *box, struct perf_event
 	return 0;
 }
 
-static u64 nhmex_rbox_shared_reg_config(struct intel_uncore_box *box, int idx)
-{
-	struct intel_uncore_extra_reg *er;
-	unsigned long flags;
-	u64 config;
-
-	er = &box->shared_regs[idx];
-
-	raw_spin_lock_irqsave(&er->lock, flags);
-	config = er->config;
-	raw_spin_unlock_irqrestore(&er->lock, flags);
-
-	return config;
-}
-
 static void nhmex_rbox_msr_enable_event(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
@@ -1750,7 +1953,7 @@ static void nhmex_rbox_msr_enable_event(struct intel_uncore_box *box, struct per
 	case 2:
 	case 3:
 		wrmsrl(NHMEX_R_MSR_PORTN_QLX_CFG(port),
-			nhmex_rbox_shared_reg_config(box, 2 + (idx / 6) * 5));
+			uncore_shared_reg_config(box, 2 + (idx / 6) * 5));
 		break;
 	case 4:
 		wrmsrl(NHMEX_R_MSR_PORTN_XBR_SET1_MM_CFG(port),
@@ -2276,7 +2479,7 @@ out:
 	return ret;
 }
 
-int uncore_pmu_event_init(struct perf_event *event)
+static int uncore_pmu_event_init(struct perf_event *event)
 {
 	struct intel_uncore_pmu *pmu;
 	struct intel_uncore_box *box;
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
index e68a455..3e382f1 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -148,9 +148,20 @@
 #define SNBEP_C0_MSR_PMON_CTL0			0xd10
 #define SNBEP_C0_MSR_PMON_BOX_CTL		0xd04
 #define SNBEP_C0_MSR_PMON_BOX_FILTER		0xd14
-#define SNBEP_CB0_MSR_PMON_BOX_FILTER_MASK	0xfffffc1f
 #define SNBEP_CBO_MSR_OFFSET			0x20
 
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_TID	0x1f
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_NID	0x3fc00
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_STATE	0x7c0000
+#define SNBEP_CB0_MSR_PMON_BOX_FILTER_OPC	0xff800000
+
+#define CBO_EVENT_EXTRA_REG(e, m, f) {	\
+	.event = (e),				\
+	.msr = SNBEP_C0_MSR_PMON_BOX_FILTER,	\
+	.config_mask = (m),			\
+	.idx = f				\
+}
+
 /* SNB-EP PCU register */
 #define SNBEP_PCU_MSR_PMON_CTR0			0xc36
 #define SNBEP_PCU_MSR_PMON_CTL0			0xc30
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-10-22  7:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-20 16:22 [PATCH] perf/x86: fix SNB-EP CBO and PCU uncore PMU filter management Stephane Eranian
2012-08-21  8:00 ` Yan, Zheng
2012-08-21 10:46 ` Peter Zijlstra
2012-08-21 11:45   ` Stephane Eranian
2012-10-22  6:02 Yan, Zheng
2012-10-22  6:14 Yan, Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).