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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 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

[PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

First, this patch move arch/x86/events/amd/iommu.h to
arch/x86/include/asm/perf/amd/iommu.h so that we easily include
it in both perf-amd-iommu and amd-iommu drivers.

Then, we consolidate declaration of AMD IOMMU performance counter
APIs into one file.

Reviewed-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/events/amd/iommu.c           |  2 +-
 arch/x86/events/amd/iommu.h           | 40 ---------------------------------
 arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c        |  2 ++
 drivers/iommu/amd_iommu_proto.h       |  7 ------
 5 files changed, 45 insertions(+), 48 deletions(-)
 delete mode 100644 arch/x86/events/amd/iommu.h
 create mode 100644 arch/x86/include/asm/perf/amd/iommu.h

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 9da0d16..fb4aa7b 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -15,9 +15,9 @@
 #include <linux/module.h>
 #include <linux/cpumask.h>
 #include <linux/slab.h>
+#include <asm/perf/amd/iommu.h>
 
 #include "../../kernel/cpu/perf_event.h"
-#include "iommu.h"
 
 #define COUNTER_SHIFT		16
 
diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
deleted file mode 100644
index 845d173..0000000
--- a/arch/x86/events/amd/iommu.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Copyright (C) 2013 Advanced Micro Devices, Inc.
- *
- * Author: Steven Kinney <Steven.Kinney@amd.com>
- * Author: Suravee Suthikulpanit <Suraveee.Suthikulpanit@amd.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef _PERF_EVENT_AMD_IOMMU_H_
-#define _PERF_EVENT_AMD_IOMMU_H_
-
-/* iommu pc mmio region register indexes */
-#define IOMMU_PC_COUNTER_REG			0x00
-#define IOMMU_PC_COUNTER_SRC_REG		0x08
-#define IOMMU_PC_PASID_MATCH_REG		0x10
-#define IOMMU_PC_DOMID_MATCH_REG		0x18
-#define IOMMU_PC_DEVID_MATCH_REG		0x20
-#define IOMMU_PC_COUNTER_REPORT_REG		0x28
-
-/* maximun specified bank/counters */
-#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 */
-extern bool amd_iommu_pc_supported(void);
-
-extern u8 amd_iommu_pc_get_max_banks(u16 devid);
-
-extern u8 amd_iommu_pc_get_max_counters(u16 devid);
-
-extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr,
-			u8 fxn, u64 *value, bool is_write);
-
-#endif /*_PERF_EVENT_AMD_IOMMU_H_*/
diff --git a/arch/x86/include/asm/perf/amd/iommu.h b/arch/x86/include/asm/perf/amd/iommu.h
new file mode 100644
index 0000000..72f64b7
--- /dev/null
+++ b/arch/x86/include/asm/perf/amd/iommu.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2013 Advanced Micro Devices, Inc.
+ *
+ * Author: Steven Kinney <Steven.Kinney@amd.com>
+ * Author: Suravee Suthikulpanit <Suraveee.Suthikulpanit@amd.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _PERF_EVENT_AMD_IOMMU_H_
+#define _PERF_EVENT_AMD_IOMMU_H_
+
+/* iommu pc mmio region register indexes */
+#define IOMMU_PC_COUNTER_REG			0x00
+#define IOMMU_PC_COUNTER_SRC_REG		0x08
+#define IOMMU_PC_PASID_MATCH_REG		0x10
+#define IOMMU_PC_DOMID_MATCH_REG		0x18
+#define IOMMU_PC_DEVID_MATCH_REG		0x20
+#define IOMMU_PC_COUNTER_REPORT_REG		0x28
+
+/* maximum specified bank/counters */
+#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_counters(u16 devid);
+
+int amd_iommu_pc_set_reg_val(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);
+
+#endif /*_PERF_EVENT_AMD_IOMMU_H_*/
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..d30f4b2 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -27,6 +27,8 @@
 #include <linux/amd-iommu.h>
 #include <linux/export.h>
 #include <linux/iommu.h>
+#include <asm/perf/amd/iommu.h>
+
 #include <asm/pci-direct.h>
 #include <asm/iommu.h>
 #include <asm/gart.h>
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 0bd9eb3..ac2da91 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -55,13 +55,6 @@ extern int amd_iommu_domain_set_gcr3(struct iommu_domain *dom, int pasid,
 extern int amd_iommu_domain_clear_gcr3(struct iommu_domain *dom, int pasid);
 extern struct iommu_domain *amd_iommu_get_v2_domain(struct pci_dev *pdev);
 
-/* IOMMU Performance Counter functions */
-extern bool amd_iommu_pc_supported(void);
-extern u8 amd_iommu_pc_get_max_banks(u16 devid);
-extern u8 amd_iommu_pc_get_max_counters(u16 devid);
-extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
-				    u64 *value, bool is_write);
-
 #ifdef CONFIG_IRQ_REMAP
 extern int amd_iommu_create_irq_domain(struct amd_iommu *iommu);
 #else
-- 
1.9.1

[PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>

First, this patch move arch/x86/events/amd/iommu.h to
arch/x86/include/asm/perf/amd/iommu.h so that we easily include
it in both perf-amd-iommu and amd-iommu drivers.

Then, we consolidate declaration of AMD IOMMU performance counter
APIs into one file.

Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 arch/x86/events/amd/iommu.c           |  2 +-
 arch/x86/events/amd/iommu.h           | 40 ---------------------------------
 arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c        |  2 ++
 drivers/iommu/amd_iommu_proto.h       |  7 ------
 5 files changed, 45 insertions(+), 48 deletions(-)
 delete mode 100644 arch/x86/events/amd/iommu.h
 create mode 100644 arch/x86/include/asm/perf/amd/iommu.h

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 9da0d16..fb4aa7b 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -15,9 +15,9 @@
 #include <linux/module.h>
 #include <linux/cpumask.h>
 #include <linux/slab.h>
+#include <asm/perf/amd/iommu.h>
 
 #include "../../kernel/cpu/perf_event.h"
-#include "iommu.h"
 
 #define COUNTER_SHIFT		16
 
diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
deleted file mode 100644
index 845d173..0000000
--- a/arch/x86/events/amd/iommu.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/*
- * Copyright (C) 2013 Advanced Micro Devices, Inc.
- *
- * Author: Steven Kinney <Steven.Kinney-5C7GfCeVMHo@public.gmane.org>
- * Author: Suravee Suthikulpanit <Suraveee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef _PERF_EVENT_AMD_IOMMU_H_
-#define _PERF_EVENT_AMD_IOMMU_H_
-
-/* iommu pc mmio region register indexes */
-#define IOMMU_PC_COUNTER_REG			0x00
-#define IOMMU_PC_COUNTER_SRC_REG		0x08
-#define IOMMU_PC_PASID_MATCH_REG		0x10
-#define IOMMU_PC_DOMID_MATCH_REG		0x18
-#define IOMMU_PC_DEVID_MATCH_REG		0x20
-#define IOMMU_PC_COUNTER_REPORT_REG		0x28
-
-/* maximun specified bank/counters */
-#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 */
-extern bool amd_iommu_pc_supported(void);
-
-extern u8 amd_iommu_pc_get_max_banks(u16 devid);
-
-extern u8 amd_iommu_pc_get_max_counters(u16 devid);
-
-extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr,
-			u8 fxn, u64 *value, bool is_write);
-
-#endif /*_PERF_EVENT_AMD_IOMMU_H_*/
diff --git a/arch/x86/include/asm/perf/amd/iommu.h b/arch/x86/include/asm/perf/amd/iommu.h
new file mode 100644
index 0000000..72f64b7
--- /dev/null
+++ b/arch/x86/include/asm/perf/amd/iommu.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2013 Advanced Micro Devices, Inc.
+ *
+ * Author: Steven Kinney <Steven.Kinney-5C7GfCeVMHo@public.gmane.org>
+ * Author: Suravee Suthikulpanit <Suraveee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _PERF_EVENT_AMD_IOMMU_H_
+#define _PERF_EVENT_AMD_IOMMU_H_
+
+/* iommu pc mmio region register indexes */
+#define IOMMU_PC_COUNTER_REG			0x00
+#define IOMMU_PC_COUNTER_SRC_REG		0x08
+#define IOMMU_PC_PASID_MATCH_REG		0x10
+#define IOMMU_PC_DOMID_MATCH_REG		0x18
+#define IOMMU_PC_DEVID_MATCH_REG		0x20
+#define IOMMU_PC_COUNTER_REPORT_REG		0x28
+
+/* maximum specified bank/counters */
+#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_counters(u16 devid);
+
+int amd_iommu_pc_set_reg_val(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);
+
+#endif /*_PERF_EVENT_AMD_IOMMU_H_*/
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..d30f4b2 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -27,6 +27,8 @@
 #include <linux/amd-iommu.h>
 #include <linux/export.h>
 #include <linux/iommu.h>
+#include <asm/perf/amd/iommu.h>
+
 #include <asm/pci-direct.h>
 #include <asm/iommu.h>
 #include <asm/gart.h>
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 0bd9eb3..ac2da91 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -55,13 +55,6 @@ extern int amd_iommu_domain_set_gcr3(struct iommu_domain *dom, int pasid,
 extern int amd_iommu_domain_clear_gcr3(struct iommu_domain *dom, int pasid);
 extern struct iommu_domain *amd_iommu_get_v2_domain(struct pci_dev *pdev);
 
-/* IOMMU Performance Counter functions */
-extern bool amd_iommu_pc_supported(void);
-extern u8 amd_iommu_pc_get_max_banks(u16 devid);
-extern u8 amd_iommu_pc_get_max_counters(u16 devid);
-extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
-				    u64 *value, bool is_write);
-
 #ifdef CONFIG_IRQ_REMAP
 extern int amd_iommu_create_irq_domain(struct amd_iommu *iommu);
 #else
-- 
1.9.1

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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-5C7GfCeVMHo@public.gmane.org>
---
 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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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-5C7GfCeVMHo@public.gmane.org>
---
 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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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

Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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

Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 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

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

[Suravee Suthikulpanit]

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

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

[Suravee Suthikulpanit]

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-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 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

[PATCH V5 09/10] iommu/amd: Introduce amd_iommu_get_num_iommus()

[Suravee Suthikulpanit]

This patch introduces amd_iommu_get_num_iommus(). This is intended for
Perf AMD IOMMU driver.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 arch/x86/include/asm/perf/amd/iommu.h | 2 ++
 drivers/iommu/amd_iommu_init.c        | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/perf/amd/iommu.h b/arch/x86/include/asm/perf/amd/iommu.h
index 60ddfb6..b4c3fd2 100644
--- a/arch/x86/include/asm/perf/amd/iommu.h
+++ b/arch/x86/include/asm/perf/amd/iommu.h
@@ -25,6 +25,8 @@
 #define PC_MAX_SPEC_CNTRS			16
 
 /* amd_iommu_init.c external support functions */
+int amd_iommu_get_num_iommus(void);
+
 bool amd_iommu_pc_supported(void);
 
 u8 amd_iommu_pc_get_max_banks(int idx);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index b68a2d6..59fc4c6 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1030,7 +1030,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 
 	/* Add IOMMU to internal data structures */
 	list_add_tail(&iommu->list, &amd_iommu_list);
-	iommu->index             = amd_iommus_present++;
+	iommu->index = amd_iommus_present++;
 
 	if (unlikely(iommu->index >= MAX_IOMMUS)) {
 		WARN(1, "AMD-Vi: System has more IOMMUs than supported by this driver\n");
@@ -2259,6 +2259,11 @@ static struct amd_iommu *get_amd_iommu(int idx)
 	return iommu;
 }
 
+int amd_iommu_get_num_iommus(void)
+{
+	return amd_iommus_present;
+}
+
 /****************************************************************************
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
-- 
1.9.1

[PATCH V5 09/10] iommu/amd: Introduce amd_iommu_get_num_iommus()

[Suravee Suthikulpanit]

This patch introduces amd_iommu_get_num_iommus(). This is intended for
Perf AMD IOMMU driver.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 arch/x86/include/asm/perf/amd/iommu.h | 2 ++
 drivers/iommu/amd_iommu_init.c        | 7 ++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/perf/amd/iommu.h b/arch/x86/include/asm/perf/amd/iommu.h
index 60ddfb6..b4c3fd2 100644
--- a/arch/x86/include/asm/perf/amd/iommu.h
+++ b/arch/x86/include/asm/perf/amd/iommu.h
@@ -25,6 +25,8 @@
 #define PC_MAX_SPEC_CNTRS			16
 
 /* amd_iommu_init.c external support functions */
+int amd_iommu_get_num_iommus(void);
+
 bool amd_iommu_pc_supported(void);
 
 u8 amd_iommu_pc_get_max_banks(int idx);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index b68a2d6..59fc4c6 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1030,7 +1030,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 
 	/* Add IOMMU to internal data structures */
 	list_add_tail(&iommu->list, &amd_iommu_list);
-	iommu->index             = amd_iommus_present++;
+	iommu->index = amd_iommus_present++;
 
 	if (unlikely(iommu->index >= MAX_IOMMUS)) {
 		WARN(1, "AMD-Vi: System has more IOMMUs than supported by this driver\n");
@@ -2259,6 +2259,11 @@ static struct amd_iommu *get_amd_iommu(int idx)
 	return iommu;
 }
 
+int amd_iommu_get_num_iommus(void)
+{
+	return amd_iommus_present;
+}
+
 /****************************************************************************
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
-- 
1.9.1

[PATCH V5 10/10] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

This patch adds multi-IOMMU support for perf by exposing
an AMD IOMMU PMU for each IOMMU found in the system via:

  /sys/device/amd_iommu_x     /* where x is the IOMMU index. */

This allows users to specify different events to be programed
onto performance counters of each IOMMU.

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 | 115 ++++++++++++++++++++++++--------------------
 1 file changed, 62 insertions(+), 53 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 35a7b18..0ebadc6 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -35,10 +35,13 @@
 #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_iommu __perf_iommu;
+#define PERF_AMD_IOMMU_NAME_SZ 16
 
 struct perf_iommu {
+	struct list_head list;
 	struct pmu pmu;
+	int idx;
+	char name[PERF_AMD_IOMMU_NAME_SZ];
 	u8 max_banks;
 	u8 max_counters;
 	u64 cntr_assign_mask;
@@ -46,6 +49,8 @@ struct perf_iommu {
 	const struct attribute_group *attr_groups[4];
 };
 
+LIST_HEAD(perf_amd_iommu_list);
+
 #define format_group	attr_groups[0]
 #define cpumask_group	attr_groups[1]
 #define events_group	attr_groups[2]
@@ -196,8 +201,7 @@ static int clear_avail_iommu_bnk_cntr(struct perf_iommu *pi, u8 bank, u8 cntr)
 static int perf_iommu_event_init(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
-	struct perf_iommu *pi;
-	u64 config, config1;
+	struct perf_iommu *pi = container_of(event->pmu, struct perf_iommu, pmu);
 
 	/* test the event attr type check for PMU enumeration */
 	if (event->attr.type != event->pmu->type)
@@ -219,27 +223,17 @@ static int perf_iommu_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
-	pi = &__perf_iommu;
-
-	if (event->pmu != &pi->pmu)
-		return -ENOENT;
-
-	if (pi) {
-		config = event->attr.config;
-		config1 = event->attr.config1;
-	} else {
-		return -EINVAL;
-	}
-
 	/* update the hw_perf_event struct with the iommu config data */
-	hwc->config = config;
-	hwc->extra_reg.config = config1;
+	hwc->idx = pi->idx;
+	hwc->config = event->attr.config;
+	hwc->extra_reg.config = event->attr.config1;
 
 	return 0;
 }
 
 static void perf_iommu_enable_event(struct perf_event *ev)
 {
+	struct hw_perf_event *hwc = &ev->hw;
 	u8 csource = _GET_CSOURCE(ev);
 	u16 devid = _GET_DEVID(ev);
 	u8 bank = _GET_BANK(ev);
@@ -247,33 +241,34 @@ static void perf_iommu_enable_event(struct perf_event *ev)
 	u64 reg = 0ULL;
 
 	reg = csource;
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_COUNTER_SRC_REG, &reg);
 
 	reg = devid | (_GET_DEVID_MASK(ev) << 32);
 	if (reg)
 		reg |= BIT(31);
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_DEVID_MATCH_REG, &reg);
 
 	reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
 	if (reg)
 		reg |= BIT(31);
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_PASID_MATCH_REG, &reg);
 
 	reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
 	if (reg)
 		reg |= BIT(31);
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_DOMID_MATCH_REG, &reg);
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
+	struct hw_perf_event *hwc = &event->hw;
 	u64 reg = 0ULL;
 
-	amd_iommu_pc_set_reg(0, _GET_DEVID(event), _GET_BANK(event),
+	amd_iommu_pc_set_reg(hwc->idx, _GET_DEVID(event), _GET_BANK(event),
 			     _GET_CNTR(event), IOMMU_PC_COUNTER_SRC_REG, &reg);
 }
 
@@ -293,7 +288,7 @@ static void perf_iommu_start(struct perf_event *event, int flags)
 
 	val = local64_read(&hwc->prev_count);
 
-	amd_iommu_pc_set_counter(0, _GET_BANK(event), _GET_CNTR(event), &val);
+	amd_iommu_pc_set_counter(hwc->idx, _GET_BANK(event), _GET_CNTR(event), &val);
 enable:
 	perf_iommu_enable_event(event);
 	perf_event_update_userpage(event);
@@ -306,7 +301,7 @@ static void perf_iommu_read(struct perf_event *event)
 	s64 delta;
 	struct hw_perf_event *hwc = &event->hw;
 
-	if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), &cnt))
+	if (amd_iommu_pc_get_counter(hwc->idx, _GET_BANK(event), _GET_CNTR(event), &cnt))
 		return;
 
 	/* IOMMU pc counter register is only 48 bits */
@@ -405,13 +400,19 @@ static __init int _init_events_attrs(struct perf_iommu *pi)
 
 static __init void amd_iommu_pc_exit(void)
 {
-	if (__perf_iommu.events_group != NULL) {
-		kfree(__perf_iommu.events_group);
-		__perf_iommu.events_group = NULL;
+	struct perf_iommu *pi, *next;
+
+	list_for_each_entry_safe(pi, next, &perf_amd_iommu_list, list) {
+		list_del(&pi->list);
+
+		kfree(pi->events_group);
+		pi->events_group = NULL;
+
+		kfree(pi);
 	}
 }
 
-static __init int _init_perf_amd_iommu(struct perf_iommu *pi, char *name)
+static __init int init_one_perf_amd_iommu(struct perf_iommu *pi, int idx)
 {
 	int ret;
 
@@ -428,53 +429,61 @@ static __init int _init_perf_amd_iommu(struct perf_iommu *pi, char *name)
 	if (_init_events_attrs(pi) != 0)
 		pr_err("Only support raw events.\n");
 
-	pi->max_banks = amd_iommu_pc_get_max_banks(0);
-	pi->max_counters = amd_iommu_pc_get_max_counters(0);
+	snprintf(pi->name, PERF_AMD_IOMMU_NAME_SZ, "amd_iommu_%u", idx);
+	pi->idx = idx;
+	pi->max_banks = amd_iommu_pc_get_max_banks(idx);
+	pi->max_counters = amd_iommu_pc_get_max_counters(idx);
 	if (!pi->max_banks || !pi->max_counters)
 		return -EINVAL;
 
 	/* Init null attributes */
 	pi->null_group = NULL;
+
+	/* Setting up PMU */
+	pi->pmu.event_init = perf_iommu_event_init,
+	pi->pmu.add = perf_iommu_add,
+	pi->pmu.del = perf_iommu_del,
+	pi->pmu.start = perf_iommu_start,
+	pi->pmu.stop = perf_iommu_stop,
+	pi->pmu.read = perf_iommu_read,
 	pi->pmu.attr_groups = pi->attr_groups;
 
-	ret = perf_pmu_register(&pi->pmu, name, -1);
+	ret = perf_pmu_register(&pi->pmu, pi->name, -1);
 	if (ret) {
 		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",
-			amd_iommu_pc_get_max_banks(0),
-			amd_iommu_pc_get_max_counters(0));
+		pr_info("Detected %s, w/ %d banks, %d counters/bank\n",
+			pi->name,
+			amd_iommu_pc_get_max_banks(idx),
+			amd_iommu_pc_get_max_counters(idx));
+
+		list_add_tail(&pi->list, &perf_amd_iommu_list);
 	}
 
 	return ret;
 }
 
-static struct perf_iommu __perf_iommu = {
-	.pmu = {
-		.event_init	= perf_iommu_event_init,
-		.add		= perf_iommu_add,
-		.del		= perf_iommu_del,
-		.start		= perf_iommu_start,
-		.stop		= perf_iommu_stop,
-		.read		= perf_iommu_read,
-	},
-	.max_banks		= 0x00,
-	.max_counters		= 0x00,
-	.cntr_assign_mask	= 0ULL,
-	.format_group		= NULL,
-	.cpumask_group		= NULL,
-	.events_group		= NULL,
-	.null_group		= NULL,
-};
-
 static __init int amd_iommu_pc_init(void)
 {
+	int i;
+
 	/* Make sure the IOMMU PC resource is available */
 	if (!amd_iommu_pc_supported())
 		return -ENODEV;
 
-	_init_perf_amd_iommu(&__perf_iommu, "amd_iommu");
+	for (i = 0 ; i < amd_iommu_get_num_iommus(); i++) {
+		int ret;
+		struct perf_iommu *pi;
+
+		pi = kzalloc(sizeof(struct perf_iommu), GFP_KERNEL);
+		if (!pi)
+			return -ENOMEM;
+
+		ret = init_one_perf_amd_iommu(pi, i);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
-- 
1.9.1

[PATCH V5 10/10] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

This patch adds multi-IOMMU support for perf by exposing
an AMD IOMMU PMU for each IOMMU found in the system via:

  /sys/device/amd_iommu_x     /* where x is the IOMMU index. */

This allows users to specify different events to be programed
onto performance counters of each IOMMU.

Cc: Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
---
 arch/x86/events/amd/iommu.c | 115 ++++++++++++++++++++++++--------------------
 1 file changed, 62 insertions(+), 53 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 35a7b18..0ebadc6 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -35,10 +35,13 @@
 #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_iommu __perf_iommu;
+#define PERF_AMD_IOMMU_NAME_SZ 16
 
 struct perf_iommu {
+	struct list_head list;
 	struct pmu pmu;
+	int idx;
+	char name[PERF_AMD_IOMMU_NAME_SZ];
 	u8 max_banks;
 	u8 max_counters;
 	u64 cntr_assign_mask;
@@ -46,6 +49,8 @@ struct perf_iommu {
 	const struct attribute_group *attr_groups[4];
 };
 
+LIST_HEAD(perf_amd_iommu_list);
+
 #define format_group	attr_groups[0]
 #define cpumask_group	attr_groups[1]
 #define events_group	attr_groups[2]
@@ -196,8 +201,7 @@ static int clear_avail_iommu_bnk_cntr(struct perf_iommu *pi, u8 bank, u8 cntr)
 static int perf_iommu_event_init(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
-	struct perf_iommu *pi;
-	u64 config, config1;
+	struct perf_iommu *pi = container_of(event->pmu, struct perf_iommu, pmu);
 
 	/* test the event attr type check for PMU enumeration */
 	if (event->attr.type != event->pmu->type)
@@ -219,27 +223,17 @@ static int perf_iommu_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
-	pi = &__perf_iommu;
-
-	if (event->pmu != &pi->pmu)
-		return -ENOENT;
-
-	if (pi) {
-		config = event->attr.config;
-		config1 = event->attr.config1;
-	} else {
-		return -EINVAL;
-	}
-
 	/* update the hw_perf_event struct with the iommu config data */
-	hwc->config = config;
-	hwc->extra_reg.config = config1;
+	hwc->idx = pi->idx;
+	hwc->config = event->attr.config;
+	hwc->extra_reg.config = event->attr.config1;
 
 	return 0;
 }
 
 static void perf_iommu_enable_event(struct perf_event *ev)
 {
+	struct hw_perf_event *hwc = &ev->hw;
 	u8 csource = _GET_CSOURCE(ev);
 	u16 devid = _GET_DEVID(ev);
 	u8 bank = _GET_BANK(ev);
@@ -247,33 +241,34 @@ static void perf_iommu_enable_event(struct perf_event *ev)
 	u64 reg = 0ULL;
 
 	reg = csource;
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_COUNTER_SRC_REG, &reg);
 
 	reg = devid | (_GET_DEVID_MASK(ev) << 32);
 	if (reg)
 		reg |= BIT(31);
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_DEVID_MATCH_REG, &reg);
 
 	reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
 	if (reg)
 		reg |= BIT(31);
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_PASID_MATCH_REG, &reg);
 
 	reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
 	if (reg)
 		reg |= BIT(31);
-	amd_iommu_pc_set_reg(0, devid, bank, cntr,
+	amd_iommu_pc_set_reg(hwc->idx, devid, bank, cntr,
 			     IOMMU_PC_DOMID_MATCH_REG, &reg);
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
+	struct hw_perf_event *hwc = &event->hw;
 	u64 reg = 0ULL;
 
-	amd_iommu_pc_set_reg(0, _GET_DEVID(event), _GET_BANK(event),
+	amd_iommu_pc_set_reg(hwc->idx, _GET_DEVID(event), _GET_BANK(event),
 			     _GET_CNTR(event), IOMMU_PC_COUNTER_SRC_REG, &reg);
 }
 
@@ -293,7 +288,7 @@ static void perf_iommu_start(struct perf_event *event, int flags)
 
 	val = local64_read(&hwc->prev_count);
 
-	amd_iommu_pc_set_counter(0, _GET_BANK(event), _GET_CNTR(event), &val);
+	amd_iommu_pc_set_counter(hwc->idx, _GET_BANK(event), _GET_CNTR(event), &val);
 enable:
 	perf_iommu_enable_event(event);
 	perf_event_update_userpage(event);
@@ -306,7 +301,7 @@ static void perf_iommu_read(struct perf_event *event)
 	s64 delta;
 	struct hw_perf_event *hwc = &event->hw;
 
-	if (amd_iommu_pc_get_counter(0, _GET_BANK(event), _GET_CNTR(event), &cnt))
+	if (amd_iommu_pc_get_counter(hwc->idx, _GET_BANK(event), _GET_CNTR(event), &cnt))
 		return;
 
 	/* IOMMU pc counter register is only 48 bits */
@@ -405,13 +400,19 @@ static __init int _init_events_attrs(struct perf_iommu *pi)
 
 static __init void amd_iommu_pc_exit(void)
 {
-	if (__perf_iommu.events_group != NULL) {
-		kfree(__perf_iommu.events_group);
-		__perf_iommu.events_group = NULL;
+	struct perf_iommu *pi, *next;
+
+	list_for_each_entry_safe(pi, next, &perf_amd_iommu_list, list) {
+		list_del(&pi->list);
+
+		kfree(pi->events_group);
+		pi->events_group = NULL;
+
+		kfree(pi);
 	}
 }
 
-static __init int _init_perf_amd_iommu(struct perf_iommu *pi, char *name)
+static __init int init_one_perf_amd_iommu(struct perf_iommu *pi, int idx)
 {
 	int ret;
 
@@ -428,53 +429,61 @@ static __init int _init_perf_amd_iommu(struct perf_iommu *pi, char *name)
 	if (_init_events_attrs(pi) != 0)
 		pr_err("Only support raw events.\n");
 
-	pi->max_banks = amd_iommu_pc_get_max_banks(0);
-	pi->max_counters = amd_iommu_pc_get_max_counters(0);
+	snprintf(pi->name, PERF_AMD_IOMMU_NAME_SZ, "amd_iommu_%u", idx);
+	pi->idx = idx;
+	pi->max_banks = amd_iommu_pc_get_max_banks(idx);
+	pi->max_counters = amd_iommu_pc_get_max_counters(idx);
 	if (!pi->max_banks || !pi->max_counters)
 		return -EINVAL;
 
 	/* Init null attributes */
 	pi->null_group = NULL;
+
+	/* Setting up PMU */
+	pi->pmu.event_init = perf_iommu_event_init,
+	pi->pmu.add = perf_iommu_add,
+	pi->pmu.del = perf_iommu_del,
+	pi->pmu.start = perf_iommu_start,
+	pi->pmu.stop = perf_iommu_stop,
+	pi->pmu.read = perf_iommu_read,
 	pi->pmu.attr_groups = pi->attr_groups;
 
-	ret = perf_pmu_register(&pi->pmu, name, -1);
+	ret = perf_pmu_register(&pi->pmu, pi->name, -1);
 	if (ret) {
 		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",
-			amd_iommu_pc_get_max_banks(0),
-			amd_iommu_pc_get_max_counters(0));
+		pr_info("Detected %s, w/ %d banks, %d counters/bank\n",
+			pi->name,
+			amd_iommu_pc_get_max_banks(idx),
+			amd_iommu_pc_get_max_counters(idx));
+
+		list_add_tail(&pi->list, &perf_amd_iommu_list);
 	}
 
 	return ret;
 }
 
-static struct perf_iommu __perf_iommu = {
-	.pmu = {
-		.event_init	= perf_iommu_event_init,
-		.add		= perf_iommu_add,
-		.del		= perf_iommu_del,
-		.start		= perf_iommu_start,
-		.stop		= perf_iommu_stop,
-		.read		= perf_iommu_read,
-	},
-	.max_banks		= 0x00,
-	.max_counters		= 0x00,
-	.cntr_assign_mask	= 0ULL,
-	.format_group		= NULL,
-	.cpumask_group		= NULL,
-	.events_group		= NULL,
-	.null_group		= NULL,
-};
-
 static __init int amd_iommu_pc_init(void)
 {
+	int i;
+
 	/* Make sure the IOMMU PC resource is available */
 	if (!amd_iommu_pc_supported())
 		return -ENODEV;
 
-	_init_perf_amd_iommu(&__perf_iommu, "amd_iommu");
+	for (i = 0 ; i < amd_iommu_get_num_iommus(); i++) {
+		int ret;
+		struct perf_iommu *pi;
+
+		pi = kzalloc(sizeof(struct perf_iommu), GFP_KERNEL);
+		if (!pi)
+			return -ENOMEM;
+
+		ret = init_one_perf_amd_iommu(pi, i);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }
-- 
1.9.1

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

[Joerg Roedel]

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

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

[Joerg Roedel]

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

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

[Suravee Suthikulpanit]

Hi Joerg,

On 2/25/16 21:54, Joerg Roedel wrote:
> 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
>

I'll do that and send out V6.

Thanks,
Suravee

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

[Suravee Suthikulpanit]

Hi Joerg,

On 2/25/16 21:54, Joerg Roedel wrote:
> 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
>

I'll do that and send out V6.

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> First, this patch move arch/x86/events/amd/iommu.h to
> arch/x86/include/asm/perf/amd/iommu.h so that we easily include
> it in both perf-amd-iommu and amd-iommu drivers.
> 
> Then, we consolidate declaration of AMD IOMMU performance counter
> APIs into one file.

These seem two independent thingies; should this therefore not be 2
patches?

> Reviewed-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
>  arch/x86/events/amd/iommu.c           |  2 +-
>  arch/x86/events/amd/iommu.h           | 40 ---------------------------------
>  arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++

That seems somewhat excessive. Not only do you create
arch/x86/include/asm/perf/ you then put another directory on top of
that.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> 
> First, this patch move arch/x86/events/amd/iommu.h to
> arch/x86/include/asm/perf/amd/iommu.h so that we easily include
> it in both perf-amd-iommu and amd-iommu drivers.
> 
> Then, we consolidate declaration of AMD IOMMU performance counter
> APIs into one file.

These seem two independent thingies; should this therefore not be 2
patches?

> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> ---
>  arch/x86/events/amd/iommu.c           |  2 +-
>  arch/x86/events/amd/iommu.h           | 40 ---------------------------------
>  arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++

That seems somewhat excessive. Not only do you create
arch/x86/include/asm/perf/ you then put another directory on top of
that.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi,

On 03/12/2016 08:22 PM, Peter Zijlstra wrote:
> On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>
>> First, this patch move arch/x86/events/amd/iommu.h to
>> arch/x86/include/asm/perf/amd/iommu.h so that we easily include
>> it in both perf-amd-iommu and amd-iommu drivers.
>>
>> Then, we consolidate declaration of AMD IOMMU performance counter
>> APIs into one file.
>
> These seem two independent thingies; should this therefore not be 2
> patches?
>
>> Reviewed-by: Joerg Roedel <jroedel@suse.de>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> ---
>>   arch/x86/events/amd/iommu.c           |  2 +-
>>   arch/x86/events/amd/iommu.h           | 40 ---------------------------------
>>   arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
>
> That seems somewhat excessive. Not only do you create
> arch/x86/include/asm/perf/ you then put another directory on top of
> that.
>

The original header files (arch/x86/events/amd/iommu.h and 
drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. 
So, with the new header file being in the 
arch/x86/include/asm/perf/amd/iommu.h, we can just have one function 
declaration.

So, you just want to separate the file moving part and the part that 
removes of the duplication?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi,

On 03/12/2016 08:22 PM, Peter Zijlstra wrote:
> On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
>>
>> First, this patch move arch/x86/events/amd/iommu.h to
>> arch/x86/include/asm/perf/amd/iommu.h so that we easily include
>> it in both perf-amd-iommu and amd-iommu drivers.
>>
>> Then, we consolidate declaration of AMD IOMMU performance counter
>> APIs into one file.
>
> These seem two independent thingies; should this therefore not be 2
> patches?
>
>> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
>> ---
>>   arch/x86/events/amd/iommu.c           |  2 +-
>>   arch/x86/events/amd/iommu.h           | 40 ---------------------------------
>>   arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
>
> That seems somewhat excessive. Not only do you create
> arch/x86/include/asm/perf/ you then put another directory on top of
> that.
>

The original header files (arch/x86/events/amd/iommu.h and 
drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. 
So, with the new header file being in the 
arch/x86/include/asm/perf/amd/iommu.h, we can just have one function 
declaration.

So, you just want to separate the file moving part and the part that 
removes of the duplication?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Mon, Mar 14, 2016 at 12:26:00PM +0700, Suravee Suthikulpanit wrote:
> Hi,
> 
> On 03/12/2016 08:22 PM, Peter Zijlstra wrote:
> >On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
> >>From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >>
> >>First, this patch move arch/x86/events/amd/iommu.h to
> >>arch/x86/include/asm/perf/amd/iommu.h so that we easily include
> >>it in both perf-amd-iommu and amd-iommu drivers.
> >>
> >>Then, we consolidate declaration of AMD IOMMU performance counter
> >>APIs into one file.
> >
> >These seem two independent thingies; should this therefore not be 2
> >patches?
> >
> >>Reviewed-by: Joerg Roedel <jroedel@suse.de>
> >>Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>---
> >>  arch/x86/events/amd/iommu.c           |  2 +-
> >>  arch/x86/events/amd/iommu.h           | 40 ---------------------------------
> >>  arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
> >
> >That seems somewhat excessive. Not only do you create
> >arch/x86/include/asm/perf/ you then put another directory on top of
> >that.
> >
> 
> The original header files (arch/x86/events/amd/iommu.h and
> drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. So,
> with the new header file being in the arch/x86/include/asm/perf/amd/iommu.h,
> we can just have one function declaration.
> 
> So, you just want to separate the file moving part and the part that removes
> of the duplication?

I'm fine with a new header, it just seems putting it in a two deep
direcotry hierarchy of its own that seems excessive.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Mon, Mar 14, 2016 at 12:26:00PM +0700, Suravee Suthikulpanit wrote:
> Hi,
> 
> On 03/12/2016 08:22 PM, Peter Zijlstra wrote:
> >On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
> >>From: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> >>
> >>First, this patch move arch/x86/events/amd/iommu.h to
> >>arch/x86/include/asm/perf/amd/iommu.h so that we easily include
> >>it in both perf-amd-iommu and amd-iommu drivers.
> >>
> >>Then, we consolidate declaration of AMD IOMMU performance counter
> >>APIs into one file.
> >
> >These seem two independent thingies; should this therefore not be 2
> >patches?
> >
> >>Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> >>Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
> >>---
> >>  arch/x86/events/amd/iommu.c           |  2 +-
> >>  arch/x86/events/amd/iommu.h           | 40 ---------------------------------
> >>  arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
> >
> >That seems somewhat excessive. Not only do you create
> >arch/x86/include/asm/perf/ you then put another directory on top of
> >that.
> >
> 
> The original header files (arch/x86/events/amd/iommu.h and
> drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. So,
> with the new header file being in the arch/x86/include/asm/perf/amd/iommu.h,
> we can just have one function declaration.
> 
> So, you just want to separate the file moving part and the part that removes
> of the duplication?

I'm fine with a new header, it just seems putting it in a two deep
direcotry hierarchy of its own that seems excessive.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi,

On 3/14/16 16:58, Peter Zijlstra wrote:
> On Mon, Mar 14, 2016 at 12:26:00PM +0700, Suravee Suthikulpanit wrote:
>> Hi,
>>
>> On 03/12/2016 08:22 PM, Peter Zijlstra wrote:
>>> On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
>>>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>>
>>>> First, this patch move arch/x86/events/amd/iommu.h to
>>>> arch/x86/include/asm/perf/amd/iommu.h so that we easily include
>>>> it in both perf-amd-iommu and amd-iommu drivers.
>>>>
>>>> Then, we consolidate declaration of AMD IOMMU performance counter
>>>> APIs into one file.
>>>
>>> These seem two independent thingies; should this therefore not be 2
>>> patches?
>>>
>>>> Reviewed-by: Joerg Roedel <jroedel@suse.de>
>>>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>> ---
>>>>   arch/x86/events/amd/iommu.c           |  2 +-
>>>>   arch/x86/events/amd/iommu.h           | 40 ---------------------------------
>>>>   arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
>>>
>>> That seems somewhat excessive. Not only do you create
>>> arch/x86/include/asm/perf/ you then put another directory on top of
>>> that.
>>>
>>
>> The original header files (arch/x86/events/amd/iommu.h and
>> drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. So,
>> with the new header file being in the arch/x86/include/asm/perf/amd/iommu.h,
>> we can just have one function declaration.
>>
>> So, you just want to separate the file moving part and the part that removes
>> of the duplication?
>
> I'm fine with a new header, it just seems putting it in a two deep
> direcotry hierarchy of its own that seems excessive.
>

Basically, we are trying to match the current Perf hierarchy for AMD 
IOMMU (arch/x86/events/amd/iommu.c). I can put it into 
arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi,

On 3/14/16 16:58, Peter Zijlstra wrote:
> On Mon, Mar 14, 2016 at 12:26:00PM +0700, Suravee Suthikulpanit wrote:
>> Hi,
>>
>> On 03/12/2016 08:22 PM, Peter Zijlstra wrote:
>>> On Tue, Feb 23, 2016 at 08:12:36AM -0600, Suravee Suthikulpanit wrote:
>>>> From: Suravee Suthikulpanit <suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>
>>>>
>>>> First, this patch move arch/x86/events/amd/iommu.h to
>>>> arch/x86/include/asm/perf/amd/iommu.h so that we easily include
>>>> it in both perf-amd-iommu and amd-iommu drivers.
>>>>
>>>> Then, we consolidate declaration of AMD IOMMU performance counter
>>>> APIs into one file.
>>>
>>> These seem two independent thingies; should this therefore not be 2
>>> patches?
>>>
>>>> Reviewed-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
>>>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit-5C7GfCeVMHo@public.gmane.org>
>>>> ---
>>>>   arch/x86/events/amd/iommu.c           |  2 +-
>>>>   arch/x86/events/amd/iommu.h           | 40 ---------------------------------
>>>>   arch/x86/include/asm/perf/amd/iommu.h | 42 +++++++++++++++++++++++++++++++++++
>>>
>>> That seems somewhat excessive. Not only do you create
>>> arch/x86/include/asm/perf/ you then put another directory on top of
>>> that.
>>>
>>
>> The original header files (arch/x86/events/amd/iommu.h and
>> drivers/iommu/amd_iommu_proto.h) has duplicate function declarations. So,
>> with the new header file being in the arch/x86/include/asm/perf/amd/iommu.h,
>> we can just have one function declaration.
>>
>> So, you just want to separate the file moving part and the part that removes
>> of the duplication?
>
> I'm fine with a new header, it just seems putting it in a two deep
> direcotry hierarchy of its own that seems excessive.
>

Basically, we are trying to match the current Perf hierarchy for AMD 
IOMMU (arch/x86/events/amd/iommu.c). I can put it into 
arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote:
> Basically, we are trying to match the current Perf hierarchy for AMD IOMMU
> (arch/x86/events/amd/iommu.c). I can put it into
> arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?

Yeah, I was going to say the same thing - match the hierarchy so that
there are no confusions between paths. Makes sense to me.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote:
> Basically, we are trying to match the current Perf hierarchy for AMD IOMMU
> (arch/x86/events/amd/iommu.c). I can put it into
> arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?

Yeah, I was going to say the same thing - match the hierarchy so that
there are no confusions between paths. Makes sense to me.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Mon, Mar 14, 2016 at 03:19:45PM +0100, Borislav Petkov wrote:
> On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote:
> > Basically, we are trying to match the current Perf hierarchy for AMD IOMMU
> > (arch/x86/events/amd/iommu.c). I can put it into
> > arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?
> 
> Yeah, I was going to say the same thing - match the hierarchy so that
> there are no confusions between paths. Makes sense to me.

Well there's still the 'perf' vs' events' thing, but also what other
files did you want to put there?

For now I think I prefer a filename without extra directories; we can
always move files about if there's more use later.

Also, since its being used by both events/amd/iommu.c and
drivers/iommu/amd_iommu.c you can also chose a name in the latter
namespace.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Mon, Mar 14, 2016 at 03:19:45PM +0100, Borislav Petkov wrote:
> On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote:
> > Basically, we are trying to match the current Perf hierarchy for AMD IOMMU
> > (arch/x86/events/amd/iommu.c). I can put it into
> > arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?
> 
> Yeah, I was going to say the same thing - match the hierarchy so that
> there are no confusions between paths. Makes sense to me.

Well there's still the 'perf' vs' events' thing, but also what other
files did you want to put there?

For now I think I prefer a filename without extra directories; we can
always move files about if there's more use later.

Also, since its being used by both events/amd/iommu.c and
drivers/iommu/amd_iommu.c you can also chose a name in the latter
namespace.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi Peter/Boris/Joerg,

On 3/14/16 23:39, Peter Zijlstra wrote:
> On Mon, Mar 14, 2016 at 03:19:45PM +0100, Borislav Petkov wrote:
>> On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote:
>>> Basically, we are trying to match the current Perf hierarchy for AMD IOMMU
>>> (arch/x86/events/amd/iommu.c). I can put it into
>>> arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?
>>
>> Yeah, I was going to say the same thing - match the hierarchy so that
>> there are no confusions between paths. Makes sense to me.
>
> Well there's still the 'perf' vs' events' thing, but also what other
> files did you want to put there?
>
> For now I think I prefer a filename without extra directories; we can
> always move files about if there's more use later.
>
> Also, since its being used by both events/amd/iommu.c and
> drivers/iommu/amd_iommu.c you can also chose a name in the latter
> namespace.
>

Actually, I also found that there is currently the 
include/linux/amd-iommu.h, which contains extern function declarations 
defined in drivers/iommu/amd_iommu_init.c and drivers/iommu/amd_iommu_v2.c.

What if I just merge the newly introduced 
arch/x86/include/perf/amd/iommu.h into the include/linux/amd-iommu.h? I 
do not see the point of having to separate things out into two files.

Joerg, since you were maintaining that file, do you have any objection?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi Peter/Boris/Joerg,

On 3/14/16 23:39, Peter Zijlstra wrote:
> On Mon, Mar 14, 2016 at 03:19:45PM +0100, Borislav Petkov wrote:
>> On Mon, Mar 14, 2016 at 08:37:02PM +0700, Suravee Suthikulpanit wrote:
>>> Basically, we are trying to match the current Perf hierarchy for AMD IOMMU
>>> (arch/x86/events/amd/iommu.c). I can put it into
>>> arch/x86/include/asm/perf_amd_iommu.h. What would you prefer?
>>
>> Yeah, I was going to say the same thing - match the hierarchy so that
>> there are no confusions between paths. Makes sense to me.
>
> Well there's still the 'perf' vs' events' thing, but also what other
> files did you want to put there?
>
> For now I think I prefer a filename without extra directories; we can
> always move files about if there's more use later.
>
> Also, since its being used by both events/amd/iommu.c and
> drivers/iommu/amd_iommu.c you can also chose a name in the latter
> namespace.
>

Actually, I also found that there is currently the 
include/linux/amd-iommu.h, which contains extern function declarations 
defined in drivers/iommu/amd_iommu_init.c and drivers/iommu/amd_iommu_v2.c.

What if I just merge the newly introduced 
arch/x86/include/perf/amd/iommu.h into the include/linux/amd-iommu.h? I 
do not see the point of having to separate things out into two files.

Joerg, since you were maintaining that file, do you have any objection?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
> What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
> into the include/linux/amd-iommu.h? I do not see the point of having to
> separate things out into two files.
> 

Works for me. Thanks!

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
> What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
> into the include/linux/amd-iommu.h? I do not see the point of having to
> separate things out into two files.
> 

Works for me. Thanks!

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
> What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
> into the include/linux/amd-iommu.h? I do not see the point of having to
> separate things out into two files.

Except that this header has x86-specific stuff and include/linux/ is
arch-agnostic.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
> What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
> into the include/linux/amd-iommu.h? I do not see the point of having to
> separate things out into two files.

Except that this header has x86-specific stuff and include/linux/ is
arch-agnostic.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Tue, Mar 15, 2016 at 11:40:17AM +0100, Borislav Petkov wrote:
> On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
> > What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
> > into the include/linux/amd-iommu.h? I do not see the point of having to
> > separate things out into two files.
> 
> Except that this header has x86-specific stuff and include/linux/ is
> arch-agnostic.

Which would suggest that header is placed wrong, because I would expect
the amd-iommu to really be rather x86 specific. Or is AMD making ARM
parts for which this is useful too?

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Peter Zijlstra]

On Tue, Mar 15, 2016 at 11:40:17AM +0100, Borislav Petkov wrote:
> On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
> > What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
> > into the include/linux/amd-iommu.h? I do not see the point of having to
> > separate things out into two files.
> 
> Except that this header has x86-specific stuff and include/linux/ is
> arch-agnostic.

Which would suggest that header is placed wrong, because I would expect
the amd-iommu to really be rather x86 specific. Or is AMD making ARM
parts for which this is useful too?

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi

On 03/15/2016 05:53 PM, Peter Zijlstra wrote:
> On Tue, Mar 15, 2016 at 11:40:17AM +0100, Borislav Petkov wrote:
>> On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
>>> What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
>>> into the include/linux/amd-iommu.h? I do not see the point of having to
>>> separate things out into two files.
>>
>> Except that this header has x86-specific stuff and include/linux/ is
>> arch-agnostic.
>
> Which would suggest that header is placed wrong, because I would expect
> the amd-iommu to really be rather x86 specific. Or is AMD making ARM
> parts for which this is useful too?
>

Actually the exposed APIs (in both files) are from the AMD IOMMU driver, 
which is not necessary x86-specific. They mostly use struct pci_dev, 
which is also arch-agnostic. It is correct that the current IOMMU IP is 
only available in x86 systems. However, if AMD plans to use the IOMMU IP 
in the ARM-based processors in the future, putting these into 
include/linux/amd-iommu.h would work better.

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi

On 03/15/2016 05:53 PM, Peter Zijlstra wrote:
> On Tue, Mar 15, 2016 at 11:40:17AM +0100, Borislav Petkov wrote:
>> On Tue, Mar 15, 2016 at 07:39:31AM +0700, Suravee Suthikulpanit wrote:
>>> What if I just merge the newly introduced arch/x86/include/perf/amd/iommu.h
>>> into the include/linux/amd-iommu.h? I do not see the point of having to
>>> separate things out into two files.
>>
>> Except that this header has x86-specific stuff and include/linux/ is
>> arch-agnostic.
>
> Which would suggest that header is placed wrong, because I would expect
> the amd-iommu to really be rather x86 specific. Or is AMD making ARM
> parts for which this is useful too?
>

Actually the exposed APIs (in both files) are from the AMD IOMMU driver, 
which is not necessary x86-specific. They mostly use struct pci_dev, 
which is also arch-agnostic. It is correct that the current IOMMU IP is 
only available in x86 systems. However, if AMD plans to use the IOMMU IP 
in the ARM-based processors in the future, putting these into 
include/linux/amd-iommu.h would work better.

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 02:07:25PM +0700, Suravee Suthikulpanit wrote:
> Actually the exposed APIs (in both files) are from the AMD IOMMU driver,
> which is not necessary x86-specific. They mostly use struct pci_dev, which
> is also arch-agnostic. It is correct that the current IOMMU IP is only
> available in x86 systems. However, if AMD plans to use the IOMMU IP in the
> ARM-based processors in the future, putting these into
> include/linux/amd-iommu.h would work better.

Let's wait until AMD does that then. Moving the header is the easiest part.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 02:07:25PM +0700, Suravee Suthikulpanit wrote:
> Actually the exposed APIs (in both files) are from the AMD IOMMU driver,
> which is not necessary x86-specific. They mostly use struct pci_dev, which
> is also arch-agnostic. It is correct that the current IOMMU IP is only
> available in x86 systems. However, if AMD plans to use the IOMMU IP in the
> ARM-based processors in the future, putting these into
> include/linux/amd-iommu.h would work better.

Let's wait until AMD does that then. Moving the header is the easiest part.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi Boris,

On 03/18/2016 04:04 PM, Borislav Petkov wrote:
> On Fri, Mar 18, 2016 at 02:07:25PM +0700, Suravee Suthikulpanit wrote:
>> Actually the exposed APIs (in both files) are from the AMD IOMMU driver,
>> which is not necessary x86-specific. They mostly use struct pci_dev, which
>> is also arch-agnostic. It is correct that the current IOMMU IP is only
>> available in x86 systems. However, if AMD plans to use the IOMMU IP in the
>> ARM-based processors in the future, putting these into
>> include/linux/amd-iommu.h would work better.
>
> Let's wait until AMD does that then. Moving the header is the easiest part.
>

But the whole point is that since we are moving it to consolidate these 
duplicated declarations, I think we should just put it in the most 
common place. The include/linux/amd-iommu.h file is already there. It's 
not like we have to create a brand new file, and then having to move it 
later.

Regards,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

Hi Boris,

On 03/18/2016 04:04 PM, Borislav Petkov wrote:
> On Fri, Mar 18, 2016 at 02:07:25PM +0700, Suravee Suthikulpanit wrote:
>> Actually the exposed APIs (in both files) are from the AMD IOMMU driver,
>> which is not necessary x86-specific. They mostly use struct pci_dev, which
>> is also arch-agnostic. It is correct that the current IOMMU IP is only
>> available in x86 systems. However, if AMD plans to use the IOMMU IP in the
>> ARM-based processors in the future, putting these into
>> include/linux/amd-iommu.h would work better.
>
> Let's wait until AMD does that then. Moving the header is the easiest part.
>

But the whole point is that since we are moving it to consolidate these 
duplicated declarations, I think we should just put it in the most 
common place. The include/linux/amd-iommu.h file is already there. It's 
not like we have to create a brand new file, and then having to move it 
later.

Regards,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 04:09:33PM +0700, Suravee Suthikulpanit wrote:
> But the whole point is that since we are moving it to consolidate these
> duplicated declarations, I think we should just put it in the most common
> place. The include/linux/amd-iommu.h file is already there. It's not like we
> have to create a brand new file, and then having to move it later.

Strictly speaking, it was wrong to put it there in the first place as it
is x86-specific.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

On 03/18/2016 04:29 PM, Borislav Petkov wrote:
> On Fri, Mar 18, 2016 at 04:09:33PM +0700, Suravee Suthikulpanit wrote:
>> But the whole point is that since we are moving it to consolidate these
>> duplicated declarations, I think we should just put it in the most common
>> place. The include/linux/amd-iommu.h file is already there. It's not like we
>> have to create a brand new file, and then having to move it later.
>
> Strictly speaking, it was wrong to put it there in the first place as it
> is x86-specific.
>

If not here, where would you prefer? Consolidating it to 
arch/x86/include/asm/amd-iommu.h)?

And if that's the case, should I also move include/linux/amd-iommu.h as 
well?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Suravee Suthikulpanit]

On 03/18/2016 04:29 PM, Borislav Petkov wrote:
> On Fri, Mar 18, 2016 at 04:09:33PM +0700, Suravee Suthikulpanit wrote:
>> But the whole point is that since we are moving it to consolidate these
>> duplicated declarations, I think we should just put it in the most common
>> place. The include/linux/amd-iommu.h file is already there. It's not like we
>> have to create a brand new file, and then having to move it later.
>
> Strictly speaking, it was wrong to put it there in the first place as it
> is x86-specific.
>

If not here, where would you prefer? Consolidating it to 
arch/x86/include/asm/amd-iommu.h)?

And if that's the case, should I also move include/linux/amd-iommu.h as 
well?

Thanks,
Suravee

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 05:06:23PM +0700, Suravee Suthikulpanit wrote:
> If not here, where would you prefer? Consolidating it to
> arch/x86/include/asm/amd-iommu.h)?
> 
> And if that's the case, should I also move include/linux/amd-iommu.h as
> well?

Yeah, so arch/x86/include/asm/ has all the x86-specific stuff which is
not exported to userspace, so moving stuff there makes sense to me.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 05:06:23PM +0700, Suravee Suthikulpanit wrote:
> If not here, where would you prefer? Consolidating it to
> arch/x86/include/asm/amd-iommu.h)?
> 
> And if that's the case, should I also move include/linux/amd-iommu.h as
> well?

Yeah, so arch/x86/include/asm/ has all the x86-specific stuff which is
not exported to userspace, so moving stuff there makes sense to me.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Joerg Roedel]

On Fri, Mar 18, 2016 at 11:39:18AM +0100, Borislav Petkov wrote:
> Yeah, so arch/x86/include/asm/ has all the x86-specific stuff which is
> not exported to userspace, so moving stuff there makes sense to me.

While the AMD IOMMU is only available on x86 today, there is nothing
x86-specific in its architecture, so I don't think its header files
belong to arch/x86.

By that reasoning we could also move all of the intel gpu stuff to
arch/x86. But we don't do it, because it doesn't make sense and because
it doesn't belong there.


Regards,

	Joerg

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 12:11:01PM +0100, Joerg Roedel wrote:
> On Fri, Mar 18, 2016 at 11:39:18AM +0100, Borislav Petkov wrote:
> > Yeah, so arch/x86/include/asm/ has all the x86-specific stuff which is
> > not exported to userspace, so moving stuff there makes sense to me.
> 
> While the AMD IOMMU is only available on x86 today, there is nothing
> x86-specific in its architecture, so I don't think its header files
> belong to arch/x86.
> 
> By that reasoning we could also move all of the intel gpu stuff to
> arch/x86. But we don't do it, because it doesn't make sense and because
> it doesn't belong there.

And according to that argument of yours, we should move everything
which is not too close to the arch it is being used on, to generic
include/linux/. Because it might possibly get used some day by other
arches. Yeah right.

And looking at include/linux - it looks like a dumping ground for
headers. :-\

I guess anything is better than putting non-generic enough stuff in
include/linux/. Maybe drivers/include or something in that direction
would be better...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

Re: [PATCH V5 02/10] perf/amd/iommu: Consolidate and move perf_event_amd_iommu header

[Borislav Petkov]

On Fri, Mar 18, 2016 at 12:11:01PM +0100, Joerg Roedel wrote:
> On Fri, Mar 18, 2016 at 11:39:18AM +0100, Borislav Petkov wrote:
> > Yeah, so arch/x86/include/asm/ has all the x86-specific stuff which is
> > not exported to userspace, so moving stuff there makes sense to me.
> 
> While the AMD IOMMU is only available on x86 today, there is nothing
> x86-specific in its architecture, so I don't think its header files
> belong to arch/x86.
> 
> By that reasoning we could also move all of the intel gpu stuff to
> arch/x86. But we don't do it, because it doesn't make sense and because
> it doesn't belong there.

And according to that argument of yours, we should move everything
which is not too close to the arch it is being used on, to generic
include/linux/. Because it might possibly get used some day by other
arches. Yeah right.

And looking at include/linux - it looks like a dumping ground for
headers. :-\

I guess anything is better than putting non-generic enough stuff in
include/linux/. Maybe drivers/include or something in that direction
would be better...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.