perf/amd/iommu: Enable multi-IOMMU support

Currently, amd_iommu_pc_get_max_[banks|counters]() require devid, which should not be the case.

Also, these don't properly support
multi-IOMMU system.

Current and future AMD systems with IOMMU that support perf counter

would likely contain homogeneous IOMMUs where multiple IOMMUs are

availalbe. So, this patch modifies these function to iterate all IOMMU

arch/x86/events/amd/iommu.c +4 -2

+		return -EINVAL;
+
 	/* Init null attributes */
 	perf_iommu->null_group = NULL;
 	perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;

@@ -460,8 +457,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(),
+			amd_iommu_pc_get_max_counters());
 	}
 
 	return ret;

arch/x86/include/asm/perf/amd/iommu.h +2 -5

index 72f64b7..97e1be5 100644
@@ -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(void);
 
-u8 amd_iommu_pc_get_max_counters(u16 devid);
+u8 amd_iommu_pc_get_max_counters(void);
 
 int amd_iommu_pc_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn, u64 *value);
 

drivers/iommu/amd_iommu_init.c +1 -1

index d30f4b2..a62b5ce 100644
@@ -2251,15 +2251,17 @@ EXPORT_SYMBOL(amd_iommu_v2_supported);
  *
  ****************************************************************************/
 
-u8 amd_iommu_pc_get_max_banks(u16 devid)
+u8 amd_iommu_pc_get_max_banks(void)

drivers/iommu/amd_iommu_init.c +3 -1

+			return 0;
 		ret = iommu->max_banks;
+	}
 
 	return ret;
 }

@@ -2271,15 +2273,17 @@ 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(void)

Quoted 2 lines · Suravee Suthikulpanit

Currently, amd_iommu_pc_get_max_[banks|counters]() require devid, which should not be the case.

Why?

Commit message could use an explanation.

Quoted 3 lines

Also, these don't properly support
multi-IOMMU system.

Quoted 1 line

Current and future AMD systems with IOMMU that support perf counter

"with an IOMMU that supports performance counters"

Quoted 1 line

would likely contain homogeneous IOMMUs where multiple IOMMUs are

What are homogeneous IOMMUs?

Quoted 2 lines

availalbe. So, this patch modifies these function to iterate all IOMMU

Please integrate a spellchecker in your patch creation workflow:

s/availalbe/available/

arch/x86/events/amd/iommu.c static __init int _init_perf_amd_iommu(

Quoted 29 lines

Reviewed-by Joerg Roedel <[email protected]>

Signed-off-by Suravee Suthikulpanit <[email protected]>

---
  arch/x86/events/amd/iommu.c           | 17 +++++++----------
  arch/x86/include/asm/perf/amd/iommu.h |  7 ++-----
  drivers/iommu/amd_iommu_init.c        | 20 ++++++++++++--------
  3 files changed, 21 insertions(+), 23 deletions(-)

arch/x86/events/amd/iommu.c +0 -8

index 2f96072..debf22d 100644
@@ -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;

@@ -450,6 +442,11 @@ static __init int _init_perf_amd_iommu(

Btw, that _init_perf_amd_iommu is unnecessarily split from amd_iommu_pc_init(). You should merge those two. But that's another patch. In that same cleanup patch you could do

s/perf_iommu/pi/g

or so because that perf_iommu local var is unnecesarily long and impairs readability.

arch/x86/events/amd/iommu.c static __init int _init_perf_amd_iommu( · line 447

Quoted 6 lines

arch/x86/events/amd/iommu.c +3 -0

  	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();
+	perf_iommu->max_counters = amd_iommu_pc_get_max_counters();
+	if ((perf_iommu->max_banks == 0) || (perf_iommu->max_counters == 0))

Simplify:

	if (!perf_iommu->max_banks ||
	    !perf_iommu->max_counters)

Quoted 10 lines

drivers/iommu/amd_iommu_init.c +1 -1

index d30f4b2..a62b5ce 100644
@@ -2251,15 +2251,17 @@ EXPORT_SYMBOL(amd_iommu_v2_supported);
   *
   ****************************************************************************/
 
-u8 amd_iommu_pc_get_max_banks(u16 devid)
+u8 amd_iommu_pc_get_max_banks(void)

drivers/iommu/amd_iommu_init.c EXPORT_SYMBOL(amd_iommu_v2_supported); · line 2261

Quoted 10 lines

drivers/iommu/amd_iommu_init.c +3 -3

  {
  	struct amd_iommu *iommu;
  	u8 ret = 0;
 
-	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
-	if (iommu)
+	for_each_iommu(iommu) {
+		if (!iommu->max_banks ||
+		    (ret && (iommu->max_banks != ret)))

What is that supposed to do?

Check that the max_banks of a previous IOMMU are == to the max_banks of the current IOMMU?

Why? Could definitely use a comment.

Introduce a helper function to calculate bit-index for assigning performance counter assignment.

Signed-off-by Suravee Suthikulpanit <[email protected]>

---
 arch/x86/events/amd/iommu.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

arch/x86/events/amd/iommu.c +0 -0

index debf22d..812eff2 100644

arch/x86/events/amd/iommu.c +7 -0

+{
+	int cntrs_per_iommu = perf_iommu->max_banks * perf_iommu->max_counters;
+	int index = (perf_iommu->max_counters * bank_index) + cntr_index;
+
+	return (cntrs_per_iommu * iommu_index) + index;
+}
+
 static int get_next_avail_iommu_bnk_cntr(struct perf_amd_iommu *perf_iommu)
 {
 	unsigned long flags;

arch/x86/events/amd/iommu.c +0 -0

 			if (perf_iommu->cntr_assign_mask & (1ULL<<shift)) {
 				continue;
 			} else {

Signature · 1 line

2.5.0

static void perf_iommu_read(struct perf_event *event) {

arch/x86/events/amd/iommu.c +25 -0

+	int i;
 	u64 delta = 0ULL;
 	struct hw_perf_event *hwc = &event->hw;
+	struct perf_amd_iommu *perf_iommu = container_of(event->pmu,
+							 struct perf_amd_iommu,
+							 pmu);
 
+	if (amd_iommu_pc_get_counters(_GET_BANK(event), _GET_CNTR(event),
+				      amd_iommu_get_num_iommus(),
+				      perf_iommu_cnts))
 		return;
 
+	/*
+	 * Now we re-aggregating event counts and prev-counts
+	 * from all IOMMUs
+	 */
+	local64_set(&hwc->prev_count, 0);
+
+	for (i = 0; i < amd_iommu_get_num_iommus(); i++) {
+		int indx = get_iommu_bnk_cnt_evt_idx(perf_iommu, i,
+						     _GET_BANK(event),
+						     _GET_CNTR(event));
+		u64 prev_raw_count = local64_read(&perf_iommu->prev_cnts[indx]);
+
+		/* IOMMU pc counter register is only 48 bits */
+		perf_iommu_cnts[i] &= GENMASK_ULL(48, 0);
+
+		/*
+		 * Since we do not enable counter overflow interrupts,
+		 * we do not have to worry about prev_count changing on us

Quoted 32 lines · Suravee Suthikulpanit

  static void perf_iommu_read(struct perf_event *event)
  {

arch/x86/events/amd/iommu.c +25 -0

+	int i;
  	u64 delta = 0ULL;
  	struct hw_perf_event *hwc = &event->hw;
+	struct perf_amd_iommu *perf_iommu = container_of(event->pmu,
+							 struct perf_amd_iommu,
+							 pmu);
 
+	if (amd_iommu_pc_get_counters(_GET_BANK(event), _GET_CNTR(event),
+				      amd_iommu_get_num_iommus(),
+				      perf_iommu_cnts))
  		return;
 
+	/*
+	 * Now we re-aggregating event counts and prev-counts
+	 * from all IOMMUs
+	 */
+	local64_set(&hwc->prev_count, 0);
+
+	for (i = 0; i < amd_iommu_get_num_iommus(); i++) {
+		int indx = get_iommu_bnk_cnt_evt_idx(perf_iommu, i,
+						     _GET_BANK(event),
+						     _GET_CNTR(event));
+		u64 prev_raw_count = local64_read(&perf_iommu->prev_cnts[indx]);
+
+		/* IOMMU pc counter register is only 48 bits */
+		perf_iommu_cnts[i] &= GENMASK_ULL(48, 0);
+
+		/*
+		 * Since we do not enable counter overflow interrupts,
+		 * we do not have to worry about prev_count changing on us

arch/x86/events/amd/iommu.c static void perf_iommu_start(struct perf_event *event, int flags) · line 377

Quoted 10 lines

arch/x86/events/amd/iommu.c +9 -0

+		 */
+		local64_set(&perf_iommu->prev_cnts[indx], perf_iommu_cnts[i]);
+		local64_add(prev_raw_count, &hwc->prev_count);
+
+		/* Handle 48-bit counter overflow */
+		delta = (perf_iommu_cnts[i] << COUNTER_SHIFT) - (prev_raw_count << COUNTER_SHIFT);
+		delta >>= COUNTER_SHIFT;
+		local64_add(delta, &event->count);
+	}
  }

So I really don't have time to review new muck while I'm hunting perf core fail, but Boris made me look at this.

This is crazy, if you have multiple IOMMUs then create an event per IOMMU, do _NOT_ fold them all into a single event.

In any case, the reason Boris asked me to look at this is that your overflow handling is broken, you want delta to be s64. Otherwise:

	delta >>= COUNTER_SHIFT;

ends up as a SHR and you loose the MSB sign bits.

Quoted 5 lines

So I really don't have time to review new muck while I'm hunting perf core fail, but Boris made me look at this.

This is crazy, if you have multiple IOMMUs then create an event per IOMMU, do _NOT_ fold them all into a single event.

These are system-wide events, which are programmed on every IOMMU the same way. I am not sure what you meant by creating an event per IOMMU. Do you mean I should create internal per-IOMMU struct perf_event for each event?

Quoted 6 lines · Suravee Suthikulpanit

Quoted 5 lines

So I really don't have time to review new muck while I'm hunting perf core fail, but Boris made me look at this.

This is crazy, if you have multiple IOMMUs then create an event per IOMMU, do _NOT_ fold them all into a single event.

Quoted 3 lines

These are system-wide events, which are programmed on every IOMMU the same way. I am not sure what you meant by creating an event per IOMMU. Do you mean I should create internal per-IOMMU struct perf_event for each event?

No, I meant to expose each IOMMU individually to userspace, as a separate device.

Is there never a case to profile just one of the IOMMUs ?

Hi

Quoted 7 lines · Peter Zijlstra

Quoted 6 lines · Suravee Suthikulpanit

Quoted 5 lines

So I really don't have time to review new muck while I'm hunting perf core fail, but Boris made me look at this.

This is crazy, if you have multiple IOMMUs then create an event per IOMMU, do _NOT_ fold them all into a single event.

Quoted 9 lines

Quoted 3 lines

These are system-wide events, which are programmed on every IOMMU the same way. I am not sure what you meant by creating an event per IOMMU. Do you mean I should create internal per-IOMMU struct perf_event for each event?

No, I meant to expose each IOMMU individually to userspace, as a separate device.

Is there never a case to profile just one of the IOMMUs ?

I see. That's definitely doable and simpler to implement.

I was not sure if making users specify the IOMMU instance (e.g. amd_iommu_0/<ev name> , amd_iommu_1/<ev_name>, ....) would be too tedious. However, this would actually give users better control of the performance events, which is a good trade-off. I think it is acceptable.

I'll make the change and send this out in V5.

Quoted 27 lines · On Tue, 23 Feb 2016 12:12:42 +0700 Suravee Suthikulpanit…

Hi

Quoted 7 lines · Peter Zijlstra

Quoted 6 lines · Suravee Suthikulpanit

Quoted 5 lines

So I really don't have time to review new muck while I'm hunting perf core fail, but Boris made me look at this.

This is crazy, if you have multiple IOMMUs then create an event per IOMMU, do _NOT_ fold them all into a single event.

Quoted 9 lines

Quoted 3 lines

These are system-wide events, which are programmed on every IOMMU the same way. I am not sure what you meant by creating an event per IOMMU. Do you mean I should create internal per-IOMMU struct perf_event for each event?

No, I meant to expose each IOMMU individually to userspace, as a separate device.

Is there never a case to profile just one of the IOMMUs ?

I see. That's definitely doable and simpler to implement.

I was not sure if making users specify the IOMMU instance (e.g. amd_iommu_0/<ev name> , amd_iommu_1/<ev_name>, ....) would be too tedious. However, this would actually give users better control of the performance events, which is a good trade-off. I think it is acceptable.

Quoted 1 line

I'll make the change and send this out in V5.

We already expose individual IOMMU hardware units in /sys/class/iommu/, you might consider trying to match the names there for the convenience of the user. Looks like we use ivhd%d for AMD. Thanks,

Alex

The current amd_iommu_pc_get_set_reg_val() does not support muli-IOMMU

system. This patch replace amd_iommu_pc_get_set_reg_val() with

amd_iommu_pc_set_reg_val() and amd_iommu_pc_[set|get]_cnt_vals().

Also, the current struct hw_perf_event.prev_count can only store the previous counter value only from one IOMMU. So, this patch introduces a new data structure, perf_amd_iommu.prev_cnts, to track previous value of each performance counter of each bank of each IOMMU.

Last, this patch introduce perf_iommu_cnts to temporary hold counter values from each IOMMU for internal use when manages counters among multiple IOMMUs.

Note that this implementation makes an assumption that the counters on all IOMMUs will be programmed the same way (i.e with the same events).

Signed-off-by Suravee Suthikulpanit <[email protected]>

---
 arch/x86/events/amd/iommu.c           | 146 +++++++++++++++++++++++-----------
 arch/x86/include/asm/perf/amd/iommu.h |   7 +-
 drivers/iommu/amd_iommu_init.c        | 107 ++++++++++++++++++++++---
 3 files changed, 201 insertions(+), 59 deletions(-)

arch/x86/events/amd/iommu.c +7 -0

index 812eff2..947ae8a 100644
@@ -42,6 +42,13 @@ struct perf_amd_iommu {
 	u64 cntr_assign_mask;
 	raw_spinlock_t lock;
 	const struct attribute_group *attr_groups[4];
+
+	/*
+	 * This is a 3D array used to store the previous count values
+	 * from each performance counter of each bank of each IOMMU.
+	 * I.E. size of array = (num iommus * num banks * num counters)
+	 */
+	local64_t *prev_cnts;
 };
 
 #define format_group	attr_groups[0]

arch/x86/events/amd/iommu.c +12 -14

+
 /*---------------------------------------------
  * sysfs cpumask attributes
  *---------------------------------------------*/

@@ -255,44 +268,42 @@ static void perf_iommu_enable_event(struct perf_event *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(devid, _GET_BANK(ev), _GET_CNTR(ev),
+			     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);
-	amd_iommu_pc_get_set_reg_val(devid,
-			_GET_BANK(ev), _GET_CNTR(ev) ,
-			 IOMMU_PC_DEVID_MATCH_REG, &reg, true);
+		reg |= BIT(31);
+	amd_iommu_pc_set_reg(devid, _GET_BANK(ev), _GET_CNTR(ev),
+			     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);
-	amd_iommu_pc_get_set_reg_val(devid,
-			_GET_BANK(ev), _GET_CNTR(ev) ,
-			 IOMMU_PC_PASID_MATCH_REG, &reg, true);
+		reg |= BIT(31);
+	amd_iommu_pc_set_reg(devid, _GET_BANK(ev), _GET_CNTR(ev),
+			     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);

arch/x86/events/amd/iommu.c +14 -7

-		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);
+		int i;
+
+		for (i = 0; i < amd_iommu_get_num_iommus(); i++) {
+			int index = get_iommu_bnk_cnt_evt_idx(perf_iommu, i,
+							      _GET_BANK(event),
+							      _GET_CNTR(event));
+
+			perf_iommu_cnts[i] = local64_read(&perf_iommu->prev_cnts[index]);
+		}
+
+		amd_iommu_pc_set_counters(_GET_BANK(event), _GET_CNTR(event),
+					  amd_iommu_get_num_iommus(),
+					  perf_iommu_cnts);
 	}
 
 	perf_iommu_enable_event(event);

@@ -315,29 +335,45 @@ 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;
+	int i;
 	u64 delta = 0ULL;
 	struct hw_perf_event *hwc = &event->hw;
-	pr_debug("perf: amd_iommu:perf_iommu_read\n");

arch/x86/events/amd/iommu.c +3 -3

-	prev_raw_count =  local64_read(&hwc->prev_count);
-	if (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
-					count) != prev_raw_count)
+	if (amd_iommu_pc_get_counters(_GET_BANK(event), _GET_CNTR(event),
+				      amd_iommu_get_num_iommus(),
+				      perf_iommu_cnts))

arch/x86/events/amd/iommu.c +17 -4

+		 */
+		local64_set(&perf_iommu->prev_cnts[indx], perf_iommu_cnts[i]);
+		local64_add(prev_raw_count, &hwc->prev_count);
+
+		/* Handle 48-bit counter overflow */
+		delta = (perf_iommu_cnts[i] << COUNTER_SHIFT) - (prev_raw_count << COUNTER_SHIFT);
+		delta >>= COUNTER_SHIFT;
+		local64_add(delta, &event->count);
+	}
 }
 
 static void perf_iommu_stop(struct perf_event *event, int flags)

@@ -427,10 +463,14 @@ static __init int _init_events_attrs(struct perf_amd_iommu *perf_iommu)
 
 static __init void amd_iommu_pc_exit(void)
 {
-	if (__perf_iommu.events_group != NULL) {
-		kfree(__perf_iommu.events_group);
-		__perf_iommu.events_group = NULL;
-	}
+	kfree(__perf_iommu.events_group);
+	__perf_iommu.events_group = NULL;
+
+	kfree(__perf_iommu.prev_cnts);
+	__perf_iommu.prev_cnts = NULL;
+
+	kfree(perf_iommu_cnts);
+	perf_iommu_cnts = NULL;
 }
 
 static __init int _init_perf_amd_iommu(

@@ -460,6 +500,18 @@ static __init int _init_perf_amd_iommu(
 	perf_iommu->null_group = NULL;
 	perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;

arch/x86/events/amd/iommu.c +3 -0

+	if (!perf_iommu_cnts)
+		return -ENOMEM;
+
 	ret = perf_pmu_register(&perf_iommu->pmu, name, -1);
 	if (ret) {
 		pr_err("perf: amd_iommu: Failed to initialized.\n");

arch/x86/include/asm/perf/amd/iommu.h +4 -3

index 40919eb..e0aa9be 100644
@@ -33,9 +33,10 @@ u8 amd_iommu_pc_get_max_banks(void);
 
 u8 amd_iommu_pc_get_max_counters(void);
 
-int amd_iommu_pc_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn, u64 *value);
+int amd_iommu_pc_set_reg(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_counters(u8 bank, u8 cntr, int num, u64 *value);
+
+int amd_iommu_pc_get_counters(u8 bank, u8 cntr, int num, u64 *value);
 
 #endif /*_PERF_EVENT_AMD_IOMMU_H_*/

drivers/iommu/amd_iommu_init.c +0 -0

index ffa057e..fd4f2b1 100644

drivers/iommu/amd_iommu_init.c +2 -2

 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;

From: Suravee Suthikulpanit <[email protected]>

This patch declares pr_fmt for perf/amd_iommu, removes unnecessary pr_debug, and clean up error messages.

From: Suravee Suthikulpanit <[email protected]>

This patch series modifies the existing perf_event_amd_iommu driver to support systems with multiple IOMMUs. It introduces new AMD IOMMU APIs, which are used by the AMD IOMMU Perf driver to access performance counters in multiple IOMMUs.

In addition, this series should also fix current AMD IOMMU PMU driver initialization issue in some existing KV and CZ platform, where it fails to write to IOMMU perf counter as reported by Andreas Hartmann here
(http://comments.gmane.org/gmane.linux.kernel.pci/49147).

Git branch containing this patch series is available here:

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

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 variable since they are the same thing

Suravee Suthikulpanit (6):
perf/amd/iommu: Consolidate and move perf_event_amd_iommu header perf/amd/iommu: Modify functions to query max banks and counters iommu/amd: Introduce amd_iommu_get_num_iommus() perf/amd/iommu: Introduce get_iommu_bnk_cnt_evt_idx perf/amd/iommu: Enable support for multiple IOMMUs perf/amd/iommu: Declar pr_fmt and remove unnecessary pr_debug

  arch/x86/events/amd/iommu.c           | 197 ++++++++++++++++++++++------------
  arch/x86/events/amd/iommu.h           |  40 -------
  arch/x86/include/asm/perf/amd/iommu.h |  42 ++++++++
  drivers/iommu/amd_iommu_init.c        | 136 +++++++++++++++++++----
  drivers/iommu/amd_iommu_proto.h       |   7 --
  5 files changed, 286 insertions(+), 136 deletions(-)

delete mode 100644 arch/x86/events/amd/iommu.h create mode 100644 arch/x86/include/asm/perf/amd/iommu.h

From: Suravee Suthikulpanit <[email protected]>

This patch series modifies the existing perf_event_amd_iommu driver to support systems with multiple IOMMUs. It introduces new AMD IOMMU APIs, which are used by the AMD IOMMU Perf driver to access performance counters in multiple IOMMUs.

In addition, this series should also fix current AMD IOMMU PMU driver initialization issue in some existing KV and CZ platform, where it fails to write to IOMMU perf counter as reported by Andreas Hartmann here
(http://comments.gmane.org/gmane.linux.kernel.pci/49147).

Hi Suravee,

Quoted 11 lines · Suravee

From: Suravee Suthikulpanit <[email protected]>

This patch series modifies the existing perf_event_amd_iommu driver to support systems with multiple IOMMUs. It introduces new AMD IOMMU APIs, which are used by the AMD IOMMU Perf driver to access performance counters in multiple IOMMUs.

In addition, this series should also fix current AMD IOMMU PMU driver initialization issue in some existing KV and CZ platform, where it fails to write to IOMMU perf counter as reported by Andreas Hartmann here
(http://comments.gmane.org/gmane.linux.kernel.pci/49147).

Since that problem also affects older kernels, we need a stand-alone fix for this initialization issue. We can't backport new features to fix bugs in old kernels.

Joerg

Hi,

Quoted 21 lines · Joerg Roedel

Hi Suravee,

Quoted 8 lines · Suravee

From: Suravee Suthikulpanit <[email protected]>

This patch series modifies the existing perf_event_amd_iommu driver to support systems with multiple IOMMUs. It introduces new AMD IOMMU APIs,
which are used by the AMD IOMMU Perf driver to access performance counters in multiple IOMMUs.

Quoted 4 lines

In addition, this series should also fix current AMD IOMMU PMU driver initialization issue in some existing KV and CZ platform, where it fails to write to IOMMU perf counter as reported by Andreas Hartmann here
(http://comments.gmane.org/gmane.linux.kernel.pci/49147).

Since that problem also affects older kernels, we need a stand-alone fix for this initialization issue. We can't backport new features to fix bugs in old kernels.

Quoted 2 lines

Joerg

OK, I am spliting V5 into two part. First would be just to fix the issue. Then the second part would be to add the multiple IOMMU support.

Suravee

Actually, my V5 is rebased from tips which has moved several files. I think we might need to create a separate patch series to fix this issue in older kernels. Which branch should I rebase my changes for the older kernel?

From b91309eedd77374fdecc379942c44f903e2dedff Mon Sep 17 00:00:00 2001