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

Groups > linux.kernel > #1615299 > unrolled thread

[PATCH v6 00/11] IMC Instrumentation Support

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 00/11] IMC Instrumentation Support Madhavan Srinivasan <maddy@linux.vnet.ibm.com> - 2017-04-03 17:10 +0200
    [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module Madhavan Srinivasan <maddy@linux.vnet.ibm.com> - 2017-04-03 17:10 +0200
      Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module Daniel Axtens <dja@axtens.net> - 2017-04-04 03:00 +0200
        Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver  module Madhavan Srinivasan <maddy@linux.vnet.ibm.com> - 2017-04-05 08:40 +0200
      Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module Daniel Axtens <dja@axtens.net> - 2017-04-04 03:50 +0200
        Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver  module Madhavan Srinivasan <maddy@linux.vnet.ibm.com> - 2017-04-05 08:40 +0200
      Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module Stewart Smith <stewart@linux.vnet.ibm.com> - 2017-04-06 09:10 +0200
    [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions Madhavan Srinivasan <maddy@linux.vnet.ibm.com> - 2017-04-03 17:10 +0200
      Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions Daniel Axtens <dja@axtens.net> - 2017-04-04 03:50 +0200
        Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros  definitions Madhavan Srinivasan <maddy@linux.vnet.ibm.com> - 2017-04-05 06:30 +0200
      Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions Stewart Smith <stewart@linux.vnet.ibm.com> - 2017-04-06 10:10 +0200
      Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions Stewart Smith <stewart@linux.vnet.ibm.com> - 2017-04-06 10:50 +0200

#1615299 — [PATCH v6 00/11] IMC Instrumentation Support

Power9 has In-Memory-Collection (IMC) infrastructure which contains
various Performance Monitoring Units (PMUs) at Nest level (these are
on-chip but off-core), Core level and Thread level.

The Nest PMU counters are handled by a Nest IMC microcode which runs
in the OCC (On-Chip Controller) complex. The microcode collects the
counter data and moves the nest IMC counter data to memory.

The Core and Thread IMC PMU counters are handled in the core. Core
level PMU counters give us the IMC counters' data per core and thread
level PMU counters give us the IMC counters' data per CPU thread.

This patchset enables the nest IMC, core IMC and thread IMC
PMUs and is based on the initial work done by Madhavan Srinivasan.
"Nest Instrumentation Support" :
https://lists.ozlabs.org/pipermail/linuxppc-dev/2015-August/132078.html

v1 for this patchset can be found here :
https://lwn.net/Articles/705475/

Nest events:
Per-chip nest instrumentation provides various per-chip metrics
such as memory, powerbus, Xlink and Alink bandwidth.

Core events:
Per-core IMC instrumentation provides various per-core metrics
such as non-idle cycles, non-idle instructions, various cache and
memory related metrics etc.

Thread events:
All the events for thread level are same as core level with the
difference being in the domain. These are per-cpu metrics.

PMU Events' Information:
OPAL obtains the IMC PMU and event information from the IMC Catalog
and passes on to the kernel via the device tree. The events' information
contains :
 - Event name
 - Event Offset
 - Event description
and, maybe :
 - Event scale
 - Event unit

Some PMUs may have a common scale and unit values for all their
supported events. For those cases, the scale and unit properties for
those events must be inherited from the PMU.

The event offset in the memory is where the counter data gets
accumulated.

The OPAL-side patches are posted upstream :
https://lists.ozlabs.org/pipermail/skiboot/2017-March/006531.html

The kernel discovers the IMC counters information in the device tree
at the "imc-counters" device node which has a compatible field
"ibm,opal-in-memory-counters".

Parsing of the Events' information:
To parse the IMC PMUs and events information, the kernel has to
discover the "imc-counters" node and walk through the pmu and event
nodes.

Here is an excerpt of the dt showing the imc-counters with
mcs0 (nest), core and thread node:

https://github.com/open-power/ima-catalog/blob/master/81E00612.4E0100.dts

/dts-v1/;

[...]

/dts-v1/;

/ {
        name = "";
        compatible = "ibm,opal-in-memory-counters";
        #address-cells = <0x1>;
        #size-cells = <0x1>;
        imc-nest-offset = <0x320000>;
        imc-nest-size = <0x30000>;
        version-id = "";

        NEST_MCS: nest-mcs-events {
                #address-cells = <0x1>;
                #size-cells = <0x1>;

                event@0 {
                        event-name = "RRTO_QFULL_NO_DISP" ;
                        reg = <0x0 0x8>;
                        desc = "RRTO not dispatched in MCS0 due to capacity - pulses once for each time a valid RRTO op is not dispatched due to a command list full condition" ;
                };
                event@8 {
                        event-name = "WRTO_QFULL_NO_DISP" ;
                        reg = <0x8 0x8>;
                        desc = "WRTO not dispatched in MCS0 due to capacity - pulses once for each time a valid WRTO op is not dispatched due to a command list full condition" ;
                };
		[...]
	mcs0 {
                compatible = "ibm,imc-counters-nest";
                events-prefix = "PM_MCS0_";
                unit = "";
                scale = "";
                reg = <0x118 0x8>;
                events = < &NEST_MCS >;
        };

        mcs1 {
                compatible = "ibm,imc-counters-nest";
                events-prefix = "PM_MCS1_";
                unit = "";
                scale = "";
                reg = <0x198 0x8>;
                events = < &NEST_MCS >;
        };
	[...]

	CORE_EVENTS: core-events {
                #address-cells = <0x1>;
                #size-cells = <0x1>;

                event@e0 {
                        event-name = "0THRD_NON_IDLE_PCYC" ;
                        reg = <0xe0 0x8>;
                        desc = "The number of processor cycles when all threads are idle" ;
                };
                event@120 {
                        event-name = "1THRD_NON_IDLE_PCYC" ;
                        reg = <0x120 0x8>;
                        desc = "The number of processor cycles when exactly one SMT thread is executing non-idle code" ;
                };
		[...]
       core {
                compatible = "ibm,imc-counters-core";
                events-prefix = "CPM_";
                unit = "";
                scale = "";
                reg = <0x0 0x8>;
                events = < &CORE_EVENTS >;
        };

        thread {
                compatible = "ibm,imc-counters-core";
                events-prefix = "CPM_";
                unit = "";
                scale = "";
                reg = <0x0 0x8>;
                events = < &CORE_EVENTS >;
        };
};

From the device tree, the kernel parses the PMUs and their events'
information.

After parsing the IMC PMUs and their events, the PMUs and their
attributes are registered in the kernel.

This patchset (patches 9 and 10) configure the thread level IMC PMUs
to count for tasks, which give us the thread level metric values per
task.

Example Usage :
 # perf list

  [...]
  nest_mcs0/PM_MCS_DOWN_128B_DATA_XFER_MC0/           [Kernel PMU event]
  nest_mcs0/PM_MCS_DOWN_128B_DATA_XFER_MC0_LAST_SAMPLE/ [Kernel PMU event]
  [...]
  core_imc/CPM_NON_IDLE_INST/                        [Kernel PMU event]
  core_imc/CPM_NON_IDLE_PCYC/                        [Kernel PMU event]
  [...]
  thread_imc/CPM_NON_IDLE_INST/                      [Kernel PMU event]
  thread_imc/CPM_NON_IDLE_PCYC/                      [Kernel PMU event]

To see per chip data for nest_mcs0/PM_MCS_DOWN_128B_DATA_XFER_MC0/ :
 # perf stat -e "nest_mcs0/PM_MCS_DOWN_128B_DATA_XFER_MC0/" -a --per-socket

To see non-idle instructions for core 0 :
 # ./perf stat -e "core_imc/CPM_NON_IDLE_INST/" -C 0 -I 1000

To see non-idle instructions for a "make" :
 # ./perf stat -e "thread_imc/CPM_NON_IDLE_PCYC/" make

Comments/feedback/suggestions are welcome.

TODO:
1)Add a sysfs interface to disable the Core imc (both for ldbar and pdbar)
2)Add disable_imc_nest and diable_imc_core kernel parameters to help out in debug

Changelog:
v5 -> v6:
 - merged few patches for the readability and code flow
 - Updated the commit message and code comments.
 - Added kdump check.
 - updated cpuhotplug code and added checks for perf migration context
 - Added READ_ONCE() when reading the counter data.
 - replaced of_property_read_u32() with of_get_address() for "reg" property read
 - replaced UNKNOWN_DOMAIN with IMC_DOMAIN_UNKNOWN
 v4 -> v5:
 - Updated opal call numbers
 - Added a patch to disable Core-IMC device using shutdown callback
 - Added patch to support cpuhotplug for thread-imc
 - Added patch to disable and enable core imc engine in cpuhot plug path
 v3 -> v4 :
 - Changed the events parser code to discover the PMU and events because
   of the changed format of the IMC DTS file (Patch 3).
 - Implemented the two TODOs to include core and thread IMC support with
   this patchset (Patches 7 through 10).
 - Changed the CPU hotplug code of Nest IMC PMUs to include a new state
   CPUHP_AP_PERF_POWERPC_NEST_ONLINE (Patch 6).
 v2 -> v3 :
 - Changed all references for IMA (In-Memory Accumulation) to IMC (In-Memory
   Collection).
 v1 -> v2 :
 - Account for the cases where a PMU can have a common scale and unit
   values for all its supported events (Patch 3/6).
 - Fixed a Build error (for maple_defconfig) by enabling imc_pmu.o
   only for CONFIG_PPC_POWERNV=y (Patch 4/6)
 - Read from the "event-name" property instead of "name" for an event
   node (Patch 3/6).

Anju T Sudhakar (1):
  powerpc/perf: Thread imc cpuhotplug support

Hemant Kumar (10):
  powerpc/powernv: Data structure and macros definitions
  powerpc/powernv: Autoload IMC device driver module
  powerpc/powernv: Detect supported IMC units and its events
  powerpc/perf: Add event attribute and group to IMC pmus
  powerpc/perf: Generic imc pmu event functions
  powerpc/perf: IMC pmu cpumask and cpu hotplug support
  powerpc/powernv: Core IMC events detection
  powerpc/perf: PMU functions for Core IMC and hotplugging
  powerpc/powernv: Thread IMC events detection
  powerpc/perf: Thread IMC PMU functions

 arch/powerpc/include/asm/imc-pmu.h             |  81 +++
 arch/powerpc/include/asm/opal-api.h            |  21 +-
 arch/powerpc/include/asm/opal.h                |   5 +
 arch/powerpc/perf/Makefile                     |   6 +-
 arch/powerpc/perf/imc-pmu.c                    | 837 +++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/Makefile        |   2 +-
 arch/powerpc/platforms/powernv/opal-imc.c      | 575 +++++++++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S |   2 +
 arch/powerpc/platforms/powernv/opal.c          |  14 +
 include/linux/cpuhotplug.h                     |   3 +
 10 files changed, 1543 insertions(+), 3 deletions(-)
 create mode 100644 arch/powerpc/include/asm/imc-pmu.h
 create mode 100644 arch/powerpc/perf/imc-pmu.c
 create mode 100644 arch/powerpc/platforms/powernv/opal-imc.c

-- 
2.7.4

[toc] | [next] | [standalone]

#1615301 — [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module

From: Hemant Kumar <hemant@linux.vnet.ibm.com>

This patch does three things :
 - Enables "opal.c" to create a platform device for the IMC interface
   according to the appropriate compatibility string.
 - Find the reserved-memory region details from the system device tree
   and get the base address of HOMER (Reserved memory) region address for each chip.
 - We also get the Nest PMU counter data offsets (in the HOMER region)
   and their sizes. The offsets for the counters' data are fixed and
   won't change from chip to chip.

The device tree parsing logic is separated from the PMU creation
functions (which is done in subsequent patches).

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/Makefile   |   2 +-
 arch/powerpc/platforms/powernv/opal-imc.c | 126 ++++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c     |  14 ++++
 3 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/opal-imc.c

diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index b5d98cb3f482..44909fec1121 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -2,7 +2,7 @@ obj-y			+= setup.o opal-wrappers.o opal.o opal-async.o idle.o
 obj-y			+= opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
 obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
 obj-y			+= opal-msglog.o opal-hmi.o opal-power.o opal-irqchip.o
-obj-y			+= opal-kmsg.o
+obj-y			+= opal-kmsg.o opal-imc.o
 
 obj-$(CONFIG_SMP)	+= smp.o subcore.o subcore-asm.o
 obj-$(CONFIG_PCI)	+= pci.o pci-ioda.o npu-dma.o
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
new file mode 100644
index 000000000000..c476d596c6a8
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -0,0 +1,126 @@
+/*
+ * OPAL IMC interface detection driver
+ * Supported on POWERNV platform
+ *
+ * Copyright	(C) 2016 Madhavan Srinivasan, IBM Corporation.
+ *		(C) 2016 Hemant K Shaw, IBM Corporation.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/miscdevice.h>
+#include <linux/fs.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/poll.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/crash_dump.h>
+#include <asm/opal.h>
+#include <asm/io.h>
+#include <asm/uaccess.h>
+#include <asm/cputable.h>
+#include <asm/imc-pmu.h>
+
+struct perchip_nest_info nest_perchip_info[IMC_MAX_CHIPS];
+
+static int opal_imc_counters_probe(struct platform_device *pdev)
+{
+	struct device_node *child, *imc_dev, *rm_node = NULL;
+	struct perchip_nest_info *pcni;
+	u32 pages, nest_offset, nest_size, idx;
+	int i = 0;
+	const char *node_name;
+	const __be32 *addrp;
+	u64 reg_addr, reg_size;
+
+	if (!pdev || !pdev->dev.of_node)
+		return -ENODEV;
+
+	/*
+	 * Check whether this kdump kernel. If yes, just return.
+	 */
+	if (is_kdump_kernel())
+		return -ENODEV;
+
+	imc_dev = pdev->dev.of_node;
+
+	/*
+	 * nest_offset : where the nest-counters' data start.
+	 * size : size of the entire nest-counters region
+	 */
+	if (of_property_read_u32(imc_dev, "imc-nest-offset", &nest_offset))
+		goto err;
+
+	if (of_property_read_u32(imc_dev, "imc-nest-size", &nest_size))
+		goto err;
+
+	/* Find the "homer region" for each chip */
+	rm_node = of_find_node_by_path("/reserved-memory");
+	if (!rm_node)
+		goto err;
+
+	for_each_child_of_node(rm_node, child) {
+		if (of_property_read_string_index(child, "name", 0,
+						  &node_name))
+			continue;
+		if (strncmp("ibm,homer-image", node_name,
+			    strlen("ibm,homer-image")))
+			continue;
+
+		/* Get the chip id to which the above homer region belongs to */
+		if (of_property_read_u32(child, "ibm,chip-id", &idx))
+			goto err;
+
+		pcni = &nest_perchip_info[idx];
+		addrp = of_get_address(child, 0, &reg_size, NULL);
+		if (!addrp)
+			goto err;
+
+		/* Fetch the homer region base address */
+		reg_addr = of_read_number(addrp, 2);
+		pcni->pbase = reg_addr;
+		/* Add the nest IMC Base offset */
+		pcni->pbase = pcni->pbase + nest_offset;
+		/* Fetch the size of the homer region */
+		pcni->size = nest_size;
+
+		do {
+			pages = PAGE_SIZE * i;
+			pcni->vbase[i++] = (u64)phys_to_virt(pcni->pbase +
+							     pages);
+		} while (i < (pcni->size / PAGE_SIZE));
+	}
+
+	return 0;
+err:
+	return -ENODEV;
+}
+
+static const struct of_device_id opal_imc_match[] = {
+	{ .compatible = IMC_DTB_COMPAT },
+	{},
+};
+
+static struct platform_driver opal_imc_driver = {
+	.driver = {
+		.name = "opal-imc-counters",
+		.of_match_table = opal_imc_match,
+	},
+	.probe = opal_imc_counters_probe,
+};
+
+MODULE_DEVICE_TABLE(of, opal_imc_match);
+module_platform_driver(opal_imc_driver);
+MODULE_DESCRIPTION("PowerNV OPAL IMC driver");
+MODULE_LICENSE("GPL");
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index e0f856bfbfe8..85ea1296f030 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -14,6 +14,7 @@
 #include <linux/printk.h>
 #include <linux/types.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_fdt.h>
 #include <linux/of_platform.h>
 #include <linux/interrupt.h>
@@ -30,6 +31,7 @@
 #include <asm/opal.h>
 #include <asm/firmware.h>
 #include <asm/mce.h>
+#include <asm/imc-pmu.h>
 
 #include "powernv.h"
 
@@ -631,6 +633,15 @@ static void opal_pdev_init(const char *compatible)
 		of_platform_device_create(np, NULL, NULL);
 }
 
+static void opal_imc_init_dev(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);
+	if (np)
+		of_platform_device_create(np, NULL, NULL);
+}
+
 static int kopald(void *unused)
 {
 	unsigned long timeout = msecs_to_jiffies(opal_heartbeat) + 1;
@@ -704,6 +715,9 @@ static int __init opal_init(void)
 	/* Setup a heatbeat thread if requested by OPAL */
 	opal_init_heartbeat();
 
+	/* Detect IMC pmu counters support and create PMUs */
+	opal_imc_init_dev();
+
 	/* Create leds platform devices */
 	leds = of_find_node_by_path("/ibm,opal/leds");
 	if (leds) {
-- 
2.7.4

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

#1615640 — Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module

Hi all,

I'm trying to get my head around these patches - at this point I'm just
doing a first pass, so I may have more substantive structural comments
later on. In the mean time - here are some minor C nits:

> + * Copyright	(C) 2016 Madhavan Srinivasan, IBM Corporation.
> + *		(C) 2016 Hemant K Shaw, IBM Corporation.

Should these be bumped to 2017?

> +
> +		do {
> +			pages = PAGE_SIZE * i;
> +			pcni->vbase[i++] = (u64)phys_to_virt(pcni->pbase +
> +							     pages);
> +		} while (i < (pcni->size / PAGE_SIZE));
> +	}
I had to scroll back up to the top of this function to make sure I
understood what this loop does. Would it be better to write it as:

for (i = 0; i < (pcni->size / PAGE_SIZE); i++) {
    pages = PAGE_SIZE * i;
    pcni->vbase[i] = (u64)....
}

And, just checking - this is expected to work on both 4 and 64kB pages?

> +
> +	return 0;
> +err:
> +	return -ENODEV;

You're not releasing any resources here - would it be better to just
replace the gotos with this return? I haven't checked to see if you
change the function later on to allocate memory - if so please ignore :)

> +}
> +
> +static const struct of_device_id opal_imc_match[] = {
> +	{ .compatible = IMC_DTB_COMPAT },
> +	{},
> +};
> +
> +static struct platform_driver opal_imc_driver = {
> +	.driver = {
> +		.name = "opal-imc-counters",
> +		.of_match_table = opal_imc_match,
> +	},
> +	.probe = opal_imc_counters_probe,
> +};
> +
> +MODULE_DEVICE_TABLE(of, opal_imc_match);
> +module_platform_driver(opal_imc_driver);
> +MODULE_DESCRIPTION("PowerNV OPAL IMC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index e0f856bfbfe8..85ea1296f030 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -14,6 +14,7 @@
>  #include <linux/printk.h>
>  #include <linux/types.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/of_fdt.h>
>  #include <linux/of_platform.h>
>  #include <linux/interrupt.h>
> @@ -30,6 +31,7 @@
>  #include <asm/opal.h>
>  #include <asm/firmware.h>
>  #include <asm/mce.h>
> +#include <asm/imc-pmu.h>
>  
>  #include "powernv.h"
>  
> @@ -631,6 +633,15 @@ static void opal_pdev_init(const char *compatible)
>  		of_platform_device_create(np, NULL, NULL);
>  }
>  
> +static void opal_imc_init_dev(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);
> +	if (np)
> +		of_platform_device_create(np, NULL, NULL);
> +}

Should this function be tagged __init?

> +
>  static int kopald(void *unused)
>  {
>  	unsigned long timeout = msecs_to_jiffies(opal_heartbeat) + 1;
> @@ -704,6 +715,9 @@ static int __init opal_init(void)
>  	/* Setup a heatbeat thread if requested by OPAL */
>  	opal_init_heartbeat();
>  
> +	/* Detect IMC pmu counters support and create PMUs */
> +	opal_imc_init_dev();
> +
>  	/* Create leds platform devices */
>  	leds = of_find_node_by_path("/ibm,opal/leds");
>  	if (leds) {
> -- 
> 2.7.4

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

#1616618 — Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module

On Tuesday 04 April 2017 06:28 AM, Daniel Axtens wrote:
> Hi all,
>
> I'm trying to get my head around these patches - at this point I'm just
> doing a first pass, so I may have more substantive structural comments
> later on. In the mean time - here are some minor C nits:
>
>> + * Copyright	(C) 2016 Madhavan Srinivasan, IBM Corporation.
>> + *		(C) 2016 Hemant K Shaw, IBM Corporation.
> Should these be bumped to 2017?

Facepalm. my bad. Will fix it.
>> +
>> +		do {
>> +			pages = PAGE_SIZE * i;
>> +			pcni->vbase[i++] = (u64)phys_to_virt(pcni->pbase +
>> +							     pages);
>> +		} while (i < (pcni->size / PAGE_SIZE));
>> +	}
> I had to scroll back up to the top of this function to make sure I
> understood what this loop does. Would it be better to write it as:
>
> for (i = 0; i < (pcni->size / PAGE_SIZE); i++) {
>      pages = PAGE_SIZE * i;
>      pcni->vbase[i] = (u64)....
> }
Idea is to map all of the nest counter area since event
offset could be anywhere within this region.
Will document that here.

>
> And, just checking - this is expected to work on both 4 and 64kB pages?
Yes. thats the intended. That said, i need to fix the
IMC_NEST_MAX_PAGES value for 4K page size.
Reason being, there was a recent change in the
size of memory allocated for nest counters in the
HOMER region for the microcode.

>
>> +
>> +	return 0;
>> +err:
>> +	return -ENODEV;
> You're not releasing any resources here - would it be better to just
> replace the gotos with this return? I haven't checked to see if you
> change the function later on to allocate memory - if so please ignore :)

We check in multiple places in the function and return on fail.
Thats why we made it as a generic return with goto.


>> +}
>> +
>> +static const struct of_device_id opal_imc_match[] = {
>> +	{ .compatible = IMC_DTB_COMPAT },
>> +	{},
>> +};
>> +
>> +static struct platform_driver opal_imc_driver = {
>> +	.driver = {
>> +		.name = "opal-imc-counters",
>> +		.of_match_table = opal_imc_match,
>> +	},
>> +	.probe = opal_imc_counters_probe,
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, opal_imc_match);
>> +module_platform_driver(opal_imc_driver);
>> +MODULE_DESCRIPTION("PowerNV OPAL IMC driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>> index e0f856bfbfe8..85ea1296f030 100644
>> --- a/arch/powerpc/platforms/powernv/opal.c
>> +++ b/arch/powerpc/platforms/powernv/opal.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/printk.h>
>>   #include <linux/types.h>
>>   #include <linux/of.h>
>> +#include <linux/of_address.h>
>>   #include <linux/of_fdt.h>
>>   #include <linux/of_platform.h>
>>   #include <linux/interrupt.h>
>> @@ -30,6 +31,7 @@
>>   #include <asm/opal.h>
>>   #include <asm/firmware.h>
>>   #include <asm/mce.h>
>> +#include <asm/imc-pmu.h>
>>   
>>   #include "powernv.h"
>>   
>> @@ -631,6 +633,15 @@ static void opal_pdev_init(const char *compatible)
>>   		of_platform_device_create(np, NULL, NULL);
>>   }
>>   
>> +static void opal_imc_init_dev(void)
>> +{
>> +	struct device_node *np;
>> +
>> +	np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);
>> +	if (np)
>> +		of_platform_device_create(np, NULL, NULL);
>> +}
> Should this function be tagged __init?

Yes. Thats right. Will make the changes.

Thanks for review
Maddy

>
>> +
>>   static int kopald(void *unused)
>>   {
>>   	unsigned long timeout = msecs_to_jiffies(opal_heartbeat) + 1;
>> @@ -704,6 +715,9 @@ static int __init opal_init(void)
>>   	/* Setup a heatbeat thread if requested by OPAL */
>>   	opal_init_heartbeat();
>>   
>> +	/* Detect IMC pmu counters support and create PMUs */
>> +	opal_imc_init_dev();
>> +
>>   	/* Create leds platform devices */
>>   	leds = of_find_node_by_path("/ibm,opal/leds");
>>   	if (leds) {
>> -- 
>> 2.7.4

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

#1615646 — Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module

Hi,

> +		do {
> +			pages = PAGE_SIZE * i;
> +			pcni->vbase[i++] = (u64)phys_to_virt(pcni->pbase +
> +							     pages);
> +		} while (i < (pcni->size / PAGE_SIZE));

I also just noticed that there's no check here against
IMC_NEST_MAX_PAGES: should that be inserted? (If for no other reason
than to stop every static analysis tool complaining!)

Daniel

> +	}
> +
> +	return 0;
> +err:
> +	return -ENODEV;
> +}
> +
> +static const struct of_device_id opal_imc_match[] = {
> +	{ .compatible = IMC_DTB_COMPAT },
> +	{},
> +};
> +
> +static struct platform_driver opal_imc_driver = {
> +	.driver = {
> +		.name = "opal-imc-counters",
> +		.of_match_table = opal_imc_match,
> +	},
> +	.probe = opal_imc_counters_probe,
> +};
> +
> +MODULE_DEVICE_TABLE(of, opal_imc_match);
> +module_platform_driver(opal_imc_driver);
> +MODULE_DESCRIPTION("PowerNV OPAL IMC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index e0f856bfbfe8..85ea1296f030 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -14,6 +14,7 @@
>  #include <linux/printk.h>
>  #include <linux/types.h>
>  #include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <linux/of_fdt.h>
>  #include <linux/of_platform.h>
>  #include <linux/interrupt.h>
> @@ -30,6 +31,7 @@
>  #include <asm/opal.h>
>  #include <asm/firmware.h>
>  #include <asm/mce.h>
> +#include <asm/imc-pmu.h>
>  
>  #include "powernv.h"
>  
> @@ -631,6 +633,15 @@ static void opal_pdev_init(const char *compatible)
>  		of_platform_device_create(np, NULL, NULL);
>  }
>  
> +static void opal_imc_init_dev(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);
> +	if (np)
> +		of_platform_device_create(np, NULL, NULL);
> +}
> +
>  static int kopald(void *unused)
>  {
>  	unsigned long timeout = msecs_to_jiffies(opal_heartbeat) + 1;
> @@ -704,6 +715,9 @@ static int __init opal_init(void)
>  	/* Setup a heatbeat thread if requested by OPAL */
>  	opal_init_heartbeat();
>  
> +	/* Detect IMC pmu counters support and create PMUs */
> +	opal_imc_init_dev();
> +
>  	/* Create leds platform devices */
>  	leds = of_find_node_by_path("/ibm,opal/leds");
>  	if (leds) {
> -- 
> 2.7.4

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

#1616611 — Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module

On Tuesday 04 April 2017 07:18 AM, Daniel Axtens wrote:
> Hi,
>
>> +		do {
>> +			pages = PAGE_SIZE * i;
>> +			pcni->vbase[i++] = (u64)phys_to_virt(pcni->pbase +
>> +							     pages);
>> +		} while (i < (pcni->size / PAGE_SIZE));
> I also just noticed that there's no check here against
> IMC_NEST_MAX_PAGES: should that be inserted? (If for no other reason
> than to stop every static analysis tool complaining!)
Yes make sense. Can add that in the next version.

Thanks for review
Maddy


> Daniel
>
>> +	}
>> +
>> +	return 0;
>> +err:
>> +	return -ENODEV;
>> +}
>> +
>> +static const struct of_device_id opal_imc_match[] = {
>> +	{ .compatible = IMC_DTB_COMPAT },
>> +	{},
>> +};
>> +
>> +static struct platform_driver opal_imc_driver = {
>> +	.driver = {
>> +		.name = "opal-imc-counters",
>> +		.of_match_table = opal_imc_match,
>> +	},
>> +	.probe = opal_imc_counters_probe,
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, opal_imc_match);
>> +module_platform_driver(opal_imc_driver);
>> +MODULE_DESCRIPTION("PowerNV OPAL IMC driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
>> index e0f856bfbfe8..85ea1296f030 100644
>> --- a/arch/powerpc/platforms/powernv/opal.c
>> +++ b/arch/powerpc/platforms/powernv/opal.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/printk.h>
>>   #include <linux/types.h>
>>   #include <linux/of.h>
>> +#include <linux/of_address.h>
>>   #include <linux/of_fdt.h>
>>   #include <linux/of_platform.h>
>>   #include <linux/interrupt.h>
>> @@ -30,6 +31,7 @@
>>   #include <asm/opal.h>
>>   #include <asm/firmware.h>
>>   #include <asm/mce.h>
>> +#include <asm/imc-pmu.h>
>>   
>>   #include "powernv.h"
>>   
>> @@ -631,6 +633,15 @@ static void opal_pdev_init(const char *compatible)
>>   		of_platform_device_create(np, NULL, NULL);
>>   }
>>   
>> +static void opal_imc_init_dev(void)
>> +{
>> +	struct device_node *np;
>> +
>> +	np = of_find_compatible_node(NULL, NULL, IMC_DTB_COMPAT);
>> +	if (np)
>> +		of_platform_device_create(np, NULL, NULL);
>> +}
>> +
>>   static int kopald(void *unused)
>>   {
>>   	unsigned long timeout = msecs_to_jiffies(opal_heartbeat) + 1;
>> @@ -704,6 +715,9 @@ static int __init opal_init(void)
>>   	/* Setup a heatbeat thread if requested by OPAL */
>>   	opal_init_heartbeat();
>>   
>> +	/* Detect IMC pmu counters support and create PMUs */
>> +	opal_imc_init_dev();
>> +
>>   	/* Create leds platform devices */
>>   	leds = of_find_node_by_path("/ibm,opal/leds");
>>   	if (leds) {
>> -- 
>> 2.7.4

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

#1617554 — Re: [PATCH v6 02/11] powerpc/powernv: Autoload IMC device driver module

Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> From: Hemant Kumar <hemant@linux.vnet.ibm.com>
>
> This patch does three things :
>  - Enables "opal.c" to create a platform device for the IMC interface
>    according to the appropriate compatibility string.
>  - Find the reserved-memory region details from the system device tree
>    and get the base address of HOMER (Reserved memory) region address for each chip.
>  - We also get the Nest PMU counter data offsets (in the HOMER region)
>    and their sizes. The offsets for the counters' data are fixed and
>    won't change from chip to chip.
>
> The device tree parsing logic is separated from the PMU creation
> functions (which is done in subsequent patches).
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/Makefile   |   2 +-
>  arch/powerpc/platforms/powernv/opal-imc.c | 126 ++++++++++++++++++++++++++++++
>  arch/powerpc/platforms/powernv/opal.c     |  14 ++++
>  3 files changed, 141 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/platforms/powernv/opal-imc.c
>
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index b5d98cb3f482..44909fec1121 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -2,7 +2,7 @@ obj-y			+= setup.o opal-wrappers.o opal.o opal-async.o idle.o
>  obj-y			+= opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
>  obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
>  obj-y			+= opal-msglog.o opal-hmi.o opal-power.o opal-irqchip.o
> -obj-y			+= opal-kmsg.o
> +obj-y			+= opal-kmsg.o opal-imc.o
>
>  obj-$(CONFIG_SMP)	+= smp.o subcore.o subcore-asm.o
>  obj-$(CONFIG_PCI)	+= pci.o pci-ioda.o npu-dma.o
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> new file mode 100644
> index 000000000000..c476d596c6a8
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -0,0 +1,126 @@
> +/*
> + * OPAL IMC interface detection driver
> + * Supported on POWERNV platform
> + *
> + * Copyright	(C) 2016 Madhavan Srinivasan, IBM Corporation.
> + *		(C) 2016 Hemant K Shaw, IBM Corporation.
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/miscdevice.h>
> +#include <linux/fs.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/poll.h>
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/crash_dump.h>
> +#include <asm/opal.h>
> +#include <asm/io.h>
> +#include <asm/uaccess.h>
> +#include <asm/cputable.h>
> +#include <asm/imc-pmu.h>
> +
> +struct perchip_nest_info nest_perchip_info[IMC_MAX_CHIPS];
> +
> +static int opal_imc_counters_probe(struct platform_device *pdev)
> +{
> +	struct device_node *child, *imc_dev, *rm_node = NULL;
> +	struct perchip_nest_info *pcni;
> +	u32 pages, nest_offset, nest_size, idx;
> +	int i = 0;
> +	const char *node_name;
> +	const __be32 *addrp;
> +	u64 reg_addr, reg_size;
> +
> +	if (!pdev || !pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	/*
> +	 * Check whether this kdump kernel. If yes, just return.
> +	 */
> +	if (is_kdump_kernel())
> +		return -ENODEV;
> +
> +	imc_dev = pdev->dev.of_node;
> +
> +	/*
> +	 * nest_offset : where the nest-counters' data start.
> +	 * size : size of the entire nest-counters region
> +	 */
> +	if (of_property_read_u32(imc_dev, "imc-nest-offset", &nest_offset))
> +		goto err;
> +
> +	if (of_property_read_u32(imc_dev, "imc-nest-size", &nest_size))
> +		goto err;
> +
> +	/* Find the "homer region" for each chip */
> +	rm_node = of_find_node_by_path("/reserved-memory");
> +	if (!rm_node)
> +		goto err;
> +
> +	for_each_child_of_node(rm_node, child) {
> +		if (of_property_read_string_index(child, "name", 0,
> +						  &node_name))
> +			continue;
> +		if (strncmp("ibm,homer-image", node_name,
> +			    strlen("ibm,homer-image")))
> +			continue;

A better way to do this would be to reference the memory region, like
what's shown in
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt

just reference the phandle of the memory region.

seeing as these are per chip, why not just have something linking
together chip-id and the IMC layout node?

-- 
Stewart Smith
OPAL Architect, IBM.

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

#1615303 — [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions

From: Hemant Kumar <hemant@linux.vnet.ibm.com>

Create new header file "imc-pmu.h" to add the data structures
and macros needed for IMC pmu support.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/imc-pmu.h | 68 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 arch/powerpc/include/asm/imc-pmu.h

diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
new file mode 100644
index 000000000000..a3d4f1bf9492
--- /dev/null
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -0,0 +1,68 @@
+#ifndef PPC_POWERNV_IMC_PMU_DEF_H
+#define PPC_POWERNV_IMC_PMU_DEF_H
+
+/*
+ * IMC Nest Performance Monitor counter support.
+ *
+ * Copyright (C) 2016 Madhavan Srinivasan, IBM Corporation.
+ *           (C) 2016 Hemant K Shaw, IBM Corporation.
+ *
+ * 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.
+ */
+
+#include <linux/perf_event.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <asm/opal.h>
+
+#define IMC_MAX_CHIPS			32
+#define IMC_MAX_PMUS			32
+#define IMC_MAX_PMU_NAME_LEN		256
+
+#define IMC_NEST_MAX_PAGES		16
+
+#define IMC_DTB_COMPAT			"ibm,opal-in-memory-counters"
+#define IMC_DTB_NEST_COMPAT		"ibm,imc-counters-nest"
+
+/*
+ * Structure to hold per chip specific memory address
+ * information for nest pmus. Nest Counter data are exported
+ * in per-chip reserved memory region by the PORE Engine.
+ */
+struct perchip_nest_info {
+	u32 chip_id;
+	u64 pbase;
+	u64 vbase[IMC_NEST_MAX_PAGES];
+	u64 size;
+};
+
+/*
+ * Place holder for nest pmu events and values.
+ */
+struct imc_events {
+	char *ev_name;
+	char *ev_value;
+};
+
+/*
+ * Device tree parser code detects IMC pmu support and
+ * registers new IMC pmus. This structure will
+ * hold the pmu functions and attrs for each imc pmu and
+ * will be referenced at the time of pmu registration.
+ */
+struct imc_pmu {
+	struct pmu pmu;
+	int domain;
+	const struct attribute_group *attr_groups[4];
+};
+
+/*
+ * Domains for IMC PMUs
+ */
+#define IMC_DOMAIN_NEST		1
+#define IMC_DOMAIN_UNKNOWN	-1
+
+#endif /* PPC_POWERNV_IMC_PMU_DEF_H */
-- 
2.7.4

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

#1615647 — Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions

Hi,

> +#define IMC_MAX_CHIPS			32
> +#define IMC_MAX_PMUS			32
> +#define IMC_MAX_PMU_NAME_LEN		256
I've noticed this is used as both the maximum length for event names and
event value strings. Would another name suit better?

> +
> +#define IMC_NEST_MAX_PAGES		16
> +
> +#define IMC_DTB_COMPAT			"ibm,opal-in-memory-counters"
> +#define IMC_DTB_NEST_COMPAT		"ibm,imc-counters-nest"
> +
> +/*
> + * Structure to hold per chip specific memory address
> + * information for nest pmus. Nest Counter data are exported
> + * in per-chip reserved memory region by the PORE Engine.
> + */
> +struct perchip_nest_info {
> +	u32 chip_id;
> +	u64 pbase;
> +	u64 vbase[IMC_NEST_MAX_PAGES];
> +	u64 size;
> +};
> +
> +/*
> + * Place holder for nest pmu events and values.
> + */
> +struct imc_events {
> +	char *ev_name;
> +	char *ev_value;
> +};
> +
> +/*
> + * Device tree parser code detects IMC pmu support and
> + * registers new IMC pmus. This structure will
> + * hold the pmu functions and attrs for each imc pmu and
> + * will be referenced at the time of pmu registration.
> + */
> +struct imc_pmu {
> +	struct pmu pmu;
> +	int domain;
> +	const struct attribute_group *attr_groups[4];
> +};
> +
> +/*
> + * Domains for IMC PMUs
> + */
> +#define IMC_DOMAIN_NEST		1
> +#define IMC_DOMAIN_UNKNOWN	-1
> +
> +#endif /* PPC_POWERNV_IMC_PMU_DEF_H */
> -- 
> 2.7.4

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

#1616563 — Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions

On Tuesday 04 April 2017 07:18 AM, Daniel Axtens wrote:
> Hi,
>
>> +#define IMC_MAX_CHIPS			32
>> +#define IMC_MAX_PMUS			32
>> +#define IMC_MAX_PMU_NAME_LEN		256
> I've noticed this is used as both the maximum length for event names and
> event value strings. Would another name suit better?

This is used in the value string length comparison also. So yes, will
change the name to suit better.

Thanks for review
Maddy

>
>> +
>> +#define IMC_NEST_MAX_PAGES		16
>> +
>> +#define IMC_DTB_COMPAT			"ibm,opal-in-memory-counters"
>> +#define IMC_DTB_NEST_COMPAT		"ibm,imc-counters-nest"
>> +
>> +/*
>> + * Structure to hold per chip specific memory address
>> + * information for nest pmus. Nest Counter data are exported
>> + * in per-chip reserved memory region by the PORE Engine.
>> + */
>> +struct perchip_nest_info {
>> +	u32 chip_id;
>> +	u64 pbase;
>> +	u64 vbase[IMC_NEST_MAX_PAGES];
>> +	u64 size;
>> +};
>> +
>> +/*
>> + * Place holder for nest pmu events and values.
>> + */
>> +struct imc_events {
>> +	char *ev_name;
>> +	char *ev_value;
>> +};
>> +
>> +/*
>> + * Device tree parser code detects IMC pmu support and
>> + * registers new IMC pmus. This structure will
>> + * hold the pmu functions and attrs for each imc pmu and
>> + * will be referenced at the time of pmu registration.
>> + */
>> +struct imc_pmu {
>> +	struct pmu pmu;
>> +	int domain;
>> +	const struct attribute_group *attr_groups[4];
>> +};
>> +
>> +/*
>> + * Domains for IMC PMUs
>> + */
>> +#define IMC_DOMAIN_NEST		1
>> +#define IMC_DOMAIN_UNKNOWN	-1
>> +
>> +#endif /* PPC_POWERNV_IMC_PMU_DEF_H */
>> -- 
>> 2.7.4

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

#1617579 — Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions

Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:

> From: Hemant Kumar <hemant@linux.vnet.ibm.com>
>
> Create new header file "imc-pmu.h" to add the data structures
> and macros needed for IMC pmu support.
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/imc-pmu.h | 68 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 arch/powerpc/include/asm/imc-pmu.h
>
> diff --git a/arch/powerpc/include/asm/imc-pmu.h b/arch/powerpc/include/asm/imc-pmu.h
> new file mode 100644
> index 000000000000..a3d4f1bf9492
> --- /dev/null
> +++ b/arch/powerpc/include/asm/imc-pmu.h
> @@ -0,0 +1,68 @@
> +#ifndef PPC_POWERNV_IMC_PMU_DEF_H
> +#define PPC_POWERNV_IMC_PMU_DEF_H
> +
> +/*
> + * IMC Nest Performance Monitor counter support.
> + *
> + * Copyright (C) 2016 Madhavan Srinivasan, IBM Corporation.
> + *           (C) 2016 Hemant K Shaw, IBM Corporation.
> + *
> + * 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.
> + */
> +
> +#include <linux/perf_event.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <asm/opal.h>
> +
> +#define IMC_MAX_CHIPS			32
> +#define IMC_MAX_PMUS			32
> +#define IMC_MAX_PMU_NAME_LEN		256

Why do we need a max length? We get the actual lengths from the device
tree, so we know at each point in time what the length of any new string
should be, right?

Otherwise you appear to be, in the general case, using 10x the memory
than you could.

-- 
Stewart Smith
OPAL Architect, IBM.

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

#1617600 — Re: [PATCH v6 01/11] powerpc/powernv: Data structure and macros definitions

Madhavan Srinivasan <maddy@linux.vnet.ibm.com> writes:
> +#define IMC_MAX_CHIPS			32
> +#define IMC_MAX_PMUS			32

The max chips and PMUs we'd be able to work out from the device tre
though, right? We could just allocate the correct amount of memory on
boot.

We may hot plug/unplug CPUs, but we're not doing that from a hardware
level, what CPUs you get in the DT on PowerNV on boot is all you're
getting.

-- 
Stewart Smith
OPAL Architect, IBM.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web