[PATCH v2 0/5] Re-enable IMC trace-mode

[PATCH v2 0/5] Re-enable IMC trace-mode

[Anju T Sudhakar]

commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"               
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent       
mode switching between accumulation mode and trace mode via the spr LDBAR  
in the hardware can trigger a checkstop(system crash).

This patch series re-enables IMC trace mode and fixes the mode switching
issue by global lock mechanism.

Patch 3/5,4/5 and 5/5 provides a selftest to verify the global-lock
mechanism.

Changes from v1 -> v2:
-------------------------
- Added self test patches to the series.

Anju T Sudhakar (2):
  powerpc/powernv: Re-enable imc trace-mode in kernel
  powerpc/perf: Implement a global lock to avoid races between trace,
    core and thread imc events.

Madhavan Srinivasan (3):
  powerpc/perf: Add an interface sub-folder to imc pmu
  selftest/powerpc/pmc: Support to include interface test for Memory
    Counter PMUs
  selftest/powerpc/pmu: Testcase for imc global lock mechanism


 arch/powerpc/include/asm/imc-pmu.h            |  11 +-
 arch/powerpc/perf/imc-pmu.c                   | 196 +++++++++++++++---
 arch/powerpc/platforms/powernv/opal-imc.c     |   9 +-
 tools/testing/selftests/powerpc/pmu/Makefile  |   7 +-
 .../powerpc/pmu/mem_counters/Makefile         |  21 ++
 .../pmu/mem_counters/imc_global_lock_test.c   |  68 ++++++
 .../powerpc/pmu/mem_counters/mem_counters.c   |  99 +++++++++
 .../powerpc/pmu/mem_counters/mem_counters.h   |  36 ++++
 8 files changed, 408 insertions(+), 39 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h

-- 
2.18.1

[PATCH v2 1/5] powerpc/powernv: Re-enable imc trace-mode in kernel

[Anju T Sudhakar]

commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
mode switching between accumulation mode and trace mode via the spr LDBAR
in the hardware can trigger a checkstop(system crash).

Patch to re-enable imc-trace mode in kernel.

The following patch in this series will address the mode switching issue
by implementing a global lock, and will restrict the usage of
accumulation and trace-mode at a time.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/opal-imc.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 000b350d4060..3b4518f4b643 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -278,14 +278,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
 			domain = IMC_DOMAIN_THREAD;
 			break;
 		case IMC_TYPE_TRACE:
-			/*
-			 * FIXME. Using trace_imc events to monitor application
-			 * or KVM thread performance can cause a checkstop
-			 * (system crash).
-			 * Disable it for now.
-			 */
-			pr_info_once("IMC: disabling trace_imc PMU\n");
-			domain = -1;
+			domain = IMC_DOMAIN_TRACE;
 			break;
 		default:
 			pr_warn("IMC Unknown Device type \n");
-- 
2.20.1

Re: [PATCH v2 1/5] powerpc/powernv: Re-enable imc trace-mode in kernel

[maddy]

On 1/21/20 3:47 PM, Anju T Sudhakar wrote:
> commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
> disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
> mode switching between accumulation mode and trace mode via the spr LDBAR
> in the hardware can trigger a checkstop(system crash).
>
> Patch to re-enable imc-trace mode in kernel.
>
> The following patch in this series will address the mode switching issue
> by implementing a global lock, and will restrict the usage of
> accumulation and trace-mode at a time.

Reviewed-by: MAdhavan Srinivasan <maddy@linux.ibm.com>

>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>   arch/powerpc/platforms/powernv/opal-imc.c | 9 +--------
>   1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> index 000b350d4060..3b4518f4b643 100644
> --- a/arch/powerpc/platforms/powernv/opal-imc.c
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -278,14 +278,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>   			domain = IMC_DOMAIN_THREAD;
>   			break;
>   		case IMC_TYPE_TRACE:
> -			/*
> -			 * FIXME. Using trace_imc events to monitor application
> -			 * or KVM thread performance can cause a checkstop
> -			 * (system crash).
> -			 * Disable it for now.
> -			 */
> -			pr_info_once("IMC: disabling trace_imc PMU\n");
> -			domain = -1;
> +			domain = IMC_DOMAIN_TRACE;
>   			break;
>   		default:
>   			pr_warn("IMC Unknown Device type \n");

[PATCH v2 2/5] powerpc/perf: Implement a global lock to avoid races between trace, core and thread imc events.

[Anju T Sudhakar]

IMC(In-memory Collection Counters) does performance monitoring in
two different modes, i.e accumulation mode(core-imc and thread-imc events),
and trace mode(trace-imc events). A cpu thread can either be in
accumulation-mode or trace-mode at a time and this is done via the LDBAR
register in POWER architecture. The current design does not address the
races between thread-imc and trace-imc events.

Patch implements a global id and lock to avoid the races between
core, trace and thread imc events. With this global id-lock
implementation, the system can either run core, thread or trace imc
events at a time. i.e. to run any core-imc events, thread/trace imc events
should not be enabled/monitored.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
---
 arch/powerpc/perf/imc-pmu.c | 177 +++++++++++++++++++++++++++++++-----
 1 file changed, 153 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..2e220f199530 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -44,6 +44,16 @@ static DEFINE_PER_CPU(u64 *, trace_imc_mem);
 static struct imc_pmu_ref *trace_imc_refc;
 static int trace_imc_mem_size;
 
+/*
+ * Global data structure used to avoid races between thread,
+ * core and trace-imc
+ */
+static struct imc_pmu_ref imc_global_refc = {
+	.lock = __MUTEX_INITIALIZER(imc_global_refc.lock),
+	.id = 0,
+	.refc = 0,
+};
+
 static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
 	return container_of(event->pmu, struct imc_pmu, pmu);
@@ -759,6 +769,20 @@ static void core_imc_counters_release(struct perf_event *event)
 		ref->refc = 0;
 	}
 	mutex_unlock(&ref->lock);
+
+	mutex_lock(&imc_global_refc.lock);
+	if (imc_global_refc.id == IMC_DOMAIN_CORE) {
+		imc_global_refc.refc--;
+		/*
+		 * If no other thread is running any core-imc
+		 * event, set the global id to zero.
+		 */
+		if (imc_global_refc.refc <= 0) {
+			imc_global_refc.refc = 0;
+			imc_global_refc.id = 0;
+		}
+	}
+	mutex_unlock(&imc_global_refc.lock);
 }
 
 static int core_imc_event_init(struct perf_event *event)
@@ -779,6 +803,22 @@ static int core_imc_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
+	/*
+	 * Take the global lock, and make sure
+	 * no other thread is running any trace OR thread imc event
+	 */
+	mutex_lock(&imc_global_refc.lock);
+	if (imc_global_refc.id == 0) {
+		imc_global_refc.id = IMC_DOMAIN_CORE;
+		imc_global_refc.refc++;
+	} else if (imc_global_refc.id == IMC_DOMAIN_CORE) {
+		imc_global_refc.refc++;
+	} else {
+		mutex_unlock(&imc_global_refc.lock);
+		return -EBUSY;
+	}
+	mutex_unlock(&imc_global_refc.lock);
+
 	event->hw.idx = -1;
 	pmu = imc_event_to_pmu(event);
 
@@ -877,7 +917,16 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
 
 static int ppc_thread_imc_cpu_offline(unsigned int cpu)
 {
-	mtspr(SPRN_LDBAR, 0);
+	/*
+	 * Toggle the bit 0 of LDBAR.
+	 *
+	 * If bit 0 of LDBAR is unset, it will stop posting
+	 * the counetr data to memory.
+	 * For thread-imc, bit 0 of LDBAR will be set to 1 in the
+	 * event_add function. So toggle this bit here, to stop the updates
+	 * to memory in the cpu_offline path.
+	 */
+	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
 	return 0;
 }
 
@@ -889,6 +938,24 @@ static int thread_imc_cpu_init(void)
 			  ppc_thread_imc_cpu_offline);
 }
 
+static void thread_imc_counters_release(struct perf_event *event)
+{
+
+	mutex_lock(&imc_global_refc.lock);
+	if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
+		imc_global_refc.refc--;
+		/*
+		 * If no other thread is running any thread-imc
+		 * event, set the global id to zero.
+		 */
+		if (imc_global_refc.refc <= 0) {
+			imc_global_refc.refc = 0;
+			imc_global_refc.id = 0;
+		}
+	}
+	mutex_unlock(&imc_global_refc.lock);
+}
+
 static int thread_imc_event_init(struct perf_event *event)
 {
 	u32 config = event->attr.config;
@@ -905,6 +972,27 @@ static int thread_imc_event_init(struct perf_event *event)
 	if (event->hw.sample_period)
 		return -EINVAL;
 
+	mutex_lock(&imc_global_refc.lock);
+	/*
+	 * Check if any other thread is running
+	 * core-engine, if not set the global id to
+	 * thread-imc.
+	 */
+	if (imc_global_refc.id == 0) {
+		imc_global_refc.id = IMC_DOMAIN_THREAD;
+		imc_global_refc.refc++;
+	} else if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
+		/*
+		 * Increase the ref count if the global id is
+		 * set to thread-imc.
+		 */
+		imc_global_refc.refc++;
+	} else {
+		mutex_unlock(&imc_global_refc.lock);
+		return -EBUSY;
+	}
+	mutex_unlock(&imc_global_refc.lock);
+
 	event->hw.idx = -1;
 	pmu = imc_event_to_pmu(event);
 
@@ -917,6 +1005,7 @@ static int thread_imc_event_init(struct perf_event *event)
 		return -EINVAL;
 
 	event->pmu->task_ctx_nr = perf_sw_context;
+	event->destroy = thread_imc_counters_release;
 	return 0;
 }
 
@@ -1063,10 +1152,12 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
 	int core_id;
 	struct imc_pmu_ref *ref;
 
-	mtspr(SPRN_LDBAR, 0);
-
 	core_id = smp_processor_id() / threads_per_core;
 	ref = &core_imc_refc[core_id];
+	if (!ref) {
+		pr_debug("imc: Failed to get event reference count\n");
+		return;
+	}
 
 	mutex_lock(&ref->lock);
 	ref->refc--;
@@ -1082,6 +1173,10 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
 		ref->refc = 0;
 	}
 	mutex_unlock(&ref->lock);
+
+	/* Toggle bit 0 of LDBAR */
+	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
+
 	/*
 	 * Take a snapshot and calculate the delta and update
 	 * the event counter values.
@@ -1133,7 +1228,8 @@ static int ppc_trace_imc_cpu_online(unsigned int cpu)
 
 static int ppc_trace_imc_cpu_offline(unsigned int cpu)
 {
-	mtspr(SPRN_LDBAR, 0);
+	/* Toggle bit 0 of LDBAR. */
+	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
 	return 0;
 }
 
@@ -1226,15 +1322,14 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
 	local_mem = get_trace_imc_event_base_addr();
 	ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) | TRACE_IMC_ENABLE;
 
-	if (core_imc_refc)
-		ref = &core_imc_refc[core_id];
+	/* trace-imc reference count */
+	if (trace_imc_refc)
+		ref = &trace_imc_refc[core_id];
 	if (!ref) {
-		/* If core-imc is not enabled, use trace-imc reference count */
-		if (trace_imc_refc)
-			ref = &trace_imc_refc[core_id];
-		if (!ref)
-			return -EINVAL;
+		pr_debug("imc: Failed to get the event reference count\n");
+		return -EINVAL;
 	}
+
 	mtspr(SPRN_LDBAR, ldbar_value);
 	mutex_lock(&ref->lock);
 	if (ref->refc == 0) {
@@ -1242,13 +1337,11 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
 				get_hard_smp_processor_id(smp_processor_id()))) {
 			mutex_unlock(&ref->lock);
 			pr_err("trace-imc: Unable to start the counters for core %d\n", core_id);
-			mtspr(SPRN_LDBAR, 0);
 			return -EINVAL;
 		}
 	}
 	++ref->refc;
 	mutex_unlock(&ref->lock);
-
 	return 0;
 }
 
@@ -1274,16 +1367,13 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
 	int core_id = smp_processor_id() / threads_per_core;
 	struct imc_pmu_ref *ref = NULL;
 
-	if (core_imc_refc)
-		ref = &core_imc_refc[core_id];
+	if (trace_imc_refc)
+		ref = &trace_imc_refc[core_id];
 	if (!ref) {
-		/* If core-imc is not enabled, use trace-imc reference count */
-		if (trace_imc_refc)
-			ref = &trace_imc_refc[core_id];
-		if (!ref)
-			return;
+		pr_debug("imc: Failed to get event reference count\n");
+		return;
 	}
-	mtspr(SPRN_LDBAR, 0);
+
 	mutex_lock(&ref->lock);
 	ref->refc--;
 	if (ref->refc == 0) {
@@ -1297,9 +1387,30 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
 		ref->refc = 0;
 	}
 	mutex_unlock(&ref->lock);
+
+	/* Toggle bit 0 of LDBAR */
+	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
+
 	trace_imc_event_stop(event, flags);
 }
 
+static void trace_imc_counters_release(struct perf_event *event)
+{
+	mutex_lock(&imc_global_refc.lock);
+	if (imc_global_refc.id == IMC_DOMAIN_TRACE) {
+		imc_global_refc.refc--;
+		/*
+		 * If no other thread is running any trace-imc
+		 * event, set the global id to zero.
+		 */
+		if (imc_global_refc.refc <= 0) {
+			imc_global_refc.refc = 0;
+			imc_global_refc.id = 0;
+		}
+	}
+	mutex_unlock(&imc_global_refc.lock);
+}
+
 static int trace_imc_event_init(struct perf_event *event)
 {
 	struct task_struct *target;
@@ -1314,10 +1425,28 @@ static int trace_imc_event_init(struct perf_event *event)
 	if (event->attr.sample_period == 0)
 		return -ENOENT;
 
+	/*
+	 * Take the global lock, and make sure
+	 * no other thread is running any core/thread imc
+	 * event
+	 */
+	mutex_lock(&imc_global_refc.lock);
+	if (imc_global_refc.id == 0) {
+		imc_global_refc.id = IMC_DOMAIN_TRACE;
+		imc_global_refc.refc++;
+	} else if (imc_global_refc.id == IMC_DOMAIN_TRACE) {
+		imc_global_refc.refc++;
+	} else {
+		mutex_unlock(&imc_global_refc.lock);
+		return -EBUSY;
+	}
+	mutex_unlock(&imc_global_refc.lock);
+
 	event->hw.idx = -1;
 	target = event->hw.target;
 
 	event->pmu->task_ctx_nr = perf_hw_context;
+	event->destroy = trace_imc_counters_release;
 	return 0;
 }
 
@@ -1429,10 +1558,10 @@ static void cleanup_all_core_imc_memory(void)
 static void thread_imc_ldbar_disable(void *dummy)
 {
 	/*
-	 * By Zeroing LDBAR, we disable thread-imc
-	 * updates.
+	 * By toggling 0th bit of LDBAR, we disable thread-imc
+	 * updates to memory.
 	 */
-	mtspr(SPRN_LDBAR, 0);
+	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
 }
 
 void thread_imc_disable(void)
-- 
2.20.1

Re: [PATCH v2 2/5] powerpc/perf: Implement a global lock to avoid races between trace, core and thread imc events.

[maddy]

On 1/21/20 3:47 PM, Anju T Sudhakar wrote:
> IMC(In-memory Collection Counters) does performance monitoring in
> two different modes, i.e accumulation mode(core-imc and thread-imc events),
> and trace mode(trace-imc events). A cpu thread can either be in
> accumulation-mode or trace-mode at a time and this is done via the LDBAR
> register in POWER architecture. The current design does not address the
> races between thread-imc and trace-imc events.
>
> Patch implements a global id and lock to avoid the races between
> core, trace and thread imc events. With this global id-lock
> implementation, the system can either run core, thread or trace imc
> events at a time. i.e. to run any core-imc events, thread/trace imc events
> should not be enabled/monitored.

Changes looks fine to me.

Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>

> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> ---
>   arch/powerpc/perf/imc-pmu.c | 177 +++++++++++++++++++++++++++++++-----
>   1 file changed, 153 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
> index cb50a9e1fd2d..2e220f199530 100644
> --- a/arch/powerpc/perf/imc-pmu.c
> +++ b/arch/powerpc/perf/imc-pmu.c
> @@ -44,6 +44,16 @@ static DEFINE_PER_CPU(u64 *, trace_imc_mem);
>   static struct imc_pmu_ref *trace_imc_refc;
>   static int trace_imc_mem_size;
>
> +/*
> + * Global data structure used to avoid races between thread,
> + * core and trace-imc
> + */
> +static struct imc_pmu_ref imc_global_refc = {
> +	.lock = __MUTEX_INITIALIZER(imc_global_refc.lock),
> +	.id = 0,
> +	.refc = 0,
> +};
> +
>   static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
>   {
>   	return container_of(event->pmu, struct imc_pmu, pmu);
> @@ -759,6 +769,20 @@ static void core_imc_counters_release(struct perf_event *event)
>   		ref->refc = 0;
>   	}
>   	mutex_unlock(&ref->lock);
> +
> +	mutex_lock(&imc_global_refc.lock);
> +	if (imc_global_refc.id == IMC_DOMAIN_CORE) {
> +		imc_global_refc.refc--;
> +		/*
> +		 * If no other thread is running any core-imc
> +		 * event, set the global id to zero.
> +		 */
> +		if (imc_global_refc.refc <= 0) {
> +			imc_global_refc.refc = 0;
> +			imc_global_refc.id = 0;
> +		}
> +	}
> +	mutex_unlock(&imc_global_refc.lock);
>   }
>
>   static int core_imc_event_init(struct perf_event *event)
> @@ -779,6 +803,22 @@ static int core_imc_event_init(struct perf_event *event)
>   	if (event->cpu < 0)
>   		return -EINVAL;
>
> +	/*
> +	 * Take the global lock, and make sure
> +	 * no other thread is running any trace OR thread imc event
> +	 */
> +	mutex_lock(&imc_global_refc.lock);
> +	if (imc_global_refc.id == 0) {
> +		imc_global_refc.id = IMC_DOMAIN_CORE;
> +		imc_global_refc.refc++;
> +	} else if (imc_global_refc.id == IMC_DOMAIN_CORE) {
> +		imc_global_refc.refc++;
> +	} else {
> +		mutex_unlock(&imc_global_refc.lock);
> +		return -EBUSY;
> +	}
> +	mutex_unlock(&imc_global_refc.lock);
> +
>   	event->hw.idx = -1;
>   	pmu = imc_event_to_pmu(event);
>
> @@ -877,7 +917,16 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
>
>   static int ppc_thread_imc_cpu_offline(unsigned int cpu)
>   {
> -	mtspr(SPRN_LDBAR, 0);
> +	/*
> +	 * Toggle the bit 0 of LDBAR.
> +	 *
> +	 * If bit 0 of LDBAR is unset, it will stop posting
> +	 * the counetr data to memory.
> +	 * For thread-imc, bit 0 of LDBAR will be set to 1 in the
> +	 * event_add function. So toggle this bit here, to stop the updates
> +	 * to memory in the cpu_offline path.
> +	 */
> +	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
>   	return 0;
>   }
>
> @@ -889,6 +938,24 @@ static int thread_imc_cpu_init(void)
>   			  ppc_thread_imc_cpu_offline);
>   }
>
> +static void thread_imc_counters_release(struct perf_event *event)
> +{
> +
> +	mutex_lock(&imc_global_refc.lock);
> +	if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
> +		imc_global_refc.refc--;
> +		/*
> +		 * If no other thread is running any thread-imc
> +		 * event, set the global id to zero.
> +		 */
> +		if (imc_global_refc.refc <= 0) {
> +			imc_global_refc.refc = 0;
> +			imc_global_refc.id = 0;
> +		}
> +	}
> +	mutex_unlock(&imc_global_refc.lock);
> +}
> +
>   static int thread_imc_event_init(struct perf_event *event)
>   {
>   	u32 config = event->attr.config;
> @@ -905,6 +972,27 @@ static int thread_imc_event_init(struct perf_event *event)
>   	if (event->hw.sample_period)
>   		return -EINVAL;
>
> +	mutex_lock(&imc_global_refc.lock);
> +	/*
> +	 * Check if any other thread is running
> +	 * core-engine, if not set the global id to
> +	 * thread-imc.
> +	 */
> +	if (imc_global_refc.id == 0) {
> +		imc_global_refc.id = IMC_DOMAIN_THREAD;
> +		imc_global_refc.refc++;
> +	} else if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
> +		/*
> +		 * Increase the ref count if the global id is
> +		 * set to thread-imc.
> +		 */
> +		imc_global_refc.refc++;
> +	} else {
> +		mutex_unlock(&imc_global_refc.lock);
> +		return -EBUSY;
> +	}
> +	mutex_unlock(&imc_global_refc.lock);
> +
>   	event->hw.idx = -1;
>   	pmu = imc_event_to_pmu(event);
>
> @@ -917,6 +1005,7 @@ static int thread_imc_event_init(struct perf_event *event)
>   		return -EINVAL;
>
>   	event->pmu->task_ctx_nr = perf_sw_context;
> +	event->destroy = thread_imc_counters_release;
>   	return 0;
>   }
>
> @@ -1063,10 +1152,12 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
>   	int core_id;
>   	struct imc_pmu_ref *ref;
>
> -	mtspr(SPRN_LDBAR, 0);
> -
>   	core_id = smp_processor_id() / threads_per_core;
>   	ref = &core_imc_refc[core_id];
> +	if (!ref) {
> +		pr_debug("imc: Failed to get event reference count\n");
> +		return;
> +	}
>
>   	mutex_lock(&ref->lock);
>   	ref->refc--;
> @@ -1082,6 +1173,10 @@ static void thread_imc_event_del(struct perf_event *event, int flags)
>   		ref->refc = 0;
>   	}
>   	mutex_unlock(&ref->lock);
> +
> +	/* Toggle bit 0 of LDBAR */
> +	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> +
>   	/*
>   	 * Take a snapshot and calculate the delta and update
>   	 * the event counter values.
> @@ -1133,7 +1228,8 @@ static int ppc_trace_imc_cpu_online(unsigned int cpu)
>
>   static int ppc_trace_imc_cpu_offline(unsigned int cpu)
>   {
> -	mtspr(SPRN_LDBAR, 0);
> +	/* Toggle bit 0 of LDBAR. */
> +	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
>   	return 0;
>   }
>
> @@ -1226,15 +1322,14 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
>   	local_mem = get_trace_imc_event_base_addr();
>   	ldbar_value = ((u64)local_mem & THREAD_IMC_LDBAR_MASK) | TRACE_IMC_ENABLE;
>
> -	if (core_imc_refc)
> -		ref = &core_imc_refc[core_id];
> +	/* trace-imc reference count */
> +	if (trace_imc_refc)
> +		ref = &trace_imc_refc[core_id];
>   	if (!ref) {
> -		/* If core-imc is not enabled, use trace-imc reference count */
> -		if (trace_imc_refc)
> -			ref = &trace_imc_refc[core_id];
> -		if (!ref)
> -			return -EINVAL;
> +		pr_debug("imc: Failed to get the event reference count\n");
> +		return -EINVAL;
>   	}
> +
>   	mtspr(SPRN_LDBAR, ldbar_value);
>   	mutex_lock(&ref->lock);
>   	if (ref->refc == 0) {
> @@ -1242,13 +1337,11 @@ static int trace_imc_event_add(struct perf_event *event, int flags)
>   				get_hard_smp_processor_id(smp_processor_id()))) {
>   			mutex_unlock(&ref->lock);
>   			pr_err("trace-imc: Unable to start the counters for core %d\n", core_id);
> -			mtspr(SPRN_LDBAR, 0);
>   			return -EINVAL;
>   		}
>   	}
>   	++ref->refc;
>   	mutex_unlock(&ref->lock);
> -
>   	return 0;
>   }
>
> @@ -1274,16 +1367,13 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
>   	int core_id = smp_processor_id() / threads_per_core;
>   	struct imc_pmu_ref *ref = NULL;
>
> -	if (core_imc_refc)
> -		ref = &core_imc_refc[core_id];
> +	if (trace_imc_refc)
> +		ref = &trace_imc_refc[core_id];
>   	if (!ref) {
> -		/* If core-imc is not enabled, use trace-imc reference count */
> -		if (trace_imc_refc)
> -			ref = &trace_imc_refc[core_id];
> -		if (!ref)
> -			return;
> +		pr_debug("imc: Failed to get event reference count\n");
> +		return;
>   	}
> -	mtspr(SPRN_LDBAR, 0);
> +
>   	mutex_lock(&ref->lock);
>   	ref->refc--;
>   	if (ref->refc == 0) {
> @@ -1297,9 +1387,30 @@ static void trace_imc_event_del(struct perf_event *event, int flags)
>   		ref->refc = 0;
>   	}
>   	mutex_unlock(&ref->lock);
> +
> +	/* Toggle bit 0 of LDBAR */
> +	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
> +
>   	trace_imc_event_stop(event, flags);
>   }
>
> +static void trace_imc_counters_release(struct perf_event *event)
> +{
> +	mutex_lock(&imc_global_refc.lock);
> +	if (imc_global_refc.id == IMC_DOMAIN_TRACE) {
> +		imc_global_refc.refc--;
> +		/*
> +		 * If no other thread is running any trace-imc
> +		 * event, set the global id to zero.
> +		 */
> +		if (imc_global_refc.refc <= 0) {
> +			imc_global_refc.refc = 0;
> +			imc_global_refc.id = 0;
> +		}
> +	}
> +	mutex_unlock(&imc_global_refc.lock);
> +}
> +
>   static int trace_imc_event_init(struct perf_event *event)
>   {
>   	struct task_struct *target;
> @@ -1314,10 +1425,28 @@ static int trace_imc_event_init(struct perf_event *event)
>   	if (event->attr.sample_period == 0)
>   		return -ENOENT;
>
> +	/*
> +	 * Take the global lock, and make sure
> +	 * no other thread is running any core/thread imc
> +	 * event
> +	 */
> +	mutex_lock(&imc_global_refc.lock);
> +	if (imc_global_refc.id == 0) {
> +		imc_global_refc.id = IMC_DOMAIN_TRACE;
> +		imc_global_refc.refc++;
> +	} else if (imc_global_refc.id == IMC_DOMAIN_TRACE) {
> +		imc_global_refc.refc++;
> +	} else {
> +		mutex_unlock(&imc_global_refc.lock);
> +		return -EBUSY;
> +	}
> +	mutex_unlock(&imc_global_refc.lock);
> +
>   	event->hw.idx = -1;
>   	target = event->hw.target;
>
>   	event->pmu->task_ctx_nr = perf_hw_context;
> +	event->destroy = trace_imc_counters_release;
>   	return 0;
>   }
>
> @@ -1429,10 +1558,10 @@ static void cleanup_all_core_imc_memory(void)
>   static void thread_imc_ldbar_disable(void *dummy)
>   {
>   	/*
> -	 * By Zeroing LDBAR, we disable thread-imc
> -	 * updates.
> +	 * By toggling 0th bit of LDBAR, we disable thread-imc
> +	 * updates to memory.
>   	 */
> -	mtspr(SPRN_LDBAR, 0);
> +	mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
>   }
>
>   void thread_imc_disable(void)

[PATCH v2 3/5] powerpc/perf: Add an interface sub-folder to imc pmu

[Anju T Sudhakar]

From: Madhavan Srinivasan <maddy@linux.ibm.com>

Patch adds an interface attribute folder to imc pmu.
This is intended to include pmu intreface capabilities
which will be useful to userspace likes selftest
testcases. Patch adds a "glob_lck" file to notify to
userspace of global lock mechanism added to imc devices
like core, thread and trace.

"glob_lck" will be used by selftest file to execute
interface test for the global lock mechanism.

Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
 arch/powerpc/include/asm/imc-pmu.h | 11 ++++++-----
 arch/powerpc/perf/imc-pmu.c        | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
index 4da4fcba0684..1b2c33c30e7c 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -87,8 +87,9 @@ struct trace_imc_data {
 /* Event attribute array index */
 #define IMC_FORMAT_ATTR		0
 #define IMC_EVENT_ATTR		1
-#define IMC_CPUMASK_ATTR	2
-#define IMC_NULL_ATTR		3
+#define IMC_INTERFACE_ATTR	2
+#define IMC_CPUMASK_ATTR	3
+#define IMC_NULL_ATTR		4
 
 /* PMU Format attribute macros */
 #define IMC_EVENT_OFFSET_MASK	0xffffffffULL
@@ -114,10 +115,10 @@ struct imc_pmu {
 	/*
 	 * Attribute groups for the PMU. Slot 0 used for
 	 * format attribute, slot 1 used for cpusmask attribute,
-	 * slot 2 used for event attribute. Slot 3 keep as
-	 * NULL.
+	 * slot 2 used for event attribute. Slot 3 used for interface
+	 * attribute and Slot 4 is NULL.
 	 */
-	const struct attribute_group *attr_groups[4];
+	const struct attribute_group *attr_groups[5];
 	u32 counter_mem_size;
 	int domain;
 	/*
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 2e220f199530..3f49664f29f1 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -54,6 +54,24 @@ static struct imc_pmu_ref imc_global_refc = {
 	.refc = 0,
 };
 
+static ssize_t glob_lck_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", 1);
+}
+
+static DEVICE_ATTR_RO(glob_lck);
+
+static struct attribute *imc_interface_attrs[] = {
+	&dev_attr_glob_lck.attr,
+	NULL,
+};
+
+static struct attribute_group imc_interface_group = {
+	.name = "interface",
+	.attrs = imc_interface_attrs,
+};
+
 static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
 	return container_of(event->pmu, struct imc_pmu, pmu);
@@ -1462,6 +1480,7 @@ static int update_pmu_ops(struct imc_pmu *pmu)
 	pmu->pmu.attr_groups = pmu->attr_groups;
 	pmu->pmu.capabilities = PERF_PMU_CAP_NO_EXCLUDE;
 	pmu->attr_groups[IMC_FORMAT_ATTR] = &imc_format_group;
+	pmu->attr_groups[IMC_INTERFACE_ATTR] = &imc_interface_group;
 
 	switch (pmu->domain) {
 	case IMC_DOMAIN_NEST:
-- 
2.20.1

[PATCH v2 4/5] selftest/powerpc/pmc: Support to include interface test for Memory Counter PMUs

[Anju T Sudhakar]

From: Madhavan Srinivasan <maddy@linux.ibm.com>

Patch to add support to include interface tests for
memory counter PMUs as part of selftest.
These PMUs are primarily used to understand socket/chip/core
resourage usage. In PowerNV envirnoment, the perf interface
registered to access these counters are called "In Memory Collection"
(IMC) and in PowerVM, the perf interface registered to access
these counters are called "hv_24x7".

New folder "mem_counters" added under selftest/powerpc/pmu.
This will include interface tests for both "imc" and "hv_24x7"
pmus. Patch adds base/common functioned needed.
To make blame easier, a place-holder test function added to
this patch. Subsequent patch will fill in the actual test
content.

Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
 tools/testing/selftests/powerpc/pmu/Makefile  |  7 +-
 .../powerpc/pmu/mem_counters/Makefile         | 21 ++++
 .../pmu/mem_counters/imc_global_lock_test.c   | 21 ++++
 .../powerpc/pmu/mem_counters/mem_counters.c   | 99 +++++++++++++++++++
 .../powerpc/pmu/mem_counters/mem_counters.h   | 36 +++++++
 5 files changed, 182 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h

diff --git a/tools/testing/selftests/powerpc/pmu/Makefile b/tools/testing/selftests/powerpc/pmu/Makefile
index 19046db995fe..e352eceac0a9 100644
--- a/tools/testing/selftests/powerpc/pmu/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/Makefile
@@ -8,7 +8,7 @@ EXTRA_SOURCES := ../harness.c event.c lib.c ../utils.c
 top_srcdir = ../../../../..
 include ../../lib.mk
 
-all: $(TEST_GEN_PROGS) ebb
+all: $(TEST_GEN_PROGS) ebb mem_counters
 
 $(TEST_GEN_PROGS): $(EXTRA_SOURCES)
 
@@ -43,4 +43,7 @@ clean:
 ebb:
 	TARGET=$@; BUILD_TARGET=$$OUTPUT/$$TARGET; mkdir -p $$BUILD_TARGET; $(MAKE) OUTPUT=$$BUILD_TARGET -k -C $$TARGET all
 
-.PHONY: all run_tests clean ebb
+mem_counters:
+	TARGET=$@; BUILD_TARGET=$$OUTPUT/$$TARGET; mkdir -p $$BUILD_TARGET; $(MAKE) OUTPUT=$$BUILD_TARGET -k -C $$TARGET all
+
+.PHONY: all run_tests clean ebb mem_counters
diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/Makefile b/tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
new file mode 100644
index 000000000000..f39ebe30ab70
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+include ../../../../../../scripts/Kbuild.include
+
+noarg:
+	$(MAKE) -C ../../
+
+CFLAGS += -m64
+
+# Toolchains may build PIE by default which breaks the assembly
+no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
+        $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)
+
+LDFLAGS += $(no-pie-option)
+
+TEST_GEN_PROGS := imc_global_lock_test
+
+top_srcdir = ../../../../../..
+include ../../../lib.mk
+
+$(TEST_GEN_PROGS): ../../harness.c ../../utils.c ../event.c ../lib.c ./mem_counters.c \
+			imc_global_lock_test.c
diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
new file mode 100644
index 000000000000..ea687ffc1990
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020, Madhavan Srinivasan, IBM Corp.
+ */
+
+#include "mem_counters.h"
+
+static int testcase(void)
+{
+	return 0;
+}
+
+static int imc_global_lock_test(void)
+{
+	return eat_cpu(testcase);
+}
+
+int main(void)
+{
+	return test_harness(imc_global_lock_test, "imc_global_lock_test");
+}
diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c b/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
new file mode 100644
index 000000000000..b0ee1319f018
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020, Madhavan Srinivasan, IBM Corp.
+ */
+
+#include "mem_counters.h"
+
+/*
+ * mem_counters.c will contain common/basic functions
+ * to support testcases for both In Memory Collection (IMC)
+ * and hv_24x7 counters.
+ */
+
+
+/*
+ * Since device type enum starts with 1,
+ * have the first entry in the array as a placeholder.
+ */
+const char mem_counters_dev_path[][30] = {
+	"",
+	"/sys/devices/thread_imc",
+	"/sys/devices/trace_imc",
+	"/sys/devices/core_imc",
+	"/sys/devices/hv_24x7",
+	"",
+};
+
+const char mem_counters_dev_type_path[][35] = {
+	"",
+	"/sys/devices/thread_imc/type",
+	"/sys/devices/trace_imc/type",
+	"/sys/devices/core_imc/type",
+	"/sys/devices/hv_24x7/type",
+	"",
+};
+
+
+static bool is_mem_counters_dev_registered(int dtype)
+{
+	if (!access(mem_counters_dev_path[dtype], F_OK))
+		return true;
+
+	return false;
+}
+
+bool is_mem_counters_device_enabled(int dtype)
+{
+	switch (dtype) {
+	case THREAD:
+		if (is_mem_counters_dev_registered(THREAD))
+			return true;
+	case TRACE:
+		if (is_mem_counters_dev_registered(TRACE))
+			return true;
+		break;
+	case CORE:
+		if (is_mem_counters_dev_registered(CORE))
+			return true;
+	case HV_24X7:
+		if (is_mem_counters_dev_registered(HV_24X7))
+			return true;
+	};
+
+	return false;
+}
+
+int get_mem_counters_pmu_type_val(int dtype)
+{
+	FILE *fp = NULL;
+	char buf[10];
+	int val;
+
+	fp = fopen(mem_counters_dev_type_path[dtype], "r");
+	if (!fp) {
+		perror("Failed to open\n");
+		return -1;
+	}
+
+	if (!fgets(buf, 10, fp)) {
+		perror("Failed to read\n");
+		return -1;
+	}
+
+	fclose(fp);
+	val = atoi(buf);
+	return val;
+}
+
+int setup_mem_counters_event(int dtype, struct event *e, u64 config, char *name)
+{
+	int val = get_mem_counters_pmu_type_val(dtype);
+
+	if (val > 0) {
+		event_init_opts(e, config, val, name);
+		return 0;
+	}
+
+	return -1;
+}
diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h b/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h
new file mode 100644
index 000000000000..16456e5fc9de
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Copyright 2020, Madhavan Srinivasan, IBM Corp.
+ */
+
+#ifndef _SELFTESTS_POWERPC_PMU_IMC_IMC_H
+#define _SELFTESTS_POWERPC_PMU_IMC_IMC_H
+
+#define _GNU_SOURCE
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <error.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/prctl.h>
+
+#include "../event.h"
+#include "../lib.h"
+
+
+enum MEM_COUNTERS_DEV_TYPE{
+	THREAD = 0x1,
+	TRACE,
+	CORE,
+	HV_24X7,
+};
+
+extern bool is_mem_counters_device_enabled(int dtype);
+extern int get_mem_counters_pmu_type_val(int dtype);
+extern int setup_mem_counters_event(int dtype, struct event *e, u64 config, char *name);
+
+#endif /* _SELFTESTS_POWERPC_PMU_IMC_IMC_H */
-- 
2.20.1

[PATCH v2 5/5] selftest/powerpc/pmu: Testcase for imc global lock mechanism

[Anju T Sudhakar]

From: Madhavan Srinivasan <maddy@linux.ibm.com>

Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
---
 .../pmu/mem_counters/imc_global_lock_test.c   | 49 ++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
index ea687ffc1990..f643dba8ecc0 100644
--- a/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
@@ -5,9 +5,56 @@
 
 #include "mem_counters.h"
 
+static  bool check_imc_interface_glob_lck(void)
+{
+	if (!access("/sys/devices/thread_imc/interface/glob_lck", F_OK))
+		return true;
+
+	return false;
+}
+
 static int testcase(void)
 {
-	return 0;
+	struct event events[2];
+
+	if (!check_imc_interface_glob_lck()) {
+		printf("Test not supported\n");
+		return MAGIC_SKIP_RETURN_VALUE;
+	}
+
+	if (!is_mem_counters_device_enabled(CORE) || !is_mem_counters_device_enabled(THREAD)) {
+		printf("%s: IMC device not found. So exiting the test\n", __FUNCTION__);
+		return -1;
+	}
+
+	if (setup_mem_counters_event(THREAD, &events[0], 0xe0, "thread_imc/cycles")) {
+		printf("%s setup_mem_counters_event for thread_imc failed\n", __FUNCTION__);
+		return -1;
+	}
+
+	if (setup_mem_counters_event(CORE, &events[1], 0xe0, "core_imc/cycles")) {
+		printf("%s setup_mem_counters_event for core_imc failed\n", __FUNCTION__);
+		return -1;
+	}
+
+	if (event_open(&events[0])) {
+		perror("thread_imc: perf_event_open");
+		return -1;
+	}
+
+	/*
+	 * If we have the Global lock patchset applied to kernel
+	 * event_open for events[1] should fail with resource busy
+	 */
+	if (event_open_with_cpu(&events[1], 0)) {
+		/*
+		 * Check for the errno to certify the test result
+		 */
+		if (errno == 16) // Resource busy (EBUSY)
+			return 0;
+	}
+
+	return -1;
 }
 
 static int imc_global_lock_test(void)
-- 
2.20.1