Skip to contentSkip to conversation

perf, x86: Implement AMD IBS

PATCH Superseded Robert Richter 6 participants 29 messages Sep 21, 2011 – Oct 12, 2011

Subsystems (2)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover perf, x86: Implement AMD IBS

This patch set adds support for AMD IBS to perf. It is a new implementation and unrelated to my previous postings last year. The main differences are:

* separate and independent from x86 perfctrs, IBS could be used without the x86 pmu,
* using dynamic pmu allocation, userspace uses sysfs to select the pmu,
* support for 64 bit counters,
* libperf based example code,
* shared IBS initialziation code for perf and oprofile.

The approach is still to collect raw sample data which should be the most important use case for application developers. The data format is the same as described in the IBS register specification.

Future work could be:

* better integration into the perf tool, use IBS for generic events where possible,
* support of the precise event sampling perf i/f,
* implementation of extended IBS features (e.g. ext. counter width),
* support of counting (perf stat),
* in-kernel IBS event parsing,
* IBS tracepoint support.

Changes for V2:

* Remove printks in pmu function stubs.
* Modify perf_event_ibs_init() to use ibs_caps directly.
* Added bit mask for msr offsets.
* Added caps field to raw sample format.
* Added caps check for IBS_OP_CUR_CNT emulation.
* Updated include header files to fix build errors on some distros.
* Note: I kept example code for reference, the patch must not be applied. I will come up with a sulution that integrates IBS into perf-report.

Changes for V3 (only patch #3 modified):

* rebased on Don's NMI V5 patch set
* disable per-task monitoring (mark pmu with perf_invalid_context), per-task monitoring can be added in a separate patch

-Robert

Robert Richter (7):
perf, x86: share IBS macros between perf and oprofile perf, x86: Implement IBS initialization perf, x86: Implement IBS event configuration perf, x86: Implement IBS interrupt handler perf, x86: Implement IBS pmu control ops perf, x86: Implement 64 bit counter support for IBS perf, x86: Example code for AMD IBS

 arch/x86/include/asm/msr-index.h           |    5 +
 arch/x86/include/asm/perf_event.h          |   42 ++-
 arch/x86/kernel/cpu/Makefile               |    2 +-
 arch/x86/kernel/cpu/perf_event_amd.c       |    4 +-
 arch/x86/kernel/cpu/perf_event_amd_ibs.c   |  716 ++++++++++++++++++++++++++++
 arch/x86/oprofile/nmi_int.c                |    2 -
 arch/x86/oprofile/op_model_amd.c           |  234 +---------
 arch/x86/oprofile/op_x86_model.h           |    1 -
 tools/perf/Documentation/examples/Makefile |   44 ++
 tools/perf/Documentation/examples/ibs.c    |  445 +++++++++++++++++
 10 files changed, 1255 insertions(+), 240 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/perf_event_amd_ibs.c
 create mode 100644 tools/perf/Documentation/examples/Makefile
 create mode 100644 tools/perf/Documentation/examples/ibs.c
#1 lore raw

PATCH v3 1/7 perf, x86: share IBS macros between perf and oprofile

Moving IBS macros from oprofile to <asm/perf_event.h> to make it available to perf. No additional changes.

---
 arch/x86/include/asm/perf_event.h    |   38 +++++++++++++++++++++++++++++++--
 arch/x86/kernel/cpu/perf_event_amd.c |    4 +-
 arch/x86/oprofile/op_model_amd.c     |   37 ++------------------------------
 3 files changed, 40 insertions(+), 39 deletions(-)
arch/x86/include/asm/perf_event.h +35 -3
index 094fb30..bc801ac 100644
@@ -43,14 +43,17 @@
 #define AMD64_RAW_EVENT_MASK		\
 	(X86_RAW_EVENT_MASK          |  \
 	 AMD64_EVENTSEL_EVENT)
+#define AMD64_NUM_COUNTERS				4
+#define AMD64_NUM_COUNTERS_F15H				6
+#define AMD64_NUM_COUNTERS_MAX				AMD64_NUM_COUNTERS_F15H
 
-#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
@@ -110,6 +113,35 @@ union cpuid10_edx {
  */
 #define X86_PMC_IDX_FIXED_BTS				(X86_PMC_IDX_FIXED + 16)
 
+/*
+ * IBS cpuid feature detection
+ */
+
+#define IBS_CPUID_FEATURES		0x8000001b
+
+/*
+ * Same bit mask as for IBS cpuid feature flags (Fn8000_001B_EAX), but
+ * bit 0 is used to indicate the existence of IBS.
+ */
+#define IBS_CAPS_AVAIL			(1U<<0)
+#define IBS_CAPS_FETCHSAM		(1U<<1)
+#define IBS_CAPS_OPSAM			(1U<<2)
+#define IBS_CAPS_RDWROPCNT		(1U<<3)
+#define IBS_CAPS_OPCNT			(1U<<4)
+#define IBS_CAPS_BRNTRGT		(1U<<5)
+#define IBS_CAPS_OPCNTEXT		(1U<<6)
+
+#define IBS_CAPS_DEFAULT		(IBS_CAPS_AVAIL		\
+					 | IBS_CAPS_FETCHSAM	\
+					 | IBS_CAPS_OPSAM)
+
+/*
+ * IBS APIC setup
+ */
+#define IBSCTL				0x1cc
+#define IBSCTL_LVT_OFFSET_VALID		(1ULL<<8)
+#define IBSCTL_LVT_OFFSET_MASK		0x0F
+
 /* IbsFetchCtl bits/masks */
 #define IBS_FETCH_RAND_EN	(1ULL<<57)
 #define IBS_FETCH_VAL		(1ULL<<49)
arch/x86/kernel/cpu/perf_event_amd.c +2 -2
index ee9436c..21c7028 100644
@@ -392,7 +392,7 @@ static __initconst const struct x86_pmu amd_pmu = {
 	.perfctr		= MSR_K7_PERFCTR0,
 	.event_map		= amd_pmu_event_map,
 	.max_events		= ARRAY_SIZE(amd_perfmon_event_map),
-	.num_counters		= 4,
+	.num_counters		= AMD64_NUM_COUNTERS,
 	.cntval_bits		= 48,
 	.cntval_mask		= (1ULL << 48) - 1,
 	.apic			= 1,
@@ -556,7 +556,7 @@ static __initconst const struct x86_pmu amd_pmu_f15h = {
 	.perfctr		= MSR_F15H_PERF_CTR,
 	.event_map		= amd_pmu_event_map,
 	.max_events		= ARRAY_SIZE(amd_perfmon_event_map),
-	.num_counters		= 6,
+	.num_counters		= AMD64_NUM_COUNTERS_F15H,
 	.cntval_bits		= 48,
 	.cntval_mask		= (1ULL << 48) - 1,
 	.apic			= 1,
arch/x86/oprofile/op_model_amd.c +3 -34
index 9cbb710..e947e5c 100644
@@ -29,8 +29,6 @@
 #include "op_x86_model.h"
 #include "op_counter.h"
 
-#define NUM_COUNTERS		4
-#define NUM_COUNTERS_F15H	6
 #ifdef CONFIG_OPROFILE_EVENT_MULTIPLEX
 #define NUM_VIRT_COUNTERS	32
 #else
@@ -70,35 +68,6 @@ static struct ibs_config ibs_config;
 static struct ibs_state ibs_state;
 
 /*
- * IBS cpuid feature detection
- */
-
-#define IBS_CPUID_FEATURES		0x8000001b
-
-/*
- * Same bit mask as for IBS cpuid feature flags (Fn8000_001B_EAX), but
- * bit 0 is used to indicate the existence of IBS.
- */
-#define IBS_CAPS_AVAIL			(1U<<0)
-#define IBS_CAPS_FETCHSAM		(1U<<1)
-#define IBS_CAPS_OPSAM			(1U<<2)
-#define IBS_CAPS_RDWROPCNT		(1U<<3)
-#define IBS_CAPS_OPCNT			(1U<<4)
-#define IBS_CAPS_BRNTRGT		(1U<<5)
-#define IBS_CAPS_OPCNTEXT		(1U<<6)
-
-#define IBS_CAPS_DEFAULT		(IBS_CAPS_AVAIL		\
-					 | IBS_CAPS_FETCHSAM	\
-					 | IBS_CAPS_OPSAM)
-
-/*
- * IBS APIC setup
- */
-#define IBSCTL				0x1cc
-#define IBSCTL_LVT_OFFSET_VALID		(1ULL<<8)
-#define IBSCTL_LVT_OFFSET_MASK		0x0F
-
-/*
  * IBS randomization macros
  */
 #define IBS_RANDOM_BITS			12
@@ -439,7 +408,7 @@ static int op_amd_fill_in_addresses(struct op_msrs * const msrs)
 			goto fail;
 		}
 		/* both registers must be reserved */
-		if (num_counters == NUM_COUNTERS_F15H) {
+		if (num_counters == AMD64_NUM_COUNTERS_F15H) {
 			msrs->counters[i].addr = MSR_F15H_PERF_CTR + (i << 1);
 			msrs->controls[i].addr = MSR_F15H_PERF_CTL + (i << 1);
 		} else {
@@ -741,9 +710,9 @@ static int op_amd_init(struct oprofile_operations *ops)
 	ops->create_files = setup_ibs_files;
 
 	if (boot_cpu_data.x86 == 0x15) {
-		num_counters = NUM_COUNTERS_F15H;
+		num_counters = AMD64_NUM_COUNTERS_F15H;
 	} else {
-		num_counters = NUM_COUNTERS;
+		num_counters = AMD64_NUM_COUNTERS;
 	}
 
 	op_amd_spec.num_counters = num_counters;
Signature · 1 line
1.7.6.1
#2 lore raw

PATCH v3 2/7 perf, x86: Implement IBS initialization

In reply to Robert Richter

This patch implements IBS feature detection and initialzation. The code is shared between perf and oprofile. If ibs is available on the system for perf, a pmu is setup.

V2:
* Remove printks in pmu function stubs.
* Modify perf_event_ibs_init() to use ibs_caps directly.

---
 arch/x86/include/asm/perf_event.h        |    2 +
 arch/x86/kernel/cpu/Makefile             |    2 +-
 arch/x86/kernel/cpu/perf_event_amd_ibs.c |  292 ++++++++++++++++++++++++++++++
 arch/x86/oprofile/nmi_int.c              |    2 -
 arch/x86/oprofile/op_model_amd.c         |  197 --------------------
 arch/x86/oprofile/op_x86_model.h         |    1 -
 6 files changed, 295 insertions(+), 201 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/perf_event_amd_ibs.c
arch/x86/include/asm/perf_event.h +2 -0
index bc801ac..e7d2f15 100644
@@ -156,6 +156,8 @@ union cpuid10_edx {
 #define IBS_OP_MAX_CNT		0x0000FFFFULL
 #define IBS_OP_MAX_CNT_EXT	0x007FFFFFULL	/* not a register bit mask */
 
+extern u32 get_ibs_caps(void);
+
 #ifdef CONFIG_PERF_EVENTS
 extern void perf_events_lapic_init(void);
 
arch/x86/kernel/cpu/Makefile +1 -1
index 6042981..ab70cd1 100644
@@ -31,7 +31,7 @@ obj-$(CONFIG_PERF_EVENTS)		+= perf_event.o
 obj-$(CONFIG_X86_MCE)			+= mcheck/
 obj-$(CONFIG_MTRR)			+= mtrr/
 
-obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o
+obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o perf_event_amd_ibs.o
 
 quiet_cmd_mkcapflags = MKCAP   $@
       cmd_mkcapflags = $(PERL) $(srctree)/$(src)/mkcapflags.pl $< $@
arch/x86/kernel/cpu/perf_event_amd_ibs.c new file +292 -0

292 lines not on this page · raw message

arch/x86/oprofile/nmi_int.c +0 -2
index adf8fb3..c04dc14 100644
@@ -385,8 +385,6 @@ static void nmi_cpu_shutdown(void *dummy)
 	apic_write(APIC_LVTPC, per_cpu(saved_lvtpc, cpu));
 	apic_write(APIC_LVTERR, v);
 	nmi_cpu_restore_registers(msrs);
-	if (model->cpu_down)
-		model->cpu_down();
 }
 
 static void nmi_cpu_up(void *dummy)
arch/x86/oprofile/op_model_amd.c +0 -197

233 lines not on this page · raw message

arch/x86/oprofile/op_x86_model.h +0 -1
index 89017fa..71e8a67 100644
@@ -43,7 +43,6 @@ struct op_x86_model_spec {
 	int		(*fill_in_addresses)(struct op_msrs * const msrs);
 	void		(*setup_ctrs)(struct op_x86_model_spec const *model,
 				      struct op_msrs const * const msrs);
-	void		(*cpu_down)(void);
 	int		(*check_ctrs)(struct pt_regs * const regs,
 				      struct op_msrs const * const msrs);
 	void		(*start)(struct op_msrs const * const msrs);
Signature · 1 line
1.7.6.1
#3 lore raw

PATCH v3 3/7 perf, x86: Implement IBS event configuration

In reply to Robert Richter

This patch implements perf configuration for AMD IBS. The IBS pmu is selected using the type attribute in sysfs. There are two types of ibs pmus, for instruction fetch (IBS_FETCH) and for instruction execution (IBS_OP):

/sys/bus/event_source/devices/ibs_fetch/type
/sys/bus/event_source/devices/ibs_op/type

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):

        type = get_pmu_type("/sys/bus/event_source/devices/ibs_fetch/type");

        memset(&attr, 0, sizeof(attr));
        attr.type        = type;
        attr.sample_type = PERF_SAMPLE_CPU | PERF_SAMPLE_RAW;
        attr.config      = IBS_FETCH_CONFIG_DEFAULT;

This implementation does not yet support 64 bit counters. It is limited to the hardware counter bit width which is 20 bits. 64 bit support can be added later.

V3:
* disable per-task monitoring (mark pmu with perf_invalid_context), per-task monitoring can be added in a separate patch

---
 arch/x86/kernel/cpu/perf_event_amd_ibs.c |   92 +++++++++++++++++++++++++++--
 1 files changed, 85 insertions(+), 7 deletions(-)
arch/x86/kernel/cpu/perf_event_amd_ibs.c +85 -7
index 1ae5154..911aeec 100644
@@ -14,12 +14,67 @@ static u32 ibs_caps;
 
 #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_CPU_SUP_AMD)
 
-static struct pmu perf_ibs;
+#define IBS_FETCH_CONFIG_MASK	(IBS_FETCH_RAND_EN | IBS_FETCH_MAX_CNT)
+#define IBS_OP_CONFIG_MASK	IBS_OP_MAX_CNT
+
+struct perf_ibs {
+	struct pmu	pmu;
+	unsigned int	msr;
+	u64		config_mask;
+	u64		cnt_mask;
+	u64		enable_mask;
+};
+
+static struct perf_ibs perf_ibs_fetch;
+static struct perf_ibs perf_ibs_op;
+
+static struct perf_ibs *get_ibs_pmu(int type)
+{
+	if (perf_ibs_fetch.pmu.type == type)
+		return &perf_ibs_fetch;
+	if (perf_ibs_op.pmu.type == type)
+		return &perf_ibs_op;
+	return NULL;
+}
 
 static int perf_ibs_init(struct perf_event *event)
 {
-	if (perf_ibs.type != event->attr.type)
+	struct hw_perf_event *hwc = &event->hw;
+	struct perf_ibs *perf_ibs;
+	u64 max_cnt, config;
+
+	perf_ibs = get_ibs_pmu(event->attr.type);
+	if (!perf_ibs)
 		return -ENOENT;
+
+	config = event->attr.config;
+	if (config & ~perf_ibs->config_mask)
+		return -EINVAL;
+
+	if (hwc->sample_period) {
+		if (config & perf_ibs->cnt_mask)
+			/* raw max_cnt may not be set */
+			return -EINVAL;
+		if (hwc->sample_period & 0x0f)
+			/* lower 4 bits can not be set in ibs max cnt */
+			return -EINVAL;
+		max_cnt = hwc->sample_period >> 4;
+		if (max_cnt & ~perf_ibs->cnt_mask)
+			/* out of range */
+			return -EINVAL;
+		config |= max_cnt;
+	} else {
+		max_cnt = config & perf_ibs->cnt_mask;
+		event->attr.sample_period = max_cnt << 4;
+		hwc->sample_period = event->attr.sample_period;
+	}
+
+	if (!max_cnt)
+		return -EINVAL;
+
+	hwc->config_base = perf_ibs->msr;
+	hwc->config = config;
+
 	return 0;
 }
 
@@ -32,10 +87,32 @@ static void perf_ibs_del(struct perf_event *event, int flags)
 {
 }
 
-static struct pmu perf_ibs = {
-	.event_init= perf_ibs_init,
-	.add= perf_ibs_add,
-	.del= perf_ibs_del,
+static struct perf_ibs perf_ibs_fetch = {
+	.pmu = {
+		.task_ctx_nr	= perf_invalid_context,
+
+		.event_init	= perf_ibs_init,
+		.add		= perf_ibs_add,
+		.del		= perf_ibs_del,
+	},
+	.msr			= MSR_AMD64_IBSFETCHCTL,
+	.config_mask		= IBS_FETCH_CONFIG_MASK,
+	.cnt_mask		= IBS_FETCH_MAX_CNT,
+	.enable_mask		= IBS_FETCH_ENABLE,
+};
+
+static struct perf_ibs perf_ibs_op = {
+	.pmu = {
+		.task_ctx_nr	= perf_invalid_context,
+
+		.event_init	= perf_ibs_init,
+		.add		= perf_ibs_add,
+		.del		= perf_ibs_del,
+	},
+	.msr			= MSR_AMD64_IBSOPCTL,
+	.config_mask		= IBS_OP_CONFIG_MASK,
+	.cnt_mask		= IBS_OP_MAX_CNT,
+	.enable_mask		= IBS_OP_ENABLE,
 };
 
 static __init int perf_event_ibs_init(void)
@@ -43,7 +120,8 @@ static __init int perf_event_ibs_init(void)
 	if (!ibs_caps)
 		return -ENODEV;	/* ibs not supported by the cpu */
 
-	perf_pmu_register(&perf_ibs, "ibs", -1);
+	perf_pmu_register(&perf_ibs_fetch.pmu, "ibs_fetch", -1);
+	perf_pmu_register(&perf_ibs_op.pmu, "ibs_op", -1);
 	printk(KERN_INFO "perf: AMD IBS detected (0x%08x)\n", ibs_caps);
 
 	return 0;
Signature · 1 line
1.7.6.1
#4 lore raw

PATCH v3 4/7 perf, x86: Implement 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.

V2:
* Added bit mask for msr offsets.
* Added caps field to raw sample format.
* Rebase on Don's NMI patches that introduce register_nmi_handler().

---
 arch/x86/include/asm/msr-index.h         |    5 ++
 arch/x86/kernel/cpu/perf_event_amd_ibs.c |   84 ++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 0 deletions(-)
arch/x86/include/asm/msr-index.h +5 -0
index d52609a..eca3363 100644
@@ -127,6 +127,8 @@
 #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_IBSFETCH_REG_MASK	((1UL<<MSR_AMD64_IBSFETCH_REG_COUNT)-1)
 #define MSR_AMD64_IBSOPCTL		0xc0011033
 #define MSR_AMD64_IBSOPRIP		0xc0011034
 #define MSR_AMD64_IBSOPDATA		0xc0011035
@@ -134,8 +136,11 @@
 #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_IBSOP_REG_MASK	((1UL<<MSR_AMD64_IBSOP_REG_COUNT)-1)
 #define MSR_AMD64_IBSCTL		0xc001103a
 #define MSR_AMD64_IBSBRTARGET		0xc001103b
+#define MSR_AMD64_IBS_REG_COUNT_MAX	8 /* includes MSR_AMD64_IBSBRTARGET */
 
 /* Fam 15h MSRs */
 #define MSR_F15H_PERF_CTL		0xc0010200
arch/x86/kernel/cpu/perf_event_amd_ibs.c +84 -0

116 lines not on this page · raw message

Signature · 1 line
1.7.6.1
#5 lore raw

Re: [V3][PATCH 4/7] perf, x86: Implement IBS interrupt handler

Quoted 8 lines · Robert Richter
arch/x86/kernel/cpu/perf_event_amd_ibs.c +7 -0
+static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
+{
+	struct perf_event *event = NULL;
+	struct hw_perf_event *hwc = &event->hw;
+	struct perf_sample_data data;
+	struct perf_raw_record raw;
+	struct pt_regs regs;

arch/x86/kernel/cpu/perf_event_amd_ibs.c static struct perf_ibs perf_ibs_op = { · line 157

Quoted 10 lines
arch/x86/kernel/cpu/perf_event_amd_ibs.c +10 -0
+	struct perf_ibs_data ibs_data;
+	int offset, size;
+	unsigned int msr;
+	u64 *buf;
+
+	msr = hwc->config_base;
+	buf = ibs_data.regs;
+	rdmsrl(msr, *buf);
+	if (!(*buf++ & perf_ibs->valid_mask))
+		return 0;

No check if the NMI is really caused by IBS? Very nasty. This will not interoperate well with other NMIs.

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only
#6 lore raw

Re: [V3][PATCH 4/7] perf, x86: Implement IBS interrupt handler

Quoted 14 lines · Andi Kleen
Quoted 13 lines · Robert Richter
arch/x86/kernel/cpu/perf_event_amd_ibs.c +12 -0
+static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
+{
+	struct perf_event *event = NULL;
+	struct hw_perf_event *hwc = &event->hw;
+	struct perf_sample_data data;
+	struct perf_raw_record raw;
+	struct pt_regs regs;
+	struct perf_ibs_data ibs_data;
+	int offset, size;
+	unsigned int msr;
+	u64 *buf;
+
Quoted 8 lines

arch/x86/kernel/cpu/perf_event_amd_ibs.c static struct perf_ibs perf_ibs_op = { · line 157

Quoted 5 lines
arch/x86/kernel/cpu/perf_event_amd_ibs.c +5 -0
+	msr = hwc->config_base;
+	buf = ibs_data.regs;
+	rdmsrl(msr, *buf);
+	if (!(*buf++ & perf_ibs->valid_mask))
+		return 0;

No check if the NMI is really caused by IBS? Very nasty. This will not interoperate well with other NMIs.

This is the check. If the valid bit is set in the config base msr, then the nmi was caused by ibs. The function returns the number of handled counters.

There is some more logic in the next patch that prevents reading the msr if ibs is disabled.

-Robert

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#7 lore raw

PATCH v3 5/7 perf, x86: Implement IBS pmu control ops

In reply to Robert Richter

Add code to control the IBS pmu. We need to maintain per-cpu states. Since some states are used and changed by the nmi handler, access to these states must be atomic.

---
 arch/x86/kernel/cpu/perf_event_amd_ibs.c |  106 +++++++++++++++++++++++++++++-
 1 files changed, 103 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/perf_event_amd_ibs.c +103 -3
index 57b38d5..9713d31 100644
@@ -22,6 +22,19 @@ static u32 ibs_caps;
 #define IBS_FETCH_CONFIG_MASK	(IBS_FETCH_RAND_EN | IBS_FETCH_MAX_CNT)
 #define IBS_OP_CONFIG_MASK	IBS_OP_MAX_CNT
 
+enum ibs_states {
+	IBS_ENABLED	= 0,
+	IBS_STARTED	= 1,
+	IBS_STOPPING	= 2,
+
+	IBS_MAX_STATES,
+};
+
+struct cpu_perf_ibs {
+	struct perf_event	*event;
+	unsigned long		state[BITS_TO_LONGS(IBS_MAX_STATES)];
+};
+
 struct perf_ibs {
 	struct pmu	pmu;
 	unsigned int	msr;
@@ -31,6 +44,7 @@ struct perf_ibs {
 	u64		valid_mask;
 	unsigned long	offset_mask[1];
 	int		offset_max;
+	struct cpu_perf_ibs __percpu *pcpu;
 };
 
 struct perf_ibs_data {
@@ -95,15 +109,66 @@ static int perf_ibs_init(struct perf_event *event)
 	return 0;
 }
 
+static void perf_ibs_start(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
+	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+
+	if (test_and_set_bit(IBS_STARTED, pcpu->state))
+		return;
+
+	wrmsrl(hwc->config_base, hwc->config | perf_ibs->enable_mask);
+}
+
+static void perf_ibs_stop(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
+	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+	u64 val;
+
+	if (!test_and_clear_bit(IBS_STARTED, pcpu->state))
+		return;
+
+	set_bit(IBS_STOPPING, pcpu->state);
+
+	rdmsrl(hwc->config_base, val);
+	val &= ~perf_ibs->enable_mask;
+	wrmsrl(hwc->config_base, val);
+}
+
 static int perf_ibs_add(struct perf_event *event, int flags)
 {
+	struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
+	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+
+	if (test_and_set_bit(IBS_ENABLED, pcpu->state))
+		return -ENOSPC;
+
+	pcpu->event = event;
+
+	if (flags & PERF_EF_START)
+		perf_ibs_start(event, PERF_EF_RELOAD);
+
 	return 0;
 }
 
 static void perf_ibs_del(struct perf_event *event, int flags)
 {
+	struct perf_ibs *perf_ibs = container_of(event->pmu, struct perf_ibs, pmu);
+	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+
+	if (!test_and_clear_bit(IBS_ENABLED, pcpu->state))
+		return;
+
+	perf_ibs_stop(event, 0);
+
+	pcpu->event = NULL;
 }
 
+static void perf_ibs_read(struct perf_event *event) { }
+
 static struct perf_ibs perf_ibs_fetch = {
 	.pmu = {
 		.task_ctx_nr	= perf_invalid_context,
@@ -111,6 +176,9 @@ static struct perf_ibs perf_ibs_fetch = {
 		.event_init	= perf_ibs_init,
 		.add		= perf_ibs_add,
 		.del		= perf_ibs_del,
+		.start		= perf_ibs_start,
+		.stop		= perf_ibs_stop,
+		.read		= perf_ibs_read,
 	},
 	.msr			= MSR_AMD64_IBSFETCHCTL,
 	.config_mask		= IBS_FETCH_CONFIG_MASK,
@@ -128,6 +196,9 @@ static struct perf_ibs perf_ibs_op = {
 		.event_init	= perf_ibs_init,
 		.add		= perf_ibs_add,
 		.del		= perf_ibs_del,
+		.start		= perf_ibs_start,
+		.stop		= perf_ibs_stop,
+		.read		= perf_ibs_read,
 	},
 	.msr			= MSR_AMD64_IBSOPCTL,
 	.config_mask		= IBS_OP_CONFIG_MASK,
@@ -140,7 +211,8 @@ static struct perf_ibs perf_ibs_op = {
 
 static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 {
-	struct perf_event *event = NULL;
+	struct cpu_perf_ibs *pcpu = this_cpu_ptr(perf_ibs->pcpu);
+	struct perf_event *event = pcpu->event;
 	struct hw_perf_event *hwc = &event->hw;
 	struct perf_sample_data data;
 	struct perf_raw_record raw;
@@ -150,6 +222,14 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 	unsigned int msr;
 	u64 *buf;
 
+	if (!test_bit(IBS_STARTED, pcpu->state)) {
+		/* Catch spurious interrupts after stopping IBS: */
+		if (!test_and_clear_bit(IBS_STOPPING, pcpu->state))
+			return 0;
+		rdmsrl(perf_ibs->msr, *ibs_data.regs);
+		return (*ibs_data.regs & perf_ibs->valid_mask);
+	}
+
 	msr = hwc->config_base;
 	buf = ibs_data.regs;
 	rdmsrl(msr, *buf);
@@ -198,13 +278,33 @@ perf_ibs_nmi_handler(unsigned int cmd, struct pt_regs *regs)
 	return handled;
 }
 
+static __init int perf_ibs_pmu_init(struct perf_ibs *perf_ibs, char *name)
+{
+	struct cpu_perf_ibs __percpu *pcpu;
+	int ret;
+
+	pcpu = alloc_percpu(struct cpu_perf_ibs);
+	if (!pcpu)
+		return -ENOMEM;
+
+	perf_ibs->pcpu = pcpu;
+
+	ret = perf_pmu_register(&perf_ibs->pmu, name, -1);
+	if (ret) {
+		perf_ibs->pcpu = NULL;
+		free_percpu(pcpu);
+	}
+
+	return ret;
+}
+
 static __init int perf_event_ibs_init(void)
 {
 	if (!ibs_caps)
 		return -ENODEV;	/* ibs not supported by the cpu */
 
-	perf_pmu_register(&perf_ibs_fetch.pmu, "ibs_fetch", -1);
-	perf_pmu_register(&perf_ibs_op.pmu, "ibs_op", -1);
+	perf_ibs_pmu_init(&perf_ibs_fetch, "ibs_fetch");
+	perf_ibs_pmu_init(&perf_ibs_op, "ibs_op");
 	register_nmi_handler(NMI_LOCAL, &perf_ibs_nmi_handler, 0, "perf_ibs");
 	printk(KERN_INFO "perf: AMD IBS detected (0x%08x)\n", ibs_caps);
 
Signature · 1 line
1.7.6.1
#8 lore raw

PATCH v3 6/7 perf, x86: Implement 64 bit counter support for IBS

In reply to Robert Richter

This patch implements 64 bit counter support for IBS. The sampling period is no longer limited to the hw counter width.

The functions perf_event_set_period() and perf_event_try_update() can be used as generic functions. They can replace similar code that is duplicate across architectures.

V2: Added caps check for IBS_OP_CUR_CNT emulation.

---
 arch/x86/include/asm/perf_event.h        |    2 +
 arch/x86/kernel/cpu/perf_event_amd_ibs.c |  204 +++++++++++++++++++++++++++---
 2 files changed, 185 insertions(+), 21 deletions(-)
arch/x86/include/asm/perf_event.h +2 -0
index e7d2f15..cc44a1a 100644
@@ -150,6 +150,8 @@ union cpuid10_edx {
 #define IBS_FETCH_MAX_CNT	0x0000FFFFULL
 
 /* IbsOpCtl bits */
+/* lower 4 bits of the current count are ignored: */
+#define IBS_OP_CUR_CNT		(0xFFFF0ULL<<32)
 #define IBS_OP_CNT_CTL		(1ULL<<19)
 #define IBS_OP_VAL		(1ULL<<18)
 #define IBS_OP_ENABLE		(1ULL<<17)
arch/x86/kernel/cpu/perf_event_amd_ibs.c +183 -21

298 lines not on this page · raw message

Signature · 1 line
1.7.6.1
#9 lore raw

PATCH v3 7/7 perf, x86: Example code for AMD IBS

In reply to Robert Richter

This patch includes an example to use IBS via perf_event.

usage: ibs [-h]
ibs ibs_fetch | ibs_op [-s] [-C CPU] [-m BUFFERPAGES] <command>

<command>
Command to execute.

-e CONFIG
64 bit configuration value, refers to msrs IbsFetchCtl (0xC0011030) or IbsOpCtl (0xC0011033). The default sample period is set to 100000.

-c COUNT
Event period to sample (default: 100000).

-h
Print help.

-s
system wide profiling (set per default)

-C CPU
profile on CPU (not yet implemented)

-m BUFFERPAGES
Per-cpu buffer pages to allocate.

V2:
* Updated include header files to fix build errors on some distros.
* Caps field added to sampling format.
* Note: I kept example code for reference, the patch must not be applied. I will come up with a sulution that integrates IBS into perf-report.

---
 tools/perf/Documentation/examples/Makefile |   44 +++
 tools/perf/Documentation/examples/ibs.c    |  445 ++++++++++++++++++++++++++++
 2 files changed, 489 insertions(+), 0 deletions(-)
 create mode 100644 tools/perf/Documentation/examples/Makefile
 create mode 100644 tools/perf/Documentation/examples/ibs.c
tools/perf/Documentation/examples/Makefile new file +44 -0
new file mode 100644
index 0000000..cfc9647
@@ -0,0 +1,44 @@
+all:	ibs
+
+CFLAGS += -I../..
+CFLAGS += -I../../util/include
+CFLAGS += -DNO_NEWT_SUPPORT
+
+LIB_FILE=../../libperf.a
+
+INSTALL = install
+
+ifeq ("$(origin O)", "command line")
+	OUTPUT := $(O)/
+endif
+
+ifneq ($(OUTPUT),)
+# check that the output directory actually exists
+OUTDIR := $(shell cd $(OUTPUT) && /bin/pwd)
+$(if $(OUTDIR),, $(error output directory "$(OUTPUT)" does not exist))
+endif
+
+ifndef DESTDIR
+prefix = $(HOME)
+endif
+bindir_relative = bin
+bindir = $(prefix)/$(bindir_relative)
+
+DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
+bindir_SQ = $(subst ','\'',$(bindir))
+
+../../libperf.a:
+	$(MAKE) CFLAGS="-DNO_NEWT_SUPPORT" -C ../.. libperf.a
+
+$(OUTPUT)ibs: ibs.c $(LIB_FILE)
+	$(CC) $(CFLAGS) $^ -o $@
+
+clean:
+	$(MAKE) -C ../.. clean
+	$(RM) ibs
+
+install: all
+	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
+	$(INSTALL) $(OUTPUT)ibs '$(DESTDIR_SQ)$(bindir_SQ)'
+
+.PHONY:	all clean install
tools/perf/Documentation/examples/ibs.c new file +445 -0

445 lines not on this page · raw message

Signature · 1 line
1.7.6.1
maintainer
#10 lore raw
In reply to Robert Richter
Quoted 17 lines · Robert Richter

This patch set adds support for AMD IBS to perf. It is a new implementation and unrelated to my previous postings last year. The main differences are:

* separate and independent from x86 perfctrs, IBS could be used without the x86 pmu,
* using dynamic pmu allocation, userspace uses sysfs to select the pmu,
* support for 64 bit counters,
* libperf based example code,
* shared IBS initialziation code for perf and oprofile.

The approach is still to collect raw sample data which should be the most important use case for application developers. The data format is the same as described in the IBS register specification.

Future work could be:

Quoted 7 lines

* better integration into the perf tool, use IBS for generic events where possible,
* support of the precise event sampling perf i/f,
* implementation of extended IBS features (e.g. ext. counter width),
* support of counting (perf stat),
* in-kernel IBS event parsing,
* IBS tracepoint support.

There was also the thing about putting perf_event_set_period() and perf_event_try_update() in a common library and converting all pmu implementations to use them.

As well as the for_each_set_bit_continue() thing which you've got queued.

Anyway, I read through the stuff and it looks about right, so I'll wait for Don to post his NMI infrastructure bits once more (he had some pending changes) and then I'll queue this on top.

The only thing I'm not quite sure on is the userspace bits, but those are in the future work section as well, but possibly Ingo has a strong opinion here, sadly he doesn't have email atm :/

#11 lore raw
Quoted 6 lines · Peter Zijlstra

There was also the thing about putting perf_event_set_period() and perf_event_try_update() in a common library and converting all pmu implementations to use them.

As well as the for_each_set_bit_continue() thing which you've got queued.

Yeah, I will post separate patches for this.

Quoted 3 lines

Anyway, I read through the stuff and it looks about right, so I'll wait for Don to post his NMI infrastructure bits once more (he had some pending changes) and then I'll queue this on top.

Many thanks.

Quoted 3 lines

The only thing I'm not quite sure on is the userspace bits, but those are in the future work section as well, but possibly Ingo has a strong opinion here, sadly he doesn't have email atm :/

With Lin's patch 'perf: Remove perf_event_attr::type check' we already can sample with perf record.

I think we could extend perf report to parse also ibs samples. The only thing we need for it should be the pmu name/type mapping in the perf.data header and the pmu type in the sample. See my comment on Stephane's patch '[PATCH] perf: make perf.data more self-descriptive (v5)'.

Thanks,

-Robert

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#12 lore raw
Quoted 6 lines · Robert Richter

I think we could extend perf report to parse also ibs samples. The only thing we need for it should be the pmu name/type mapping in the perf.data header and the pmu type in the sample. See my comment on Stephane's patch '[PATCH] perf: make perf.data more self-descriptive (v5)'.

I would like to have a similar user tool for Intel PEBS data.

I have some hacks currently to dump more data from the PEBS record and it's useful for some situations. I was actually thinking of adapting Robert's tool, but integrating it into other tools is fine too.

Sometimes a "perf print-samples" to print individual samples would be useful too (it's already possible with -R, but very unfriendly without post processing)

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only
#13 lore raw
Quoted 6 lines · Andi Kleen

Sometimes a "perf print-samples" to print individual samples would be useful too (it's already possible with -R, but very unfriendly without post processing)

-Andi

What are you looking for that perf-script currently does not do?

David

#14 lore raw
Quoted 1 line

What are you looking for that perf-script currently does not do?

Hmm can perf-script process raw data from the perf record?

If yes perhaps it is what I want indeed.

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only.
#15 lore raw
Quoted 7 lines · Andi Kleen
Quoted 1 line

What are you looking for that perf-script currently does not do?

Hmm can perf-script process raw data from the perf record?

If yes perhaps it is what I want indeed.

-Andi

yes, it is now a misnomer and should be 'perf dump' (or your suggested 'perf print-samples'). It has a set of default output fields with the ability to customize (see the -f option for field options).

David

#16 lore raw
In reply to Andi Kleen
Quoted 9 lines · Andi Kleen
Quoted 6 lines · Robert Richter

I think we could extend perf report to parse also ibs samples. The only thing we need for it should be the pmu name/type mapping in the perf.data header and the pmu type in the sample. See my comment on Stephane's patch '[PATCH] perf: make perf.data more self-descriptive (v5)'.

Quoted 6 lines

I would like to have a similar user tool for Intel PEBS data.

I have some hacks currently to dump more data from the PEBS record and it's useful for some situations. I was actually thinking of adapting Robert's tool, but integrating it into other tools is fine too.

There is Lin Ming's patch to expose PEBS Load Latency information. I will post a patch to export more of the machine state later on.

Quoted 8 lines

Sometimes a "perf print-samples" to print individual samples would be useful too  (it's already possible with -R, but very unfriendly without post processing)

-Andi

--
[email protected] -- Speaking for myself only

#17 lore raw
Quoted 2 lines

There is Lin Ming's patch to expose PEBS Load Latency information. I will post a patch to export more of the machine state later on.

I have some patches to expose LBR and raw PEBS (similar to Robert's IBS patches, just for PEBS). But the user code still needs more work for that.

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only.
#18 lore raw
Quoted 6 lines · Andi Kleen
Quoted 2 lines

There is Lin Ming's patch to expose PEBS Load Latency information. I will post a patch to export more of the machine state later on.

I have some patches to expose LBR and raw PEBS (similar to Robert's IBS patches, just for PEBS). But the user code still needs more work for that.

I will post a complete patch to abstract LBR and update perf accordingly. I am updating it to 3.1.0-rc8. Will post later this week.

maintainer
#19 lore raw
In reply to Robert Richter
Quoted 4 lines · Robert Richter
Quoted 3 lines

The only thing I'm not quite sure on is the userspace bits, but those are in the future work section as well, but possibly Ingo has a strong opinion here, sadly he doesn't have email atm :/

Quoted 8 lines

With Lin's patch 'perf: Remove perf_event_attr::type check' we already can sample with perf record.

I think we could extend perf report to parse also ibs samples. The only thing we need for it should be the pmu name/type mapping in the perf.data header and the pmu type in the sample. See my comment on Stephane's patch '[PATCH] perf: make perf.data more self-descriptive (v5)'.

OK, so Ingo really really wants to see a little more perf integration there. Acme, what's the state of Stephane's patch?

#20 lore raw
Quoted 8 lines · Peter Zijlstra
Quoted 7 lines · Robert Richter
Quoted 3 lines

The only thing I'm not quite sure on is the userspace bits, but those are in the future work section as well, but possibly Ingo has a strong opinion here, sadly he doesn't have email atm :/

With Lin's patch 'perf: Remove perf_event_attr::type check' we already can sample with perf record.

Quoted 8 lines
Quoted 5 lines

I think we could extend perf report to parse also ibs samples. The only thing we need for it should be the pmu name/type mapping in the perf.data header and the pmu type in the sample. See my comment on Stephane's patch '[PATCH] perf: make perf.data more self-descriptive (v5)'.

OK, so Ingo really really wants to see a little more perf integration there. Acme, what's the state of Stephane's patch?

So the whole IBS thing looks quite unintegrated to me - and that's partly because the hw is admittedly weird. The way we could perhaps live with it upstream is two conditions:

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

- Only root/privileged users should be able to access it. Right now i think it's root-only due to percpu restrictions, but wanted to mention it that this is an explicit requirement.

Non-privileged IBS use could be enabled by having working integration with generic events: we could add a upos generic event for example so IBS could map to '-e uops:p' or such, to allow skid-less profiling on AMD CPUs.

Thanks,

Ingo

#21 lore raw
Quoted 9 lines · Ingo Molnar

So the whole IBS thing looks quite unintegrated to me - and that's partly because the hw is admittedly weird. The way we could perhaps live with it upstream is two conditions:

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

With Lin Ming's patch

[PATCH v2 6/6] perf tool: Parse general/raw events from sysfs

you can use perf record to collect IBS samples, e.g.:

 # perf record -e ibs_fetch:r0 -c 100000 -a bash -c someload
 [ perf record: Woken up 3 times to write data ]
 [ perf record: Captured and wrote 2.004 MB perf.data (~87572 samples) ]

With Stephane's patch

[PATCH] perf: make perf.data more self-descriptive (v8)

I could extend perf report to add an IBS parser (I need to know the pmu name/type value mapping).

Do you have something like that in mind?

As all these patches are still off-tree I would rather prefer to decouple the IBS patches from a parser implementation and add it later as a single patch series. Hmm?

Quoted 3 lines

- Only root/privileged users should be able to access it. Right now i think it's root-only due to percpu restrictions, but wanted to mention it that this is an explicit requirement.

Yes, IBS is restricted to percpu. And percpu requires root privileges like all other pmu events. What is your point here?

-Robert

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#22 lore raw
Quoted 34 lines · Robert Richter
Quoted 9 lines · Ingo Molnar

So the whole IBS thing looks quite unintegrated to me - and that's partly because the hw is admittedly weird. The way we could perhaps live with it upstream is two conditions:

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

With Lin Ming's patch

[PATCH v2 6/6] perf tool: Parse general/raw events from sysfs

you can use perf record to collect IBS samples, e.g.:

 # perf record -e ibs_fetch:r0 -c 100000 -a bash -c someload
 [ perf record: Woken up 3 times to write data ]
 [ perf record: Captured and wrote 2.004 MB perf.data (~87572 samples) ]

With Stephane's patch

[PATCH] perf: make perf.data more self-descriptive (v8)

I could extend perf report to add an IBS parser (I need to know the pmu name/type value mapping).

Do you have something like that in mind?

As all these patches are still off-tree I would rather prefer to decouple the IBS patches from a parser implementation and add it later as a single patch series. Hmm?

Quoted 6 lines
Quoted 3 lines

- Only root/privileged users should be able to access it. Right now i think it's root-only due to percpu restrictions, but wanted to mention it that this is an explicit requirement.

Yes, IBS is restricted to percpu. And percpu requires root privileges like all other pmu events. What is your point here?

Ingo, any comments?

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#23 lore raw
In reply to Robert Richter
Quoted 25 lines · Robert Richter
Quoted 9 lines · Ingo Molnar

So the whole IBS thing looks quite unintegrated to me - and that's partly because the hw is admittedly weird. The way we could perhaps live with it upstream is two conditions:

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

With Lin Ming's patch

[PATCH v2 6/6] perf tool: Parse general/raw events from sysfs

you can use perf record to collect IBS samples, e.g.:

 # perf record -e ibs_fetch:r0 -c 100000 -a bash -c someload
 [ perf record: Woken up 3 times to write data ]
 [ perf record: Captured and wrote 2.004 MB perf.data (~87572 samples) ]

With Stephane's patch

[PATCH] perf: make perf.data more self-descriptive (v8)

Quoted 8 lines

I could extend perf report to add an IBS parser (I need to know the pmu name/type value mapping).

Do you have something like that in mind?

As all these patches are still off-tree I would rather prefer to decouple the IBS patches from a parser implementation and add it later as a single patch series. Hmm?

Yeah, i'd like to have something that i can try out in practice on an AMD box without having to apply any other patches.

The perf.data extension patch from Stephane is now in perf/core. You could add Lin Ming's patch to your tree - they really interdepend. That way it could be reviewed and handled as a coherent unit offering some real, testable functionality.

Quoted 6 lines
Quoted 3 lines

- Only root/privileged users should be able to access it. Right now i think it's root-only due to percpu restrictions, but wanted to mention it that this is an explicit requirement.

Yes, IBS is restricted to percpu. And percpu requires root privileges like all other pmu events. What is your point here?

My point is that this should not just be an incidental side-effect of hardware limitations but should stay so until deeper integration is achieved. The list i gave is similar for other future PMU additions.

Anyway, if perf report [and, in latest perf/core, perf top] can do IBS output that would be a sufficient level of integration i think.

Thanks,

Ingo

#24 lore raw
In reply to Ingo Molnar
Quoted 6 lines · Ingo Molnar

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

I looked at this for PEBS, and the best solution currently seems to be a python script that works with perf script and does some post processing to present the data as summary or individual samples. I assume that could work for IBS too.

Comments?

-Andi

maintainer
#25 lore raw
Quoted 8 lines · Andi Kleen
Quoted 6 lines · Ingo Molnar

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

Quoted 6 lines

I looked at this for PEBS, and the best solution currently seems to be a python script that works with perf script and does some post processing to present the data as summary or individual samples. I assume that could work for IBS too.

Comments?

I would much prefer perf {ibs,pebs} commands implemented in C, so that normal people can change/improve them too.

I really hate all that snake foo.

#26 lore raw
Quoted 2 lines · Peter Zijlstra

I would much prefer perf {ibs,pebs} commands implemented in C, so that normal people can change/improve them too.

Would a perf {ibs,pebs} be an ascii dump or so? Then I just would better extend perf-report (-D option) to support decoding of raw samples from dynamically pmus. Something like:

# perf report -D
[...]
. ... raw event: size 88 bytes
.  0000:  01 00 00 00 02 00 58 00 6d 11 00 00 6d 11 00 00  ......X.m...m...
.  0010:  00 80 86 b7 00 00 00 00 00 10 01 00 00 00 00 00  ................
.  0020:  00 00 00 00 00 00 00 00 2f 6c 69 62 2f 6c 69 62  ......../lib/lib
.  0030:  7a 2e 73 6f 2e 31 2e 32 2e 33 00 00 00 00 00 00  z.so.1.2.3......
.  0040:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
.  0050:  00 00 00 00 00 00 00 00                          ........
.  # IBS sample (ibs_fetch):
.  # IbsFetchCtl          = 0x0000000000000000
.  #   IbsFetchMaxCnt=0
.  #   IbsFetchCnt=0
.  #   IbsFetchLat=0
.  #   IbsFetchEn=0
.  #   IbsFetchVal=0
.  #   IbsFetchComp=0
.  #   IbsIcMiss=0
.  #   IbsPhyAddrValid=0
.  #   IbsL1TlbPgSz=0
.  #   IbsL1TlbMiss=0
.  #   IbsL2TlbMiss=0
.  #   IbsRandEn=0
.  # IbsFetchLinAd        = 0x00000000813e8b80
.  #   IbsFetchLinAd=0xffffffff813e8b80
.  # IbsFetchPhysAd       = 0x00000000013e8b80
.  #   IbsFetchPhysAd=0x13e8b80
[...]

(Data decoding not correct in this example.)

This wouln't require architecture specific commands.

-Robert

Signature · 2 lines
Advanced Micro Devices, Inc.
Operating System Research Center
#27 lore raw
Quoted 5 lines · Robert Richter
Quoted 2 lines · Peter Zijlstra

I would much prefer perf {ibs,pebs} commands implemented in C, so that normal people can change/improve them too.

Would a perf {ibs,pebs} be an ascii dump or so? Then I just would

You would need a script to post process that anyways, that is why the idea was to put it into perf script (with a suitable default) But the default could be also written in C if Peter prefers that.

-Andi

#28 lore raw
In reply to Peter Zijlstra
Quoted 14 lines · Peter Zijlstra
Quoted 13 lines · Andi Kleen
Quoted 6 lines · Ingo Molnar

- Testable IBS user-space code a bit more prominently integrated than having to go down into a cellar with no working lights and finding the code on display in tools/perf/Documentation/examples/ on the bottom of a locked filing cabinet stuck in a disused lavatory with a sign on the door saying 'Beware of the Leopard.'

I looked at this for PEBS, and the best solution currently seems to be a python script that works with perf script and does some post processing to present the data as summary or individual samples. I assume that could work for IBS too.

Quoted 4 lines
Quoted 1 line

Comments?

I would much prefer perf {ibs,pebs} commands implemented in C, so that normal people can change/improve them too.

The other issue is performance, obviously. So for core functionality we are pretty sure we want to offer to everyone C is the preferred form.

Scripting integration *in addition* to that is obviously welcome too.

Thanks,

Ingo