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

Groups > linux.kernel > #1340667 > unrolled thread

[PATCH V5 00/10] perf/amd/iommu: Enable multi-IOMMU support

Back to article view | Back to linux.kernel

Contents

  [PATCH V5 00/10] perf/amd/iommu: Enable multi-IOMMU support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:20 +0100
    [PATCH V5 01/10] perf/amd/iommu: Misc fix up perf_iommu_read Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:20 +0100
    [PATCH V5 08/10] perf/amd/iommu: Rename struct perf_amd_iommu to perf_iommu Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:20 +0100
    [PATCH V5 07/10] perf/amd/iommu: Clean up get_next_available_iommu_bnk_cntr Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:20 +0100
    [PATCH V5 05/10] perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:30 +0100
    [PATCH V5 03/10] perf/amd/iommu: Modify functions to query max banks and counters Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:30 +0100
    [PATCH V5 04/10] perf/amd/iommu: Modify IOMMU API to allow specifying IOMMU index Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:30 +0100
    [PATCH V5 06/10] perf/amd/iommu: Clean up perf_iommu_enable_event Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-02-23 15:30 +0100
    Re: [PATCH V5 00/10] perf/amd/iommu: Enable multi-IOMMU support Joerg Roedel <joro@8bytes.org> - 2016-02-25 16:00 +0100

#1340667 — [PATCH V5 00/10] perf/amd/iommu: Enable multi-IOMMU support

This is a two-part patch series:

Part1: 1-4 :
Introduce a workaround for the current AMD IOMMU perf initialization issue
in some existing KV and CZ platforms, where it fails to write to IOMMU
perf counter as reported by Andreas Hartmann here
(http://comments.gmane.org/gmane.linux.kernel.pci/49147).

Part2: 5-10: Enable multi-IOMMU support 
Cleans up the AMD IOMMU perf, and modifies the existing driver
to support systems with multiple IOMMUs by exposing IOMMU an amd_iommu
PMU instance per IOMMU. This allows users to specify performance events
separately for each IOMMU.

Git branch containing this patch series is available here:

    https://github.com/ssuthiku/linux.git  perf-iommu-v5

Changes from V4 (https://lkml.org/lkml/2016/2/11/79)
  * Re-order the patch series to split patches into two parts,
    and I have tried to clean up as much as I could before adding
    new stuff.
  * Based on Boris's feedback:
    * Clean up several coding style and spelling errors.
  * Based on Peter's feedback:
    * Implement per-IOMMU PMU

Changes from V3 (https://lkml.org/lkml/2016/2/9/845)
  * Rebase the code to tip/master per Boris suggestion
  * Most changes are in patch 5/6:
    * Fix several spelling and styling issues per Boris review comment
    * Remove unnecessary pr_debug in the perf amd iommu driver (per Boris)
    * Rename several function to make it less confusing (per Boris)
    * Properly handle errors when fails to set registers/counters
      on multiple IOMMUs. (per Boris)

Changes from V2 ( https://lkml.org/lkml/2016/1/1/141)
  * Ported to 4.5.0-rc2
  * Add reviewed by Joerg for patch 1 and 2
  * Remove EXPORT_SYMBOL from patch 3 (per Joerg suggestion)
  * Merge patch 4/6 and 6/6 from V2 into 5/5 in V3 and add
    more description in the commit message and in code comment.
  * Patch 5: modify the logic to update counts to get rid off
    un-necessary local64_cmpxchg().

Changes from V1 (https://lkml.org/lkml/2015/12/22/535):
  * Update patch3 and 6 to use amd_iommus_present instead of introducing
    amd_iommu_cnt static v
Suravee Suthikulpanit (10):
  perf/amd/iommu: Misc fix up perf_iommu_read
  perf/amd/iommu: Consolidate and move perf_event_amd_iommu header
  perf/amd/iommu: Modify functions to query max banks and counters
  perf/amd/iommu: Modify IOMMU API to allow specifying IOMMU index
  perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug
  perf/amd/iommu: Clean up perf_iommu_enable_event
  perf/amd/iommu: Clean up get_next_available_iommu_bnk_cntr
  perf/amd/iommu: Rename struct perf_amd_iommu to perf_iommu
  iommu/amd: Introduce amd_iommu_get_num_iommus()
  perf/amd/iommu: Enable support for multiple IOMMUs

 arch/x86/events/amd/iommu.c           | 274 +++++++++++++++++-----------------
 arch/x86/events/amd/iommu.h           |  40 -----
 arch/x86/include/asm/perf/amd/iommu.h |  43 ++++++
 drivers/iommu/amd_iommu_init.c        | 111 +++++++++++---
 drivers/iommu/amd_iommu_proto.h       |   7 -
 5 files changed, 263 insertions(+), 212 deletions(-)
 delete mode 100644 arch/x86/events/amd/iommu.h
 create mode 100644 arch/x86/include/asm/perf/amd/iommu.h

-- 
1.9.1

[toc] | [next] | [standalone]

#1340671 — [PATCH V5 01/10] perf/amd/iommu: Misc fix up perf_iommu_read

This patch contains the follow minor fixup:
  * Fixed overflow handling since u64 delta would lose the MSB sign bit.
  * Remove unnecessary local64_set().
  * Coding style and make use of GENMASK_ULL macro.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 629bc70..9da0d16 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -314,9 +314,8 @@ static void perf_iommu_start(struct perf_event *event, int flags)
 
 static void perf_iommu_read(struct perf_event *event)
 {
-	u64 count = 0ULL;
-	u64 prev_raw_count = 0ULL;
-	u64 delta = 0ULL;
+	u64 cnt, prev;
+	s64 delta;
 	struct hw_perf_event *hwc = &event->hw;
 	pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
@@ -325,18 +324,20 @@ static void perf_iommu_read(struct perf_event *event)
 				IOMMU_PC_COUNTER_REG, &count, false);
 
 	/* IOMMU pc counter register is only 48 bits */
-	count &= 0xFFFFFFFFFFFFULL;
+	cnt &= GENMASK_ULL(48, 0);
 
-	prev_raw_count =  local64_read(&hwc->prev_count);
-	if (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
-					count) != prev_raw_count)
-		return;
+	prev = local64_read(&hwc->prev_count);
 
-	/* Handling 48-bit counter overflowing */
-	delta = (count << COUNTER_SHIFT) - (prev_raw_count << COUNTER_SHIFT);
+	/*
+	 * Since we do not enable counter overflow interrupts,
+	 * we do not have to worry about prev_count changing on us.
+	 */
+	local64_set(&hwc->prev_count, cnt);
+
+	/* Handle 48-bit counter overflow */
+	delta = (cnt << COUNTER_SHIFT) - (prev << COUNTER_SHIFT);
 	delta >>= COUNTER_SHIFT;
 	local64_add(delta, &event->count);
-
 }
 
 static void perf_iommu_stop(struct perf_event *event, int flags)
-- 
1.9.1

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

#1340674 — [PATCH V5 08/10] perf/amd/iommu: Rename struct perf_amd_iommu to perf_iommu

This patch shortens the struct name perf_amd_iommu to perf_iommu since
the old name is too long. The new name should be sufficient since
the structure is only used within this file. We also clean up variable
name for this structure in various functions.

Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c | 82 +++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 44 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index d5e4d39..35a7b18 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -35,9 +35,9 @@
 #define _GET_PASID_MASK(ev) ((ev->hw.extra_reg.config >> 16) & 0xFFFFULL)
 #define _GET_DOMID_MASK(ev) ((ev->hw.extra_reg.config >> 32) & 0xFFFFULL)
 
-static struct perf_amd_iommu __perf_iommu;
+static struct perf_iommu __perf_iommu;
 
-struct perf_amd_iommu {
+struct perf_iommu {
 	struct pmu pmu;
 	u8 max_banks;
 	u8 max_counters;
@@ -147,49 +147,48 @@ static struct attribute_group amd_iommu_cpumask_group = {
 
 /*---------------------------------------------*/
 
-static int get_next_avail_iommu_bnk_cntr(struct perf_amd_iommu *perf_iommu)
+static int get_next_avail_iommu_bnk_cntr(struct perf_iommu *pi)
 {
 	unsigned long flags;
 	int bank, cntr, retval = -ENOSPC;
 
-	raw_spin_lock_irqsave(&perf_iommu->lock, flags);
+	raw_spin_lock_irqsave(&pi->lock, flags);
 
-	for (bank = 0; bank < perf_iommu->max_banks; bank++) {
-		for (cntr = 0; cntr < perf_iommu->max_counters; cntr++) {
-			int shift = (perf_iommu->max_counters * bank) + cntr;
+	for (bank = 0; bank < pi->max_banks; bank++) {
+		for (cntr = 0; cntr < pi->max_counters; cntr++) {
+			int shift = (pi->max_counters * bank) + cntr;
 
-			if (perf_iommu->cntr_assign_mask & (1ULL << shift)) {
+			if (pi->cntr_assign_mask & (1ULL << shift)) {
 				continue;
 			} else {
-				perf_iommu->cntr_assign_mask |= (1ULL << shift);
+				pi->cntr_assign_mask |= (1ULL << shift);
 				retval = ((u16)((u16)bank << 8) | (u8)(cntr));
 				goto out;
 			}
 		}
 	}
 out:
-	raw_spin_unlock_irqrestore(&perf_iommu->lock, flags);
+	raw_spin_unlock_irqrestore(&pi->lock, flags);
 	return retval;
 }
 
-static int clear_avail_iommu_bnk_cntr(struct perf_amd_iommu *perf_iommu,
-					u8 bank, u8 cntr)
+static int clear_avail_iommu_bnk_cntr(struct perf_iommu *pi, u8 bank, u8 cntr)
 {
 	unsigned long flags;
 	int max_banks, max_cntrs;
 	int shift = 0;
 
-	max_banks = perf_iommu->max_banks;
-	max_cntrs = perf_iommu->max_counters;
+	max_banks = pi->max_banks;
+	max_cntrs = pi->max_counters;
 
 	if ((bank > max_banks) || (cntr > max_cntrs))
 		return -EINVAL;
 
 	shift = bank + cntr + (bank*3);
 
-	raw_spin_lock_irqsave(&perf_iommu->lock, flags);
-	perf_iommu->cntr_assign_mask &= ~(1ULL<<shift);
-	raw_spin_unlock_irqrestore(&perf_iommu->lock, flags);
+	raw_spin_lock_irqsave(&pi->lock, flags);
+	pi->cntr_assign_mask &= ~(1ULL<<shift);
+	raw_spin_unlock_irqrestore(&pi->lock, flags);
 
 	return 0;
 }
@@ -197,7 +196,7 @@ static int clear_avail_iommu_bnk_cntr(struct perf_amd_iommu *perf_iommu,
 static int perf_iommu_event_init(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
-	struct perf_amd_iommu *perf_iommu;
+	struct perf_iommu *pi;
 	u64 config, config1;
 
 	/* test the event attr type check for PMU enumeration */
@@ -220,12 +219,12 @@ static int perf_iommu_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
-	perf_iommu = &__perf_iommu;
+	pi = &__perf_iommu;
 
-	if (event->pmu != &perf_iommu->pmu)
+	if (event->pmu != &pi->pmu)
 		return -ENOENT;
 
-	if (perf_iommu) {
+	if (pi) {
 		config = event->attr.config;
 		config1 = event->attr.config1;
 	} else {
@@ -350,13 +349,12 @@ static void perf_iommu_stop(struct perf_event *event, int flags)
 static int perf_iommu_add(struct perf_event *event, int flags)
 {
 	int retval;
-	struct perf_amd_iommu *perf_iommu =
-			container_of(event->pmu, struct perf_amd_iommu, pmu);
+	struct perf_iommu *pi = container_of(event->pmu, struct perf_iommu, pmu);
 
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
 	/* request an iommu bank/counter */
-	retval = get_next_avail_iommu_bnk_cntr(perf_iommu);
+	retval = get_next_avail_iommu_bnk_cntr(pi);
 	if (retval != -ENOSPC)
 		event->hw.extra_reg.reg = (u16)retval;
 	else
@@ -370,20 +368,17 @@ static int perf_iommu_add(struct perf_event *event, int flags)
 
 static void perf_iommu_del(struct perf_event *event, int flags)
 {
-	struct perf_amd_iommu *perf_iommu =
-			container_of(event->pmu, struct perf_amd_iommu, pmu);
+	struct perf_iommu *pi = container_of(event->pmu, struct perf_iommu, pmu);
 
 	perf_iommu_stop(event, PERF_EF_UPDATE);
 
 	/* clear the assigned iommu bank/counter */
-	clear_avail_iommu_bnk_cntr(perf_iommu,
-				     _GET_BANK(event),
-				     _GET_CNTR(event));
+	clear_avail_iommu_bnk_cntr(pi, _GET_BANK(event), _GET_CNTR(event));
 
 	perf_event_update_userpage(event);
 }
 
-static __init int _init_events_attrs(struct perf_amd_iommu *perf_iommu)
+static __init int _init_events_attrs(struct perf_iommu *pi)
 {
 	struct attribute **attrs;
 	struct attribute_group *attr_group;
@@ -403,7 +398,7 @@ static __init int _init_events_attrs(struct perf_amd_iommu *perf_iommu)
 
 	attr_group->name = "events";
 	attr_group->attrs = attrs;
-	perf_iommu->events_group = attr_group;
+	pi->events_group = attr_group;
 
 	return 0;
 }
@@ -416,34 +411,33 @@ static __init void amd_iommu_pc_exit(void)
 	}
 }
 
-static __init int _init_perf_amd_iommu(
-	struct perf_amd_iommu *perf_iommu, char *name)
+static __init int _init_perf_amd_iommu(struct perf_iommu *pi, char *name)
 {
 	int ret;
 
-	raw_spin_lock_init(&perf_iommu->lock);
+	raw_spin_lock_init(&pi->lock);
 
 	/* Init format attributes */
-	perf_iommu->format_group = &amd_iommu_format_group;
+	pi->format_group = &amd_iommu_format_group;
 
 	/* Init cpumask attributes to only core 0 */
 	cpumask_set_cpu(0, &iommu_cpumask);
-	perf_iommu->cpumask_group = &amd_iommu_cpumask_group;
+	pi->cpumask_group = &amd_iommu_cpumask_group;
 
 	/* Init events attributes */
-	if (_init_events_attrs(perf_iommu) != 0)
+	if (_init_events_attrs(pi) != 0)
 		pr_err("Only support raw events.\n");
 
-	perf_iommu->max_banks = amd_iommu_pc_get_max_banks(0);
-	perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
-	if (!perf_iommu->max_banks || !perf_iommu->max_counters)
+	pi->max_banks = amd_iommu_pc_get_max_banks(0);
+	pi->max_counters = amd_iommu_pc_get_max_counters(0);
+	if (!pi->max_banks || !pi->max_counters)
 		return -EINVAL;
 
 	/* Init null attributes */
-	perf_iommu->null_group = NULL;
-	perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
+	pi->null_group = NULL;
+	pi->pmu.attr_groups = pi->attr_groups;
 
-	ret = perf_pmu_register(&perf_iommu->pmu, name, -1);
+	ret = perf_pmu_register(&pi->pmu, name, -1);
 	if (ret) {
 		pr_err("Error initializing AMD IOMMU perf counters.\n");
 		amd_iommu_pc_exit();
@@ -456,7 +450,7 @@ static __init int _init_perf_amd_iommu(
 	return ret;
 }
 
-static struct perf_amd_iommu __perf_iommu = {
+static struct perf_iommu __perf_iommu = {
 	.pmu = {
 		.event_init	= perf_iommu_event_init,
 		.add		= perf_iommu_add,
-- 
1.9.1

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

#1340679 — [PATCH V5 07/10] perf/amd/iommu: Clean up get_next_available_iommu_bnk_cntr

This patch cleans up the coding style of this function.
This should not affect the logic and functionality.

Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 1a678b9..d5e4d39 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -150,25 +150,23 @@ static struct attribute_group amd_iommu_cpumask_group = {
 static int get_next_avail_iommu_bnk_cntr(struct perf_amd_iommu *perf_iommu)
 {
 	unsigned long flags;
-	int shift, bank, cntr, retval;
-	int max_banks = perf_iommu->max_banks;
-	int max_cntrs = perf_iommu->max_counters;
+	int bank, cntr, retval = -ENOSPC;
 
 	raw_spin_lock_irqsave(&perf_iommu->lock, flags);
 
-	for (bank = 0, shift = 0; bank < max_banks; bank++) {
-		for (cntr = 0; cntr < max_cntrs; cntr++) {
-			shift = bank + (bank*3) + cntr;
-			if (perf_iommu->cntr_assign_mask & (1ULL<<shift)) {
+	for (bank = 0; bank < perf_iommu->max_banks; bank++) {
+		for (cntr = 0; cntr < perf_iommu->max_counters; cntr++) {
+			int shift = (perf_iommu->max_counters * bank) + cntr;
+
+			if (perf_iommu->cntr_assign_mask & (1ULL << shift)) {
 				continue;
 			} else {
-				perf_iommu->cntr_assign_mask |= (1ULL<<shift);
-				retval = ((u16)((u16)bank<<8) | (u8)(cntr));
+				perf_iommu->cntr_assign_mask |= (1ULL << shift);
+				retval = ((u16)((u16)bank << 8) | (u8)(cntr));
 				goto out;
 			}
 		}
 	}
-	retval = -ENOSPC;
 out:
 	raw_spin_unlock_irqrestore(&perf_iommu->lock, flags);
 	return retval;
-- 
1.9.1

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

#1340685 — [PATCH V5 05/10] perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug

This patch declare pr_fmt for perf/amd_iommu and remove unnecessary
pr_debug.

Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 8a29754..ee7b4d3 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -11,6 +11,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt)	"perf/amd_iommu: " fmt
+
 #include <linux/perf_event.h>
 #include <linux/module.h>
 #include <linux/cpumask.h>
@@ -283,7 +285,6 @@ static void perf_iommu_start(struct perf_event *event, int flags)
 	u64 val;
 	struct hw_perf_event *hwc = &event->hw;
 
-	pr_debug("perf: amd_iommu:perf_iommu_start\n");
 	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
 		return;
 
@@ -307,7 +308,6 @@ static void perf_iommu_read(struct perf_event *event)
 	u64 cnt, prev;
 	s64 delta;
 	struct hw_perf_event *hwc = &event->hw;
-	pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
 	if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), &cnt))
 		return;
@@ -334,8 +334,6 @@ static void perf_iommu_stop(struct perf_event *event, int flags)
 	struct hw_perf_event *hwc = &event->hw;
 	u64 config;
 
-	pr_debug("perf: amd_iommu:perf_iommu_stop\n");
-
 	if (hwc->state & PERF_HES_UPTODATE)
 		return;
 
@@ -357,7 +355,6 @@ static int perf_iommu_add(struct perf_event *event, int flags)
 	struct perf_amd_iommu *perf_iommu =
 			container_of(event->pmu, struct perf_amd_iommu, pmu);
 
-	pr_debug("perf: amd_iommu:perf_iommu_add\n");
 	event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
 	/* request an iommu bank/counter */
@@ -378,7 +375,6 @@ static void perf_iommu_del(struct perf_event *event, int flags)
 	struct perf_amd_iommu *perf_iommu =
 			container_of(event->pmu, struct perf_amd_iommu, pmu);
 
-	pr_debug("perf: amd_iommu:perf_iommu_del\n");
 	perf_iommu_stop(event, PERF_EF_UPDATE);
 
 	/* clear the assigned iommu bank/counter */
@@ -438,7 +434,7 @@ static __init int _init_perf_amd_iommu(
 
 	/* Init events attributes */
 	if (_init_events_attrs(perf_iommu) != 0)
-		pr_err("perf: amd_iommu: Only support raw events.\n");
+		pr_err("Only support raw events.\n");
 
 	perf_iommu->max_banks = amd_iommu_pc_get_max_banks(0);
 	perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
@@ -451,7 +447,7 @@ static __init int _init_perf_amd_iommu(
 
 	ret = perf_pmu_register(&perf_iommu->pmu, name, -1);
 	if (ret) {
-		pr_err("perf: amd_iommu: Failed to initialized.\n");
+		pr_err("Error initializing AMD IOMMU perf counters.\n");
 		amd_iommu_pc_exit();
 	} else {
 		pr_info("perf: amd_iommu: Detected. (%d banks, %d counters/bank)\n",
-- 
1.9.1

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

#1340689 — [PATCH V5 03/10] perf/amd/iommu: Modify functions to query max banks and counters

Currently, amd_iommu_pc_get_max_[banks|counters]() use end-point
device ID to locate an IOMMU and check the reported max banks/counters.
The logic assumes that the IOMMU_BASE_DEVID belongs to the first IOMMU,
and uses it to acquire a reference to the first IOMMU, which does not work
on certain systems. Instead, we modify the function to take IOMMU index,
and use it to query the corresponded AMD IOMMU instance.

Note that we currently hard-code the IOMMU index to 0, since the current
AMD IOMMU perf implementation only supports single IOMMU. Subsequent patch
will add support for multi-IOMMU, and will use proper IOMMU index.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c           | 17 +++++++----------
 arch/x86/include/asm/perf/amd/iommu.h |  7 ++-----
 drivers/iommu/amd_iommu_init.c        | 35 +++++++++++++++++++++--------------
 3 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index fb4aa7b..67ff3f1 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -232,14 +232,6 @@ static int perf_iommu_event_init(struct perf_event *event)
 		return -EINVAL;
 	}
 
-	/* integrate with iommu base devid (0000), assume one iommu */
-	perf_iommu->max_banks =
-		amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID);
-	perf_iommu->max_counters =
-		amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID);
-	if ((perf_iommu->max_banks == 0) || (perf_iommu->max_counters == 0))
-		return -EINVAL;
-
 	/* update the hw_perf_event struct with the iommu config data */
 	hwc->config = config;
 	hwc->extra_reg.config = config1;
@@ -451,6 +443,11 @@ static __init int _init_perf_amd_iommu(
 	if (_init_events_attrs(perf_iommu) != 0)
 		pr_err("perf: amd_iommu: Only support raw events.\n");
 
+	perf_iommu->max_banks = amd_iommu_pc_get_max_banks(0);
+	perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
+	if (!perf_iommu->max_banks || !perf_iommu->max_counters)
+		return -EINVAL;
+
 	/* Init null attributes */
 	perf_iommu->null_group = NULL;
 	perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
@@ -461,8 +458,8 @@ static __init int _init_perf_amd_iommu(
 		amd_iommu_pc_exit();
 	} else {
 		pr_info("perf: amd_iommu: Detected. (%d banks, %d counters/bank)\n",
-			amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID),
-			amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID));
+			amd_iommu_pc_get_max_banks(0),
+			amd_iommu_pc_get_max_counters(0));
 	}
 
 	return ret;
diff --git a/arch/x86/include/asm/perf/amd/iommu.h b/arch/x86/include/asm/perf/amd/iommu.h
index 72f64b7..466be63 100644
--- a/arch/x86/include/asm/perf/amd/iommu.h
+++ b/arch/x86/include/asm/perf/amd/iommu.h
@@ -24,15 +24,12 @@
 #define PC_MAX_SPEC_BNKS			64
 #define PC_MAX_SPEC_CNTRS			16
 
-/* iommu pc reg masks*/
-#define IOMMU_BASE_DEVID			0x0000
-
 /* amd_iommu_init.c external support functions */
 bool amd_iommu_pc_supported(void);
 
-u8 amd_iommu_pc_get_max_banks(u16 devid);
+u8 amd_iommu_pc_get_max_banks(int idx);
 
-u8 amd_iommu_pc_get_max_counters(u16 devid);
+u8 amd_iommu_pc_get_max_counters(int idx);
 
 int amd_iommu_pc_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn, u64 *value);
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index d30f4b2..dde960f 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2244,6 +2244,19 @@ bool amd_iommu_v2_supported(void)
 }
 EXPORT_SYMBOL(amd_iommu_v2_supported);
 
+static struct amd_iommu *get_amd_iommu(int idx)
+{
+	int i = 0;
+	struct amd_iommu *iommu = NULL;
+
+	for_each_iommu(iommu) {
+		if (i == idx)
+			break;
+		i++;
+	}
+	return iommu;
+}
+
 /****************************************************************************
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
@@ -2251,17 +2264,14 @@ EXPORT_SYMBOL(amd_iommu_v2_supported);
  *
  ****************************************************************************/
 
-u8 amd_iommu_pc_get_max_banks(u16 devid)
+u8 amd_iommu_pc_get_max_banks(int idx)
 {
-	struct amd_iommu *iommu;
-	u8 ret = 0;
+	struct amd_iommu *iommu = get_amd_iommu(idx);
 
-	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
 	if (iommu)
-		ret = iommu->max_banks;
+		return iommu->max_banks;
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(amd_iommu_pc_get_max_banks);
 
@@ -2271,17 +2281,14 @@ bool amd_iommu_pc_supported(void)
 }
 EXPORT_SYMBOL(amd_iommu_pc_supported);
 
-u8 amd_iommu_pc_get_max_counters(u16 devid)
+u8 amd_iommu_pc_get_max_counters(int idx)
 {
-	struct amd_iommu *iommu;
-	u8 ret = 0;
+	struct amd_iommu *iommu = get_amd_iommu(idx);
 
-	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
 	if (iommu)
-		ret = iommu->max_counters;
+		return iommu->max_counters;
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(amd_iommu_pc_get_max_counters);
 
-- 
1.9.1

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

#1340691 — [PATCH V5 04/10] perf/amd/iommu: Modify IOMMU API to allow specifying IOMMU index

The current amd_iommu_pc_set_reg_val() cann not support multi-IOMMU.
So, this patch rename and modifies them to allow callers to specify
IOMMU index.

The function amd_iommu_pc_get_set_reg_val() is too confusing,
and does not support multi-IOMMU. So, this patch breaks it down
to amd_iommu_pc_[get|set]_counter(), and modifies them to allow
callers to specify IOMMU index.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c           | 45 +++++++++++------------
 arch/x86/include/asm/perf/amd/iommu.h |  8 +++--
 drivers/iommu/amd_iommu_init.c        | 67 ++++++++++++++++++++++++++++++-----
 3 files changed, 84 insertions(+), 36 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 67ff3f1..8a29754 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -243,46 +243,44 @@ static void perf_iommu_enable_event(struct perf_event *ev)
 {
 	u8 csource = _GET_CSOURCE(ev);
 	u16 devid = _GET_DEVID(ev);
+	u8 bank = _GET_BANK(ev);
+	u8 cntr = _GET_CNTR(ev);
 	u64 reg = 0ULL;
 
 	reg = csource;
-	amd_iommu_pc_get_set_reg_val(devid,
-			_GET_BANK(ev), _GET_CNTR(ev) ,
-			 IOMMU_PC_COUNTER_SRC_REG, &reg, true);
+	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+			     IOMMU_PC_COUNTER_SRC_REG, &reg);
 
 	reg = 0ULL | devid | (_GET_DEVID_MASK(ev) << 32);
 	if (reg)
 		reg |= (1UL << 31);
-	amd_iommu_pc_get_set_reg_val(devid,
-			_GET_BANK(ev), _GET_CNTR(ev) ,
-			 IOMMU_PC_DEVID_MATCH_REG, &reg, true);
+	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+			     IOMMU_PC_DEVID_MATCH_REG, &reg);
 
 	reg = 0ULL | _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
 	if (reg)
 		reg |= (1UL << 31);
-	amd_iommu_pc_get_set_reg_val(devid,
-			_GET_BANK(ev), _GET_CNTR(ev) ,
-			 IOMMU_PC_PASID_MATCH_REG, &reg, true);
+	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+			     IOMMU_PC_PASID_MATCH_REG, &reg);
 
 	reg = 0ULL | _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
 	if (reg)
 		reg |= (1UL << 31);
-	amd_iommu_pc_get_set_reg_val(devid,
-			_GET_BANK(ev), _GET_CNTR(ev) ,
-			 IOMMU_PC_DOMID_MATCH_REG, &reg, true);
+	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+			     IOMMU_PC_DOMID_MATCH_REG, &reg);
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
 	u64 reg = 0ULL;
 
-	amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-			_GET_BANK(event), _GET_CNTR(event),
-			IOMMU_PC_COUNTER_SRC_REG, &reg, true);
+	amd_iommu_pc_set_reg(0, _GET_DEVID(event), _GET_BANK(event),
+			     _GET_CNTR(event), IOMMU_PC_COUNTER_SRC_REG, &reg);
 }
 
 static void perf_iommu_start(struct perf_event *event, int flags)
 {
+	u64 val;
 	struct hw_perf_event *hwc = &event->hw;
 
 	pr_debug("perf: amd_iommu:perf_iommu_start\n");
@@ -292,13 +290,13 @@ static void perf_iommu_start(struct perf_event *event, int flags)
 	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
 	hwc->state = 0;
 
-	if (flags & PERF_EF_RELOAD) {
-		u64 prev_raw_count =  local64_read(&hwc->prev_count);
-		amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-				_GET_BANK(event), _GET_CNTR(event),
-				IOMMU_PC_COUNTER_REG, &prev_raw_count, true);
-	}
+	if (!(flags & PERF_EF_RELOAD))
+		goto enable;
+
+	val = local64_read(&hwc->prev_count);
 
+	amd_iommu_pc_set_counter(0, _GET_BANK(event), _GET_CNTR(event), &val);
+enable:
 	perf_iommu_enable_event(event);
 	perf_event_update_userpage(event);
 
@@ -311,9 +309,8 @@ static void perf_iommu_read(struct perf_event *event)
 	struct hw_perf_event *hwc = &event->hw;
 	pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
-	amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-				_GET_BANK(event), _GET_CNTR(event),
-				IOMMU_PC_COUNTER_REG, &count, false);
+	if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), &cnt))
+		return;
 
 	/* IOMMU pc counter register is only 48 bits */
 	cnt &= GENMASK_ULL(48, 0);
diff --git a/arch/x86/include/asm/perf/amd/iommu.h b/arch/x86/include/asm/perf/amd/iommu.h
index 466be63..60ddfb6 100644
--- a/arch/x86/include/asm/perf/amd/iommu.h
+++ b/arch/x86/include/asm/perf/amd/iommu.h
@@ -31,9 +31,11 @@ u8 amd_iommu_pc_get_max_banks(int idx);
 
 u8 amd_iommu_pc_get_max_counters(int idx);
 
-int amd_iommu_pc_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn, u64 *value);
+int amd_iommu_pc_set_reg(int idx, u16 devid, u8 bank, u8 cntr,
+			 u8 fxn, u64 *value);
 
-int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
-				 u64 *value, bool is_write);
+int amd_iommu_pc_set_counter(int idx, u8 bank, u8 cntr, u64 *value);
+
+int amd_iommu_pc_get_counter(int idx, u8 bank, u8 cntr, u64 *value);
 
 #endif /*_PERF_EVENT_AMD_IOMMU_H_*/
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index dde960f..b68a2d6 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1133,6 +1133,8 @@ static int __init init_iommu_all(struct acpi_table_header *table)
 	return 0;
 }
 
+static int iommu_pc_get_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
+				u8 fxn, u64 *value, bool is_write);
 
 static void init_iommu_perf_ctr(struct amd_iommu *iommu)
 {
@@ -1144,8 +1146,8 @@ static void init_iommu_perf_ctr(struct amd_iommu *iommu)
 	amd_iommu_pc_present = true;
 
 	/* Check if the performance counters can be written to */
-	if ((0 != amd_iommu_pc_get_set_reg_val(0, 0, 0, 0, &val, true)) ||
-	    (0 != amd_iommu_pc_get_set_reg_val(0, 0, 0, 0, &val2, false)) ||
+	if ((iommu_pc_get_set_reg(iommu, 0, 0, 0, &val, true)) ||
+	    (iommu_pc_get_set_reg(iommu, 0, 0, 0, &val2, false)) ||
 	    (val != val2)) {
 		pr_err("AMD-Vi: Unable to write to IOMMU perf counter.\n");
 		amd_iommu_pc_present = false;
@@ -2292,10 +2294,9 @@ u8 amd_iommu_pc_get_max_counters(int idx)
 }
 EXPORT_SYMBOL(amd_iommu_pc_get_max_counters);
 
-int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
-				    u64 *value, bool is_write)
+static int iommu_pc_get_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
+				u8 fxn, u64 *value, bool is_write)
 {
-	struct amd_iommu *iommu;
 	u32 offset;
 	u32 max_offset_lim;
 
@@ -2303,9 +2304,6 @@ int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
 	if (!amd_iommu_pc_present)
 		return -ENODEV;
 
-	/* Locate the iommu associated with the device ID */
-	iommu = amd_iommu_rlookup_table[devid];
-
 	/* Check for valid iommu and pc register indexing */
 	if (WARN_ON((iommu == NULL) || (fxn > 0x28) || (fxn & 7)))
 		return -ENODEV;
@@ -2330,4 +2328,55 @@ int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
 
 	return 0;
 }
-EXPORT_SYMBOL(amd_iommu_pc_get_set_reg_val);
+
+int amd_iommu_pc_set_reg(int idx, u16 devid, u8 bank, u8 cntr,
+			 u8 fxn, u64 *value)
+{
+	struct amd_iommu *iommu = get_amd_iommu(idx);
+
+	if (!iommu)
+		return -EINVAL;
+
+	return iommu_pc_get_set_reg(iommu, bank, cntr, fxn, value, true);
+}
+EXPORT_SYMBOL(amd_iommu_pc_set_reg);
+
+int amd_iommu_pc_set_counter(int idx, u8 bank, u8 cntr, u64 *value)
+{
+	struct amd_iommu *iommu = get_amd_iommu(idx);
+
+	if (!iommu)
+		return -EINVAL;
+
+	return iommu_pc_get_set_reg(iommu, bank, cntr,
+				    IOMMU_PC_COUNTER_REG,
+				    value, true);
+}
+EXPORT_SYMBOL(amd_iommu_pc_set_counter);
+
+int amd_iommu_pc_get_counter(int idx, u8 bank, u8 cntr, u64 *value)
+{
+	struct amd_iommu *iommu = get_amd_iommu(idx);
+	int ret;
+	u64 tmp;
+
+	if (!value || !iommu)
+		return -EINVAL;
+	/*
+	 * Here, we read the specified counters on all IOMMUs,
+	 * which should have been programmed the same way and
+	 * aggregate the counter values.
+	 */
+
+	ret = iommu_pc_get_set_reg(iommu, bank, cntr,
+				   IOMMU_PC_COUNTER_REG,
+				   &tmp, false);
+	if (ret)
+		return ret;
+
+	/* IOMMU pc counter register is only 48 bits */
+	*value = tmp & GENMASK_ULL(48, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL(amd_iommu_pc_get_counter);
-- 
1.9.1

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

#1340692 — [PATCH V5 06/10] perf/amd/iommu: Clean up perf_iommu_enable_event

This patch cleans up:
  * Various bitwise operations in perf_iommu_enable_event
  * Make use macros BIT(x)

This should not affect logic and functionality.

Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index ee7b4d3..1a678b9 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -253,21 +253,21 @@ static void perf_iommu_enable_event(struct perf_event *ev)
 	amd_iommu_pc_set_reg(0, devid, bank, cntr,
 			     IOMMU_PC_COUNTER_SRC_REG, &reg);
 
-	reg = 0ULL | devid | (_GET_DEVID_MASK(ev) << 32);
+	reg = devid | (_GET_DEVID_MASK(ev) << 32);
 	if (reg)
-		reg |= (1UL << 31);
+		reg |= BIT(31);
 	amd_iommu_pc_set_reg(0, devid, bank, cntr,
 			     IOMMU_PC_DEVID_MATCH_REG, &reg);
 
-	reg = 0ULL | _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
+	reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
 	if (reg)
-		reg |= (1UL << 31);
+		reg |= BIT(31);
 	amd_iommu_pc_set_reg(0, devid, bank, cntr,
 			     IOMMU_PC_PASID_MATCH_REG, &reg);
 
-	reg = 0ULL | _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
+	reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
 	if (reg)
-		reg |= (1UL << 31);
+		reg |= BIT(31);
 	amd_iommu_pc_set_reg(0, devid, bank, cntr,
 			     IOMMU_PC_DOMID_MATCH_REG, &reg);
 }
-- 
1.9.1

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

#1343231

Hi Suravee,

On Tue, Feb 23, 2016 at 08:12:34AM -0600, Suravee Suthikulpanit wrote:
> This is a two-part patch series:
> 
> Part1: 1-4 :
> Introduce a workaround for the current AMD IOMMU perf initialization issue
> in some existing KV and CZ platforms, where it fails to write to IOMMU
> perf counter as reported by Andreas Hartmann here
> (http://comments.gmane.org/gmane.linux.kernel.pci/49147).

Okay, these 4 patches fix the issue, but they also refactor the code and
so a lot more. They are not suitable for stable kernels, so to just fix
the issue I take the small patch I extracted from yours and posted
previously. I retains your authorship, hope that is fine with you.

You can rebase these patches on-top of that minimal fix.


	Joerg

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web