Skip to contentSkip to conversation

perf: implement AMD IBS (v2)

PATCH In review Robert Richter 6 participants 27 messages May 19, 2010 – Nov 23, 2012

Subsystems (7)
Arm (32-bit) x86 PowerPC perf events +3 more subsystems
lore
Share & tools
lore
#0 lore raw

cover perf: implement AMD IBS (v2)

This is an updated version of my patch set that introduces AMD IBS for perf.

Changes made:

* rebased to latest tip/perf/core
* dropped the approach using a model_spec flag
* introduced an attribute to specify a raw hardware event type
* renamed *_SIZE macros to *_REG_COUNT
* introduced AMD_IBS_EVENT_CONSTRAINT() macro
* ease ibs initialization code
* made code cpu hotplug capable (using hotplug hooks)
* introduced ibs_map[] to better describe an ibs event which eases interfaces of ibs functions
* implemented support for setup using sample_period parameter
* fixed irq statistic counter
* adjust raw sample size to end the buffer at a 64 bit boundary

See also the diff below.

-Robert

arch/arm/kernel/perf_event.c +5 -0
index 9e70f20..73d680c 100644
@@ -388,6 +388,11 @@ __hw_perf_event_init(struct perf_event *event)
 	} else if (PERF_TYPE_HW_CACHE == event->attr.type) {
 		mapping = armpmu_map_cache_event(event->attr.config);
 	} else if (PERF_TYPE_RAW == event->attr.type) {
+		if (event->attr.raw_type) {
+			pr_debug("invalid raw type %x\n",
+				 event->attr.raw_type);
+			return -EINVAL;
+		}
 		mapping = armpmu->raw_event(event->attr.config);
 	} else {
 		pr_debug("event type %x not supported\n", event->attr.type);
arch/powerpc/kernel/perf_event.c +2 -3
index 0b1f0f2..c8fb3cf 100644
@@ -1036,6 +1036,8 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
 			return ERR_PTR(err);
 		break;
 	case PERF_TYPE_RAW:
+		if (event->attr.raw_type)
+			return ERR_PTR(-EINVAL);
 		ev = event->attr.config;
 		break;
 	default:
@@ -1044,9 +1046,6 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
 	event->hw.config_base = ev;
 	event->hw.idx = 0;
 
-	if (attr->model_spec)
-		return ERR_PTR(-EOPNOTSUPP);
-
 	/*
 	 * If we are not running on a hypervisor, force the
 	 * exclude_hv bit to 0 so that we don't care what
arch/powerpc/kernel/perf_event_fsl_emb.c +2 -0
index 369872f..7547e96 100644
@@ -452,6 +452,8 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
 		break;
 
 	case PERF_TYPE_RAW:
+		if (event->attr.raw_type)
+			return ERR_PTR(-EINVAL);
 		ev = event->attr.config;
 		break;
 
arch/sh/kernel/perf_event.c +2 -3
index eef545a..482cf48 100644
@@ -109,9 +109,6 @@ static int __hw_perf_event_init(struct perf_event *event)
 	if (!sh_pmu_initialized())
 		return -ENODEV;
 
-	if (attr->model_spec)
-		return -EOPNOTSUPP;
-
 	/*
 	 * All of the on-chip counters are "limited", in that they have
 	 * no interrupts, and are therefore unable to do sampling without
@@ -145,6 +142,8 @@ static int __hw_perf_event_init(struct perf_event *event)
 
 	switch (attr->type) {
 	case PERF_TYPE_RAW:
+		if (attr->raw_type)
+			return -EINVAL;
 		config = attr->config & sh_pmu->raw_event_mask;
 		break;
 	case PERF_TYPE_HW_CACHE:
arch/sparc/kernel/perf_event.c +0 -3
index b3ae28e..cf4ce26 100644
@@ -1047,9 +1047,6 @@ static int __hw_perf_event_init(struct perf_event *event)
 	} else
 		return -EOPNOTSUPP;
 
-	if (attr->model_spec)
-		return -EOPNOTSUPP;
-
 	/* We save the enable bits in the config_base.  */
 	hwc->config_base = sparc_pmu->irq_bit;
 	if (!attr->exclude_user)
arch/x86/include/asm/msr-index.h +3 -3
index a7e4aa5..8b9929f 100644
@@ -113,7 +113,7 @@
 #define MSR_AMD64_IBSFETCHCTL		0xc0011030
 #define MSR_AMD64_IBSFETCHLINAD		0xc0011031
 #define MSR_AMD64_IBSFETCHPHYSAD	0xc0011032
-#define MSR_AMD64_IBSFETCH_SIZE		3
+#define MSR_AMD64_IBSFETCH_REG_COUNT	3
 #define MSR_AMD64_IBSOPCTL		0xc0011033
 #define MSR_AMD64_IBSOPRIP		0xc0011034
 #define MSR_AMD64_IBSOPDATA		0xc0011035
@@ -121,9 +121,9 @@
 #define MSR_AMD64_IBSOPDATA3		0xc0011037
 #define MSR_AMD64_IBSDCLINAD		0xc0011038
 #define MSR_AMD64_IBSDCPHYSAD		0xc0011039
-#define MSR_AMD64_IBSOP_SIZE		7
+#define MSR_AMD64_IBSOP_REG_COUNT	7
 #define MSR_AMD64_IBSCTL		0xc001103a
-#define MSR_AMD64_IBS_SIZE_MAX		MSR_AMD64_IBSOP_SIZE
+#define MSR_AMD64_IBS_REG_COUNT_MAX	MSR_AMD64_IBSOP_REG_COUNT
 
 /* Fam 10h MSRs */
 #define MSR_FAM10H_MMIO_CONF_BASE	0xc0010058
arch/x86/include/asm/perf_event.h +4 -3
index e787d01..dace4e2 100644
@@ -44,14 +44,15 @@
 #define AMD64_RAW_EVENT_MASK		\
 	(X86_RAW_EVENT_MASK          |  \
 	 AMD64_EVENTSEL_EVENT)
+#define AMD64_NUM_COUNTERS				4
 
-#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_SEL		      0x3c
+#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_SEL		0x3c
 #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_UMASK		(0x00 << 8)
-#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_INDEX			 0
+#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_INDEX		0
 #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_PRESENT \
 		(1 << (ARCH_PERFMON_UNHALTED_CORE_CYCLES_INDEX))
 
-#define ARCH_PERFMON_BRANCH_MISSES_RETIRED			 6
+#define ARCH_PERFMON_BRANCH_MISSES_RETIRED		6
 
 /*
  * Intel "Architectural Performance Monitoring" CPUID
arch/x86/kernel/cpu/perf_event.c +16 -30

103 lines not on this page · raw message

arch/x86/kernel/cpu/perf_event_amd.c +129 -90

335 lines not on this page · raw message

arch/x86/kernel/cpu/perf_event_intel.c +3 -0
index 8a16205..dfbbe69 100644
@@ -770,6 +770,9 @@ static int intel_pmu_hw_config(struct perf_event *event)
 	if (event->attr.type != PERF_TYPE_RAW)
 		return 0;
 
+	if (event->attr.raw_type)
+		return -EINVAL;
+
 	if (!(event->attr.config & ARCH_PERFMON_EVENTSEL_ANY))
 		return 0;
 
arch/x86/kernel/cpu/perf_event_p4.c +5 -0
index 87e1803..1001892 100644
@@ -437,6 +437,11 @@ static int p4_hw_config(struct perf_event *event)
 		event->hw.config = p4_set_ht_bit(event->hw.config);
 
 	if (event->attr.type == PERF_TYPE_RAW) {
+		/* only raw perfctr config supported */
+		if (event->attr.raw_type) {
+			rc = -EINVAL;
+			goto out;
+		}
 
 		/* user data may have out-of-bound event index */
 		evnt = p4_config_unpack_event(event->attr.config);
include/linux/perf_event.h +5 -3
index b50f4cf..f9d2d5e 100644
@@ -214,16 +214,18 @@ struct perf_event_attr {
 				 *  See also PERF_RECORD_MISC_EXACT_IP
 				 */
 				precise_ip     :  2, /* skid constraint       */
-				model_spec     :  1, /* model specific hw event */
 
-				__reserved_1   : 46;
+				__reserved_1   : 47;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
 		__u32		wakeup_watermark; /* bytes before wakeup   */
 	};
 
-	__u32			bp_type;
+	union {
+		__u32		bp_type;
+		__u32		raw_type;
+	};
 	__u64			bp_addr;
 	__u64			bp_len;
 };
#1 lore raw

PATCH 1/7 perf: introduce raw_type attribute to specify the type of a raw sample

This patch introduces a method to specify the type of a raw sample. This can be used to setup hardware events other than generic performance counters by passing special config data to the pmu. The config data can be interpreted different from generic events and thus can be used for other purposes.

The raw_type attribute is an extension of the ABI. It reuses the unused bp_type space for this. Generic performance counters can be setup by setting the raw_type attribute to null. Thus special raw events must have a type other than null.

Raw types can be defined as needed for cpu models or architectures. To keep backward compatibility all architectures must return an error for an event with a raw_type other than null that is not supported.

E.g., raw_type can be used to setup IBS on an AMD cpu. IBS is not common to pmu features from other vendors or architectures. The pmu must be setup with a special config value. Sample data is returned in a certain format back to the userland. An IBS event is created by setting a raw event and encoding the IBS type in raw_type. The pmu handles this raw event then and passes raw sample data back.

Raw type could be architecure specific, e.g. for x86:

enum perf_raw_type {

        PERF_RAW_PERFCTR                        = 0,
        PERF_RAW_IBS_FETCH                      = 1,
        PERF_RAW_IBS_OP                         = 2,

PERF_RAW_MAX,
};

Null is the architecture's default, meaning for x86 a perfctr.

Maybe the raw type definition could also be part of the ABI with one definition for all architectures.

To use raw events with perf, the raw event syntax could be suffixed by the type (as for breakpoints):

-e rNNN[:TYPE]

Example:

perf record -e r186A:1 # ... meaning IBS fetch, cycle count 100000
perf record -e r0:1 -c 100000 # ... the same

Or with named types:

perf record -e r186A:IBS_FETCH ...
perf record -e r0:IBS_FETCH -c 100000 ...

This solution has a number of advantages: A raw event type may be specified without to encode the type in the config value. The attr flags are not 'polluted'. We can follow the already existing breakpoint concept in syntax and encoding.

---
 arch/arm/kernel/perf_event.c             |    5 +++++
 arch/powerpc/kernel/perf_event.c         |    2 ++
 arch/powerpc/kernel/perf_event_fsl_emb.c |    2 ++
 arch/sh/kernel/perf_event.c              |    2 ++
 arch/x86/kernel/cpu/perf_event.c         |    5 ++++-
 arch/x86/kernel/cpu/perf_event_amd.c     |    3 +++
 arch/x86/kernel/cpu/perf_event_intel.c   |    3 +++
 arch/x86/kernel/cpu/perf_event_p4.c      |    5 +++++
 include/linux/perf_event.h               |    5 ++++-
 9 files changed, 30 insertions(+), 2 deletions(-)
arch/arm/kernel/perf_event.c +5 -0
index 9e70f20..73d680c 100644
@@ -388,6 +388,11 @@ __hw_perf_event_init(struct perf_event *event)
 	} else if (PERF_TYPE_HW_CACHE == event->attr.type) {
 		mapping = armpmu_map_cache_event(event->attr.config);
 	} else if (PERF_TYPE_RAW == event->attr.type) {
+		if (event->attr.raw_type) {
+			pr_debug("invalid raw type %x\n",
+				 event->attr.raw_type);
+			return -EINVAL;
+		}
 		mapping = armpmu->raw_event(event->attr.config);
 	} else {
 		pr_debug("event type %x not supported\n", event->attr.type);
arch/powerpc/kernel/perf_event.c +2 -0
index 43b83c3..c8fb3cf 100644
@@ -1036,6 +1036,8 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
 			return ERR_PTR(err);
 		break;
 	case PERF_TYPE_RAW:
+		if (event->attr.raw_type)
+			return ERR_PTR(-EINVAL);
 		ev = event->attr.config;
 		break;
 	default:
arch/powerpc/kernel/perf_event_fsl_emb.c +2 -0
index 369872f..7547e96 100644
@@ -452,6 +452,8 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
 		break;
 
 	case PERF_TYPE_RAW:
+		if (event->attr.raw_type)
+			return ERR_PTR(-EINVAL);
 		ev = event->attr.config;
 		break;
 
arch/sh/kernel/perf_event.c +2 -0
index 81b6de4..482cf48 100644
@@ -142,6 +142,8 @@ static int __hw_perf_event_init(struct perf_event *event)
 
 	switch (attr->type) {
 	case PERF_TYPE_RAW:
+		if (attr->raw_type)
+			return -EINVAL;
 		config = attr->config & sh_pmu->raw_event_mask;
 		break;
 	case PERF_TYPE_HW_CACHE:
arch/x86/kernel/cpu/perf_event.c +4 -1
index fd4db0d..3539b53 100644
@@ -449,8 +449,11 @@ static int x86_setup_perfctr(struct perf_event *event)
 			return -EOPNOTSUPP;
 	}
 
-	if (attr->type == PERF_TYPE_RAW)
+	if (attr->type == PERF_TYPE_RAW) {
+		if (attr->raw_type)
+			return -EINVAL;
 		return 0;
+	}
 
 	if (attr->type == PERF_TYPE_HW_CACHE)
 		return set_ext_hw_attr(hwc, attr);
arch/x86/kernel/cpu/perf_event_amd.c +3 -0
index 611df11..87e5ae4 100644
@@ -121,6 +121,9 @@ static int amd_pmu_hw_config(struct perf_event *event)
 	if (event->attr.type != PERF_TYPE_RAW)
 		return 0;
 
+	if (event->attr.raw_type)
+		return -EINVAL;
+
 	event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
 
 	return 0;
arch/x86/kernel/cpu/perf_event_intel.c +3 -0
index fdbc652..d15faf5 100644
@@ -770,6 +770,9 @@ static int intel_pmu_hw_config(struct perf_event *event)
 	if (event->attr.type != PERF_TYPE_RAW)
 		return 0;
 
+	if (event->attr.raw_type)
+		return -EINVAL;
+
 	if (!(event->attr.config & ARCH_PERFMON_EVENTSEL_ANY))
 		return 0;
 
arch/x86/kernel/cpu/perf_event_p4.c +5 -0
index 87e1803..1001892 100644
@@ -437,6 +437,11 @@ static int p4_hw_config(struct perf_event *event)
 		event->hw.config = p4_set_ht_bit(event->hw.config);
 
 	if (event->attr.type == PERF_TYPE_RAW) {
+		/* only raw perfctr config supported */
+		if (event->attr.raw_type) {
+			rc = -EINVAL;
+			goto out;
+		}
 
 		/* user data may have out-of-bound event index */
 		evnt = p4_config_unpack_event(event->attr.config);
include/linux/perf_event.h +4 -1
index fe50347..f9d2d5e 100644
@@ -222,7 +222,10 @@ struct perf_event_attr {
 		__u32		wakeup_watermark; /* bytes before wakeup   */
 	};
 
-	__u32			bp_type;
+	union {
+		__u32		bp_type;
+		__u32		raw_type;
+	};
 	__u64			bp_addr;
 	__u64			bp_len;
 };
Signature · 1 line
1.7.1
#2 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Hi Robert,

Quoted 44 lines · Robert Richter

This patch introduces a method to specify the type of a raw sample. This can be used to setup hardware events other than generic performance counters by passing special config data to the pmu. The config data can be interpreted different from generic events and thus can be used for other purposes.

The raw_type attribute is an extension of the ABI. It reuses the unused bp_type space for this. Generic performance counters can be setup by setting the raw_type attribute to null. Thus special raw events must have a type other than null.

Raw types can be defined as needed for cpu models or architectures. To keep backward compatibility all architectures must return an error for an event with a raw_type other than null that is not supported.

E.g., raw_type can be used to setup IBS on an AMD cpu. IBS is not common to pmu features from other vendors or architectures. The pmu must be setup with a special config value. Sample data is returned in a certain format back to the userland. An IBS event is created by setting a raw event and encoding the IBS type in raw_type. The pmu handles this raw event then and passes raw sample data back.

Raw type could be architecure specific, e.g. for x86:

enum perf_raw_type {

        PERF_RAW_PERFCTR                        = 0,
        PERF_RAW_IBS_FETCH                      = 1,
        PERF_RAW_IBS_OP                         = 2,

PERF_RAW_MAX,
};

Null is the architecture's default, meaning for x86 a perfctr.

Maybe the raw type definition could also be part of the ABI with one definition for all architectures.

To use raw events with perf, the raw event syntax could be suffixed by the type (as for breakpoints):

   -e rNNN[:TYPE]

Example:

Quoted 7 lines

perf record -e r186A:1 # ... meaning IBS fetch, cycle count 100000
perf record -e r0:1 -c 100000 # ... the same

Or with named types:

perf record -e r186A:IBS_FETCH ...
perf record -e r0:IBS_FETCH -c 100000 ...

Should this raw value have been 186A0 instead of 186A?

Where is the named type translation coming from? Is this something that needs to be hard-coded into perf?

Have you looked at Lin Ming's patch series? I think it offers another way to support IBS and other arch-specific and off-chip PMUs in a more general way, though it's not quite fully-baked yet.

- Corey

#3 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 6 lines · Corey Ashford

[...]

Have you looked at Lin Ming's patch series? I think it offers another way to support IBS and other arch-specific and off-chip PMUs in a more general way, though it's not quite fully-baked yet.

Robert, check out this thread on lkml:

[RFC][PATCH v2 06/11] perf: core, export pmus via sysfs

Here is Peter's post that describes the high level design:

http://groups.google.com/group/linux.kernel/msg/ab9aa075016c639e

And please help out Lin Ming with this work - we dont want to add new, special-purpose ABI extensions for enumeration purposes, it should be introduced using a new event_source node via the sysfs enumeration.

Whether IBS fits that category is an open question - ideally it should have similar high level usage as the PEBS/LBR bits on Intel CPUs. (at least as far as tooling goes)

Thanks,

Ingo

#4 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 4 lines · Ingo Molnar

Whether IBS fits that category is an open question - ideally it should have similar high level usage as the PEBS/LBR bits on Intel CPUs. (at least as far as tooling goes)

Ingo,

high level usage is from the perspective of perf tools. IBS delivers high detailed diagnostic data that is also processed by userland tools other than perf. If we would introduce another interface layer it becomes much harder for such tools to use the pmu via this layer. The raw data is of much more interest here for users. If we think of perf_event as a subsystem controlling the pmu in a single instance, it should provide access to set and get raw pmu registers. I see this as a necessary feature.

Nevertheless I agree that key features should be exposed also with a high level interface and if possible in an abstracted non-model specific way.

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
#5 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

In reply to Corey Ashford
Quoted 4 lines · Corey Ashford
Quoted 2 lines

perf record -e r186A:IBS_FETCH ...
perf record -e r0:IBS_FETCH -c 100000 ...

Should this raw value have been 186A0 instead of 186A?

This is the 20 bit value of the cycle count, but only bits 19:4 are encoded in bits 15:0 of the raw ibs register. Lower 4 bits of the cycle count must be null and thus not pushed to the register. The raw register setup is correct.

Quoted 2 lines

Where is the named type translation coming from? Is this something that needs to be hard-coded into perf?

I was thinking of an enum or macro definition for the values in the kernel. A name mapping table could be implemented at least in the userspace, if needed maybe also in the kernel.

Quoted 3 lines

Have you looked at Lin Ming's patch series? I think it offers another way to support IBS and other arch-specific and off-chip PMUs in a more general way, though it's not quite fully-baked yet.

Yes, this could be an option too. The proposal was some days ago and a little hidden in its subject, so I missed it first. But the concept of registering a pmu feature looks good and could be an alternative to the raw_value approach.

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
#6 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

In reply to Robert Richter

Robert,

I still don't understand why you need all of this to encode IBS. I still believe that with attr.config there is plenty of bits to choose from. I do understand the need for PERF_SAMPLE_RAW. I think there is no other way.

You simply need to pick an encoding to mark the config as IBS. You need two bits for this: 00 regular counters, 01 IBS Fetch, 10 IBS op. Regular counters use 43 bits, IBS fetch uses 58, IBS op uses 52. So you could use bits 62-63 for instance. You don't need to encode the sampling period in attr.config for either IBS. You can use attr.sample_period, so you free up 16 bits.

I understand that IBS may evolve and thus may use more bits. But you still have at least 16 bits of margin.

Users and tools would rely on an library to provide the event encoding. No need to come up with some raw hex number on the cmdline.

Quoted 207 lines · Robert Richter

This patch introduces a method to specify the type of a raw sample. This can be used to setup hardware events other than generic performance counters by passing special config data to the pmu. The config data can be interpreted different from generic events and thus can be used for other purposes.

The raw_type attribute is an extension of the ABI. It reuses the unused bp_type space for this. Generic performance counters can be setup by setting the raw_type attribute to null. Thus special raw events must have a type other than null.

Raw types can be defined as needed for cpu models or architectures. To keep backward compatibility all architectures must return an error for an event with a raw_type other than null that is not supported.

E.g., raw_type can be used to setup IBS on an AMD cpu. IBS is not common to pmu features from other vendors or architectures. The pmu must be setup with a special config value. Sample data is returned in a certain format back to the userland. An IBS event is created by setting a raw event and encoding the IBS type in raw_type. The pmu handles this raw event then and passes raw sample data back.

Raw type could be architecure specific, e.g. for x86:

enum perf_raw_type {
       PERF_RAW_PERFCTR                        = 0, PERF_RAW_IBS_FETCH                      = 1, PERF_RAW_IBS_OP                         = 2,

       PERF_RAW_MAX,
};

Null is the architecture's default, meaning for x86 a perfctr.

Maybe the raw type definition could also be part of the ABI with one definition for all architectures.

To use raw events with perf, the raw event syntax could be suffixed by the type (as for breakpoints):

  -e rNNN[:TYPE]

Example:

 perf record -e r186A:1          # ... meaning IBS fetch, cycle count 100000 perf record -e r0:1 -c 100000   # ... the same

Or with named types:

 perf record -e r186A:IBS_FETCH ...
 perf record -e r0:IBS_FETCH -c 100000 ...

This solution has a number of advantages: A raw event type may be specified without to encode the type in the config value. The attr flags are not 'polluted'. We can follow the already existing breakpoint concept in syntax and encoding.

---
 arch/arm/kernel/perf_event.c             |    5 +++++ arch/powerpc/kernel/perf_event.c         |    2 ++ arch/powerpc/kernel/perf_event_fsl_emb.c |    2 ++ arch/sh/kernel/perf_event.c              |    2 ++ arch/x86/kernel/cpu/perf_event.c         |    5 ++++- arch/x86/kernel/cpu/perf_event_amd.c     |    3 +++ arch/x86/kernel/cpu/perf_event_intel.c   |    3 +++ arch/x86/kernel/cpu/perf_event_p4.c      |    5 +++++ include/linux/perf_event.h               |    5 ++++-

 9 files changed, 30 insertions(+), 2 deletions(-)
arch/arm/kernel/perf_event.c +5 -0
index 9e70f20..73d680c 100644
@@ -388,6 +388,11 @@ __hw_perf_event_init(struct perf_event *event)
        } else if (PERF_TYPE_HW_CACHE == event->attr.type) {
                mapping = armpmu_map_cache_event(event->attr.config);
        } else if (PERF_TYPE_RAW == event->attr.type) {
+               if (event->attr.raw_type) {
+                       pr_debug("invalid raw type %x\n",
+                                event->attr.raw_type);
+                       return -EINVAL;
+               }
                mapping = armpmu->raw_event(event->attr.config);
        } else {
                pr_debug("event type %x not supported\n", event->attr.type);
arch/powerpc/kernel/perf_event.c +0 -0
index 43b83c3..c8fb3cf 100644
@@ -1036,6 +1036,8 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
                        return ERR_PTR(err);
                break;

       case PERF_TYPE_RAW:
+               if (event->attr.raw_type)
+                       return ERR_PTR(-EINVAL);
               ev = event->attr.config;
               break;
       default:

arch/powerpc/kernel/perf_event_fsl_emb.c +0 -0
index 369872f..7547e96 100644
@@ -452,6 +452,8 @@ const struct pmu *hw_perf_event_init(struct perf_event *event)
                break;
 

       case PERF_TYPE_RAW:
+               if (event->attr.raw_type)
+                       return ERR_PTR(-EINVAL);
               ev = event->attr.config;
               break;

arch/sh/kernel/perf_event.c +0 -0
index 81b6de4..482cf48 100644
@@ -142,6 +142,8 @@ static int __hw_perf_event_init(struct perf_event *event)
 
        switch (attr->type) {

       case PERF_TYPE_RAW:
+               if (attr->raw_type)
+                       return -EINVAL;
               config = attr->config & sh_pmu->raw_event_mask; break;
       case PERF_TYPE_HW_CACHE:

arch/x86/kernel/cpu/perf_event.c +4 -1
index fd4db0d..3539b53 100644
@@ -449,8 +449,11 @@ static int x86_setup_perfctr(struct perf_event *event)
                        return -EOPNOTSUPP;
        }
 
-       if (attr->type == PERF_TYPE_RAW)
+       if (attr->type == PERF_TYPE_RAW) {
+               if (attr->raw_type)
+                       return -EINVAL;
                return 0;
+       }
 
        if (attr->type == PERF_TYPE_HW_CACHE)
                return set_ext_hw_attr(hwc, attr);
arch/x86/kernel/cpu/perf_event_amd.c +3 -0
index 611df11..87e5ae4 100644
@@ -121,6 +121,9 @@ static int amd_pmu_hw_config(struct perf_event *event)
        if (event->attr.type != PERF_TYPE_RAW)
                return 0;
 
+       if (event->attr.raw_type)
+               return -EINVAL;
+
        event->hw.config |= event->attr.config & AMD64_RAW_EVENT_MASK;
 
        return 0;
arch/x86/kernel/cpu/perf_event_intel.c +3 -0
index fdbc652..d15faf5 100644
@@ -770,6 +770,9 @@ static int intel_pmu_hw_config(struct perf_event *event)
        if (event->attr.type != PERF_TYPE_RAW)
                return 0;
 
+       if (event->attr.raw_type)
+               return -EINVAL;
+
        if (!(event->attr.config & ARCH_PERFMON_EVENTSEL_ANY))
                return 0;
 
arch/x86/kernel/cpu/perf_event_p4.c +5 -0
index 87e1803..1001892 100644
@@ -437,6 +437,11 @@ static int p4_hw_config(struct perf_event *event)
                event->hw.config = p4_set_ht_bit(event->hw.config);
 
        if (event->attr.type == PERF_TYPE_RAW) {
+               /* only raw perfctr config supported */
+               if (event->attr.raw_type) {
+                       rc = -EINVAL;
+                       goto out;
+               }
 
                /* user data may have out-of-bound event index */
                evnt = p4_config_unpack_event(event->attr.config);
include/linux/perf_event.h +0 -0
index fe50347..f9d2d5e 100644
@@ -222,7 +222,10 @@ struct perf_event_attr {

               __u32           wakeup_watermark; /* bytes before wakeup   */ };

-       __u32                   bp_type;
+       union {
+               __u32           bp_type;
+               __u32           raw_type;
+       };
       __u64                   bp_addr;
       __u64                   bp_len;
 };
--
1.7.1

maintainer
#7 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 12 lines · Stephane Eranian

I still don't understand why you need all of this to encode IBS. I still believe that with attr.config there is plenty of bits to choose from. I do understand the need for PERF_SAMPLE_RAW. I think there is no other way.

You simply need to pick an encoding to mark the config as IBS. You need two bits for this: 00 regular counters, 01 IBS Fetch, 10 IBS op. Regular counters use 43 bits, IBS fetch uses 58, IBS op uses 52. So you could use bits 62-63 for instance. You don't need to encode the sampling period in attr.config for either IBS. You can use attr.sample_period, so you free up 16 bits.

Quoted 5 lines

I understand that IBS may evolve and thus may use more bits. But you still have at least 16 bits of margin.

Users and tools would rely on an library to provide the event encoding. No need to come up with some raw hex number on the cmdline.

No need for any of that afaict, how about:

For Instruction-Fetch:

0:15 sample-period (r/w)

 16:31 cnt           (r/w)
 32:47 latency       (r/w)
    48 enable        (r/w)
    49 valid         (r/w)
 50:56               (ro)
    57 randomized    (r/w)

For Instruction-Execution:

0:15 sample-period (r/w)
17 enable (r/w)
18 valid (r/w)

So if we add perf_event_attr::latency (can also be used for PEBS-load-latency, can Sparc/PowerPC/ARM/SH use this too?), we can encode these IBS things as:

0x87 Instruction Fetch Stall -- Ins-Fetch
0xC0 Retired Instructions -- Ins-Exec

When we set perf_event_attr::precise > 0

The Ins-Exec will have to re-construct the actual event->count by adding sample-period on each interrupt, as it seems we lack an actual counter in hardware.

Furthermore, these counters will have to deal with sample-period > 2^16 by 'ignoring' interrupts until we get ->period_left down to 0.

The extra data could possibly be exposed through attaching non-sampling group events and using SAMPLE_READ, like L1-misses, although reconstructing the count from just one bit seems 'interesting'.

The IbsFetchLinAd/IbsOpRip would go straight into PERF_SAMPLE_IP by replacing pt_regs->ip I guess.

IbsDcLinAd goes into PERF_SAMPLE_ADDR

#8 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 23 lines · Peter Zijlstra
Quoted 12 lines · Stephane Eranian

I still don't understand why you need all of this to encode IBS. I still believe that with attr.config there is plenty of bits to choose from. I do understand the need for PERF_SAMPLE_RAW. I think there is no other way.

You simply need to pick an encoding to mark the config as IBS. You need two bits for this: 00 regular counters, 01 IBS Fetch, 10 IBS op. Regular counters use 43 bits, IBS fetch uses 58, IBS op uses 52. So you could use bits 62-63 for instance. You don't need to encode the sampling period in attr.config for either IBS. You can use attr.sample_period, so you free up 16 bits.

Quoted 5 lines

I understand that IBS may evolve and thus may use more bits. But you still have at least 16 bits of margin.

Users and tools would rely on an library to provide the event encoding. No need to come up with some raw hex number on the cmdline.

No need for any of that afaict, how about:

For Instruction-Fetch:

Quoted 8 lines

 0:15 sample-period (r/w)
 16:31 cnt           (r/w)
 32:47 latency       (r/w)
   48 enable        (r/w)
   49 valid         (r/w)
 50:56               (ro)
   57 randomized    (r/w)

Your are mixing output and input parameters.

The only input parameters you have are:
- sample-period, enable, random
The rest is output only.

So I would say: you don't need to encode anything
sample-period -> attr.sample_period
random -> attr.random_width (once we add that)

Quoted 7 lines

For Instruction-Execution:

 0:15 sample-period (r/w)
   17 enable        (r/w)
   18 valid         (r/w)

Same thing here, sample-period is the only input parameter. so sample-period -> attr.sample_period

Both IBFETCHCTL, IBSOP would also be included in
the PERF_SAMPLE_RAW. Because they have the valid
bits and latency.

Quoted 2 lines

So if we add perf_event_attr::latency (can also be used for PEBS-load-latency, can Sparc/PowerPC/ARM/SH use this too?), we can

I suspect you can encode the latency with the event code used for PEBS load-latency (PEBS-LL). It can only be used with one event.

Quoted 5 lines

encode these IBS things as:

 0x87 Instruction Fetch Stall -- Ins-Fetch
 0xC0 Retired Instructions    -- Ins-Exec

I think those events do not map to the behavior of IBS. We have add that discussion before.

Quoted 2 lines

When we set perf_event_attr::precise > 0

Well, then you'd have to document that when you do that you use IBS and that the event do not count the same things.

Quoted 4 lines

The Ins-Exec will have to re-construct the actual event->count by adding sample-period on each interrupt, as it seems we lack an actual counter in hardware.

For what? counting mode?

Quoted 3 lines

Furthermore, these counters will have to deal with sample-period > 2^16 by 'ignoring' interrupts until we get ->period_left down to 0.

Well, it's not 2^16, it's 2^20 but bottom 4 bits must be zero. What about simply failing perf_event_open() is sample_period does not fit the constraint?

Quoted 9 lines

The extra data could possibly be exposed through attaching non-sampling group events and using SAMPLE_READ, like L1-misses, although reconstructing the count from just one bit seems 'interesting'.

The IbsFetchLinAd/IbsOpRip would go straight into PERF_SAMPLE_IP by replacing pt_regs->ip I guess.

IbsDcLinAd goes into PERF_SAMPLE_ADDR

What about the rest, the TLB, alignment, data sources?

maintainer
#9 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 9 lines · Stephane Eranian
Quoted 3 lines

For Instruction-Fetch:

32:47 latency (r/w)

Your are mixing output and input parameters.

The only input parameters you have are:
- sample-period, enable, random
The rest is output only.

Ah, my bad, I thought it was a r/w field.

Quoted 7 lines
Quoted 5 lines

encode these IBS things as:

0x87 Instruction Fetch Stall -- Ins-Fetch
0xC0 Retired Instructions -- Ins-Exec

I think those events do not map to the behavior of IBS. We have add that discussion before.

Hrm,. so there are no regular events that count the same thing as the IBS things? That really sucks.

So yeah, you might as well expose it as a whole separate PMU using Lin's stuff.

Quoted 5 lines
Quoted 4 lines

The Ins-Exec will have to re-construct the actual event->count by adding sample-period on each interrupt, as it seems we lack an actual counter in hardware.

For what? counting mode?

Yeah, events are supposed to count.

Quoted 6 lines
Quoted 3 lines

Furthermore, these counters will have to deal with sample-period > 2^16 by 'ignoring' interrupts until we get ->period_left down to 0.

Well, it's not 2^16, it's 2^20 but bottom 4 bits must be zero. What about simply failing perf_event_open() is sample_period does not fit the constraint?

Why, its simple enough to ignore a few interrupts, we do the same for all other implementations.

Quoted 10 lines
Quoted 9 lines

The extra data could possibly be exposed through attaching non-sampling group events and using SAMPLE_READ, like L1-misses, although reconstructing the count from just one bit seems 'interesting'.

The IbsFetchLinAd/IbsOpRip would go straight into PERF_SAMPLE_IP by replacing pt_regs->ip I guess.

IbsDcLinAd goes into PERF_SAMPLE_ADDR

What about the rest, the TLB, alignment, data sources?

Dunno, reconstruct sensible counters? Surely the software that uses IBS does something useful with that data? What does libpfm do with the IBS data?

#10 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 19 lines · Peter Zijlstra
Quoted 9 lines · Stephane Eranian
Quoted 3 lines

For Instruction-Fetch:

 32:47 latency       (r/w)

Your are mixing output and input parameters.

The only input parameters you have are:
- sample-period, enable, random
The rest is output only.

Ah, my bad, I thought it was a r/w field.

Quoted 5 lines
Quoted 5 lines

encode these IBS things as:

 0x87 Instruction Fetch Stall -- Ins-Fetch
 0xC0 Retired Instructions    -- Ins-Exec

Quoted 8 lines
Quoted 2 lines

I think those events do not map to the behavior of IBS. We have add that discussion before.

Hrm,. so there are no regular events that count the same thing as the IBS things? That really sucks.

So yeah, you might as well expose it as a whole separate PMU using Lin's stuff.

What's wrong with creating pseudo-events for IBS? We'd have to pick two unused event codes. That would have the advantage of making it explicit you're using IBS. I think we could still use the precise_ip field if people are only interested in the IP. They would use PERF_SAMPLE_RAW if they need more.

Quoted 9 lines
Quoted 6 lines
Quoted 5 lines

The Ins-Exec will have to re-construct the actual event->count by adding sample-period on each interrupt, as it seems we lack an actual counter in hardware.

For what? counting mode?

Yeah, events are supposed to count.

IBS is a sampling only feature. I suspect it would be okay to return 0 here or do as you said, count the number of IBS interrupts and multiply by the sampling period.

Quoted 17 lines
Quoted 6 lines
Quoted 3 lines

Furthermore, these counters will have to deal with sample-period > 2^16 by 'ignoring' interrupts until we get ->period_left down to 0.

Well, it's not 2^16, it's 2^20 but bottom 4 bits must be zero. What about simply failing perf_event_open() is sample_period does not fit the constraint?

Why, its simple enough to ignore a few interrupts, we do the same for all other implementations.

Quoted 7 lines
Quoted 7 lines

The extra data could possibly be exposed through attaching non-sampling group events and using SAMPLE_READ, like L1-misses, although reconstructing the count from just one bit seems 'interesting'.

The IbsFetchLinAd/IbsOpRip would go straight into PERF_SAMPLE_IP by replacing pt_regs->ip I guess.

Quoted 8 lines
Quoted 3 lines
Quoted 2 lines

IbsDcLinAd goes into PERF_SAMPLE_ADDR

What about the rest, the TLB, alignment, data sources?

Dunno, reconstruct sensible counters? Surely the software that uses IBS does something useful with that data? What does libpfm do with the IBS data?

The common usage model is you gather the IBSop data (all of it), you save samples into a file and then you have scripts that extract whatever fields they need to compute the metric you want. For instance, if you want data cache misses you extract [IP, data address, data source, miss latency], if you care about instruction latencies, you extract [IP, tag2ret, comp2ret], and so on.

Libpfm does not handle IBS output. Its goal is to help applications setup the events/counters. With perf_events, it does the mapping from symbolic event names+attributes -> struct perf_event_attr. It does not make any perf_event syscalls for you.

#11 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 5 lines · Stephane Eranian

What's wrong with creating pseudo-events for IBS? We'd have to pick two unused event codes. That would have the advantage of making it explicit you're using IBS. I think we could still use the precise_ip field if people are only interested in the IP. They would use PERF_SAMPLE_RAW if they need more.

For some key events this is fine and useful. But, if this has to be used to programm all IBS features, it will introduce too much implementation and maintenance effort. What you get then will probably of less interest since users/tools want to have the raw sample data for further processing. But maybe we agree already in this point, if there is a demand we should provide pseudo events for important or offen used events.

Quoted 11 lines
Quoted 8 lines
Quoted 5 lines
Quoted 4 lines

The Ins-Exec will have to re-construct the actual event->count by adding sample-period on each interrupt, as it seems we lack an actual counter in hardware.

For what? counting mode?

Yeah, events are supposed to count.

IBS is a sampling only feature. I suspect it would be okay to return 0 here or do as you said, count the number of IBS interrupts and multiply by the sampling period.

True, ibs can be used as an additional counter using the interrupt only and dropping sampling data. I was already thinking of adding it somehow to the list of available counters. But this is also later work as its 64 bit extension.

Quoted 3 lines
Quoted 3 lines

Dunno, reconstruct sensible counters? Surely the software that uses IBS does something useful with that data? What does libpfm do with the IBS data?

Here are some use cases described:

http://developer.amd.com/assets/AMD_IBS_paper_EN.pdf
http://developer.amd.com/cpu/CodeAnalyst/assets/ISPASS2010_IBS_CA_abstract.pdf

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
#12 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 8 lines · Robert Richter
Quoted 3 lines
Quoted 3 lines

Dunno, reconstruct sensible counters? Surely the software that uses IBS does something useful with that data? What does libpfm do with the IBS data?

Here are some use cases described:

http://developer.amd.com/assets/AMD_IBS_paper_EN.pdf
http://developer.amd.com/cpu/CodeAnalyst/assets/ISPASS2010_IBS_CA_abstract.pdf

A copy of this document can be found here:

http://www.kernel.org/pub/linux/kernel/people/rric/ISPASS2010_IBS_CA_abstract.pdf

-Robert

#13 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

In reply to Peter Zijlstra
Quoted 2 lines · Peter Zijlstra

So yeah, you might as well expose it as a whole separate PMU using Lin's stuff.

This seems to be the most promising alternative to raw_type. But it's also much more bloated requiring sysfs and additional file descriptors.

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
#14 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 7 lines · Robert Richter
Quoted 2 lines · Peter Zijlstra

So yeah, you might as well expose it as a whole separate PMU using Lin's stuff.

This seems to be the most promising alternative to raw_type. But it's also much more bloated requiring sysfs and additional file descriptors.

Not really, fds are really cheap on Linux, and in any case it's only needed when the event is created - can be closed afterwards.

Ingo

#15 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 2 lines · Ingo Molnar

Not really, fds are really cheap on Linux, and in any case it's only needed when the event is created - can be closed afterwards.

I was wondering if we could get the perf event fd directly with the open("/sys/...") command? This would be really easy to set up.

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
maintainer
#16 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 6 lines · Robert Richter
Quoted 2 lines · Ingo Molnar

Not really, fds are really cheap on Linux, and in any case it's only needed when the event is created - can be closed afterwards.

I was wondering if we could get the perf event fd directly with the open("/sys/...") command? This would be really easy to set up.

I guess we could sorta do that, but then we'd need a ioctl to change the perf_event_attr.

#17 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

In reply to Peter Zijlstra
Quoted 13 lines · Peter Zijlstra
Quoted 12 lines · Stephane Eranian

I still don't understand why you need all of this to encode IBS. I still believe that with attr.config there is plenty of bits to choose from. I do understand the need for PERF_SAMPLE_RAW. I think there is no other way.

You simply need to pick an encoding to mark the config as IBS. You need two bits for this: 00 regular counters, 01 IBS Fetch, 10 IBS op. Regular counters use 43 bits, IBS fetch uses 58, IBS op uses 52. So you could use bits 62-63 for instance. You don't need to encode the sampling period in attr.config for either IBS. You can use attr.sample_period, so you free up 16 bits.

Quoted 2 lines
Quoted 2 lines

I understand that IBS may evolve and thus may use more bits. But you still have at least 16 bits of margin.

We have some bits in the msrs that are reserved now, for perfctr and also for ibs. We could start reusing it to mark a special sample type and so on. So far, ok. Somewhen in the future there is a hw extensions and these bits are not reserved anymore, what now?

In case the register layout changes, you start workaround this shifting and masking bits back and forth. This will end in a mess. We don't know what will change and when, but *reserved* means it could be subject to change. So, actually you have to assume arbitrary 64 bit config values for perfctrs and ibs. Thus, you cannot use reserved bits for encoding, doing this would be a hack.

The approach is also not extendable for new pmu features. The question will rise again then how to encode it.

So encoding this information in some other attribute like raw_type or any other field is a much cleaner and a more stable solution, easier to program and to maintain. And there is enough space in the attribute structure.

Quoted 2 lines
Quoted 2 lines

Users and tools would rely on an library to provide the event encoding. No need to come up with some raw hex number on the cmdline.

We can pack everything in tools and libraries and hide direct access to the user. Now, one reads the IBS spec and wants to start using it. He will have to learn (by reading the source or documentation) now, how to pass parameters using the tool or library to set certain bits in the registers. This adds an additional i/f layer what needs to be defined, documented, implemented and maintained. Why not keep things simple and let the user pass a register value to the pmu? Only the config has to be validated in the kernel and that's it. The hw spec is then the reference for using the interface. We have already raw config values for counters and there was a reason for it.

Quoted 11 lines

For Instruction-Fetch:

0:15 sample-period (r/w)
57 randomized (r/w)

For Instruction-Execution:

0:15 sample-period (r/w)

0x87 Instruction Fetch Stall -- Ins-Fetch
0xC0 Retired Instructions -- Ins-Exec

I agree, where possible we should reuse attributes or existing events for configuration. But since the pmu behavior differs, there is the risk of mixing different things together. "precise" would be different on Intel and AMD. This will be hidden to the user leading to wrong assumptions and results. And, what about new options that don't have equivalent attribute flags?

But it's not the question how to pass the config, the question is how to mark the event configuration as a certain IBS event.

Quoted 2 lines

Furthermore, these counters will have to deal with sample-period > 2^16 by 'ignoring' interrupts until we get ->period_left down to 0.

No question here, ibs counters should be extended to 64 bit counters, but this is not addressed with this patch set, I have kept this for later work.

Quoted 8 lines

The extra data could possibly be exposed through attaching non-sampling group events and using SAMPLE_READ, like L1-misses, although reconstructing the count from just one bit seems 'interesting'.

The IbsFetchLinAd/IbsOpRip would go straight into PERF_SAMPLE_IP by replacing pt_regs->ip I guess.

IbsDcLinAd goes into PERF_SAMPLE_ADDR

It's the same for sampling data as with config data. We can spread it all over its data structures, but a user will then recollect all of it again needing to know where the information can be found. And, it will never be the same kind of information if the pmus are different.

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
#18 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 14 lines · Robert Richter
Quoted 13 lines · Peter Zijlstra
Quoted 12 lines · Stephane Eranian

I still don't understand why you need all of this to encode IBS. I still believe that with attr.config there is plenty of bits to choose from. I do understand the need for PERF_SAMPLE_RAW. I think there is no other way.

You simply need to pick an encoding to mark the config as IBS. You need two bits for this: 00 regular counters, 01 IBS Fetch, 10 IBS op. Regular counters use 43 bits, IBS fetch uses 58, IBS op uses 52. So you could use bits 62-63 for instance. You don't need to encode the sampling period in attr.config for either IBS. You can use attr.sample_period, so you free up 16 bits.

Quoted 8 lines
Quoted 2 lines
Quoted 2 lines

I understand that IBS may evolve and thus may use more bits. But you still have at least 16 bits of margin.

We have some bits in the msrs that are reserved now, for perfctr and also for ibs. We could start reusing it to mark a special sample type and so on. So far, ok. Somewhen in the future there is a hw extensions and these bits are not reserved anymore, what now?

You will always have the sampling period bits. Those will always be handled by attr->sample_period. And they correspond to the lower bits which are also used to encode the event select on regular counters. So you could identify IBS with special event selects in the lower 8 bits for instance.

IBS is a very model specific feature, as such the method you use to encode it may change.

Quoted 7 lines

In case the register layout changes, you start workaround this shifting and masking bits back and forth. This will end in a mess. We don't know what will change and when, but *reserved* means it could be subject to change. So, actually you have to assume arbitrary 64 bit config values for perfctrs and ibs. Thus, you cannot use reserved bits for encoding, doing this would be a hack.

In my new proposal, I am not using reserved bits, I am using bits which are stored elsewhere in the attr structure.

Quoted 3 lines

The approach is also not extendable for new pmu features. The question will rise again then how to encode it.

There is no guarantee your scheme will help there either. Nobody knows the interface to those new features. It may be that you will need more than 64 bits.

Quoted 5 lines

So encoding this information in some other attribute like raw_type or any other field is a much cleaner and a more stable solution, easier to program and to maintain. And there is enough space in the attribute structure.

What I am missing with raw_type is how it plays out with attr->type which already has RAW? Seems like you have a second level of raw.

Quoted 26 lines
Quoted 2 lines
Quoted 2 lines

Users and tools would rely on an library to provide the event encoding. No need to come up with some raw hex number on the cmdline.

We can pack everything in tools and libraries and hide direct access to the user. Now, one reads the IBS spec and wants to start using it. He will have to learn (by reading the source or documentation) now, how to pass parameters using the tool or library to set certain bits in the registers. This adds an additional i/f layer what needs to be defined, documented, implemented and maintained. Why not keep things simple and let the user pass a register value to the pmu? Only the config has to be validated in the kernel and that's it. The hw spec is then the reference for using the interface. We have already raw config values for counters and there was a reason for it.

Quoted 11 lines

For Instruction-Fetch:

  0:15 sample-period (r/w)
    57 randomized    (r/w)

For Instruction-Execution:

  0:15 sample-period (r/w)

  0x87 Instruction Fetch Stall -- Ins-Fetch
  0xC0 Retired Instructions    -- Ins-Exec

Quoted 7 lines

I agree, where possible we should reuse attributes or existing events for configuration. But since the pmu behavior differs, there is the risk of mixing different things together. "precise" would be different on Intel and AMD. This will be hidden to the user leading to wrong assumptions and results. And, what about new options that don't have equivalent attribute flags?

I agree.

Quoted 3 lines

But it's not the question how to pass the config, the question is how to mark the event configuration as a certain IBS event.

Yes, we agree on that.

Quoted 7 lines
Quoted 2 lines

Furthermore, these counters will have to deal with sample-period > 2^16 by 'ignoring' interrupts until we get ->period_left down to 0.

No question here, ibs counters should be extended to 64 bit counters, but this is not addressed with this patch set, I have kept this for later work.

That's fine with me.

Quoted 7 lines
Quoted 7 lines

The extra data could possibly be exposed through attaching non-sampling group events and using SAMPLE_READ, like L1-misses, although reconstructing the count from just one bit seems 'interesting'.

The IbsFetchLinAd/IbsOpRip would go straight into PERF_SAMPLE_IP by replacing pt_regs->ip I guess.

Quoted 7 lines
Quoted 1 line

IbsDcLinAd goes into PERF_SAMPLE_ADDR

It's the same for sampling data as with config data. We can spread it all over its data structures, but a user will then recollect all of it again needing to know where the information can be found. And, it will never be the same kind of information if the pmus are different.

Well, I don't think you want to define an attribute for each and every bit of info returned by IBS. I think you need to return the IBSDATA regs as is and let the user pick and choose. Of course, the information can change, but IBS is model specific, it returns something richer than just a count.

#19 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

Quoted 5 lines
Quoted 3 lines

But it's not the question how to pass the config, the question is how to mark the event configuration as a certain IBS event.

Yes, we agree on that.

IBS uses at most 58 out of 64 bits today. We know we can move sampling period, random out to other fields in attr. In fact all we would need would be two bits. But to distinguish with regular events, I think we could reuse the event select field of 8 bits. So you would be using 8 out of 64. IBS could go from 58 to 64 = 6 more bits. Those could be encoded in the config field, where you have > 50 bits left.

#20 lore raw

Re: [PATCH 1/7] perf: introduce raw_type attribute to specify the type of a raw sample

In reply to Stephane Eranian
Quoted 11 lines · Stephane Eranian
Quoted 5 lines

We have some bits in the msrs that are reserved now, for perfctr and also for ibs. We could start reusing it to mark a special sample type and so on. So far, ok. Somewhen in the future there is a hw extensions and these bits are not reserved anymore, what now?

You will always have the sampling period bits. Those will always be handled by attr->sample_period. And they correspond to the lower bits which are also used to encode the event select on regular counters. So you could identify IBS with special event selects in the lower 8 bits for instance.

Quoted 5 lines

IBS is a very model specific feature, as such the method you use to encode it may change.

In my new proposal, I am not using reserved bits, I am using bits which are stored elsewhere in the attr structure.

Ok, maybe technically it is possible for ibs and perfctrs. You are moving the sample count from the raw config out to sample_count attribute, then the type is written to the new free space in the raw config. In the kernel you do all of this in reverse order. This makes programming events really difficult and confusing. I don't see an advantage compared to the use of a raw_type.

Quoted 3 lines

There is no guarantee your scheme will help there either. Nobody knows the interface to those new features. It may be that you will need more than 64 bits.

Right, for more than 64 bit the need another extension.

Quoted 2 lines

What I am missing with raw_type is how it plays out with attr->type which already has RAW? Seems like you have a second level of raw.

"raw_type" is only valid together with (attr->type == PERF_TYPE_RAW). Currently it is null (bp_type is unused and zeroed). So (raw_type ==
0) will leave everything as it is and uses the archtectures default raw config. If we set raw_type to a value other than zero, we have a special configuration for a special pmu feature. It looks sane to me.

Quoted 5 lines

Well, I don't think you want to define an attribute for each and every bit of info returned by IBS. I think you need to return the IBSDATA regs as is and let the user pick and choose. Of course, the information can change, but IBS is model specific, it returns something richer than just a count.

I think, we all agree the ibs sample should be send back to userland also as raw sample.

-Robert

Signature · 3 lines
Advanced Micro Devices, Inc.
Operating System Research Center
email: [email protected]
#21 lore raw

PATCH 2/7 perf, x86: introduce bit range for special pmu events

In reply to Robert Richter

There are some pmu events such as Intel BTS or AMD IBS that do not fit in the generic or fixed performance counter scheme. The upper bits starting at bit 48 of the 64 bit counter mask are reserved for such events and can be used to handle them. The events can be identified by its index in the bit mask.

---
 arch/x86/include/asm/perf_event.h         |    3 ++-
 arch/x86/kernel/cpu/perf_event.c          |    6 +++---
 arch/x86/kernel/cpu/perf_event_intel.c    |   10 +++++-----
 arch/x86/kernel/cpu/perf_event_intel_ds.c |    4 ++--
 4 files changed, 12 insertions(+), 11 deletions(-)
arch/x86/include/asm/perf_event.h +2 -1
index 254883d..7e51c75 100644
@@ -10,6 +10,7 @@
 
 #define X86_PMC_IDX_GENERIC				        0
 #define X86_PMC_IDX_FIXED				       32
+#define X86_PMC_IDX_SPECIAL				       48
 #define X86_PMC_IDX_MAX					       64
 
 #define MSR_ARCH_PERFMON_PERFCTR0			      0xc1
@@ -107,7 +108,7 @@ union cpuid10_edx {
  * values are used by actual fixed events and higher values are used
  * to indicate other overflow conditions in the PERF_GLOBAL_STATUS msr.
  */
-#define X86_PMC_IDX_FIXED_BTS				(X86_PMC_IDX_FIXED + 16)
+#define X86_PMC_IDX_SPECIAL_BTS				(X86_PMC_IDX_SPECIAL + 0)
 
 /* IbsFetchCtl bits/masks */
 #define IBS_FETCH_RAND_EN		(1ULL<<57)
arch/x86/kernel/cpu/perf_event.c +3 -3
index 3539b53..75c0a44 100644
@@ -283,7 +283,7 @@ x86_perf_event_update(struct perf_event *event)
 	int idx = hwc->idx;
 	s64 delta;
 
-	if (idx == X86_PMC_IDX_FIXED_BTS)
+	if (idx == X86_PMC_IDX_SPECIAL_BTS)
 		return 0;
 
 	/*
@@ -775,7 +775,7 @@ static inline void x86_assign_hw_event(struct perf_event *event,
 	hwc->last_cpu = smp_processor_id();
 	hwc->last_tag = ++cpuc->tags[i];
 
-	if (hwc->idx == X86_PMC_IDX_FIXED_BTS) {
+	if (hwc->idx == X86_PMC_IDX_SPECIAL_BTS) {
 		hwc->config_base = 0;
 		hwc->event_base	= 0;
 	} else if (hwc->idx >= X86_PMC_IDX_FIXED) {
@@ -891,7 +891,7 @@ x86_perf_event_set_period(struct perf_event *event)
 	s64 period = hwc->sample_period;
 	int ret = 0, idx = hwc->idx;
 
-	if (idx == X86_PMC_IDX_FIXED_BTS)
+	if (idx == X86_PMC_IDX_SPECIAL_BTS)
 		return 0;
 
 	/*
arch/x86/kernel/cpu/perf_event_intel.c +5 -5
index d15faf5..dfbbe69 100644
@@ -458,7 +458,7 @@ static void intel_pmu_disable_all(void)
 
 	wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, 0);
 
-	if (test_bit(X86_PMC_IDX_FIXED_BTS, cpuc->active_mask))
+	if (test_bit(X86_PMC_IDX_SPECIAL_BTS, cpuc->active_mask))
 		intel_pmu_disable_bts();
 
 	intel_pmu_pebs_disable_all();
@@ -473,9 +473,9 @@ static void intel_pmu_enable_all(int added)
 	intel_pmu_lbr_enable_all();
 	wrmsrl(MSR_CORE_PERF_GLOBAL_CTRL, x86_pmu.intel_ctrl);
 
-	if (test_bit(X86_PMC_IDX_FIXED_BTS, cpuc->active_mask)) {
+	if (test_bit(X86_PMC_IDX_SPECIAL_BTS, cpuc->active_mask)) {
 		struct perf_event *event =
-			cpuc->events[X86_PMC_IDX_FIXED_BTS];
+			cpuc->events[X86_PMC_IDX_SPECIAL_BTS];
 
 		if (WARN_ON_ONCE(!event))
 			return;
@@ -550,7 +550,7 @@ static void intel_pmu_disable_event(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
 
-	if (unlikely(hwc->idx == X86_PMC_IDX_FIXED_BTS)) {
+	if (unlikely(hwc->idx == X86_PMC_IDX_SPECIAL_BTS)) {
 		intel_pmu_disable_bts();
 		intel_pmu_drain_bts_buffer();
 		return;
@@ -602,7 +602,7 @@ static void intel_pmu_enable_event(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
 
-	if (unlikely(hwc->idx == X86_PMC_IDX_FIXED_BTS)) {
+	if (unlikely(hwc->idx == X86_PMC_IDX_SPECIAL_BTS)) {
 		if (!__get_cpu_var(cpu_hw_events).enabled)
 			return;
 
arch/x86/kernel/cpu/perf_event_intel_ds.c +2 -2
index 18018d1..bd46bbd 100644
@@ -176,7 +176,7 @@ static int reserve_ds_buffers(void)
  */
 
 static struct event_constraint bts_constraint =
-	EVENT_CONSTRAINT(0, 1ULL << X86_PMC_IDX_FIXED_BTS, 0);
+	EVENT_CONSTRAINT(0, 1ULL << X86_PMC_IDX_SPECIAL_BTS, 0);
 
 static void intel_pmu_enable_bts(u64 config)
 {
@@ -223,7 +223,7 @@ static void intel_pmu_drain_bts_buffer(void)
 		u64	to;
 		u64	flags;
 	};
-	struct perf_event *event = cpuc->events[X86_PMC_IDX_FIXED_BTS];
+	struct perf_event *event = cpuc->events[X86_PMC_IDX_SPECIAL_BTS];
 	struct bts_record *at, *top;
 	struct perf_output_handle handle;
 	struct perf_event_header header;
Signature · 1 line
1.7.1
#22 lore raw

PATCH 3/7 perf, x86: modify some code to allow the introduction of ibs events

In reply to Robert Richter

The changes are needed to introduce ibs events that are implemented as special x86 events.

---
 arch/x86/kernel/cpu/perf_event.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)
arch/x86/kernel/cpu/perf_event.c +9 -9
index 75c0a44..e64502c 100644
@@ -283,7 +283,7 @@ x86_perf_event_update(struct perf_event *event)
 	int idx = hwc->idx;
 	s64 delta;
 
-	if (idx == X86_PMC_IDX_SPECIAL_BTS)
+	if (idx >= X86_PMC_IDX_SPECIAL)
 		return 0;
 
 	/*
@@ -775,10 +775,10 @@ static inline void x86_assign_hw_event(struct perf_event *event,
 	hwc->last_cpu = smp_processor_id();
 	hwc->last_tag = ++cpuc->tags[i];
 
-	if (hwc->idx == X86_PMC_IDX_SPECIAL_BTS) {
-		hwc->config_base = 0;
-		hwc->event_base	= 0;
-	} else if (hwc->idx >= X86_PMC_IDX_FIXED) {
+	if (hwc->idx < X86_PMC_IDX_FIXED) {
+		hwc->config_base = x86_pmu.eventsel;
+		hwc->event_base  = x86_pmu.perfctr;
+	} else if (hwc->idx < X86_PMC_IDX_SPECIAL) {
 		hwc->config_base = MSR_ARCH_PERFMON_FIXED_CTR_CTRL;
 		/*
 		 * We set it so that event_base + idx in wrmsr/rdmsr maps to
@@ -786,9 +786,9 @@ static inline void x86_assign_hw_event(struct perf_event *event,
 		 */
 		hwc->event_base =
 			MSR_ARCH_PERFMON_FIXED_CTR0 - X86_PMC_IDX_FIXED;
-	} else {
-		hwc->config_base = x86_pmu.eventsel;
-		hwc->event_base  = x86_pmu.perfctr;
+	} else if (hwc->idx == X86_PMC_IDX_SPECIAL_BTS) {
+		hwc->config_base = 0;
+		hwc->event_base	= 0;
 	}
 }
 
@@ -891,7 +891,7 @@ x86_perf_event_set_period(struct perf_event *event)
 	s64 period = hwc->sample_period;
 	int ret = 0, idx = hwc->idx;
 
-	if (idx == X86_PMC_IDX_SPECIAL_BTS)
+	if (idx >= X86_PMC_IDX_SPECIAL_BTS)
 		return 0;
 
 	/*
Signature · 1 line
1.7.1
#23 lore raw

PATCH 4/7 perf, x86: implement IBS feature detection

In reply to Robert Richter

The new code checks if IBS is available on the cpu. It implements only a basic detection that should be later extended to read ibs cpuid feature flags.

---
 arch/x86/kernel/cpu/perf_event.c     |    5 +++++
 arch/x86/kernel/cpu/perf_event_amd.c |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/perf_event.c +5 -0
index e64502c..e186d3b 100644
@@ -244,6 +244,11 @@ struct x86_pmu {
 	 */
 	unsigned long	lbr_tos, lbr_from, lbr_to; /* MSR base regs       */
 	int		lbr_nr;			   /* hardware stack size */
+
+	/*
+	 * AMD IBS
+	 */
+	int		ibs;			/* cpuid flags */
 };
 
 static struct x86_pmu x86_pmu __read_mostly;
arch/x86/kernel/cpu/perf_event_amd.c +2 -0
index 87e5ae4..cda8475 100644
@@ -405,6 +405,8 @@ static __init int amd_pmu_init(void)
 		return -ENODEV;
 
 	x86_pmu = amd_pmu;
+	if (boot_cpu_has(X86_FEATURE_IBS))
+		x86_pmu.ibs = 1;
 
 	/* Events are common for all AMDs */
 	memcpy(hw_cache_event_ids, amd_hw_cache_event_ids,
Signature · 1 line
1.7.1
#24 lore raw

PATCH 5/7 perf, x86: setup NMI handler for IBS

In reply to Robert Richter

This implements the perf nmi handler for ibs interrupts. The code was copied from oprofile and should be merged somewhen.

---
 arch/x86/kernel/cpu/perf_event.c     |    5 ++
 arch/x86/kernel/cpu/perf_event_amd.c |   85 ++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 0 deletions(-)
arch/x86/kernel/cpu/perf_event.c +5 -0
index e186d3b..91c48b2 100644
@@ -533,6 +533,8 @@ static int x86_pmu_hw_config(struct perf_event *event)
 	return x86_setup_perfctr(event);
 }
 
+static inline void init_ibs_nmi(void);
+
 /*
  * Setup the hardware configuration for a given attr_type
  */
@@ -554,6 +556,8 @@ static int __hw_perf_event_init(struct perf_event *event)
 				if (err)
 					release_pmc_hardware();
 			}
+			if (!err)
+				init_ibs_nmi();
 		}
 		if (!err)
 			atomic_inc(&active_events);
@@ -1294,6 +1298,7 @@ static void __init pmu_check_apic(void)
 		return;
 
 	x86_pmu.apic = 0;
+	x86_pmu.ibs = 0;
 	pr_info("no APIC, boot with the \"lapic\" boot parameter to force-enable it.\n");
 	pr_info("no hardware sampling interrupt available.\n");
 }
arch/x86/kernel/cpu/perf_event_amd.c +85 -0

108 lines not on this page · raw message

Signature · 1 line
1.7.1
#25 lore raw

PATCH 6/7 perf, x86: implement AMD IBS event configuration

In reply to Robert Richter

This patch implements IBS for perf_event. It extends the AMD pmu to handle IBS and implements special raw events for this.

An IBS hw event can be selected by setting the event's raw type. There are two types of IBS events for instruction fetch (IBS_FETCH) and instruction execution (IBS_OP). Both events are implemented as special x86 events. The event allocation is implemented by using special event constraints for ibs. This way, the generic event scheduler can be used to allocate ibs events.

Except for the sample period IBS can only be set up with raw config values and raw data samples. The event attributes for the syscall should be programmed like this (IBS_FETCH):

        memset(&attr, 0, sizeof(attr));
        attr.type        = PERF_TYPE_RAW;
        attr.raw_type    = PERF_RAW_IBS_FETCH;
        attr.sample_type = PERF_SAMPLE_CPU | PERF_SAMPLE_RAW;
        attr.config      = IBS_FETCH_CONFIG_DEFAULT;
Cc Stephane Eranian <[email protected]>
---
 arch/x86/include/asm/perf_event.h    |   11 ++-
 arch/x86/kernel/cpu/perf_event.c     |   11 ++
 arch/x86/kernel/cpu/perf_event_amd.c |  194 +++++++++++++++++++++++++++++++++-
 3 files changed, 207 insertions(+), 9 deletions(-)
arch/x86/include/asm/perf_event.h +7 -4
index 7e51c75..dace4e2 100644
@@ -44,14 +44,15 @@
 #define AMD64_RAW_EVENT_MASK		\
 	(X86_RAW_EVENT_MASK          |  \
 	 AMD64_EVENTSEL_EVENT)
+#define AMD64_NUM_COUNTERS				4
 
-#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_SEL		      0x3c
+#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_SEL		0x3c
 #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_UMASK		(0x00 << 8)
-#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_INDEX			 0
+#define ARCH_PERFMON_UNHALTED_CORE_CYCLES_INDEX		0
 #define ARCH_PERFMON_UNHALTED_CORE_CYCLES_PRESENT \
 		(1 << (ARCH_PERFMON_UNHALTED_CORE_CYCLES_INDEX))
 
-#define ARCH_PERFMON_BRANCH_MISSES_RETIRED			 6
+#define ARCH_PERFMON_BRANCH_MISSES_RETIRED		6
 
 /*
  * Intel "Architectural Performance Monitoring" CPUID
@@ -102,13 +103,15 @@ union cpuid10_edx {
 #define X86_PMC_IDX_FIXED_BUS_CYCLES			(X86_PMC_IDX_FIXED + 2)
 
 /*
- * We model BTS tracing as another fixed-mode PMC.
+ * Masks for special PMU features
  *
  * We choose a value in the middle of the fixed event range, since lower
  * values are used by actual fixed events and higher values are used
  * to indicate other overflow conditions in the PERF_GLOBAL_STATUS msr.
  */
 #define X86_PMC_IDX_SPECIAL_BTS				(X86_PMC_IDX_SPECIAL + 0)
+#define X86_PMC_IDX_SPECIAL_IBS_FETCH			(X86_PMC_IDX_SPECIAL + 1)
+#define X86_PMC_IDX_SPECIAL_IBS_OP			(X86_PMC_IDX_SPECIAL + 2)
 
 /* IbsFetchCtl bits/masks */
 #define IBS_FETCH_RAND_EN		(1ULL<<57)
arch/x86/kernel/cpu/perf_event.c +11 -0
index 91c48b2..fe7ba91 100644
@@ -148,6 +148,9 @@ struct cpu_hw_events {
 #define INTEL_EVENT_CONSTRAINT(c, n)	\
 	EVENT_CONSTRAINT(c, n, ARCH_PERFMON_EVENTSEL_EVENT)
 
+#define AMD_IBS_EVENT_CONSTRAINT(idx)	\
+	__EVENT_CONSTRAINT(0, 1ULL << (idx), 0, AMD64_NUM_COUNTERS + 1)
+
 /*
  * Constraint on the Event code + UMask + fixed-mask
  *
@@ -186,6 +189,14 @@ union perf_capabilities {
 };
 
 /*
+ * Raw hardware event types
+ */
+#define PERF_RAW_IBS_FETCH	1
+#define PERF_RAW_IBS_OP		2
+
+#define PERF_RAW_IBS_BASE	PERF_RAW_IBS_FETCH
+
+/*
  * struct x86_pmu - generic x86 pmu
  */
 struct x86_pmu {
arch/x86/kernel/cpu/perf_event_amd.c +189 -5

240 lines not on this page · raw message

Signature · 1 line
1.7.1
#26 lore raw

PATCH 7/7 perf, x86: implement the ibs interrupt handler

In reply to Robert Richter

This patch implements code to handle ibs interrupts. If ibs data is available a raw perf_event data sample is created and sent back to the userland. This patch only implements the storage of ibs data in the raw sample, but this could be extended in a later patch by generating generic event data such as the rip from the ibs sampling data.

---
 arch/x86/include/asm/msr-index.h     |    3 +
 arch/x86/kernel/cpu/perf_event_amd.c |   70 +++++++++++++++++++++++++++++++++-
 2 files changed, 72 insertions(+), 1 deletions(-)
arch/x86/include/asm/msr-index.h +3 -0
index bc473ac..8b9929f 100644
@@ -113,6 +113,7 @@
 #define MSR_AMD64_IBSFETCHCTL		0xc0011030
 #define MSR_AMD64_IBSFETCHLINAD		0xc0011031
 #define MSR_AMD64_IBSFETCHPHYSAD	0xc0011032
+#define MSR_AMD64_IBSFETCH_REG_COUNT	3
 #define MSR_AMD64_IBSOPCTL		0xc0011033
 #define MSR_AMD64_IBSOPRIP		0xc0011034
 #define MSR_AMD64_IBSOPDATA		0xc0011035
@@ -120,7 +121,9 @@
 #define MSR_AMD64_IBSOPDATA3		0xc0011037
 #define MSR_AMD64_IBSDCLINAD		0xc0011038
 #define MSR_AMD64_IBSDCPHYSAD		0xc0011039
+#define MSR_AMD64_IBSOP_REG_COUNT	7
 #define MSR_AMD64_IBSCTL		0xc001103a
+#define MSR_AMD64_IBS_REG_COUNT_MAX	MSR_AMD64_IBSOP_REG_COUNT
 
 /* Fam 10h MSRs */
 #define MSR_FAM10H_MMIO_CONF_BASE	0xc0010058
arch/x86/kernel/cpu/perf_event_amd.c +69 -1

104 lines not on this page · raw message

Signature · 1 line
1.7.1