[PATCH 0/6] perf/x86/amd: Add memory controller events

[PATCH 0/6] perf/x86/amd: Add memory controller events

[Sandipan Das]

Unified Memory Controller (UMC) events were introduced with Zen 4 as a
part of the Performance Monitoring Version 2 (PerfMonV2) enhancements.
Currently, Zen 4 supports upto 12 channels of DDR5 memory and each of
them are controlled by a dedicated UMC. Each UMC, in turn, has its own
set of performance monitoring counters. These counters can provide info
on UMC command activity which in turn can be used to derive utilization
and bandwidth. Using perf tool, users can profile activity either on a
combined basis (includes all active UMCs) or for individual UMCs.

E.g. measurement across all UMCs

  $ sudo perf stat -e amd_umc/umc_cas_cmd.all/ -a -- sleep 1

   Performance counter stats for 'system wide':

             544,810      amd_umc/umc_cas_cmd.all/

         1.002012663 seconds time elapsed

E.g. measurement specific to certain UMCs

  $ sudo perf stat -e amd_umc_0/umc_cas_cmd.all/ -e amd_umc_6/umc_cas_cmd.all/ -a -- sleep 1

   Performance counter stats for 'system wide':

              21,096      amd_umc_0/umc_cas_cmd.all/
              35,428      amd_umc_6/umc_cas_cmd.all/

         1.001802611 seconds time elapsed

The available UMCs can be found from sysfs and the socket to which they
belong can be derived from the cpumask.

E.g.

  $ find /sys/devices/ -maxdepth 1 -name "amd_umc_*"

  /sys/devices/amd_umc_9
  /sys/devices/amd_umc_7
  /sys/devices/amd_umc_5
  /sys/devices/amd_umc_3
  /sys/devices/amd_umc_1
  /sys/devices/amd_umc_10
  /sys/devices/amd_umc_8
  /sys/devices/amd_umc_6
  /sys/devices/amd_umc_4
  /sys/devices/amd_umc_2
  /sys/devices/amd_umc_11
  /sys/devices/amd_umc_0

  $ cat /sys/devices/amd_umc_0/cpumask
  0

  $ cat /sys/devices/amd_umc_6/cpumask
  96

All of the output above comes from a dual socket Genoa system having
96 cores and 6 populated memory channels per socket.

Sandipan Das (6):
  perf/x86/amd/uncore: Refactor uncore management
  perf/x86/amd/uncore: Use rdmsr if rdpmc is unavailable
  x86/cpuid: Add smp helper
  perf/x86/amd/uncore: Add group exclusivity
  perf/x86/amd/uncore: Add memory controller support
  perf vendor events amd: Add Zen 4 memory controller events

 arch/x86/events/amd/uncore.c                  | 893 +++++++++++-------
 arch/x86/include/asm/cpuid.h                  |  14 +
 arch/x86/include/asm/msr-index.h              |   4 +
 arch/x86/include/asm/perf_event.h             |   9 +
 arch/x86/lib/Makefile                         |   2 +-
 arch/x86/lib/cpuid-smp.c                      |  36 +
 .../arch/x86/amdzen4/memory-controller.json   | 101 ++
 .../arch/x86/amdzen4/recommended.json         |  84 ++
 tools/perf/pmu-events/jevents.py              |   2 +
 9 files changed, 807 insertions(+), 338 deletions(-)
 create mode 100644 arch/x86/lib/cpuid-smp.c
 create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json

-- 
2.34.1

[PATCH 1/6] perf/x86/amd/uncore: Refactor uncore management

[Sandipan Das]

Since struct amd_uncore is used to manage per-cpu contexts, rename it to
amd_uncore_ctx in order to better reflect its purpose. Add a new struct
amd_uncore and encapsulate all attributes which are shared by per-cpu
contexts for the corresponding PMU. These attributes include the number
of counters, active mask, MSR and RDPMC base addresses. Since the struct
pmu is now embedded, the corresponding amd_uncore for a given event can
be found by simply using container_of().

Finally, move all PMU-specific code to separate functions and use the
original event management functions for base functionality. Determining
the PMU type (such as DF or L3) is no longer required for applying any
customizations or handling any quirks.

The motivation is to simplify the management of uncore PMUs.

Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/events/amd/uncore.c | 711 ++++++++++++++++++-----------------
 1 file changed, 366 insertions(+), 345 deletions(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 83f15fe411b3..b0afbe6d9eb4 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -27,56 +27,41 @@
 
 #define COUNTER_SHIFT		16
 
+#define NUM_UNCORES_MAX		2	/* DF (or NB) and L3 (or L2) */
+#define UNCORE_NAME_LEN		16
+
 #undef pr_fmt
 #define pr_fmt(fmt)	"amd_uncore: " fmt
 
 static int pmu_version;
-static int num_counters_llc;
-static int num_counters_nb;
-static bool l3_mask;
 
 static HLIST_HEAD(uncore_unused_list);
 
-struct amd_uncore {
+struct amd_uncore_ctx {
 	int id;
 	int refcnt;
 	int cpu;
-	int num_counters;
-	int rdpmc_base;
-	u32 msr_base;
-	cpumask_t *active_mask;
-	struct pmu *pmu;
 	struct perf_event **events;
 	struct hlist_node node;
 };
 
-static struct amd_uncore * __percpu *amd_uncore_nb;
-static struct amd_uncore * __percpu *amd_uncore_llc;
-
-static struct pmu amd_nb_pmu;
-static struct pmu amd_llc_pmu;
-
-static cpumask_t amd_nb_active_mask;
-static cpumask_t amd_llc_active_mask;
-
-static bool is_nb_event(struct perf_event *event)
-{
-	return event->pmu->type == amd_nb_pmu.type;
-}
+struct amd_uncore {
+	char name[UNCORE_NAME_LEN];
+	int num_counters;
+	int rdpmc_base;
+	u32 msr_base;
+	cpumask_t active_mask;
+	struct pmu pmu;
+	struct amd_uncore_ctx * __percpu *ctx;
+	int (*id)(unsigned int cpu);
+};
 
-static bool is_llc_event(struct perf_event *event)
-{
-	return event->pmu->type == amd_llc_pmu.type;
-}
+static struct amd_uncore uncores[NUM_UNCORES_MAX];
+static int num_uncores __read_mostly;
 
 static struct amd_uncore *event_to_amd_uncore(struct perf_event *event)
 {
-	if (is_nb_event(event) && amd_uncore_nb)
-		return *per_cpu_ptr(amd_uncore_nb, event->cpu);
-	else if (is_llc_event(event) && amd_uncore_llc)
-		return *per_cpu_ptr(amd_uncore_llc, event->cpu);
-
-	return NULL;
+	return container_of(event->pmu, struct amd_uncore, pmu);
 }
 
 static void amd_uncore_read(struct perf_event *event)
@@ -118,7 +103,7 @@ static void amd_uncore_stop(struct perf_event *event, int flags)
 	hwc->state |= PERF_HES_STOPPED;
 
 	if ((flags & PERF_EF_UPDATE) && !(hwc->state & PERF_HES_UPTODATE)) {
-		amd_uncore_read(event);
+		event->pmu->read(event);
 		hwc->state |= PERF_HES_UPTODATE;
 	}
 }
@@ -127,14 +112,15 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 {
 	int i;
 	struct amd_uncore *uncore = event_to_amd_uncore(event);
+	struct amd_uncore_ctx *ctx = *per_cpu_ptr(uncore->ctx, event->cpu);
 	struct hw_perf_event *hwc = &event->hw;
 
 	/* are we already assigned? */
-	if (hwc->idx != -1 && uncore->events[hwc->idx] == event)
+	if (hwc->idx != -1 && ctx->events[hwc->idx] == event)
 		goto out;
 
 	for (i = 0; i < uncore->num_counters; i++) {
-		if (uncore->events[i] == event) {
+		if (ctx->events[i] == event) {
 			hwc->idx = i;
 			goto out;
 		}
@@ -143,7 +129,7 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	/* if not, take the first available counter */
 	hwc->idx = -1;
 	for (i = 0; i < uncore->num_counters; i++) {
-		if (cmpxchg(&uncore->events[i], NULL, event) == NULL) {
+		if (cmpxchg(&ctx->events[i], NULL, event) == NULL) {
 			hwc->idx = i;
 			break;
 		}
@@ -158,18 +144,8 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	hwc->event_base_rdpmc = uncore->rdpmc_base + hwc->idx;
 	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
-	/*
-	 * The first four DF counters are accessible via RDPMC index 6 to 9
-	 * followed by the L3 counters from index 10 to 15. For processors
-	 * with more than four DF counters, the DF RDPMC assignments become
-	 * discontiguous as the additional counters are accessible starting
-	 * from index 16.
-	 */
-	if (is_nb_event(event) && hwc->idx >= NUM_COUNTERS_NB)
-		hwc->event_base_rdpmc += NUM_COUNTERS_L3;
-
 	if (flags & PERF_EF_START)
-		amd_uncore_start(event, PERF_EF_RELOAD);
+		event->pmu->start(event, PERF_EF_RELOAD);
 
 	return 0;
 }
@@ -178,54 +154,35 @@ static void amd_uncore_del(struct perf_event *event, int flags)
 {
 	int i;
 	struct amd_uncore *uncore = event_to_amd_uncore(event);
+	struct amd_uncore_ctx *ctx = *per_cpu_ptr(uncore->ctx, event->cpu);
 	struct hw_perf_event *hwc = &event->hw;
 
-	amd_uncore_stop(event, PERF_EF_UPDATE);
+	event->pmu->stop(event, PERF_EF_UPDATE);
 
 	for (i = 0; i < uncore->num_counters; i++) {
-		if (cmpxchg(&uncore->events[i], event, NULL) == event)
+		if (cmpxchg(&ctx->events[i], event, NULL) == event)
 			break;
 	}
 
 	hwc->idx = -1;
 }
 
-/*
- * Return a full thread and slice mask unless user
- * has provided them
- */
-static u64 l3_thread_slice_mask(u64 config)
-{
-	if (boot_cpu_data.x86 <= 0x18)
-		return ((config & AMD64_L3_SLICE_MASK) ? : AMD64_L3_SLICE_MASK) |
-		       ((config & AMD64_L3_THREAD_MASK) ? : AMD64_L3_THREAD_MASK);
-
-	/*
-	 * If the user doesn't specify a threadmask, they're not trying to
-	 * count core 0, so we enable all cores & threads.
-	 * We'll also assume that they want to count slice 0 if they specify
-	 * a threadmask and leave sliceid and enallslices unpopulated.
-	 */
-	if (!(config & AMD64_L3_F19H_THREAD_MASK))
-		return AMD64_L3_F19H_THREAD_MASK | AMD64_L3_EN_ALL_SLICES |
-		       AMD64_L3_EN_ALL_CORES;
-
-	return config & (AMD64_L3_F19H_THREAD_MASK | AMD64_L3_SLICEID_MASK |
-			 AMD64_L3_EN_ALL_CORES | AMD64_L3_EN_ALL_SLICES |
-			 AMD64_L3_COREID_MASK);
-}
-
 static int amd_uncore_event_init(struct perf_event *event)
 {
 	struct amd_uncore *uncore;
+	struct amd_uncore_ctx *ctx;
 	struct hw_perf_event *hwc = &event->hw;
-	u64 event_mask = AMD64_RAW_EVENT_MASK_NB;
 
 	if (event->attr.type != event->pmu->type)
 		return -ENOENT;
 
-	if (pmu_version >= 2 && is_nb_event(event))
-		event_mask = AMD64_PERFMON_V2_RAW_EVENT_MASK_NB;
+	if (event->cpu < 0)
+		return -EINVAL;
+
+	uncore = event_to_amd_uncore(event);
+	ctx = *per_cpu_ptr(uncore->ctx, event->cpu);
+	if (!ctx)
+		return -ENODEV;
 
 	/*
 	 * NB and Last level cache counters (MSRs) are shared across all cores
@@ -235,28 +192,14 @@ static int amd_uncore_event_init(struct perf_event *event)
 	 * out. So we do not support sampling and per-thread events via
 	 * CAP_NO_INTERRUPT, and we do not enable counter overflow interrupts:
 	 */
-	hwc->config = event->attr.config & event_mask;
+	hwc->config = event->attr.config;
 	hwc->idx = -1;
 
-	if (event->cpu < 0)
-		return -EINVAL;
-
-	/*
-	 * SliceMask and ThreadMask need to be set for certain L3 events.
-	 * For other events, the two fields do not affect the count.
-	 */
-	if (l3_mask && is_llc_event(event))
-		hwc->config |= l3_thread_slice_mask(event->attr.config);
-
-	uncore = event_to_amd_uncore(event);
-	if (!uncore)
-		return -ENODEV;
-
 	/*
 	 * since request can come in to any of the shared cores, we will remap
 	 * to a single common cpu.
 	 */
-	event->cpu = uncore->cpu;
+	event->cpu = ctx->cpu;
 
 	return 0;
 }
@@ -278,17 +221,10 @@ static ssize_t amd_uncore_attr_show_cpumask(struct device *dev,
 					    struct device_attribute *attr,
 					    char *buf)
 {
-	cpumask_t *active_mask;
 	struct pmu *pmu = dev_get_drvdata(dev);
+	struct amd_uncore *uncore = container_of(pmu, struct amd_uncore, pmu);
 
-	if (pmu->type == amd_nb_pmu.type)
-		active_mask = &amd_nb_active_mask;
-	else if (pmu->type == amd_llc_pmu.type)
-		active_mask = &amd_llc_active_mask;
-	else
-		return 0;
-
-	return cpumap_print_to_pagebuf(true, buf, active_mask);
+	return cpumap_print_to_pagebuf(true, buf, &uncore->active_mask);
 }
 static DEVICE_ATTR(cpumask, S_IRUGO, amd_uncore_attr_show_cpumask, NULL);
 
@@ -396,113 +332,57 @@ static const struct attribute_group *amd_uncore_l3_attr_update[] = {
 	NULL,
 };
 
-static struct pmu amd_nb_pmu = {
-	.task_ctx_nr	= perf_invalid_context,
-	.attr_groups	= amd_uncore_df_attr_groups,
-	.name		= "amd_nb",
-	.event_init	= amd_uncore_event_init,
-	.add		= amd_uncore_add,
-	.del		= amd_uncore_del,
-	.start		= amd_uncore_start,
-	.stop		= amd_uncore_stop,
-	.read		= amd_uncore_read,
-	.capabilities	= PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
-	.module		= THIS_MODULE,
-};
-
-static struct pmu amd_llc_pmu = {
-	.task_ctx_nr	= perf_invalid_context,
-	.attr_groups	= amd_uncore_l3_attr_groups,
-	.attr_update	= amd_uncore_l3_attr_update,
-	.name		= "amd_l2",
-	.event_init	= amd_uncore_event_init,
-	.add		= amd_uncore_add,
-	.del		= amd_uncore_del,
-	.start		= amd_uncore_start,
-	.stop		= amd_uncore_stop,
-	.read		= amd_uncore_read,
-	.capabilities	= PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
-	.module		= THIS_MODULE,
-};
-
-static struct amd_uncore *amd_uncore_alloc(unsigned int cpu)
-{
-	return kzalloc_node(sizeof(struct amd_uncore), GFP_KERNEL,
-			cpu_to_node(cpu));
-}
-
-static inline struct perf_event **
-amd_uncore_events_alloc(unsigned int num, unsigned int cpu)
-{
-	return kzalloc_node(sizeof(struct perf_event *) * num, GFP_KERNEL,
-			    cpu_to_node(cpu));
-}
-
 static int amd_uncore_cpu_up_prepare(unsigned int cpu)
 {
-	struct amd_uncore *uncore_nb = NULL, *uncore_llc = NULL;
+	struct amd_uncore *uncore;
+	struct amd_uncore_ctx *ctx;
+	int i;
 
-	if (amd_uncore_nb) {
-		*per_cpu_ptr(amd_uncore_nb, cpu) = NULL;
-		uncore_nb = amd_uncore_alloc(cpu);
-		if (!uncore_nb)
-			goto fail;
-		uncore_nb->cpu = cpu;
-		uncore_nb->num_counters = num_counters_nb;
-		uncore_nb->rdpmc_base = RDPMC_BASE_NB;
-		uncore_nb->msr_base = MSR_F15H_NB_PERF_CTL;
-		uncore_nb->active_mask = &amd_nb_active_mask;
-		uncore_nb->pmu = &amd_nb_pmu;
-		uncore_nb->events = amd_uncore_events_alloc(num_counters_nb, cpu);
-		if (!uncore_nb->events)
+	for (i = 0; i < num_uncores; i++) {
+		uncore = &uncores[i];
+		*per_cpu_ptr(uncore->ctx, cpu) = NULL;
+		ctx = kzalloc_node(sizeof(struct amd_uncore_ctx), GFP_KERNEL,
+				   cpu_to_node(cpu));
+		if (!ctx)
 			goto fail;
-		uncore_nb->id = -1;
-		*per_cpu_ptr(amd_uncore_nb, cpu) = uncore_nb;
-	}
 
-	if (amd_uncore_llc) {
-		*per_cpu_ptr(amd_uncore_llc, cpu) = NULL;
-		uncore_llc = amd_uncore_alloc(cpu);
-		if (!uncore_llc)
-			goto fail;
-		uncore_llc->cpu = cpu;
-		uncore_llc->num_counters = num_counters_llc;
-		uncore_llc->rdpmc_base = RDPMC_BASE_LLC;
-		uncore_llc->msr_base = MSR_F16H_L2I_PERF_CTL;
-		uncore_llc->active_mask = &amd_llc_active_mask;
-		uncore_llc->pmu = &amd_llc_pmu;
-		uncore_llc->events = amd_uncore_events_alloc(num_counters_llc, cpu);
-		if (!uncore_llc->events)
+		ctx->cpu = cpu;
+		ctx->events = kzalloc_node(sizeof(struct perf_event *) *
+					   uncore->num_counters, GFP_KERNEL,
+					   cpu_to_node(cpu));
+		if (!ctx->events)
 			goto fail;
-		uncore_llc->id = -1;
-		*per_cpu_ptr(amd_uncore_llc, cpu) = uncore_llc;
+
+		ctx->id = -1;
+		*per_cpu_ptr(uncore->ctx, cpu) = ctx;
 	}
 
 	return 0;
 
 fail:
-	if (uncore_nb) {
-		kfree(uncore_nb->events);
-		kfree(uncore_nb);
-	}
+	/* Rollback */
+	for (; i >= 0; i--) {
+		uncore = &uncores[i];
+		ctx = *per_cpu_ptr(uncore->ctx, cpu);
+		if (!ctx)
+			continue;
 
-	if (uncore_llc) {
-		kfree(uncore_llc->events);
-		kfree(uncore_llc);
+		kfree(ctx->events);
+		kfree(ctx);
 	}
 
 	return -ENOMEM;
 }
 
-static struct amd_uncore *
-amd_uncore_find_online_sibling(struct amd_uncore *this,
-			       struct amd_uncore * __percpu *uncores)
+static struct amd_uncore_ctx *
+amd_uncore_find_online_sibling(struct amd_uncore_ctx *this,
+			       struct amd_uncore *uncore)
 {
 	unsigned int cpu;
-	struct amd_uncore *that;
+	struct amd_uncore_ctx *that;
 
 	for_each_online_cpu(cpu) {
-		that = *per_cpu_ptr(uncores, cpu);
+		that = *per_cpu_ptr(uncore->ctx, cpu);
 
 		if (!that)
 			continue;
@@ -523,24 +403,16 @@ amd_uncore_find_online_sibling(struct amd_uncore *this,
 
 static int amd_uncore_cpu_starting(unsigned int cpu)
 {
-	unsigned int eax, ebx, ecx, edx;
 	struct amd_uncore *uncore;
+	struct amd_uncore_ctx *ctx;
+	int i;
 
-	if (amd_uncore_nb) {
-		uncore = *per_cpu_ptr(amd_uncore_nb, cpu);
-		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
-		uncore->id = ecx & 0xff;
-
-		uncore = amd_uncore_find_online_sibling(uncore, amd_uncore_nb);
-		*per_cpu_ptr(amd_uncore_nb, cpu) = uncore;
-	}
-
-	if (amd_uncore_llc) {
-		uncore = *per_cpu_ptr(amd_uncore_llc, cpu);
-		uncore->id = get_llc_id(cpu);
-
-		uncore = amd_uncore_find_online_sibling(uncore, amd_uncore_llc);
-		*per_cpu_ptr(amd_uncore_llc, cpu) = uncore;
+	for (i = 0; i < num_uncores; i++) {
+		uncore = &uncores[i];
+		ctx = *per_cpu_ptr(uncore->ctx, cpu);
+		ctx->id = uncore->id(cpu);
+		ctx = amd_uncore_find_online_sibling(ctx, uncore);
+		*per_cpu_ptr(uncore->ctx, cpu) = ctx;
 	}
 
 	return 0;
@@ -548,195 +420,359 @@ static int amd_uncore_cpu_starting(unsigned int cpu)
 
 static void uncore_clean_online(void)
 {
-	struct amd_uncore *uncore;
+	struct amd_uncore_ctx *ctx;
 	struct hlist_node *n;
 
-	hlist_for_each_entry_safe(uncore, n, &uncore_unused_list, node) {
-		hlist_del(&uncore->node);
-		kfree(uncore->events);
-		kfree(uncore);
+	hlist_for_each_entry_safe(ctx, n, &uncore_unused_list, node) {
+		hlist_del(&ctx->node);
+		kfree(ctx->events);
+		kfree(ctx);
 	}
 }
 
-static void uncore_online(unsigned int cpu,
-			  struct amd_uncore * __percpu *uncores)
+static int amd_uncore_cpu_online(unsigned int cpu)
 {
-	struct amd_uncore *uncore = *per_cpu_ptr(uncores, cpu);
+	struct amd_uncore *uncore;
+	struct amd_uncore_ctx *ctx;
+	int i;
 
 	uncore_clean_online();
 
-	if (cpu == uncore->cpu)
-		cpumask_set_cpu(cpu, uncore->active_mask);
+	for (i = 0; i < num_uncores; i++) {
+		uncore = &uncores[i];
+		ctx = *per_cpu_ptr(uncore->ctx, cpu);
+		if (cpu == ctx->cpu)
+			cpumask_set_cpu(cpu, &uncore->active_mask);
+	}
+
+	return 0;
 }
 
-static int amd_uncore_cpu_online(unsigned int cpu)
+static int amd_uncore_cpu_down_prepare(unsigned int cpu)
 {
-	if (amd_uncore_nb)
-		uncore_online(cpu, amd_uncore_nb);
+	struct amd_uncore_ctx *this, *that;
+	struct amd_uncore *uncore;
+	int i, j;
+
+	for (i = 0; i < num_uncores; i++) {
+		uncore = &uncores[i];
+		this = *per_cpu_ptr(uncore->ctx, cpu);
+
+		/* this cpu is going down, migrate to a shared sibling if possible */
+		for_each_online_cpu(j) {
+			that = *per_cpu_ptr(uncore->ctx, j);
+
+			if (cpu == j)
+				continue;
+
+			if (this == that) {
+				perf_pmu_migrate_context(&uncore->pmu, cpu, j);
+				cpumask_clear_cpu(cpu, &uncore->active_mask);
+				cpumask_set_cpu(j, &uncore->active_mask);
+				that->cpu = j;
+				break;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int amd_uncore_cpu_dead(unsigned int cpu)
+{
+	struct amd_uncore_ctx *ctx;
+	struct amd_uncore *uncore;
+	int i;
+
+	for (i = 0; i < num_uncores; i++) {
+		uncore = &uncores[i];
+		ctx = *per_cpu_ptr(uncore->ctx, cpu);
+		if (cpu == ctx->cpu)
+			cpumask_clear_cpu(cpu, &uncore->active_mask);
+
+		if (!--ctx->refcnt) {
+			kfree(ctx->events);
+			kfree(ctx);
+		}
 
-	if (amd_uncore_llc)
-		uncore_online(cpu, amd_uncore_llc);
+		*per_cpu_ptr(uncore->ctx, cpu) = NULL;
+	}
 
 	return 0;
 }
 
-static void uncore_down_prepare(unsigned int cpu,
-				struct amd_uncore * __percpu *uncores)
+static int amd_uncore_df_id(unsigned int cpu)
 {
-	unsigned int i;
-	struct amd_uncore *this = *per_cpu_ptr(uncores, cpu);
+	unsigned int eax, ebx, ecx, edx;
 
-	if (this->cpu != cpu)
-		return;
+	cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
 
-	/* this cpu is going down, migrate to a shared sibling if possible */
-	for_each_online_cpu(i) {
-		struct amd_uncore *that = *per_cpu_ptr(uncores, i);
+	return ecx & 0xff;
+}
 
-		if (cpu == i)
-			continue;
+static int amd_uncore_df_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	int ret = amd_uncore_event_init(event);
 
-		if (this == that) {
-			perf_pmu_migrate_context(this->pmu, cpu, i);
-			cpumask_clear_cpu(cpu, that->active_mask);
-			cpumask_set_cpu(i, that->active_mask);
-			that->cpu = i;
-			break;
-		}
-	}
+	if (ret || pmu_version < 2)
+		return ret;
+
+	hwc->config = event->attr.config &
+		      (pmu_version >= 2 ? AMD64_PERFMON_V2_RAW_EVENT_MASK_NB :
+					  AMD64_RAW_EVENT_MASK_NB);
+
+	return 0;
 }
 
-static int amd_uncore_cpu_down_prepare(unsigned int cpu)
+static int amd_uncore_df_add(struct perf_event *event, int flags)
 {
-	if (amd_uncore_nb)
-		uncore_down_prepare(cpu, amd_uncore_nb);
+	int ret = amd_uncore_add(event, flags & ~PERF_EF_START);
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (ret)
+		return ret;
 
-	if (amd_uncore_llc)
-		uncore_down_prepare(cpu, amd_uncore_llc);
+	/*
+	 * The first four DF counters are accessible via RDPMC index 6 to 9
+	 * followed by the L3 counters from index 10 to 15. For processors
+	 * with more than four DF counters, the DF RDPMC assignments become
+	 * discontiguous as the additional counters are accessible starting
+	 * from index 16.
+	 */
+	if (hwc->idx >= NUM_COUNTERS_NB)
+		hwc->event_base_rdpmc += NUM_COUNTERS_L3;
+
+	/* Delayed start after rdpmc base update */
+	if (flags & PERF_EF_START)
+		amd_uncore_start(event, PERF_EF_RELOAD);
 
 	return 0;
 }
 
-static void uncore_dead(unsigned int cpu, struct amd_uncore * __percpu *uncores)
+static int amd_uncore_df_init(void)
 {
-	struct amd_uncore *uncore = *per_cpu_ptr(uncores, cpu);
+	struct attribute **df_attr = amd_uncore_df_format_attr;
+	struct amd_uncore *uncore = &uncores[num_uncores];
+	union cpuid_0x80000022_ebx ebx;
+	int ret;
+
+	if (!boot_cpu_has(X86_FEATURE_PERFCTR_NB))
+		return 0;
 
-	if (cpu == uncore->cpu)
-		cpumask_clear_cpu(cpu, uncore->active_mask);
+	/*
+	 * For Family 17h and above, the Northbridge counters are repurposed
+	 * as Data Fabric counters. The PMUs are exported based on family as
+	 * either NB or DF.
+	 */
+	strncpy(uncore->name, boot_cpu_data.x86 >= 0x17 ? "amd_df" : "amd_nb",
+		sizeof(uncore->name));
+
+	uncore->num_counters = NUM_COUNTERS_NB;
+	uncore->msr_base = MSR_F15H_NB_PERF_CTL;
+	uncore->rdpmc_base = RDPMC_BASE_NB;
+	uncore->id = amd_uncore_df_id;
+
+	if (pmu_version >= 2) {
+		*df_attr++ = &format_attr_event14v2.attr;
+		*df_attr++ = &format_attr_umask12.attr;
+		ebx.full = cpuid_ebx(EXT_PERFMON_DEBUG_FEATURES);
+		uncore->num_counters = ebx.split.num_df_pmc;
+	} else if (boot_cpu_data.x86 >= 0x17) {
+		*df_attr = &format_attr_event14.attr;
+	}
 
-	if (!--uncore->refcnt) {
-		kfree(uncore->events);
-		kfree(uncore);
+	uncore->ctx = alloc_percpu(struct amd_uncore_ctx *);
+	if (!uncore->ctx)
+		return -ENOMEM;
+
+	uncore->pmu = (struct pmu) {
+		.task_ctx_nr	= perf_invalid_context,
+		.attr_groups	= amd_uncore_df_attr_groups,
+		.name		= uncore->name,
+		.event_init	= amd_uncore_df_event_init,
+		.add		= amd_uncore_df_add,
+		.del		= amd_uncore_del,
+		.start		= amd_uncore_start,
+		.stop		= amd_uncore_stop,
+		.read		= amd_uncore_read,
+		.capabilities	= PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
+		.module		= THIS_MODULE,
+	};
+
+	ret = perf_pmu_register(&uncore->pmu, uncore->pmu.name, -1);
+	if (ret) {
+		free_percpu(uncore->ctx);
+		uncore->ctx = NULL;
+		return ret;
 	}
 
-	*per_cpu_ptr(uncores, cpu) = NULL;
+	pr_info("%d %s %s counters detected\n", uncore->num_counters,
+		boot_cpu_data.x86_vendor == X86_VENDOR_HYGON ?  "HYGON" : "",
+		uncore->pmu.name);
+
+	num_uncores++;
+
+	return 0;
 }
 
-static int amd_uncore_cpu_dead(unsigned int cpu)
+static int amd_uncore_l3_id(unsigned int cpu)
+{
+	return get_llc_id(cpu);
+}
+
+static int amd_uncore_l3_event_init(struct perf_event *event)
 {
-	if (amd_uncore_nb)
-		uncore_dead(cpu, amd_uncore_nb);
+	int ret = amd_uncore_event_init(event);
+	struct hw_perf_event *hwc = &event->hw;
+	u64 config = event->attr.config;
+	u64 mask;
+
+	hwc->config = config & AMD64_RAW_EVENT_MASK_NB;
+
+	/*
+	 * SliceMask and ThreadMask need to be set for certain L3 events.
+	 * For other events, the two fields do not affect the count.
+	 */
+	if (ret || boot_cpu_data.x86 < 0x17)
+		return ret;
+
+	mask = config & (AMD64_L3_F19H_THREAD_MASK | AMD64_L3_SLICEID_MASK |
+			 AMD64_L3_EN_ALL_CORES | AMD64_L3_EN_ALL_SLICES |
+			 AMD64_L3_COREID_MASK);
+
+	if (boot_cpu_data.x86 <= 0x18)
+		mask = ((config & AMD64_L3_SLICE_MASK) ? : AMD64_L3_SLICE_MASK) |
+		       ((config & AMD64_L3_THREAD_MASK) ? : AMD64_L3_THREAD_MASK);
 
-	if (amd_uncore_llc)
-		uncore_dead(cpu, amd_uncore_llc);
+	/*
+	 * If the user doesn't specify a ThreadMask, they're not trying to
+	 * count core 0, so we enable all cores & threads.
+	 * We'll also assume that they want to count slice 0 if they specify
+	 * a ThreadMask and leave SliceId and EnAllSlices unpopulated.
+	 */
+	else if (!(config & AMD64_L3_F19H_THREAD_MASK))
+		mask = AMD64_L3_F19H_THREAD_MASK | AMD64_L3_EN_ALL_SLICES |
+		       AMD64_L3_EN_ALL_CORES;
+
+	hwc->config |= mask;
 
 	return 0;
 }
 
-static int __init amd_uncore_init(void)
+static int amd_uncore_l3_init(void)
 {
-	struct attribute **df_attr = amd_uncore_df_format_attr;
 	struct attribute **l3_attr = amd_uncore_l3_format_attr;
-	union cpuid_0x80000022_ebx ebx;
-	int ret = -ENODEV;
+	struct amd_uncore *uncore = &uncores[num_uncores];
+	int ret;
 
-	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
-	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
-		return -ENODEV;
+	if (!boot_cpu_has(X86_FEATURE_PERFCTR_LLC))
+		return 0;
 
-	if (!boot_cpu_has(X86_FEATURE_TOPOEXT))
-		return -ENODEV;
+	/*
+	 * For Family 17h and above, L3 cache counters are available instead
+	 * of L2 cache counters. The PMUs are exported based on family as
+	 * either L2 or L3.
+	 */
+	strncpy(uncore->name, boot_cpu_data.x86 >= 0x17 ? "amd_l3" : "amd_l2",
+		sizeof(uncore->name));
 
-	if (boot_cpu_has(X86_FEATURE_PERFMON_V2))
-		pmu_version = 2;
+	uncore->num_counters = NUM_COUNTERS_L2;
+	uncore->msr_base = MSR_F16H_L2I_PERF_CTL;
+	uncore->rdpmc_base = RDPMC_BASE_LLC;
+	uncore->id = amd_uncore_l3_id;
 
-	num_counters_nb	= NUM_COUNTERS_NB;
-	num_counters_llc = NUM_COUNTERS_L2;
 	if (boot_cpu_data.x86 >= 0x17) {
-		/*
-		 * For F17h and above, the Northbridge counters are
-		 * repurposed as Data Fabric counters. Also, L3
-		 * counters are supported too. The PMUs are exported
-		 * based on family as either L2 or L3 and NB or DF.
-		 */
-		num_counters_llc	  = NUM_COUNTERS_L3;
-		amd_nb_pmu.name		  = "amd_df";
-		amd_llc_pmu.name	  = "amd_l3";
-		l3_mask			  = true;
+		*l3_attr++ = &format_attr_event8.attr;
+		*l3_attr++ = &format_attr_umask8.attr;
+		*l3_attr++ = boot_cpu_data.x86 >= 0x19 ?
+			     &format_attr_threadmask2.attr :
+			     &format_attr_threadmask8.attr;
+		uncore->num_counters = NUM_COUNTERS_L3;
 	}
 
-	if (boot_cpu_has(X86_FEATURE_PERFCTR_NB)) {
-		if (pmu_version >= 2) {
-			*df_attr++ = &format_attr_event14v2.attr;
-			*df_attr++ = &format_attr_umask12.attr;
-		} else if (boot_cpu_data.x86 >= 0x17) {
-			*df_attr = &format_attr_event14.attr;
-		}
+	uncore->ctx = alloc_percpu(struct amd_uncore_ctx *);
+	if (!uncore->ctx)
+		return -ENOMEM;
+
+	uncore->pmu = (struct pmu) {
+		.task_ctx_nr	= perf_invalid_context,
+		.attr_groups	= amd_uncore_l3_attr_groups,
+		.attr_update	= amd_uncore_l3_attr_update,
+		.name		= uncore->name,
+		.event_init	= amd_uncore_l3_event_init,
+		.add		= amd_uncore_add,
+		.del		= amd_uncore_del,
+		.start		= amd_uncore_start,
+		.stop		= amd_uncore_stop,
+		.read		= amd_uncore_read,
+		.capabilities	= PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
+		.module		= THIS_MODULE,
+	};
+
+	ret = perf_pmu_register(&uncore->pmu, uncore->pmu.name, -1);
+	if (ret) {
+		free_percpu(uncore->ctx);
+		uncore->ctx = NULL;
+		return ret;
+	}
 
-		amd_uncore_nb = alloc_percpu(struct amd_uncore *);
-		if (!amd_uncore_nb) {
-			ret = -ENOMEM;
-			goto fail_nb;
-		}
-		ret = perf_pmu_register(&amd_nb_pmu, amd_nb_pmu.name, -1);
-		if (ret)
-			goto fail_nb;
+	pr_info("%d %s %s counters detected\n", uncore->num_counters,
+		boot_cpu_data.x86_vendor == X86_VENDOR_HYGON ?  "HYGON" : "",
+		uncore->pmu.name);
 
-		if (pmu_version >= 2) {
-			ebx.full = cpuid_ebx(EXT_PERFMON_DEBUG_FEATURES);
-			num_counters_nb = ebx.split.num_df_pmc;
-		}
+	num_uncores++;
 
-		pr_info("%d %s %s counters detected\n", num_counters_nb,
-			boot_cpu_data.x86_vendor == X86_VENDOR_HYGON ?  "HYGON" : "",
-			amd_nb_pmu.name);
+	return 0;
+}
 
-		ret = 0;
-	}
+static void uncore_free(void)
+{
+	struct amd_uncore *uncore;
+	int i;
 
-	if (boot_cpu_has(X86_FEATURE_PERFCTR_LLC)) {
-		if (boot_cpu_data.x86 >= 0x19) {
-			*l3_attr++ = &format_attr_event8.attr;
-			*l3_attr++ = &format_attr_umask8.attr;
-			*l3_attr++ = &format_attr_threadmask2.attr;
-		} else if (boot_cpu_data.x86 >= 0x17) {
-			*l3_attr++ = &format_attr_event8.attr;
-			*l3_attr++ = &format_attr_umask8.attr;
-			*l3_attr++ = &format_attr_threadmask8.attr;
-		}
+	for (i = 0; i < num_uncores; i++) {
+		uncore = &uncores[i];
+		if (!uncore->ctx)
+			continue;
 
-		amd_uncore_llc = alloc_percpu(struct amd_uncore *);
-		if (!amd_uncore_llc) {
-			ret = -ENOMEM;
-			goto fail_llc;
-		}
-		ret = perf_pmu_register(&amd_llc_pmu, amd_llc_pmu.name, -1);
-		if (ret)
-			goto fail_llc;
-
-		pr_info("%d %s %s counters detected\n", num_counters_llc,
-			boot_cpu_data.x86_vendor == X86_VENDOR_HYGON ?  "HYGON" : "",
-			amd_llc_pmu.name);
-		ret = 0;
+		perf_pmu_unregister(&uncore->pmu);
+		free_percpu(uncore->ctx);
+		uncore->ctx = NULL;
 	}
 
+	num_uncores = 0;
+}
+
+static int __init amd_uncore_init(void)
+{
+	int ret;
+
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD &&
+	    boot_cpu_data.x86_vendor != X86_VENDOR_HYGON)
+		return -ENODEV;
+
+	if (!boot_cpu_has(X86_FEATURE_TOPOEXT))
+		return -ENODEV;
+
+	if (boot_cpu_has(X86_FEATURE_PERFMON_V2))
+		pmu_version = 2;
+
+	ret = amd_uncore_df_init();
+	if (ret)
+		goto fail;
+
+	ret = amd_uncore_l3_init();
+	if (ret)
+		goto fail;
+
 	/*
 	 * Install callbacks. Core will call them for each online cpu.
 	 */
 	if (cpuhp_setup_state(CPUHP_PERF_X86_AMD_UNCORE_PREP,
 			      "perf/x86/amd/uncore:prepare",
 			      amd_uncore_cpu_up_prepare, amd_uncore_cpu_dead))
-		goto fail_llc;
+		goto fail;
 
 	if (cpuhp_setup_state(CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING,
 			      "perf/x86/amd/uncore:starting",
@@ -753,12 +789,8 @@ static int __init amd_uncore_init(void)
 	cpuhp_remove_state(CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING);
 fail_prep:
 	cpuhp_remove_state(CPUHP_PERF_X86_AMD_UNCORE_PREP);
-fail_llc:
-	if (boot_cpu_has(X86_FEATURE_PERFCTR_NB))
-		perf_pmu_unregister(&amd_nb_pmu);
-	free_percpu(amd_uncore_llc);
-fail_nb:
-	free_percpu(amd_uncore_nb);
+fail:
+	uncore_free();
 
 	return ret;
 }
@@ -768,18 +800,7 @@ static void __exit amd_uncore_exit(void)
 	cpuhp_remove_state(CPUHP_AP_PERF_X86_AMD_UNCORE_ONLINE);
 	cpuhp_remove_state(CPUHP_AP_PERF_X86_AMD_UNCORE_STARTING);
 	cpuhp_remove_state(CPUHP_PERF_X86_AMD_UNCORE_PREP);
-
-	if (boot_cpu_has(X86_FEATURE_PERFCTR_LLC)) {
-		perf_pmu_unregister(&amd_llc_pmu);
-		free_percpu(amd_uncore_llc);
-		amd_uncore_llc = NULL;
-	}
-
-	if (boot_cpu_has(X86_FEATURE_PERFCTR_NB)) {
-		perf_pmu_unregister(&amd_nb_pmu);
-		free_percpu(amd_uncore_nb);
-		amd_uncore_nb = NULL;
-	}
+	uncore_free();
 }
 
 module_init(amd_uncore_init);
-- 
2.34.1

[PATCH 2/6] perf/x86/amd/uncore: Use rdmsr if rdpmc is unavailable

[Sandipan Das]

Not all uncore PMUs may support the use of the RDPMC instruction for
reading counters. In such cases, read the count from the corresponding
PERF_CTR register using the RDMSR instruction.

Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/events/amd/uncore.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index b0afbe6d9eb4..f17df6574ba5 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -76,7 +76,16 @@ static void amd_uncore_read(struct perf_event *event)
 	 */
 
 	prev = local64_read(&hwc->prev_count);
-	rdpmcl(hwc->event_base_rdpmc, new);
+
+	/*
+	 * Some uncore PMUs do not have RDPMC assignments. In such cases,
+	 * read counts directly from the corresponding PERF_CTR.
+	 */
+	if (hwc->event_base_rdpmc < 0)
+		rdmsrl(hwc->event_base, new);
+	else
+		rdpmcl(hwc->event_base_rdpmc, new);
+
 	local64_set(&hwc->prev_count, new);
 	delta = (new << COUNTER_SHIFT) - (prev << COUNTER_SHIFT);
 	delta >>= COUNTER_SHIFT;
@@ -144,6 +153,9 @@ static int amd_uncore_add(struct perf_event *event, int flags)
 	hwc->event_base_rdpmc = uncore->rdpmc_base + hwc->idx;
 	hwc->state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
+	if (uncore->rdpmc_base < 0)
+		hwc->event_base_rdpmc = -1;
+
 	if (flags & PERF_EF_START)
 		event->pmu->start(event, PERF_EF_RELOAD);
 
-- 
2.34.1

[PATCH 3/6] x86/cpuid: Add smp helper

[Sandipan Das]

Depending on which CPU the CPUID instruction is executed, some leaves
can report different values. There are cases where it may be required
to know all possible values.

E.g. for AMD Zen 4 processors, the ActiveUmcMask field from leaf
0x80000022 ECX, which provides a way to determine the active memory
controllers, can have different masks on CPUs belonging to different
sockets as each socket can follow a different DIMM population scheme.
Each memory channel is assigned a memory controller (UMC) and if no
DIMMs are attached to a channel, the corresponding memory controller
is inactive. There are performance monitoring counters exclusive to
each memory controller which need to be represented under separate
PMUs. So, it will be necessary to know the active memory controllers
on each socket during the initialization of the UMC PMUs irrespective
of where the uncore driver's module init runs.

Add a new helper that executes CPUID on a particular CPU and returns
the EAX, EBX, ECX and EDX values.

Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/include/asm/cpuid.h | 14 ++++++++++++++
 arch/x86/lib/Makefile        |  2 +-
 arch/x86/lib/cpuid-smp.c     | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/lib/cpuid-smp.c

diff --git a/arch/x86/include/asm/cpuid.h b/arch/x86/include/asm/cpuid.h
index 9bee3e7bf973..17e74d4584f5 100644
--- a/arch/x86/include/asm/cpuid.h
+++ b/arch/x86/include/asm/cpuid.h
@@ -150,6 +150,20 @@ static __always_inline bool cpuid_function_is_indexed(u32 function)
 	return false;
 }
 
+#ifdef CONFIG_SMP
+int cpuid_on_cpu(unsigned int cpu, unsigned int op,
+		 unsigned int *eax, unsigned int *ebx,
+		 unsigned int *ecx, unsigned int *edx);
+#else	/* CONFIG_SMP */
+static inline int cpuid_on_cpu(unsigned int cpu, unsigned int op,
+			       unsigned int *eax, unsigned int *ebx,
+			       unsigned int *ecx, unsigned int *edx)
+{
+	cpuid(op, eax, ebx, ecx, edx);
+	return 0;
+}
+#endif	/* CONFIG_SMP */
+
 #define for_each_possible_hypervisor_cpuid_base(function) \
 	for (function = 0x40000000; function < 0x40010000; function += 0x100)
 
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index ea3a28e7b613..e0097ae55edf 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -39,7 +39,7 @@ $(obj)/inat.o: $(obj)/inat-tables.c
 
 clean-files := inat-tables.c
 
-obj-$(CONFIG_SMP) += msr-smp.o cache-smp.o
+obj-$(CONFIG_SMP) += msr-smp.o cache-smp.o cpuid-smp.o
 
 lib-y := delay.o misc.o cmdline.o cpu.o
 lib-y += usercopy_$(BITS).o usercopy.o getuser.o putuser.o
diff --git a/arch/x86/lib/cpuid-smp.c b/arch/x86/lib/cpuid-smp.c
new file mode 100644
index 000000000000..87340893ff61
--- /dev/null
+++ b/arch/x86/lib/cpuid-smp.c
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/export.h>
+#include <linux/smp.h>
+#include <asm/cpuid.h>
+
+struct cpuid_info {
+	u32 op;
+	struct cpuid_regs regs;
+};
+
+static void __cpuid_smp(void *info)
+{
+	struct cpuid_info *rv = info;
+
+	cpuid(rv->op, &rv->regs.eax, &rv->regs.ebx, &rv->regs.ecx, &rv->regs.edx);
+}
+
+int cpuid_on_cpu(unsigned int cpu, unsigned int op,
+		 unsigned int *eax, unsigned int *ebx,
+		 unsigned int *ecx, unsigned int *edx)
+{
+	struct cpuid_info rv;
+	int err;
+
+	memset(&rv, 0, sizeof(rv));
+
+	rv.op = op;
+	err = smp_call_function_single(cpu, __cpuid_smp, &rv, 1);
+	*eax = rv.regs.eax;
+	*ebx = rv.regs.ebx;
+	*ecx = rv.regs.ecx;
+	*edx = rv.regs.edx;
+
+	return err;
+}
+EXPORT_SYMBOL(cpuid_on_cpu);
-- 
2.34.1

Re: [PATCH 3/6] x86/cpuid: Add smp helper

[Peter Zijlstra]

On Wed, Jul 19, 2023 at 12:25:38PM +0530, Sandipan Das wrote:
> Depending on which CPU the CPUID instruction is executed, some leaves
> can report different values. There are cases where it may be required
> to know all possible values.
> 
> E.g. for AMD Zen 4 processors, the ActiveUmcMask field from leaf
> 0x80000022 ECX, which provides a way to determine the active memory
> controllers, can have different masks on CPUs belonging to different
> sockets as each socket can follow a different DIMM population scheme.
> Each memory channel is assigned a memory controller (UMC) and if no
> DIMMs are attached to a channel, the corresponding memory controller
> is inactive. There are performance monitoring counters exclusive to
> each memory controller which need to be represented under separate
> PMUs. So, it will be necessary to know the active memory controllers
> on each socket during the initialization of the UMC PMUs irrespective
> of where the uncore driver's module init runs.
> 
> Add a new helper that executes CPUID on a particular CPU and returns
> the EAX, EBX, ECX and EDX values.
> 

So I hate all this for multiple reasons:

 - the wohle foo_on_cpu() model generally leads to atrocious code that
   does multiple IPIs, I've seen rdmsr_on_cpu() followed by
   wrmsr_on_cpu() and worse things, just don't do this.

 - The whole CPUID thing is insane; we should read CPUID -- all of it --
   *ONCE* at bringup and thereafter never touch the instruction ever
   again. It could be people are already working on patches to this
   effect.

 - Different CPUID values for different CPUs is a pain :/

Re: [PATCH 3/6] x86/cpuid: Add smp helper

[Sandipan Das]

On 7/19/2023 12:58 PM, Peter Zijlstra wrote:
> On Wed, Jul 19, 2023 at 12:25:38PM +0530, Sandipan Das wrote:
>> Depending on which CPU the CPUID instruction is executed, some leaves
>> can report different values. There are cases where it may be required
>> to know all possible values.
>>
>> E.g. for AMD Zen 4 processors, the ActiveUmcMask field from leaf
>> 0x80000022 ECX, which provides a way to determine the active memory
>> controllers, can have different masks on CPUs belonging to different
>> sockets as each socket can follow a different DIMM population scheme.
>> Each memory channel is assigned a memory controller (UMC) and if no
>> DIMMs are attached to a channel, the corresponding memory controller
>> is inactive. There are performance monitoring counters exclusive to
>> each memory controller which need to be represented under separate
>> PMUs. So, it will be necessary to know the active memory controllers
>> on each socket during the initialization of the UMC PMUs irrespective
>> of where the uncore driver's module init runs.
>>
>> Add a new helper that executes CPUID on a particular CPU and returns
>> the EAX, EBX, ECX and EDX values.
>>
> 
> So I hate all this for multiple reasons:
> 
>  - the wohle foo_on_cpu() model generally leads to atrocious code that
>    does multiple IPIs, I've seen rdmsr_on_cpu() followed by
>    wrmsr_on_cpu() and worse things, just don't do this.
> 
>  - The whole CPUID thing is insane; we should read CPUID -- all of it --
>    *ONCE* at bringup and thereafter never touch the instruction ever
>    again. It could be people are already working on patches to this
>    effect.
> 
>  - Different CPUID values for different CPUs is a pain :/

Thanks for the clarification. Will remove this.

- Sandipan

Re: [PATCH 3/6] x86/cpuid: Add smp helper

[Thomas Gleixner]

On Wed, Jul 19 2023 at 12:25, Sandipan Das wrote:
> Depending on which CPU the CPUID instruction is executed, some leaves
> can report different values. There are cases where it may be required
> to know all possible values.
>
> E.g. for AMD Zen 4 processors, the ActiveUmcMask field from leaf
> 0x80000022 ECX, which provides a way to determine the active memory
> controllers, can have different masks on CPUs belonging to different
> sockets as each socket can follow a different DIMM population scheme.
> Each memory channel is assigned a memory controller (UMC) and if no
> DIMMs are attached to a channel, the corresponding memory controller
> is inactive. There are performance monitoring counters exclusive to
> each memory controller which need to be represented under separate
> PMUs. So, it will be necessary to know the active memory controllers
> on each socket during the initialization of the UMC PMUs irrespective
> of where the uncore driver's module init runs.
>
> Add a new helper that executes CPUID on a particular CPU and returns
> the EAX, EBX, ECX and EDX values.

NAK.

This madness has to stop. The correct thing is to parse the information
in CPUID at the point where the CPU comes online and store it for easy
consumption.

I'm in the process of reworking the CPUID and topology evaluation and
that's where these things need to be stored. I'm still fighting some
nightmares with the already existing mess.

Look at the mess people created over time here:

     https://lore.kernel.org/lkml/20230717223049.327865981@linutronix.de

No need to add more insanities to it. IOW, this has to wait for a week
or two until I settled the remaining issues.

Thanks

        tglx

Re: [PATCH 3/6] x86/cpuid: Add smp helper

[Sandipan Das]

On 7/19/2023 12:59 PM, Thomas Gleixner wrote:
> On Wed, Jul 19 2023 at 12:25, Sandipan Das wrote:
>> Depending on which CPU the CPUID instruction is executed, some leaves
>> can report different values. There are cases where it may be required
>> to know all possible values.
>>
>> E.g. for AMD Zen 4 processors, the ActiveUmcMask field from leaf
>> 0x80000022 ECX, which provides a way to determine the active memory
>> controllers, can have different masks on CPUs belonging to different
>> sockets as each socket can follow a different DIMM population scheme.
>> Each memory channel is assigned a memory controller (UMC) and if no
>> DIMMs are attached to a channel, the corresponding memory controller
>> is inactive. There are performance monitoring counters exclusive to
>> each memory controller which need to be represented under separate
>> PMUs. So, it will be necessary to know the active memory controllers
>> on each socket during the initialization of the UMC PMUs irrespective
>> of where the uncore driver's module init runs.
>>
>> Add a new helper that executes CPUID on a particular CPU and returns
>> the EAX, EBX, ECX and EDX values.
> 
> NAK.
> 
> This madness has to stop. The correct thing is to parse the information
> in CPUID at the point where the CPU comes online and store it for easy
> consumption.
> 
> I'm in the process of reworking the CPUID and topology evaluation and
> that's where these things need to be stored. I'm still fighting some
> nightmares with the already existing mess.
> 
> Look at the mess people created over time here:
> 
>      https://lore.kernel.org/lkml/20230717223049.327865981@linutronix.de
> 
> No need to add more insanities to it. IOW, this has to wait for a week
> or two until I settled the remaining issues.
> 

Agreed. I'll rework the patches and remove this.

- Sandipan

Re: [PATCH 3/6] x86/cpuid: Add smp helper

[Thomas Gleixner]

On Wed, Jul 19 2023 at 09:29, Thomas Gleixner wrote:
> On Wed, Jul 19 2023 at 12:25, Sandipan Das wrote:
>> Depending on which CPU the CPUID instruction is executed, some leaves
>> can report different values. There are cases where it may be required
>> to know all possible values.
>> Add a new helper that executes CPUID on a particular CPU and returns
>> the EAX, EBX, ECX and EDX values.
>
> NAK.
>
> This madness has to stop. The correct thing is to parse the information
> in CPUID at the point where the CPU comes online and store it for easy
> consumption.
>
> I'm in the process of reworking the CPUID and topology evaluation and
> that's where these things need to be stored. I'm still fighting some
> nightmares with the already existing mess.
>
> Look at the mess people created over time here:
>
>      https://lore.kernel.org/lkml/20230717223049.327865981@linutronix.de
>
> No need to add more insanities to it. IOW, this has to wait for a week
> or two until I settled the remaining issues.

Actually you can do that today already. Either make it part of the CPUID
evaluation or use the perf hotplug callback mechanism which is invoked
early on the upcoming CPU and read that CPUID leaf.

Re: [PATCH 3/6] x86/cpuid: Add smp helper

[Sandipan Das]

On 7/19/2023 5:20 PM, Thomas Gleixner wrote:
> On Wed, Jul 19 2023 at 09:29, Thomas Gleixner wrote:
>> On Wed, Jul 19 2023 at 12:25, Sandipan Das wrote:
>>> Depending on which CPU the CPUID instruction is executed, some leaves
>>> can report different values. There are cases where it may be required
>>> to know all possible values.
>>> Add a new helper that executes CPUID on a particular CPU and returns
>>> the EAX, EBX, ECX and EDX values.
>>
>> NAK.
>>
>> This madness has to stop. The correct thing is to parse the information
>> in CPUID at the point where the CPU comes online and store it for easy
>> consumption.
>>
>> I'm in the process of reworking the CPUID and topology evaluation and
>> that's where these things need to be stored. I'm still fighting some
>> nightmares with the already existing mess.
>>
>> Look at the mess people created over time here:
>>
>>      https://lore.kernel.org/lkml/20230717223049.327865981@linutronix.de
>>
>> No need to add more insanities to it. IOW, this has to wait for a week
>> or two until I settled the remaining issues.
> 
> Actually you can do that today already. Either make it part of the CPUID
> evaluation or use the perf hotplug callback mechanism which is invoked
> early on the upcoming CPU and read that CPUID leaf.
> 

Thanks for the suggestions. I'll start with the hotplug callback mechanism.

- Sandipan

[PATCH 4/6] perf/x86/amd/uncore: Add group exclusivity

[Sandipan Das]

In some cases, it may be necessary to restrict opening PMU events to a
subset of CPUs. Uncore PMUs which require this restriction can use the
new group attribute in struct amd_uncore to set a valid uncore ID during
initialization. In the starting phase of hotplug, the per-CPU context
will be marked as unused if the ID in group does not match the uncore ID
for the CPU that is being onlined.

E.g. the Zen 4 memory controller (UMC) PMUs are specific to each active
memory channel and the MSR address space for the PERF_CTL and PERF_CTR
registers is reused on each socket. Thus, PMU events corresponding to a
memory controller should only be opened on CPUs belonging to the socket
associated with that memory controller.

Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/events/amd/uncore.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index f17df6574ba5..6653e8e164bd 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -50,6 +50,7 @@ struct amd_uncore {
 	int num_counters;
 	int rdpmc_base;
 	u32 msr_base;
+	int group;
 	cpumask_t active_mask;
 	struct pmu pmu;
 	struct amd_uncore_ctx * __percpu *ctx;
@@ -423,6 +424,17 @@ static int amd_uncore_cpu_starting(unsigned int cpu)
 		uncore = &uncores[i];
 		ctx = *per_cpu_ptr(uncore->ctx, cpu);
 		ctx->id = uncore->id(cpu);
+
+		/*
+		 * Reclaim the context if events can only be opened by CPUs
+		 * within the same group
+		 */
+		if (uncore->group >= 0 && ctx->id != uncore->group) {
+			hlist_add_head(&ctx->node, &uncore_unused_list);
+			*per_cpu_ptr(uncore->ctx, cpu) = NULL;
+			continue;
+		}
+
 		ctx = amd_uncore_find_online_sibling(ctx, uncore);
 		*per_cpu_ptr(uncore->ctx, cpu) = ctx;
 	}
@@ -453,7 +465,7 @@ static int amd_uncore_cpu_online(unsigned int cpu)
 	for (i = 0; i < num_uncores; i++) {
 		uncore = &uncores[i];
 		ctx = *per_cpu_ptr(uncore->ctx, cpu);
-		if (cpu == ctx->cpu)
+		if (ctx && cpu == ctx->cpu)
 			cpumask_set_cpu(cpu, &uncore->active_mask);
 	}
 
@@ -469,12 +481,14 @@ static int amd_uncore_cpu_down_prepare(unsigned int cpu)
 	for (i = 0; i < num_uncores; i++) {
 		uncore = &uncores[i];
 		this = *per_cpu_ptr(uncore->ctx, cpu);
+		if (!this)
+			continue;
 
 		/* this cpu is going down, migrate to a shared sibling if possible */
 		for_each_online_cpu(j) {
 			that = *per_cpu_ptr(uncore->ctx, j);
 
-			if (cpu == j)
+			if (!that || cpu == j)
 				continue;
 
 			if (this == that) {
@@ -499,6 +513,9 @@ static int amd_uncore_cpu_dead(unsigned int cpu)
 	for (i = 0; i < num_uncores; i++) {
 		uncore = &uncores[i];
 		ctx = *per_cpu_ptr(uncore->ctx, cpu);
+		if (!ctx)
+			continue;
+
 		if (cpu == ctx->cpu)
 			cpumask_clear_cpu(cpu, &uncore->active_mask);
 
@@ -584,6 +601,7 @@ static int amd_uncore_df_init(void)
 	uncore->msr_base = MSR_F15H_NB_PERF_CTL;
 	uncore->rdpmc_base = RDPMC_BASE_NB;
 	uncore->id = amd_uncore_df_id;
+	uncore->group = -1;
 
 	if (pmu_version >= 2) {
 		*df_attr++ = &format_attr_event14v2.attr;
@@ -693,6 +711,7 @@ static int amd_uncore_l3_init(void)
 	uncore->msr_base = MSR_F16H_L2I_PERF_CTL;
 	uncore->rdpmc_base = RDPMC_BASE_LLC;
 	uncore->id = amd_uncore_l3_id;
+	uncore->group = -1;
 
 	if (boot_cpu_data.x86 >= 0x17) {
 		*l3_attr++ = &format_attr_event8.attr;
-- 
2.34.1

[PATCH 5/6] perf/x86/amd/uncore: Add memory controller support

[Sandipan Das]

Unified Memory Controller (UMC) events were introduced with Zen 4 as a
part of the Performance Monitoring Version 2 (PerfMonV2) enhancements.
An event is specified using the EventSelect bits and the RdWrMask bits
can be used for additional filtering of reads and writes.

As of now, a maximum of 12 channels of DDR5 are available on each socket
and each channel controlled by a dedicated UMC. Each UMC has it own set
of counters.

Since the MSR address space for the UMC PERF_CTL and PERF_CTR registers
are reused across sockets, uncore groups are created on the basis of
socket IDs. Hence, group exclusivity is mandatory while opening events
so that events for an UMC can only be opened on CPUs which are the same
socket as the corresponding memory channel.

For each socket, the total number of available UMC counters and active
memory channels are determined from CPUID leaf 0x80000022 EBX and ECX
respectively. Usually, on Zen 4, each UMC gets 4 counters.

MSR assignments are determined on the basis of active UMCs. E.g. if
UMCs 1, 4 and 9 are active for a given socket, then

  * UMC 1 gets MSRs 0xc0010800 to 0xc0010807 as PERF_CTLs and PERF_CTRs
  * UMC 4 gets MSRs 0xc0010808 to 0xc001080f as PERF_CTLs and PERF_CTRs
  * UMC 9 gets MSRs 0xc0010810 to 0xc0010817 as PERF_CTLs and PERF_CTRs

Memory channels are generally labelled using alphabets and the mapping
of UMCs to memory channels is dependent on the family and model. This
information can be found in the "UMC and DDR Phy Logical Mapping"
section of the AMD Processor Programming Reference (PPR).

If there are sockets without any online CPUs when the amd_uncore driver
is loaded, UMCs for such sockets will not be discoverable since the
mechanism relies on executing the CPUID instruction on an online CPU
from the socket.

Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/events/amd/uncore.c      | 171 +++++++++++++++++++++++++++++-
 arch/x86/include/asm/msr-index.h  |   4 +
 arch/x86/include/asm/perf_event.h |   9 ++
 3 files changed, 182 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 6653e8e164bd..c3e1bddd4e1b 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -27,7 +27,12 @@
 
 #define COUNTER_SHIFT		16
 
-#define NUM_UNCORES_MAX		2	/* DF (or NB) and L3 (or L2) */
+/*
+ * While DF (or NB) and L3 (or L2) PMUs have a single instance, there may be
+ * multiple UMC PMUs, each corresponding to active memory channels across all
+ * sockets.
+ */
+#define NUM_UNCORES_MAX		64
 #define UNCORE_NAME_LEN		16
 
 #undef pr_fmt
@@ -264,7 +269,7 @@ static struct device_attribute format_attr_##_var =			\
 DEFINE_UNCORE_FORMAT_ATTR(event12,	event,		"config:0-7,32-35");
 DEFINE_UNCORE_FORMAT_ATTR(event14,	event,		"config:0-7,32-35,59-60"); /* F17h+ DF */
 DEFINE_UNCORE_FORMAT_ATTR(event14v2,	event,		"config:0-7,32-37");	   /* PerfMonV2 DF */
-DEFINE_UNCORE_FORMAT_ATTR(event8,	event,		"config:0-7");		   /* F17h+ L3 */
+DEFINE_UNCORE_FORMAT_ATTR(event8,	event,		"config:0-7");		   /* F17h+ L3, PerfMonV2 UMC */
 DEFINE_UNCORE_FORMAT_ATTR(umask8,	umask,		"config:8-15");
 DEFINE_UNCORE_FORMAT_ATTR(umask12,	umask,		"config:8-15,24-27");	   /* PerfMonV2 DF */
 DEFINE_UNCORE_FORMAT_ATTR(coreid,	coreid,		"config:42-44");	   /* F19h L3 */
@@ -274,6 +279,7 @@ DEFINE_UNCORE_FORMAT_ATTR(threadmask2,	threadmask,	"config:56-57");	   /* F19h L
 DEFINE_UNCORE_FORMAT_ATTR(enallslices,	enallslices,	"config:46");		   /* F19h L3 */
 DEFINE_UNCORE_FORMAT_ATTR(enallcores,	enallcores,	"config:47");		   /* F19h L3 */
 DEFINE_UNCORE_FORMAT_ATTR(sliceid,	sliceid,	"config:48-50");	   /* F19h L3 */
+DEFINE_UNCORE_FORMAT_ATTR(rdwrmask,    rdwrmask,       "config:8-9");             /* PerfMonV2 UMC */
 
 /* Common DF and NB attributes */
 static struct attribute *amd_uncore_df_format_attr[] = {
@@ -305,6 +311,13 @@ static struct attribute *amd_f19h_uncore_l3_format_attr[] = {
 	NULL,
 };
 
+/* Common UMC attributes */
+static struct attribute *amd_uncore_umc_format_attr[] = {
+	&format_attr_event8.attr,       /* event */
+	&format_attr_rdwrmask.attr,     /* rdwrmask */
+	NULL,
+};
+
 static struct attribute_group amd_uncore_df_format_group = {
 	.name = "format",
 	.attrs = amd_uncore_df_format_attr,
@@ -327,6 +340,11 @@ static struct attribute_group amd_f19h_uncore_l3_format_group = {
 	.is_visible = amd_f19h_uncore_is_visible,
 };
 
+static struct attribute_group amd_uncore_umc_format_group = {
+	.name = "format",
+	.attrs = amd_uncore_umc_format_attr,
+};
+
 static const struct attribute_group *amd_uncore_df_attr_groups[] = {
 	&amd_uncore_attr_group,
 	&amd_uncore_df_format_group,
@@ -345,6 +363,12 @@ static const struct attribute_group *amd_uncore_l3_attr_update[] = {
 	NULL,
 };
 
+static const struct attribute_group *amd_uncore_umc_attr_groups[] = {
+	&amd_uncore_attr_group,
+	&amd_uncore_umc_format_group,
+	NULL,
+};
+
 static int amd_uncore_cpu_up_prepare(unsigned int cpu)
 {
 	struct amd_uncore *uncore;
@@ -757,6 +781,145 @@ static int amd_uncore_l3_init(void)
 	return 0;
 }
 
+static int amd_uncore_umc_id(unsigned int cpu)
+{
+	return topology_die_id(cpu);
+}
+
+static int amd_uncore_umc_event_init(struct perf_event *event)
+{
+	struct hw_perf_event *hwc = &event->hw;
+	int ret = amd_uncore_event_init(event);
+
+	if (ret)
+		return ret;
+
+	hwc->config = event->attr.config & AMD64_PERFMON_V2_RAW_EVENT_MASK_UMC;
+
+	return 0;
+}
+
+static void amd_uncore_umc_start(struct perf_event *event, int flags)
+{
+	struct hw_perf_event *hwc = &event->hw;
+
+	if (flags & PERF_EF_RELOAD)
+		wrmsrl(hwc->event_base, (u64)local64_read(&hwc->prev_count));
+
+	hwc->state = 0;
+	wrmsrl(hwc->config_base, (hwc->config | AMD64_PERFMON_V2_ENABLE_UMC));
+	perf_event_update_userpage(event);
+}
+
+static int amd_uncore_umc_init(void)
+{
+	unsigned int cpu, eax, ecx, edx;
+	union cpuid_0x80000022_ebx ebx;
+	struct amd_uncore *uncore;
+	int umc_idx = 0, group_id, group_num_umc, group_umc_idx, ret, i;
+	DECLARE_BITMAP(group_mask, NUM_UNCORES_MAX) = { 0 };
+
+	if (pmu_version < 2)
+		return 0;
+
+	/*
+	 * Each group of memory controllers can have an unique configuration
+	 * based on the DIMM population scheme. If all CPUs associated with a
+	 * group of memory channels are offline, the corresponding UMC PMUs
+	 * will not be initialized since they are only discoverable via CPUID.
+	 */
+	for_each_online_cpu(cpu) {
+		group_id = amd_uncore_umc_id(cpu);
+
+		/* Check if this group has already been discovered */
+		if (test_bit(group_id, group_mask))
+			continue;
+
+		__set_bit(group_id, group_mask);
+		ret = cpuid_on_cpu(cpu, EXT_PERFMON_DEBUG_FEATURES, &eax,
+				   &ebx.full, &ecx, &edx);
+		if (ret)
+			goto fail;
+
+		group_umc_idx = 0;
+		group_num_umc = hweight32(ecx);
+
+		/*
+		 * There are more PMUs than anticipated and the max array size
+		 * needs to be increased to accommodate them
+		 */
+		if ((num_uncores + umc_idx + group_num_umc) > NUM_UNCORES_MAX) {
+			WARN(1, "some uncore PMUs cannot be initialized");
+			break;
+		}
+
+		/* Create PMUs for active UMCs in the current group */
+		for (i = 0; i < 32; i++) {
+			if (!(ecx & BIT(i)))
+				continue;
+
+			uncore = &uncores[num_uncores + umc_idx];
+			snprintf(uncore->name, sizeof(uncore->name), "amd_umc_%d", umc_idx);
+			uncore->num_counters = ebx.split.num_umc_pmc / group_num_umc;
+			uncore->msr_base = MSR_F19H_UMC_PERF_CTL + group_umc_idx * uncore->num_counters * 2;
+			uncore->rdpmc_base = -1;
+			uncore->id = amd_uncore_umc_id;
+			uncore->group = group_id;
+
+			uncore->ctx = alloc_percpu(struct amd_uncore_ctx *);
+			if (!uncore->ctx) {
+				ret = -ENOMEM;
+				goto fail;
+			}
+
+			uncore->pmu = (struct pmu) {
+				.task_ctx_nr	= perf_invalid_context,
+				.attr_groups	= amd_uncore_umc_attr_groups,
+				.name		= uncore->name,
+				.event_init	= amd_uncore_umc_event_init,
+				.add		= amd_uncore_add,
+				.del		= amd_uncore_del,
+				.start		= amd_uncore_umc_start,
+				.stop		= amd_uncore_stop,
+				.read		= amd_uncore_read,
+				.capabilities	= PERF_PMU_CAP_NO_EXCLUDE | PERF_PMU_CAP_NO_INTERRUPT,
+				.module		= THIS_MODULE,
+			};
+
+			ret = perf_pmu_register(&uncore->pmu, uncore->pmu.name, -1);
+			if (ret) {
+				free_percpu(uncore->ctx);
+				uncore->ctx = NULL;
+				goto fail;
+			}
+
+			pr_info("%d %s %s counters detected\n", uncore->num_counters,
+				boot_cpu_data.x86_vendor == X86_VENDOR_HYGON ?  "HYGON" : "",
+				uncore->pmu.name);
+
+			umc_idx++;
+			group_umc_idx++;
+		}
+	}
+
+	num_uncores += umc_idx;
+
+	return 0;
+
+fail:
+	for (i = 0; i < umc_idx; i++) {
+		uncore = &uncores[num_uncores + i];
+		if (!uncore->ctx)
+			continue;
+
+		perf_pmu_unregister(&uncore->pmu);
+		free_percpu(uncore->ctx);
+		uncore->ctx = NULL;
+	}
+
+	return ret;
+}
+
 static void uncore_free(void)
 {
 	struct amd_uncore *uncore;
@@ -797,6 +960,10 @@ static int __init amd_uncore_init(void)
 	if (ret)
 		goto fail;
 
+	ret = amd_uncore_umc_init();
+	if (ret)
+		goto fail;
+
 	/*
 	 * Install callbacks. Core will call them for each online cpu.
 	 */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 3aedae61af4f..bfcc72b20f54 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -624,6 +624,10 @@
 /* AMD Last Branch Record MSRs */
 #define MSR_AMD64_LBR_SELECT			0xc000010e
 
+/* Fam 19h MSRs */
+#define MSR_F19H_UMC_PERF_CTL		0xc0010800
+#define MSR_F19H_UMC_PERF_CTR		0xc0010801
+
 /* Fam 17h MSRs */
 #define MSR_F17H_IRPERF			0xc00000e9
 
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 85a9fd5a3ec3..2618ec7c3d1d 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -112,6 +112,13 @@
 	(AMD64_PERFMON_V2_EVENTSEL_EVENT_NB	|	\
 	 AMD64_PERFMON_V2_EVENTSEL_UMASK_NB)
 
+#define AMD64_PERFMON_V2_ENABLE_UMC			BIT_ULL(31)
+#define AMD64_PERFMON_V2_EVENTSEL_EVENT_UMC		GENMASK_ULL(7, 0)
+#define AMD64_PERFMON_V2_EVENTSEL_RDWRMASK_UMC		GENMASK_ULL(9, 8)
+#define AMD64_PERFMON_V2_RAW_EVENT_MASK_UMC		\
+	(AMD64_PERFMON_V2_EVENTSEL_EVENT_UMC	|	\
+	 AMD64_PERFMON_V2_EVENTSEL_RDWRMASK_UMC)
+
 #define AMD64_NUM_COUNTERS				4
 #define AMD64_NUM_COUNTERS_CORE				6
 #define AMD64_NUM_COUNTERS_NB				4
@@ -232,6 +239,8 @@ union cpuid_0x80000022_ebx {
 		unsigned int	lbr_v2_stack_sz:6;
 		/* Number of Data Fabric Counters */
 		unsigned int	num_df_pmc:6;
+		/* Number of Unified Memory Controller Counters */
+		unsigned int	num_umc_pmc:6;
 	} split;
 	unsigned int		full;
 };
-- 
2.34.1

[PATCH 6/6] perf vendor events amd: Add Zen 4 memory controller events

[Sandipan Das]

Make the jevents parser aware of the Unified Memory Controller (UMC) PMU
and add events taken from Section 8.2.1 "UMC Performance Monitor Events"
of the Processor Programming Reference (PPR) for AMD Family 19h Model 11h
processors. The events capture UMC command activity such as CAS, ACTIVATE,
PRECHARGE etc. while the metrics derive data bus utilization and memory
bandwidth out of these events.

Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 .../arch/x86/amdzen4/memory-controller.json   | 101 ++++++++++++++++++
 .../arch/x86/amdzen4/recommended.json         |  84 +++++++++++++++
 tools/perf/pmu-events/jevents.py              |   2 +
 3 files changed, 187 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json

diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
new file mode 100644
index 000000000000..55263e5e4f69
--- /dev/null
+++ b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
@@ -0,0 +1,101 @@
+[
+  {
+    "EventName": "umc_mem_clk",
+    "PublicDescription": "Number of memory clock cycles.",
+    "EventCode": "0x00",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_act_cmd.all",
+    "PublicDescription": "Number of ACTIVATE commands sent.",
+    "EventCode": "0x05",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_act_cmd.rd",
+    "PublicDescription": "Number of ACTIVATE commands sent for reads.",
+    "EventCode": "0x05",
+    "RdWrMask": "0x1",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_act_cmd.wr",
+    "PublicDescription": "Number of ACTIVATE commands sent for writes.",
+    "EventCode": "0x05",
+    "RdWrMask": "0x2",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_pchg_cmd.all",
+    "PublicDescription": "Number of PRECHARGE commands sent.",
+    "EventCode": "0x06",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_pchg_cmd.rd",
+    "PublicDescription": "Number of PRECHARGE commands sent for reads.",
+    "EventCode": "0x06",
+    "RdWrMask": "0x1",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_pchg_cmd.wr",
+    "PublicDescription": "Number of PRECHARGE commands sent for writes.",
+    "EventCode": "0x06",
+    "RdWrMask": "0x2",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_cas_cmd.all",
+    "PublicDescription": "Number of CAS commands sent.",
+    "EventCode": "0x0a",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_cas_cmd.rd",
+    "PublicDescription": "Number of CAS commands sent for reads.",
+    "EventCode": "0x0a",
+    "RdWrMask": "0x1",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_cas_cmd.wr",
+    "PublicDescription": "Number of CAS commands sent for writes.",
+    "EventCode": "0x0a",
+    "RdWrMask": "0x2",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_data_slot_clks.all",
+    "PublicDescription": "Number of clocks used by the data bus.",
+    "EventCode": "0x14",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_data_slot_clks.rd",
+    "PublicDescription": "Number of clocks used by the data bus for reads.",
+    "EventCode": "0x14",
+    "RdWrMask": "0x1",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  },
+  {
+    "EventName": "umc_data_slot_clks.wr",
+    "PublicDescription": "Number of clocks used by the data bus for writes.",
+    "EventCode": "0x14",
+    "RdWrMask": "0x2",
+    "PerPkg": "1",
+    "Unit": "UMCPMC"
+  }
+]
diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json b/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json
index 5e6a793acf7b..96e06401c6cb 100644
--- a/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json
+++ b/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json
@@ -330,5 +330,89 @@
     "MetricGroup": "data_fabric",
     "PerPkg": "1",
     "ScaleUnit": "6.103515625e-5MiB"
+  },
+  {
+    "MetricName": "umc_data_bus_utilization",
+    "BriefDescription": "Memory controller data bus utilization.",
+    "MetricExpr": "d_ratio(umc_data_slot_clks.all / 2, umc_mem_clk)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "100%"
+  },
+  {
+    "MetricName": "umc_cas_cmd_rate",
+    "BriefDescription": "Memory controller CAS command rate.",
+    "MetricExpr": "d_ratio(umc_cas_cmd.all * 1000, umc_mem_clk)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1"
+  },
+  {
+    "MetricName": "umc_cas_cmd_read_ratio",
+    "BriefDescription": "Ratio of memory controller CAS commands for reads.",
+    "MetricExpr": "d_ratio(umc_cas_cmd.rd, umc_cas_cmd.all)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "100%"
+  },
+  {
+    "MetricName": "umc_cas_cmd_write_ratio",
+    "BriefDescription": "Ratio of memory controller CAS commands for writes.",
+    "MetricExpr": "d_ratio(umc_cas_cmd.wr, umc_cas_cmd.all)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "100%"
+  },
+  {
+    "MetricName": "umc_mem_read_bandwidth",
+    "BriefDescription": "Estimated memory read bandwidth.",
+    "MetricExpr": "(umc_cas_cmd.rd * 64) / 1e6 / duration_time",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "1MB/s"
+  },
+  {
+    "MetricName": "umc_mem_write_bandwidth",
+    "BriefDescription": "Estimated memory write bandwidth.",
+    "MetricExpr": "(umc_cas_cmd.wr * 64) / 1e6 / duration_time",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "1MB/s"
+  },
+  {
+    "MetricName": "umc_mem_bandwidth",
+    "BriefDescription": "Estimated combined memory bandwidth.",
+    "MetricExpr": "(umc_cas_cmd.all * 64) / 1e6 / duration_time",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "1MB/s"
+  },
+  {
+    "MetricName": "umc_cas_cmd_read_ratio",
+    "BriefDescription": "Ratio of memory controller CAS commands for reads.",
+    "MetricExpr": "d_ratio(umc_cas_cmd.rd, umc_cas_cmd.all)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1",
+    "ScaleUnit": "100%"
+  },
+  {
+    "MetricName": "umc_cas_cmd_rate",
+    "BriefDescription": "Memory controller CAS command rate.",
+    "MetricExpr": "d_ratio(umc_cas_cmd.all * 1000, umc_mem_clk)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1"
+  },
+  {
+    "MetricName": "umc_activate_cmd_rate",
+    "BriefDescription": "Memory controller ACTIVATE command rate.",
+    "MetricExpr": "d_ratio(umc_act_cmd.all * 1000, umc_mem_clk)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1"
+  },
+  {
+    "MetricName": "umc_precharge_cmd_rate",
+    "BriefDescription": "Memory controller PRECHARGE command rate.",
+    "MetricExpr": "d_ratio(umc_pchg_cmd.all * 1000, umc_mem_clk)",
+    "MetricGroup": "memory_controller",
+    "PerPkg": "1"
   }
 ]
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 12e80bb7939b..c2a5728253db 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -272,6 +272,7 @@ class JsonEvent:
           'imx8_ddr': 'imx8_ddr',
           'L3PMC': 'amd_l3',
           'DFPMC': 'amd_df',
+          'UMCPMC': 'amd_umc',
           'cpu_core': 'cpu_core',
           'cpu_atom': 'cpu_atom',
       }
@@ -330,6 +331,7 @@ class JsonEvent:
         ('Invert', 'inv='),
         ('SampleAfterValue', 'period='),
         ('UMask', 'umask='),
+        ('RdWrMask', 'rdwrmask='),
     ]
     for key, value in event_fields:
       if key in jd and jd[key] != '0':
-- 
2.34.1

Re: [PATCH 6/6] perf vendor events amd: Add Zen 4 memory controller events

[Ian Rogers]

On Tue, Jul 18, 2023 at 11:58 PM Sandipan Das <sandipan.das@amd.com> wrote:
>
> Make the jevents parser aware of the Unified Memory Controller (UMC) PMU
> and add events taken from Section 8.2.1 "UMC Performance Monitor Events"
> of the Processor Programming Reference (PPR) for AMD Family 19h Model 11h
> processors. The events capture UMC command activity such as CAS, ACTIVATE,
> PRECHARGE etc. while the metrics derive data bus utilization and memory
> bandwidth out of these events.
>
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>

Acked-by: Ian Rogers <irogers@google.com>

> ---
>  .../arch/x86/amdzen4/memory-controller.json   | 101 ++++++++++++++++++
>  .../arch/x86/amdzen4/recommended.json         |  84 +++++++++++++++
>  tools/perf/pmu-events/jevents.py              |   2 +
>  3 files changed, 187 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>
> diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> new file mode 100644
> index 000000000000..55263e5e4f69
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> @@ -0,0 +1,101 @@
> +[
> +  {
> +    "EventName": "umc_mem_clk",
> +    "PublicDescription": "Number of memory clock cycles.",
> +    "EventCode": "0x00",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"

nit: Why use UMCPMC and then rewrite to amd_umc, why not just use "amd_umc" ?

Thanks,
Ian


> +  },
> +  {
> +    "EventName": "umc_act_cmd.all",
> +    "PublicDescription": "Number of ACTIVATE commands sent.",
> +    "EventCode": "0x05",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_act_cmd.rd",
> +    "PublicDescription": "Number of ACTIVATE commands sent for reads.",
> +    "EventCode": "0x05",
> +    "RdWrMask": "0x1",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_act_cmd.wr",
> +    "PublicDescription": "Number of ACTIVATE commands sent for writes.",
> +    "EventCode": "0x05",
> +    "RdWrMask": "0x2",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_pchg_cmd.all",
> +    "PublicDescription": "Number of PRECHARGE commands sent.",
> +    "EventCode": "0x06",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_pchg_cmd.rd",
> +    "PublicDescription": "Number of PRECHARGE commands sent for reads.",
> +    "EventCode": "0x06",
> +    "RdWrMask": "0x1",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_pchg_cmd.wr",
> +    "PublicDescription": "Number of PRECHARGE commands sent for writes.",
> +    "EventCode": "0x06",
> +    "RdWrMask": "0x2",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_cas_cmd.all",
> +    "PublicDescription": "Number of CAS commands sent.",
> +    "EventCode": "0x0a",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_cas_cmd.rd",
> +    "PublicDescription": "Number of CAS commands sent for reads.",
> +    "EventCode": "0x0a",
> +    "RdWrMask": "0x1",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_cas_cmd.wr",
> +    "PublicDescription": "Number of CAS commands sent for writes.",
> +    "EventCode": "0x0a",
> +    "RdWrMask": "0x2",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_data_slot_clks.all",
> +    "PublicDescription": "Number of clocks used by the data bus.",
> +    "EventCode": "0x14",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_data_slot_clks.rd",
> +    "PublicDescription": "Number of clocks used by the data bus for reads.",
> +    "EventCode": "0x14",
> +    "RdWrMask": "0x1",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  },
> +  {
> +    "EventName": "umc_data_slot_clks.wr",
> +    "PublicDescription": "Number of clocks used by the data bus for writes.",
> +    "EventCode": "0x14",
> +    "RdWrMask": "0x2",
> +    "PerPkg": "1",
> +    "Unit": "UMCPMC"
> +  }
> +]
> diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json b/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json
> index 5e6a793acf7b..96e06401c6cb 100644
> --- a/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json
> +++ b/tools/perf/pmu-events/arch/x86/amdzen4/recommended.json
> @@ -330,5 +330,89 @@
>      "MetricGroup": "data_fabric",
>      "PerPkg": "1",
>      "ScaleUnit": "6.103515625e-5MiB"
> +  },
> +  {
> +    "MetricName": "umc_data_bus_utilization",
> +    "BriefDescription": "Memory controller data bus utilization.",
> +    "MetricExpr": "d_ratio(umc_data_slot_clks.all / 2, umc_mem_clk)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "100%"
> +  },
> +  {
> +    "MetricName": "umc_cas_cmd_rate",
> +    "BriefDescription": "Memory controller CAS command rate.",
> +    "MetricExpr": "d_ratio(umc_cas_cmd.all * 1000, umc_mem_clk)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1"
> +  },
> +  {
> +    "MetricName": "umc_cas_cmd_read_ratio",
> +    "BriefDescription": "Ratio of memory controller CAS commands for reads.",
> +    "MetricExpr": "d_ratio(umc_cas_cmd.rd, umc_cas_cmd.all)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "100%"
> +  },
> +  {
> +    "MetricName": "umc_cas_cmd_write_ratio",
> +    "BriefDescription": "Ratio of memory controller CAS commands for writes.",
> +    "MetricExpr": "d_ratio(umc_cas_cmd.wr, umc_cas_cmd.all)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "100%"
> +  },
> +  {
> +    "MetricName": "umc_mem_read_bandwidth",
> +    "BriefDescription": "Estimated memory read bandwidth.",
> +    "MetricExpr": "(umc_cas_cmd.rd * 64) / 1e6 / duration_time",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "1MB/s"
> +  },
> +  {
> +    "MetricName": "umc_mem_write_bandwidth",
> +    "BriefDescription": "Estimated memory write bandwidth.",
> +    "MetricExpr": "(umc_cas_cmd.wr * 64) / 1e6 / duration_time",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "1MB/s"
> +  },
> +  {
> +    "MetricName": "umc_mem_bandwidth",
> +    "BriefDescription": "Estimated combined memory bandwidth.",
> +    "MetricExpr": "(umc_cas_cmd.all * 64) / 1e6 / duration_time",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "1MB/s"
> +  },
> +  {
> +    "MetricName": "umc_cas_cmd_read_ratio",
> +    "BriefDescription": "Ratio of memory controller CAS commands for reads.",
> +    "MetricExpr": "d_ratio(umc_cas_cmd.rd, umc_cas_cmd.all)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1",
> +    "ScaleUnit": "100%"
> +  },
> +  {
> +    "MetricName": "umc_cas_cmd_rate",
> +    "BriefDescription": "Memory controller CAS command rate.",
> +    "MetricExpr": "d_ratio(umc_cas_cmd.all * 1000, umc_mem_clk)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1"
> +  },
> +  {
> +    "MetricName": "umc_activate_cmd_rate",
> +    "BriefDescription": "Memory controller ACTIVATE command rate.",
> +    "MetricExpr": "d_ratio(umc_act_cmd.all * 1000, umc_mem_clk)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1"
> +  },
> +  {
> +    "MetricName": "umc_precharge_cmd_rate",
> +    "BriefDescription": "Memory controller PRECHARGE command rate.",
> +    "MetricExpr": "d_ratio(umc_pchg_cmd.all * 1000, umc_mem_clk)",
> +    "MetricGroup": "memory_controller",
> +    "PerPkg": "1"
>    }
>  ]
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 12e80bb7939b..c2a5728253db 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -272,6 +272,7 @@ class JsonEvent:
>            'imx8_ddr': 'imx8_ddr',
>            'L3PMC': 'amd_l3',
>            'DFPMC': 'amd_df',
> +          'UMCPMC': 'amd_umc',
>            'cpu_core': 'cpu_core',
>            'cpu_atom': 'cpu_atom',
>        }
> @@ -330,6 +331,7 @@ class JsonEvent:
>          ('Invert', 'inv='),
>          ('SampleAfterValue', 'period='),
>          ('UMask', 'umask='),
> +        ('RdWrMask', 'rdwrmask='),
>      ]
>      for key, value in event_fields:
>        if key in jd and jd[key] != '0':
> --
> 2.34.1
>

Re: [PATCH 6/6] perf vendor events amd: Add Zen 4 memory controller events

[Sandipan Das]

On 7/19/2023 9:42 PM, Ian Rogers wrote:
> On Tue, Jul 18, 2023 at 11:58 PM Sandipan Das <sandipan.das@amd.com> wrote:
>>
>> Make the jevents parser aware of the Unified Memory Controller (UMC) PMU
>> and add events taken from Section 8.2.1 "UMC Performance Monitor Events"
>> of the Processor Programming Reference (PPR) for AMD Family 19h Model 11h
>> processors. The events capture UMC command activity such as CAS, ACTIVATE,
>> PRECHARGE etc. while the metrics derive data bus utilization and memory
>> bandwidth out of these events.
>>
>> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> 
> Acked-by: Ian Rogers <irogers@google.com>
> 
>> ---
>>  .../arch/x86/amdzen4/memory-controller.json   | 101 ++++++++++++++++++
>>  .../arch/x86/amdzen4/recommended.json         |  84 +++++++++++++++
>>  tools/perf/pmu-events/jevents.py              |   2 +
>>  3 files changed, 187 insertions(+)
>>  create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>>
>> diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>> new file mode 100644
>> index 000000000000..55263e5e4f69
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>> @@ -0,0 +1,101 @@
>> +[
>> +  {
>> +    "EventName": "umc_mem_clk",
>> +    "PublicDescription": "Number of memory clock cycles.",
>> +    "EventCode": "0x00",
>> +    "PerPkg": "1",
>> +    "Unit": "UMCPMC"
> 
> nit: Why use UMCPMC and then rewrite to amd_umc, why not just use "amd_umc" ?
> 

I followed the convention that has been historically used for AMD uncore PMUs e.g.
the "Unit" for amd_df is "DFPMC" and for amd_l3 is "L3PMC". I do agree that its
simpler to use the same naming so will change this. If you prefer, I can send out
a separate patch to change the unit naming for amd_df and amd_l3.

- Sandipan

Re: [PATCH 6/6] perf vendor events amd: Add Zen 4 memory controller events

[Ian Rogers]

On Wed, Jul 19, 2023 at 10:23 PM Sandipan Das <sandipan.das@amd.com> wrote:
>
> On 7/19/2023 9:42 PM, Ian Rogers wrote:
> > On Tue, Jul 18, 2023 at 11:58 PM Sandipan Das <sandipan.das@amd.com> wrote:
> >>
> >> Make the jevents parser aware of the Unified Memory Controller (UMC) PMU
> >> and add events taken from Section 8.2.1 "UMC Performance Monitor Events"
> >> of the Processor Programming Reference (PPR) for AMD Family 19h Model 11h
> >> processors. The events capture UMC command activity such as CAS, ACTIVATE,
> >> PRECHARGE etc. while the metrics derive data bus utilization and memory
> >> bandwidth out of these events.
> >>
> >> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> >
> > Acked-by: Ian Rogers <irogers@google.com>
> >
> >> ---
> >>  .../arch/x86/amdzen4/memory-controller.json   | 101 ++++++++++++++++++
> >>  .../arch/x86/amdzen4/recommended.json         |  84 +++++++++++++++
> >>  tools/perf/pmu-events/jevents.py              |   2 +
> >>  3 files changed, 187 insertions(+)
> >>  create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> >>
> >> diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> >> new file mode 100644
> >> index 000000000000..55263e5e4f69
> >> --- /dev/null
> >> +++ b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
> >> @@ -0,0 +1,101 @@
> >> +[
> >> +  {
> >> +    "EventName": "umc_mem_clk",
> >> +    "PublicDescription": "Number of memory clock cycles.",
> >> +    "EventCode": "0x00",
> >> +    "PerPkg": "1",
> >> +    "Unit": "UMCPMC"
> >
> > nit: Why use UMCPMC and then rewrite to amd_umc, why not just use "amd_umc" ?
> >
>
> I followed the convention that has been historically used for AMD uncore PMUs e.g.
> the "Unit" for amd_df is "DFPMC" and for amd_l3 is "L3PMC". I do agree that its
> simpler to use the same naming so will change this. If you prefer, I can send out
> a separate patch to change the unit naming for amd_df and amd_l3.

Thanks for the explanation. I don't mind but it is nicer to have fewer
renames imo. If we get rid of one, perhaps we can get rid of them all?
Perhaps merge this and follow up with simplification.

Thanks,
Ian

> - Sandipan
>

Re: [PATCH 6/6] perf vendor events amd: Add Zen 4 memory controller events

[Sandipan Das]

On 7/20/2023 9:20 PM, Ian Rogers wrote:
> On Wed, Jul 19, 2023 at 10:23 PM Sandipan Das <sandipan.das@amd.com> wrote:
>>
>> On 7/19/2023 9:42 PM, Ian Rogers wrote:
>>> On Tue, Jul 18, 2023 at 11:58 PM Sandipan Das <sandipan.das@amd.com> wrote:
>>>>
>>>> Make the jevents parser aware of the Unified Memory Controller (UMC) PMU
>>>> and add events taken from Section 8.2.1 "UMC Performance Monitor Events"
>>>> of the Processor Programming Reference (PPR) for AMD Family 19h Model 11h
>>>> processors. The events capture UMC command activity such as CAS, ACTIVATE,
>>>> PRECHARGE etc. while the metrics derive data bus utilization and memory
>>>> bandwidth out of these events.
>>>>
>>>> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
>>>
>>> Acked-by: Ian Rogers <irogers@google.com>
>>>
>>>> ---
>>>>  .../arch/x86/amdzen4/memory-controller.json   | 101 ++++++++++++++++++
>>>>  .../arch/x86/amdzen4/recommended.json         |  84 +++++++++++++++
>>>>  tools/perf/pmu-events/jevents.py              |   2 +
>>>>  3 files changed, 187 insertions(+)
>>>>  create mode 100644 tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>>>>
>>>> diff --git a/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>>>> new file mode 100644
>>>> index 000000000000..55263e5e4f69
>>>> --- /dev/null
>>>> +++ b/tools/perf/pmu-events/arch/x86/amdzen4/memory-controller.json
>>>> @@ -0,0 +1,101 @@
>>>> +[
>>>> +  {
>>>> +    "EventName": "umc_mem_clk",
>>>> +    "PublicDescription": "Number of memory clock cycles.",
>>>> +    "EventCode": "0x00",
>>>> +    "PerPkg": "1",
>>>> +    "Unit": "UMCPMC"
>>>
>>> nit: Why use UMCPMC and then rewrite to amd_umc, why not just use "amd_umc" ?
>>>
>>
>> I followed the convention that has been historically used for AMD uncore PMUs e.g.
>> the "Unit" for amd_df is "DFPMC" and for amd_l3 is "L3PMC". I do agree that its
>> simpler to use the same naming so will change this. If you prefer, I can send out
>> a separate patch to change the unit naming for amd_df and amd_l3.
> 
> Thanks for the explanation. I don't mind but it is nicer to have fewer
> renames imo. If we get rid of one, perhaps we can get rid of them all?
> Perhaps merge this and follow up with simplification.
> 

Sure, sounds good.

- Sandipan