From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:06
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-May/007167.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)
Changelog:
v7 -> v8:
- opal-call API for nest and core is changed.
OPAL_NEST_IMC_COUNTERS_CONTROL and
OPAL_CORE_IMC_COUNTERS_CONTROL is replaced with
OPAL_IMC_COUNTERS_INIT, OPAL_IMC_COUNTERS_START and
OPAL_IMC_COUNTERS_STOP.
- thread_ima doesn't have CPUMASK_ATTR, hence added a
fix in patch 09/10, which will swap the IMC_EVENT_ATTR
slot with IMC_CPUMASK_ATTR.
v6 -> v7:
- Updated the commit message and code comments.
- Changed the counter init code to disable the
nest/core counters by default and enable only
when it is used.
- Updated the pmu-setup code to register the
PMUs which doesn't have events.
- replaced imc_event_info_val() to imc_event_prop_update()
- Updated the imc_pmu_setup() code, by checking for the "value"
of compatible property instead of merely checking for compatible.
- removed imc_get_domain().
- init_imc_pmu() and imc_pmu_setup() are made __init.
- update_max_val() is invoked immediately after updating the offset value.
v5 -> v6:
- merged few patches for the readability and code flow
- Updated the commit message and code comments.
- 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 (6):
powerpc/powernv: Autoload IMC device driver module
powerpc/powernv: Detect supported IMC units and its events
powerpc/perf: IMC pmu cpumask and cpuhotplug support
powerpc/powernv: Thread IMC events detection
powerpc/perf: Thread IMC PMU functions
powerpc/perf: Thread imc cpuhotplug support
Hemant Kumar (4):
powerpc/powernv: Data structure and macros definitions for IMC
powerpc/perf: Add generic IMC pmu groupand event functions
powerpc/powernv: Core IMC events detection
powerpc/perf: PMU functions for Core IMC and hotplugging
arch/powerpc/include/asm/imc-pmu.h | 118 +++
arch/powerpc/include/asm/opal-api.h | 12 +-
arch/powerpc/include/asm/opal.h | 4 +
arch/powerpc/perf/Makefile | 3 +
arch/powerpc/perf/imc-pmu.c | 1098 ++++++++++++++++++++++++
arch/powerpc/platforms/powernv/Kconfig | 10 +
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-imc.c | 607 +++++++++++++
arch/powerpc/platforms/powernv/opal-wrappers.S | 3 +
arch/powerpc/platforms/powernv/opal.c | 18 +
include/linux/cpuhotplug.h | 3 +
11 files changed, 1876 insertions(+), 1 deletion(-)
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
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:14
Parse device tree to detect IMC units. Traverse through each IMC unit
node to find supported events and corresponding unit/scale files (if any).
Here is the DTS file for reference:
https://github.com/open-power/ima-catalog/blob/master/81E00612.4E0100.dts
The device tree for IMC counters starts at the node "imc-counters".
This node contains all the IMC PMU nodes and event nodes
for these IMC PMUs. The PMU nodes have an "events" property which has a
phandle value for the actual events node. The events are separated from
the PMU nodes to abstract out the common events. For example, PMU node
"mcs0", "mcs1" etc. will contain a pointer to "nest-mcs-events" since,
the events are common between these PMUs. These events have a different
prefix based on their relation to different PMUs, and hence, the PMU
nodes themselves contain an "events-prefix" property. The value for this
property concatenated to the event name, forms the actual event
name. Also, the PMU have a "reg" field as the base offset for the events
which belong to this PMU. This "reg" field is added to event's "reg" field
in the "events" node, which gives us the location of the counter data. Kernel
code uses this offset as event configuration value.
Device tree parser code also looks for scale/unit property in the event
node and passes on the value as an event attr for perf interface to use
in the post processing by the perf tool. Some PMUs may have common scale
and unit properties which implies that all events supported by this PMU
inherit the scale and unit properties of the PMU itself. For those
events, we need to set the common unit and scale values.
For failure to initialize any unit or any event, disable that unit and
continue setting up the rest of them.
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/platforms/powernv/opal-imc.c | 413 ++++++++++++++++++++++++++++++
1 file changed, 413 insertions(+)
@@ -33,15 +33,428 @@#include<asm/cputable.h>#include<asm/imc-pmu.h>+u64nest_max_offset;structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];+structimc_pmu*per_nest_pmu_arr[IMC_MAX_PMUS];++staticintimc_event_prop_update(char*name,structimc_events*events)+{+char*buf;++if(!events||!name)+return-EINVAL;++/* memory for content */+buf=kzalloc(IMC_MAX_NAME_VAL_LEN,GFP_KERNEL);+if(!buf)+return-ENOMEM;++events->ev_name=name;+events->ev_value=buf;+return0;+}++staticintimc_event_prop_str(structproperty*pp,char*name,+structimc_events*events)+{+intret;++ret=imc_event_prop_update(name,events);+if(ret)+returnret;++if(!pp->value||(strnlen(pp->value,pp->length)==pp->length)||+(pp->length>IMC_MAX_NAME_VAL_LEN))+return-EINVAL;+strncpy(events->ev_value,(constchar*)pp->value,pp->length);++return0;+}++staticintimc_event_prop_val(char*name,u32val,+structimc_events*events)+{+intret;++ret=imc_event_prop_update(name,events);+if(ret)+returnret;+snprintf(events->ev_value,IMC_MAX_NAME_VAL_LEN,"event=0x%x",val);++return0;+}++staticintset_event_property(structproperty*pp,char*event_prop,+structimc_events*events,char*ev_name)+{+char*buf;+intret;++buf=kzalloc(IMC_MAX_NAME_VAL_LEN,GFP_KERNEL);+if(!buf)+return-ENOMEM;++sprintf(buf,"%s.%s",ev_name,event_prop);+ret=imc_event_prop_str(pp,buf,events);+if(ret){+if(events->ev_name)+kfree(events->ev_name);+if(events->ev_value)+kfree(events->ev_value);+}+returnret;+}++/*+*Updatesthemaximumoffsetforaneventinthepmuwithdomain+*"pmu_domain".+*/+staticvoidupdate_max_value(u32value,intpmu_domain)+{+switch(pmu_domain){+caseIMC_DOMAIN_NEST:+if(nest_max_offset<value)+nest_max_offset=value;+break;+default:+/* Unknown domain, return */+return;+}+}++/*+*imc_events_node_parser:Parsetheeventnode"dev"andassigntheparsed+*informationtoevent"events".+*+*Parsesthe"reg","scale"and"unit"propertiesofthisevent.+*"reg"givesustheeventoffsetinthecountermemory.+*/+staticintimc_events_node_parser(structdevice_node*dev,+structimc_events*events,+structproperty*event_scale,+structproperty*event_unit,+structproperty*name_prefix,+u32reg,intpmu_domain)+{+structproperty*name,*pp;+char*ev_name;+u32val;+intidx=0,ret;++if(!dev)+gotofail;++/* Find the event name */+name=of_find_property(dev,"event-name",NULL);+if(!name)+return-ENODEV;++if(!name->value||+(strnlen(name->value,name->length)==name->length)||+(name->length>IMC_MAX_NAME_VAL_LEN))+return-EINVAL;++ev_name=kzalloc(IMC_MAX_NAME_VAL_LEN,GFP_KERNEL);+if(!ev_name)+return-ENOMEM;++snprintf(ev_name,IMC_MAX_NAME_VAL_LEN,"%s%s",+(char*)name_prefix->value,+(char*)name->value);++/*+*Parseeachpropertyofthiseventnode"dev".Property"reg"has+*theoffsetwhichisassignedtotheeventname.Otherproperties+*like"scale"and"unit"areassignedtoevent.scaleandevent.unit+*accordingly.+*/+for_each_property_of_node(dev,pp){+/*+*Ifthereisanissueinparsingasinglepropertyof+*thisevent,wejustcleanupthebuffers,butwestill+*continuetoparse.XXX:Thiscouldberewrittentoskipthe+*entireeventnodeincaseofparsingissues,butthatcanbe+*donelater.+*/+if(strncmp(pp->name,"reg",3)==0){+of_property_read_u32(dev,pp->name,&val);+val+=reg;+update_max_value(val,pmu_domain);+ret=imc_event_prop_val(ev_name,val,&events[idx]);+if(ret){+if(events[idx].ev_name)+kfree(events[idx].ev_name);+if(events[idx].ev_value)+kfree(events[idx].ev_value);+gotofail;+}+idx++;+/*+*Ifthecommonscaleandunitpropertiesavailable,+*then,assignthemtothisevent+*/+if(event_scale){+ret=set_event_property(event_scale,"scale",+&events[idx],+ev_name);+if(ret)+gotofail;+idx++;+}+if(event_unit){+ret=set_event_property(event_unit,"unit",+&events[idx],+ev_name);+if(ret)+gotofail;+idx++;+}+}elseif(strncmp(pp->name,"unit",4)==0){+/*+*Theevent'sunitandscalepropertiescanoverridethe+*PMU'seventandscaleproperties,ifpresent.+*/+ret=set_event_property(pp,"unit",&events[idx],+ev_name);+if(ret)+gotofail;+idx++;+}elseif(strncmp(pp->name,"scale",5)==0){+ret=set_event_property(pp,"scale",&events[idx],+ev_name);+if(ret)+gotofail;+idx++;+}+}++returnidx;+fail:+return-EINVAL;+}++/*+*get_nr_children:Returnsthenumberofchildrenforapmudevicenode.+*/+staticintget_nr_children(structdevice_node*pmu_node)+{+structdevice_node*child;+inti=0;++for_each_child_of_node(pmu_node,child)+i++;+returni;+}++/*+*imc_free_events:Cleanupthe"events"listhaving"nr_entries"entries.+*/+staticvoidimc_free_events(structimc_events*events,intnr_entries)+{+inti;++/* Nothing to clean, return */+if(!events)+return;++for(i=0;i<nr_entries;i++){+if(events[i].ev_name)+kfree(events[i].ev_name);+if(events[i].ev_value)+kfree(events[i].ev_value);+}++kfree(events);+}++/*+*imc_events_setup():Firstfindstheeventnodeforthepmuand+*getsthenumberofsupportedeventsandthen+*allocatesmemoryforthesame.Finallyreturnstheaddressofevents+*memoryallocated.+*/+staticstructimc_events*imc_events_setup(structdevice_node*parent,+intpmu_index,+structimc_pmu*pmu_ptr,+u32prop,+int*idx)+{+structdevice_node*ev_node=NULL,*dir=NULL;+u32reg;+structimc_events*events;+structproperty*scale_pp,*unit_pp,*name_prefix;+intret=0,nr_children=0;++/*+*FetchtheactualnodewheretheeventsforthisPMUexist.+*/+dir=of_find_node_by_phandle(prop);+if(!dir)+returnNULL;+/*+*Getthemaximumno.ofeventsinthisnode.+*Multiplyby3toaccountfor.scaleand.unitproperties+*Thisnumbersuggeststheamountofmemoryneededtosetupthe+*eventsforthispmu.+*/+nr_children=get_nr_children(dir)*3;++events=kzalloc((sizeof(structimc_events)*nr_children),+GFP_KERNEL);+if(!events)+returnNULL;++/*+*Checkifthereisacommon"scale"and"unit"propertiesinside+*thePMUnodeforalltheeventssupportedbythisPMU.+*/+scale_pp=of_find_property(parent,"scale",NULL);+unit_pp=of_find_property(parent,"unit",NULL);++/*+*Gettheevent-prefixpropertyfromthePMUnode+*whichneedstobeattachedwiththeeventnames.+*/+name_prefix=of_find_property(parent,"events-prefix",NULL);+if(!name_prefix)+gotofree_events;++/*+*"reg"propertygivesoutthebaseoffsetofthecountersdata+*forthisPMU.+*/+of_property_read_u32(parent,"reg",®);++if(!name_prefix->value||+(strnlen(name_prefix->value,name_prefix->length)==name_prefix->length)||+(name_prefix->length>IMC_MAX_NAME_VAL_LEN))+gotofree_events;++/* Loop through event nodes */+for_each_child_of_node(dir,ev_node){+ret=imc_events_node_parser(ev_node,&events[*idx],scale_pp,+unit_pp,name_prefix,reg,pmu_ptr->domain);+if(ret<0){+/* Unable to parse this event */+if(ret==-ENOMEM)+gotofree_events;+continue;+}++/*+*imc_event_node_parserwillreturnnumberof+*evententriescreatedforthis.Thiscouldinclude+*eventscaleandunitfilesalso.+*/+*idx+=ret;+}+returnevents;++free_events:+imc_free_events(events,*idx);+returnNULL;++}++/*+*imc_pmu_create:Takestheparentdevicewhichisthepmuunitanda+*pmu_indexastheinputs.+*Allocatesmemoryforthepmu,setsupitsdomain(NEST),and+*callsimc_events_setup()toallocatememoryfortheeventssupported+*bythispmu.Assignsanameforthepmu.Callsimc_events_node_parser()+*tosetuptheindividualevents.+*Ifeverythinggoesfine,itcalls,init_imc_pmu()tosetupthepmudevice+*andregisterit.+*/+staticintimc_pmu_create(structdevice_node*parent,intpmu_index,intdomain)+{+structimc_events*events=NULL;+structimc_pmu*pmu_ptr;+u32prop=0;+structproperty*pp;+char*buf;+intidx=0,ret=0;++if(!parent)+return-EINVAL;++/* memory for pmu */+pmu_ptr=kzalloc(sizeof(structimc_pmu),GFP_KERNEL);+if(!pmu_ptr)+return-ENOMEM;++pmu_ptr->domain=domain;+if(pmu_ptr->domain==IMC_DOMAIN_UNKNOWN)+gotofree_pmu;++/* Needed for hotplug/migration */+per_nest_pmu_arr[pmu_index]=pmu_ptr;++pp=of_find_property(parent,"name",NULL);+if(!pp){+ret=-ENODEV;+gotofree_pmu;+}++if(!pp->value||+(strnlen(pp->value,pp->length)==pp->length)||+(pp->length>IMC_MAX_NAME_VAL_LEN)){+ret=-EINVAL;+gotofree_pmu;+}++buf=kzalloc(IMC_MAX_NAME_VAL_LEN,GFP_KERNEL);+if(!buf){+ret=-ENOMEM;+gotofree_pmu;+}+/* Save the name to register it later */+sprintf(buf,"nest_%s",(char*)pp->value);+pmu_ptr->pmu.name=(char*)buf;++/*+*"events"propertyinsideaPMUnodecontainsthephandlevalue+*fortheactualeventsnode.The"events"nodefortheIMCPMU+*isnotinthisnode,ratherinside"imc-counters"node,since,+*wewanttofactoroutthecommonevents(thereby,reducingthe+*sizeofthedevicetree)+*/+of_property_read_u32(parent,"events",&prop);+if(prop)+events=imc_events_setup(parent,pmu_index,pmu_ptr,+prop,&idx);+return0;++free_pmu:+kfree(pmu_ptr);+returnret;+}+/**imc_pmu_setup:SetuptheIMCPMUs(childrenof"parent").+*+*Toplevel"imc-counters"nodecontainsbothevent-nodesandpmu+*unitnodes.Weonlyconsiderthepmuunitnodehere.*/staticvoid__initimc_pmu_setup(structdevice_node*parent){+structdevice_node*child;+intpmu_count=0,rc=0,domain;+if(!parent)return;+/*+*Loopthroughtheimc-counterstreeforeachcompatible+*"ibm,imc-counters-nest",andupdate"struct imc_pmu".+*/+for_each_compatible_node(child,NULL,IMC_DTB_NEST_COMPAT){+domain=IMC_DOMAIN_NEST;+rc=imc_pmu_create(child,pmu_count,domain);+if(rc)+return;+pmu_count++;+}}staticintopal_imc_counters_probe(structplatform_device*pdev)
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:18
From: Hemant Kumar <redacted>
Device tree IMC driver code parses the IMC units and their events. It
passes the information to IMC pmu code which is placed in powerpc/perf
as "imc-pmu.c".
Patch adds a set of generic imc pmu related event functions to be
used by each imc pmu unit. Add code to setup format attribute and to
register imc pmus. Add a event_init function for nest_imc events.
Since, the IMC counters' data are periodically fed to a memory location,
the functions to read/update, start/stop, add/del can be generic and can
be used by all IMC PMU units.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 3 +
arch/powerpc/perf/Makefile | 3 +
arch/powerpc/perf/imc-pmu.c | 269 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal-imc.c | 10 +-
4 files changed, 283 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/perf/imc-pmu.c
@@ -0,0 +1,269 @@+/*+*NestPerformanceMonitorcountersupport.+*+*Copyright(C)2017MadhavanSrinivasan,IBMCorporation.+*(C)2017AnjuTSudhakar,IBMCorporation.+*(C)2017HemantKShaw,IBMCorporation.+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseversion2aspublished+*bytheFreeSoftwareFoundation.+*/+#include<linux/perf_event.h>+#include<linux/slab.h>+#include<asm/opal.h>+#include<asm/imc-pmu.h>+#include<asm/cputhreads.h>+#include<asm/smp.h>+#include<linux/string.h>++structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];+structimc_pmu*per_nest_pmu_arr[IMC_MAX_PMUS];++/* Needed for sanity check */+externu64nest_max_offset;++PMU_FORMAT_ATTR(event,"config:0-20");+staticstructattribute*imc_format_attrs[]={+&format_attr_event.attr,+NULL,+};++staticstructattribute_groupimc_format_group={+.name="format",+.attrs=imc_format_attrs,+};++staticintnest_imc_event_init(structperf_event*event)+{+intchip_id;+u32config=event->attr.config;+structperchip_nest_info*pcni;++if(event->attr.type!=event->pmu->type)+return-ENOENT;++/* Sampling not supported */+if(event->hw.sample_period)+return-EINVAL;++/* unsupported modes and filters */+if(event->attr.exclude_user||+event->attr.exclude_kernel||+event->attr.exclude_hv||+event->attr.exclude_idle||+event->attr.exclude_host||+event->attr.exclude_guest)+return-EINVAL;++if(event->cpu<0)+return-EINVAL;++/* Sanity check for config (event offset) */+if(config>nest_max_offset)+return-EINVAL;++chip_id=topology_physical_package_id(event->cpu);+pcni=&nest_perchip_info[chip_id];++/*+*MemoryforNestHWcounterdatacouldbeinmultiplepages.+*Hencecheckandpicktherighteventbasepageforchipwith+*"chip_id"andadd"config"toit".+*/+event->hw.event_base=pcni->vbase[config/PAGE_SIZE]++(config&~PAGE_MASK);++return0;+}++staticvoidimc_read_counter(structperf_event*event)+{+u64*addr,data;++/*+*In-MemoryCollection(IMC)countersarefreeflowingcounters.+*Sowetakeasnapshotofthecountervalueonenableandsaveit+*tocalculatethedeltaatlaterstagetopresenttheeventcounter+*value.+*/+addr=(u64*)event->hw.event_base;+data=__be64_to_cpu(READ_ONCE(*addr));+local64_set(&event->hw.prev_count,data);+}++staticvoidimc_perf_event_update(structperf_event*event)+{+u64counter_prev,counter_new,final_count,*addr;++addr=(u64*)event->hw.event_base;+counter_prev=local64_read(&event->hw.prev_count);+counter_new=__be64_to_cpu(READ_ONCE(*addr));+final_count=counter_new-counter_prev;++/*+*Needtoupdateprev_countisthat,countercouldbe+*readinaperiodicintervalfromthetoolside.+*/+local64_set(&event->hw.prev_count,counter_new);+/* Update the delta to the event count */+local64_add(final_count,&event->count);+}++staticvoidimc_event_start(structperf_event*event,intflags)+{+/*+*InMemoryCountersarefreeflowingcounters.HWorthemicrocode+*keepsaddingtothecounteroffsetinmemory.Togetevent+*countervalue,wesnapshotthevaluehereandwecalculate+*deltaatlaterpoint.+*/+imc_read_counter(event);+}++staticvoidimc_event_stop(structperf_event*event,intflags)+{+/*+*Takeasnapshotandcalculatethedeltaandupdate+*theeventcountervalues.+*/+imc_perf_event_update(event);+}++/*+*Thewrapperfunctionisprovidedhere,sincewewillhavereserve+*andreleaselockforimc_event_start()inthefollowingpatch.+*Sameincaseofimc_event_stop().+*/+staticvoidnest_imc_event_start(structperf_event*event,intflags)+{+imc_event_start(event,flags);+}++staticvoidnest_imc_event_stop(structperf_event*event,intflags)+{+imc_event_stop(event,flags);+}++staticintnest_imc_event_add(structperf_event*event,intflags)+{+if(flags&PERF_EF_START)+nest_imc_event_start(event,flags);++return0;+}++/* update_pmu_ops : Populate the appropriate operations for "pmu" */+staticintupdate_pmu_ops(structimc_pmu*pmu)+{+if(!pmu)+return-EINVAL;++pmu->pmu.task_ctx_nr=perf_invalid_context;+pmu->pmu.event_init=nest_imc_event_init;+pmu->pmu.add=nest_imc_event_add;+pmu->pmu.del=nest_imc_event_stop;+pmu->pmu.start=nest_imc_event_start;+pmu->pmu.stop=nest_imc_event_stop;+pmu->pmu.read=imc_perf_event_update;+pmu->attr_groups[IMC_FORMAT_ATTR]=&imc_format_group;+pmu->pmu.attr_groups=pmu->attr_groups;++return0;+}++/* dev_str_attr : Populate event "name" and string "str" in attribute */+staticstructattribute*dev_str_attr(constchar*name,constchar*str)+{+structperf_pmu_events_attr*attr;++attr=kzalloc(sizeof(*attr),GFP_KERNEL);+if(!attr)+returnNULL;+sysfs_attr_init(&attr->attr.attr);++attr->event_str=str;+attr->attr.attr.name=name;+attr->attr.attr.mode=0444;+attr->attr.show=perf_event_sysfs_show;++return&attr->attr.attr;+}++/*+*update_events_in_group:Updatethe"events"informationinanattr_group+*andassigntheattr_grouptothepmu"pmu".+*/+staticintupdate_events_in_group(structimc_events*events,+intidx,structimc_pmu*pmu)+{+structattribute_group*attr_group;+structattribute**attrs;+inti;++/* If there is no events for this pmu, just return zero */+if(!events)+return0;++/* Allocate memory for attribute group */+attr_group=kzalloc(sizeof(*attr_group),GFP_KERNEL);+if(!attr_group)+return-ENOMEM;++/* Allocate memory for attributes */+attrs=kzalloc((sizeof(structattribute*)*(idx+1)),GFP_KERNEL);+if(!attrs){+kfree(attr_group);+return-ENOMEM;+}++attr_group->name="events";+attr_group->attrs=attrs;+for(i=0;i<idx;i++,events++){+attrs[i]=dev_str_attr((char*)events->ev_name,+(char*)events->ev_value);+}++/* Save the event attribute */+pmu->attr_groups[IMC_EVENT_ATTR]=attr_group;+return0;+}++/*+*init_imc_pmu:SetupandregistertheIMCpmudevice.+*+*@events:eventsmemoryforthispmu.+*@idx:numberofevententriescreated.+*@pmu_ptr:memoryallocatedforthispmu.+*/+int__initinit_imc_pmu(structimc_events*events,intidx,+structimc_pmu*pmu_ptr)+{+intret=-ENODEV;++ret=update_events_in_group(events,idx,pmu_ptr);+if(ret)+gotoerr_free;++ret=update_pmu_ops(pmu_ptr);+if(ret)+gotoerr_free;++ret=perf_pmu_register(&pmu_ptr->pmu,pmu_ptr->pmu.name,-1);+if(ret)+gotoerr_free;++pr_info("%s performance monitor hardware support registered\n",+pmu_ptr->pmu.name);++return0;++err_free:+/* Only free the attr_groups which are dynamically allocated */+if(pmu_ptr->attr_groups[IMC_EVENT_ATTR]){+if(pmu_ptr->attr_groups[IMC_EVENT_ATTR]->attrs)+kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]->attrs);+kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]);+}++returnret;+}
@@ -423,8 +421,16 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)if(prop)events=imc_events_setup(parent,pmu_index,pmu_ptr,prop,&idx);+/* Function to register IMC pmu */+ret=init_imc_pmu(events,idx,pmu_ptr);+if(ret){+pr_err("IMC PMU %s Register failed\n",pmu_ptr->pmu.name);+gotofree_events;+}return0;+free_events:+imc_free_events(events,idx);free_pmu:kfree(pmu_ptr);returnret;
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:24
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).
Patch also adds a CONFIG_HV_PERF_IMC_CTRS for the IMC driver.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/platforms/powernv/Kconfig | 10 +++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-imc.c | 140 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal.c | 18 ++++
4 files changed, 169 insertions(+)
create mode 100644 arch/powerpc/platforms/powernv/opal-imc.c
@@ -0,0 +1,140 @@+/*+*OPALIMCinterfacedetectiondriver+*SupportedonPOWERNVplatform+*+*Copyright(C)2017MadhavanSrinivasan,IBMCorporation.+*(C)2017AnjuTSudhakar,IBMCorporation.+*(C)2017HemantKShaw,IBMCorporation.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/+#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>++structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];++/*+*imc_pmu_setup:SetuptheIMCPMUs(childrenof"parent").+*/+staticvoid__initimc_pmu_setup(structdevice_node*parent)+{+if(!parent)+return;+}++staticintopal_imc_counters_probe(structplatform_device*pdev)+{+structdevice_node*imc_dev,*dn,*rm_node=NULL;+structperchip_nest_info*pcni;+u32pages,nest_offset,nest_size,chip_id;+inti=0;+const__be32*addrp;+u64reg_addr,reg_size;++if(!pdev||!pdev->dev.of_node)+return-ENODEV;++/*+*Checkwhetherthisiskdumpkernel.Ifyes,justreturn.+*/+if(is_kdump_kernel())+return-ENODEV;++imc_dev=pdev->dev.of_node;++/*+*NestcounterdataaresavedinareservedmemorycalledHOMER.+*"imc-nest-offset"identifiesthecounterdatalocationwithinHOMER.+*size:sizeoftheentirenest-countersregion+*/+if(of_property_read_u32(imc_dev,"imc-nest-offset",&nest_offset))+gotoerr;++if(of_property_read_u32(imc_dev,"imc-nest-size",&nest_size))+gotoerr;++/* Sanity check */+if((nest_size/PAGE_SIZE)>IMC_NEST_MAX_PAGES)+gotoerr;++/* Find the "HOMER region" for each chip */+rm_node=of_find_node_by_path("/reserved-memory");+if(!rm_node)+gotoerr;++/*+*Weneedtolookforthe"ibm,homer-image"nodeinthe+*"/reserved-memory"node.+*/+for(dn=of_find_node_by_name(rm_node,"ibm,homer-image");dn;+dn=of_find_node_by_name(dn,"ibm,homer-image")){++/* Get the chip id to which the above homer region belongs to */+if(of_property_read_u32(dn,"ibm,chip-id",&chip_id))+gotoerr;++pcni=&nest_perchip_info[chip_id];+addrp=of_get_address(dn,0,®_size,NULL);+if(!addrp)+gotoerr;++/* 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;++for(i=0;i<(pcni->size/PAGE_SIZE);i++){+pages=PAGE_SIZE*i;+pcni->vbase[i]=(u64)phys_to_virt(pcni->pbase+pages);+}+}++imc_pmu_setup(imc_dev);++return0;+err:+return-ENODEV;+}++staticconststructof_device_idopal_imc_match[]={+{.compatible=IMC_DTB_COMPAT},+{},+};++staticstructplatform_driveropal_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");
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:33
From: Hemant Kumar <redacted>
This patch adds the PMU function to initialize a core IMC event. It also
adds cpumask initialization function for core IMC PMU. For
initialization, a 8KB of memory is allocated per core where the data
for core IMC counters will be accumulated. The base address for this
page is sent to OPAL via an OPAL call which initializes various SCOMs
related to Core IMC initialization. Upon any errors, the pages are
free'ed and core IMC counters are disabled using the same OPAL call.
For CPU hotplugging, a cpumask is initialized which contains an online
CPU from each core. If a cpu goes offline, we check whether that cpu
belongs to the core imc cpumask, if yes, then, we migrate the PMU
context to any other online cpu (if available) in that core. If a cpu
comes back online, then this cpu will be added to the core imc cpumask
only if there was no other cpu from that core in the previous cpumask.
To register the hotplug functions for core_imc, a new state
CPUHP_AP_PERF_POWERPC_COREIMC_ONLINE is added to the list of existing
states.
Patch also adds OPAL device shutdown callback. Needed to disable the
IMC core engine to handle kexec.
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 7 +
arch/powerpc/perf/imc-pmu.c | 380 +++++++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/opal-imc.c | 7 +
include/linux/cpuhotplug.h | 1 +
4 files changed, 384 insertions(+), 11 deletions(-)
@@ -21,9 +21,21 @@ struct imc_pmu *per_nest_pmu_arr[IMC_MAX_PMUS];staticcpumask_tnest_imc_cpumask;staticatomic_tnest_events;+staticatomic_tcore_events;/* Used to avoid races in calling enable/disable nest-pmu units*/staticDEFINE_MUTEX(imc_nest_reserve);+/* Used to avoid races in calling enable/disable core-pmu units */+staticDEFINE_MUTEX(imc_core_reserve);+/*+*Maintainsbaseaddressesforallthecores.+*MAXchipandcorearedefinedas32.Sowe+*staticallyallocate8Kforthisstructure.+*+*TODO--Couldbemadedynamic+*/+staticu64per_core_pdbar_add[IMC_MAX_CHIPS][IMC_MAX_CORES];+staticcpumask_tcore_imc_cpumask;structimc_pmu*core_imc_pmu;/* Needed for sanity check */
@@ -64,6 +82,100 @@ static struct attribute_group imc_pmu_cpumask_attr_group = {};/*+*core_imc_mem_init:Initializesmemoryforthecurrentcore.+*+*Usesalloc_pages_exact_nid()andusesthereturnedaddressasanargumentto+*anopalcalltoconfigurethepdbar.Theaddresssentasanargumentis+*convertedtophysicaladdressbeforetheopalcallismade.Thisisthe+*baseaddressatwhichthecoreimccountersarepopulated.+*/+staticint__meminitcore_imc_mem_init(void)+{+intcore_id,phys_id;+intrc=-1;++phys_id=topology_physical_package_id(smp_processor_id());+core_id=smp_processor_id()/threads_per_core;++/*+*alloc_pages_exact_nid()willallocatememoryforcoreinthe+*localnodeonly.+*/+per_core_pdbar_add[phys_id][core_id]=(u64)alloc_pages_exact_nid(phys_id,+(size_t)IMC_CORE_COUNTER_MEM,GFP_KERNEL|__GFP_ZERO);+rc=opal_imc_counters_init(OPAL_IMC_COUNTERS_CORE,+(u64)virt_to_phys((void*)per_core_pdbar_add[phys_id][core_id]));++returnrc;+}++/*+*Callscore_imc_mem_initandchecksthereturnvalue.+*/+staticvoidcore_imc_init(int*cpu_opal_rc)+{+intrc=0;++rc=core_imc_mem_init();+if(rc)+cpu_opal_rc[smp_processor_id()]=1;+}++staticvoidcore_imc_change_cpu_context(intold_cpu,intnew_cpu)+{+if(!core_imc_pmu)+return;+perf_pmu_migrate_context(&core_imc_pmu->pmu,old_cpu,new_cpu);+}+++staticintppc_core_imc_cpu_online(unsignedintcpu)+{+intret;++/* If a cpu for this core is already set, then, don't do anything */+ret=cpumask_any_and(&core_imc_cpumask,+cpu_sibling_mask(cpu));+if(ret<nr_cpu_ids)+return0;++/* Else, set the cpu in the mask, and change the context */+cpumask_set_cpu(cpu,&core_imc_cpumask);+opal_imc_counters_start(OPAL_IMC_COUNTERS_CORE);+core_imc_change_cpu_context(-1,cpu);+return0;+}++staticintppc_core_imc_cpu_offline(unsignedintcpu)+{+inttarget;+unsignedintncpu;++/*+*clearthiscpuoutofthemask,ifnotpresentinthemask,+*don'tbotherdoinganything.+*/+if(!cpumask_test_and_clear_cpu(cpu,&core_imc_cpumask))+return0;++/* Find any online cpu in that core except the current "cpu" */+ncpu=cpumask_any_but(cpu_sibling_mask(cpu),cpu);++if(ncpu<nr_cpu_ids){+target=ncpu;+cpumask_set_cpu(target,&core_imc_cpumask);+}else{+opal_imc_counters_stop(OPAL_IMC_COUNTERS_CORE);+target=-1;+}++/* migrate the context */+core_imc_change_cpu_context(cpu,target);++return0;+}++/**nest_init:Initializesthenestimcengineforthecurrentchip.*bydefaultthenestengineisdisabled.*/
@@ -195,6 +307,97 @@ static int nest_pmu_cpumask_init(void)return-ENODEV;}+staticvoidcleanup_core_imc_memory(void)+{+intphys_id,core_id;+u64addr;++phys_id=topology_physical_package_id(smp_processor_id());+core_id=smp_processor_id()/threads_per_core;++addr=per_core_pdbar_add[phys_id][core_id];++/* Only if the address is non-zero shall, we free it */+if(addr)+free_pages(addr,0);+}++staticvoidcleanup_all_core_imc_memory(void)+{+on_each_cpu_mask(&core_imc_cpumask,+(smp_call_func_t)cleanup_core_imc_memory,NULL,1);+}++/* Enabling of Core Engine needs a scom operation */+staticvoidcore_imc_control_enable(void)+{+opal_imc_counters_start(OPAL_IMC_COUNTERS_CORE);+}+++/*+*DisablingofIMCCoreEngineneedsascomoperation+*/+staticvoidcore_imc_control_disable(void)+{+opal_imc_counters_stop(OPAL_IMC_COUNTERS_CORE);+}++/*+*FunctiontodiabletheIMCCoreengineusingcoreimccpumask+*/+voidcore_imc_disable(void)+{+on_each_cpu_mask(&core_imc_cpumask,+(smp_call_func_t)core_imc_control_disable,NULL,1);+}++staticintcore_imc_pmu_cpumask_init(void)+{+intcpu,*cpus_opal_rc;++/*+*Getthemaskoffirstonlinecpusforeverycore.+*/+core_imc_cpumask=cpu_online_cores_map();++/*+*MemoryforOPALcallreturnvalue.+*/+cpus_opal_rc=kzalloc((sizeof(int)*nr_cpu_ids),GFP_KERNEL);+if(!cpus_opal_rc)+gotofail;++/*+*InitializethecoreIMCPMUoneachcoreusingthe+*core_imc_cpumaskbycallingcore_imc_init().+*/+on_each_cpu_mask(&core_imc_cpumask,(smp_call_func_t)core_imc_init,+(void*)cpus_opal_rc,1);++/* Check return value array for any OPAL call failure */+for_each_cpu(cpu,&core_imc_cpumask){+if(cpus_opal_rc[cpu]){+kfree(cpus_opal_rc);+gotofail;+}+}++kfree(cpus_opal_rc);++cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_COREIMC_ONLINE,+"POWER_CORE_IMC_ONLINE",+ppc_core_imc_cpu_online,+ppc_core_imc_cpu_offline);++return0;++fail:+/* Free up the allocated pages */+cleanup_all_core_imc_memory();+return-ENODEV;+}+staticintnest_imc_event_init(structperf_event*event){intchip_id;
@@ -238,6 +441,44 @@ static int nest_imc_event_init(struct perf_event *event)return0;}+staticintcore_imc_event_init(structperf_event*event)+{+intcore_id,phys_id;+u64config=event->attr.config;++if(event->attr.type!=event->pmu->type)+return-ENOENT;++/* Sampling not supported */+if(event->hw.sample_period)+return-EINVAL;++/* unsupported modes and filters */+if(event->attr.exclude_user||+event->attr.exclude_kernel||+event->attr.exclude_hv||+event->attr.exclude_idle||+event->attr.exclude_host||+event->attr.exclude_guest)+return-EINVAL;++if(event->cpu<0)+return-EINVAL;++event->hw.idx=-1;++/* Sanity check for config (event offset) */+if(config>core_max_offset)+return-EINVAL;++core_id=event->cpu/threads_per_core;+phys_id=topology_physical_package_id(event->cpu);+event->hw.event_base=+per_core_pdbar_add[phys_id][core_id]+config;++return0;+}+staticvoidimc_read_counter(structperf_event*event){u64*addr,data;
@@ -384,6 +625,100 @@ static int nest_imc_event_add(struct perf_event *event, int flags)return0;}+staticintcore_imc_control(intoperation)+{+intcpu,*cpus_opal_rc;++/*+*MemoryforOPALcallreturnvalue.+*/+cpus_opal_rc=kzalloc((sizeof(int)*nr_cpu_ids),GFP_KERNEL);+if(!cpus_opal_rc)+gotofail;++/*+*InitializethecoreIMCPMUoneachcoreusingthe+*core_imc_cpumaskbycallingcore_imc_init().+*/+switch(operation){++caseIMC_COUNTER_DISABLE:+on_each_cpu_mask(&core_imc_cpumask,+(smp_call_func_t)core_imc_control_disable,+(void*)cpus_opal_rc,1);+break;+caseIMC_COUNTER_ENABLE:+on_each_cpu_mask(&core_imc_cpumask,+(smp_call_func_t)core_imc_control_enable,+(void*)cpus_opal_rc,1);+break;+default:+gotofail;+}++/* Check return value array for any OPAL call failure */+for_each_cpu(cpu,&core_imc_cpumask){+if(cpus_opal_rc[cpu])+gotofail;+}++return0;+fail:+if(cpus_opal_rc)+kfree(cpus_opal_rc);+return-EINVAL;+}+++staticvoidcore_imc_event_start(structperf_event*event,intflags)+{+intrc;++/*+*Corepmuunitsareenabledonlywhenitisused.+*Seeifthisistriggeredforthefirsttime.+*Ifyes,takethemutexlockandenablethecorecounters.+*Ifnot,justincrementthecountincore_events.+*/+if(atomic_inc_return(&core_events)==1){+mutex_lock(&imc_core_reserve);+rc=core_imc_control(IMC_COUNTER_ENABLE);+mutex_unlock(&imc_core_reserve);+if(rc)+pr_err("IMC: Unbale to start the counters\n");+}+imc_event_start(event,flags);+}++staticvoidcore_imc_event_stop(structperf_event*event,intflags)+{+intrc;++imc_event_stop(event,flags);+/*+*SeeifweneedtodisabletheIMCPMU.+*Ifnoeventsarecurrentlyinuse,thenwehavetotakea+*mutextoensurethatwedon'tracewithanothertaskdoing+*enableordisablethecorecounters.+*/+if(atomic_dec_return(&core_events)==0){+mutex_lock(&imc_core_reserve);+rc=core_imc_control(IMC_COUNTER_DISABLE);+mutex_unlock(&imc_core_reserve);+if(rc)+pr_err("IMC: Disable counters failed\n");+}+}++staticintcore_imc_event_add(structperf_event*event,intflags)+{+if(flags&PERF_EF_START)+core_imc_event_start(event,flags);++return0;+}++/* update_pmu_ops : Populate the appropriate operations for "pmu" */staticintupdate_pmu_ops(structimc_pmu*pmu){
@@ -391,13 +726,22 @@ static int update_pmu_ops(struct imc_pmu *pmu)return-EINVAL;pmu->pmu.task_ctx_nr=perf_invalid_context;-pmu->pmu.event_init=nest_imc_event_init;-pmu->pmu.add=nest_imc_event_add;-pmu->pmu.del=nest_imc_event_stop;-pmu->pmu.start=nest_imc_event_start;-pmu->pmu.stop=nest_imc_event_stop;+if(pmu->domain==IMC_DOMAIN_NEST){+pmu->pmu.event_init=nest_imc_event_init;+pmu->pmu.add=nest_imc_event_add;+pmu->pmu.del=nest_imc_event_stop;+pmu->pmu.start=nest_imc_event_start;+pmu->pmu.stop=nest_imc_event_stop;+pmu->attr_groups[IMC_CPUMASK_ATTR]=&imc_pmu_cpumask_attr_group;+}elseif(pmu->domain==IMC_DOMAIN_CORE){+pmu->pmu.event_init=core_imc_event_init;+pmu->pmu.add=core_imc_event_add;+pmu->pmu.del=core_imc_event_stop;+pmu->pmu.start=core_imc_event_start;+pmu->pmu.stop=core_imc_event_stop;+pmu->attr_groups[IMC_CPUMASK_ATTR]=&imc_pmu_cpumask_attr_group;+}pmu->pmu.read=imc_perf_event_update;-pmu->attr_groups[IMC_CPUMASK_ATTR]=&imc_pmu_cpumask_attr_group;pmu->attr_groups[IMC_FORMAT_ATTR]=&imc_format_group;pmu->pmu.attr_groups=pmu->attr_groups;
@@ -477,9 +821,20 @@ int __init init_imc_pmu(struct imc_events *events, int idx,intret=-ENODEV;/* Add cpumask and register for hotplug notification */-ret=nest_pmu_cpumask_init();-if(ret)-returnret;+switch(pmu_ptr->domain){+caseIMC_DOMAIN_NEST:+ret=nest_pmu_cpumask_init();+if(ret)+returnret;+break;+caseIMC_DOMAIN_CORE:+ret=core_imc_pmu_cpumask_init();+if(ret)+returnret;+break;+default:+return-1;/* Unknown domain */+}ret=update_events_in_group(events,idx,pmu_ptr);if(ret)
@@ -505,6 +860,9 @@ int __init init_imc_pmu(struct imc_events *events, int idx,kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]->attrs);kfree(pmu_ptr->attr_groups[IMC_EVENT_ATTR]);}+/* For core_imc, we have allocated memory, we need to free it */+if(pmu_ptr->domain==IMC_DOMAIN_CORE)+cleanup_all_core_imc_memory();returnret;}
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:36
Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any
online CPU) from each chip for nest PMUs is designated to read counters.
On CPU hotplug, dying CPU is checked to see whether it is one of the
designated cpus, if yes, next online cpu from the same chip (for nest
units) is designated as new cpu to read counters. For this purpose, we
introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_ONLINE.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 4 +
arch/powerpc/include/asm/opal-api.h | 12 +-
arch/powerpc/include/asm/opal.h | 4 +
arch/powerpc/perf/imc-pmu.c | 248 ++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/opal-wrappers.S | 3 +
include/linux/cpuhotplug.h | 1 +
6 files changed, 266 insertions(+), 6 deletions(-)
@@ -18,6 +18,11 @@structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];structimc_pmu*per_nest_pmu_arr[IMC_MAX_PMUS];+staticcpumask_tnest_imc_cpumask;++staticatomic_tnest_events;+/* Used to avoid races in calling enable/disable nest-pmu units*/+staticDEFINE_MUTEX(imc_nest_reserve);/* Needed for sanity check */externu64nest_max_offset;
@@ -33,6 +38,160 @@ static struct attribute_group imc_format_group = {.attrs=imc_format_attrs,};+/* Get the cpumask printed to a buffer "buf" */+staticssize_timc_pmu_cpumask_get_attr(structdevice*dev,+structdevice_attribute*attr,+char*buf)+{+cpumask_t*active_mask;++active_mask=&nest_imc_cpumask;+returncpumap_print_to_pagebuf(true,buf,active_mask);+}++staticDEVICE_ATTR(cpumask,S_IRUGO,imc_pmu_cpumask_get_attr,NULL);++staticstructattribute*imc_pmu_cpumask_attrs[]={+&dev_attr_cpumask.attr,+NULL,+};++staticstructattribute_groupimc_pmu_cpumask_attr_group={+.attrs=imc_pmu_cpumask_attrs,+};++/*+*nest_init:Initializesthenestimcengineforthecurrentchip.+*bydefaultthenestengineisdisabled.+*/+staticvoidnest_init(int*cpu_opal_rc)+{+intrc;++/*+*OPALfiguresoutwhichCPUtostartbasedontheCPUthatis+*currentlyrunningwhenwecallintoOPAL+*/+rc=opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);+if(rc)+cpu_opal_rc[smp_processor_id()]=1;+}++staticvoidnest_change_cpu_context(intold_cpu,intnew_cpu)+{+inti;++for(i=0;+(per_nest_pmu_arr[i]!=NULL)&&(i<IMC_MAX_PMUS);i++)+perf_pmu_migrate_context(&per_nest_pmu_arr[i]->pmu,+old_cpu,new_cpu);+}++staticintppc_nest_imc_cpu_online(unsignedintcpu)+{+intnid;+conststructcpumask*l_cpumask;+structcpumasktmp_mask;++/* Find the cpumask of this node */+nid=cpu_to_node(cpu);+l_cpumask=cpumask_of_node(nid);++/*+*Ifanyofthecpufromthisnodeisalreadypresentinthemask,+*justreturn,ifnot,thensetthiscpuinthemask.+*/+if(!cpumask_and(&tmp_mask,l_cpumask,&nest_imc_cpumask)){+cpumask_set_cpu(cpu,&nest_imc_cpumask);+nest_change_cpu_context(-1,cpu);+return0;+}++return0;+}++staticintppc_nest_imc_cpu_offline(unsignedintcpu)+{+intnid,target=-1;+conststructcpumask*l_cpumask;++/*+*Checkinthedesignatedlistforthiscpu.Dontbother+*ifnotoneofthem.+*/+if(!cpumask_test_and_clear_cpu(cpu,&nest_imc_cpumask))+return0;++/*+*Nowthatthiscpuisoneofthedesignated,+*findanextcpua)whichisonlineandb)insamechip.+*/+nid=cpu_to_node(cpu);+l_cpumask=cpumask_of_node(nid);+target=cpumask_next(cpu,l_cpumask);++/*+*Updatethecpumaskwiththetargetcpuand+*migratethecontextifneeded+*/+if(target>=0&&target<=nr_cpu_ids){+cpumask_set_cpu(target,&nest_imc_cpumask);+nest_change_cpu_context(cpu,target);+}+return0;+}++staticintnest_pmu_cpumask_init(void)+{+conststructcpumask*l_cpumask;+intcpu,nid;+int*cpus_opal_rc;++if(!cpumask_empty(&nest_imc_cpumask))+return0;++/*+*MemoryforOPALcallreturnvalue.+*/+cpus_opal_rc=kzalloc((sizeof(int)*nr_cpu_ids),GFP_KERNEL);+if(!cpus_opal_rc)+gotofail;++/*+*NestPMUsareper-chipcounters.Sodesignateacpu+*fromeachchipforcountercollection.+*/+for_each_online_node(nid){+l_cpumask=cpumask_of_node(nid);++/* designate first online cpu in this node */+cpu=cpumask_first(l_cpumask);+cpumask_set_cpu(cpu,&nest_imc_cpumask);+}++/* Initialize Nest PMUs in each node using designated cpus */+on_each_cpu_mask(&nest_imc_cpumask,(smp_call_func_t)nest_init,+(void*)cpus_opal_rc,1);++/* Check return value array for any OPAL call failure */+for_each_cpu(cpu,&nest_imc_cpumask){+if(cpus_opal_rc[cpu])+gotofail;+}++cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_NEST_ONLINE,+"POWER_NEST_IMC_ONLINE",+ppc_nest_imc_cpu_online,+ppc_nest_imc_cpu_offline);++return0;++fail:+if(cpus_opal_rc)+kfree(cpus_opal_rc);+return-ENODEV;+}+staticintnest_imc_event_init(structperf_event*event){intchip_id;
@@ -109,6 +268,51 @@ static void imc_perf_event_update(struct perf_event *event)local64_add(final_count,&event->count);}+staticvoidnest_imc_start(int*cpu_opal_rc)+{+intrc;++/* Enable nest engine */+rc=opal_imc_counters_start(OPAL_IMC_COUNTERS_NEST);+if(rc)+cpu_opal_rc[smp_processor_id()]=1;++}++staticintnest_imc_control(intoperation)+{+int*cpus_opal_rc,cpu;++/*+*MemoryforOPALcallreturnvalue.+*/+cpus_opal_rc=kzalloc((sizeof(int)*nr_cpu_ids),GFP_KERNEL);+if(!cpus_opal_rc)+return-ENOMEM;+switch(operation){++caseIMC_COUNTER_ENABLE:+/* Initialize Nest PMUs in each node using designated cpus */+on_each_cpu_mask(&nest_imc_cpumask,(smp_call_func_t)nest_imc_start,+(void*)cpus_opal_rc,1);+break;+caseIMC_COUNTER_DISABLE:+/* Disable the counters */+on_each_cpu_mask(&nest_imc_cpumask,(smp_call_func_t)nest_init,+(void*)cpus_opal_rc,1);+break;+default:return-EINVAL;++}++/* Check return value array for any OPAL call failure */+for_each_cpu(cpu,&nest_imc_cpumask){+if(cpus_opal_rc[cpu])+return-ENODEV;+}+return0;+}+staticvoidimc_event_start(structperf_event*event,intflags){/*
@@ -129,19 +333,44 @@ static void imc_event_stop(struct perf_event *event, int flags)imc_perf_event_update(event);}-/*-*Thewrapperfunctionisprovidedhere,sincewewillhavereserve-*andreleaselockforimc_event_start()inthefollowingpatch.-*Sameincaseofimc_event_stop().-*/staticvoidnest_imc_event_start(structperf_event*event,intflags){+intrc;++/*+*Nestpmuunitsareenabledonlywhenitisused.+*Seeifthisistriggeredforthefirsttime.+*Ifyes,takethemutexlockandenablethenestcounters.+*Ifnot,justincrementthecountinnest_events.+*/+if(atomic_inc_return(&nest_events)==1){+mutex_lock(&imc_nest_reserve);+rc=nest_imc_control(IMC_COUNTER_ENABLE);+mutex_unlock(&imc_nest_reserve);+if(rc)+pr_err("IMC: Unbale to start the counters\n");+}imc_event_start(event,flags);}staticvoidnest_imc_event_stop(structperf_event*event,intflags){+intrc;+imc_event_stop(event,flags);+/*+*SeeifweneedtodisablethenestPMU.+*Ifnoeventsarecurrentlyinuse,thenwehavetotakea+*mutextoensurethatwedon'tracewithanothertaskdoing+*enableordisablethenestcounters.+*/+if(atomic_dec_return(&nest_events)==0){+mutex_lock(&imc_nest_reserve);+rc=nest_imc_control(IMC_COUNTER_DISABLE);+mutex_unlock(&imc_nest_reserve);+if(rc)+pr_err("IMC: Disable counters failed\n");+}}staticintnest_imc_event_add(structperf_event*event,intflags)
@@ -165,6 +394,7 @@ static int update_pmu_ops(struct imc_pmu *pmu)pmu->pmu.start=nest_imc_event_start;pmu->pmu.stop=nest_imc_event_stop;pmu->pmu.read=imc_perf_event_update;+pmu->attr_groups[IMC_CPUMASK_ATTR]=&imc_pmu_cpumask_attr_group;pmu->attr_groups[IMC_FORMAT_ATTR]=&imc_format_group;pmu->pmu.attr_groups=pmu->attr_groups;
@@ -234,12 +464,20 @@ static int update_events_in_group(struct imc_events *events,*@events:eventsmemoryforthispmu.*@idx:numberofevententriescreated.*@pmu_ptr:memoryallocatedforthispmu.+*+*init_imc_pmu()setupthecpumaskinformationforthesepmusandsetup+*thestatemachinehotplugnotifiersaswell.*/int__initinit_imc_pmu(structimc_events*events,intidx,structimc_pmu*pmu_ptr){intret=-ENODEV;+/* Add cpumask and register for hotplug notification */+ret=nest_pmu_cpumask_init();+if(ret)+returnret;+ret=update_events_in_group(events,idx,pmu_ptr);if(ret)gotoerr_free;
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:39
From: Hemant Kumar <redacted>
This patch adds support for detection of core IMC events along with the
Nest IMC events. It adds a new domain IMC_DOMAIN_CORE and its determined
with the help of the compatibility string "ibm,imc-counters-core" based
on the IMC device tree.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 4 +++-
arch/powerpc/perf/imc-pmu.c | 3 +++
arch/powerpc/platforms/powernv/opal-imc.c | 28 +++++++++++++++++++++++++---
3 files changed, 31 insertions(+), 4 deletions(-)
@@ -386,7 +391,10 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)gotofree_pmu;/* Needed for hotplug/migration */-per_nest_pmu_arr[pmu_index]=pmu_ptr;+if(pmu_ptr->domain==IMC_DOMAIN_CORE)+core_imc_pmu=pmu_ptr;+elseif(pmu_ptr->domain==IMC_DOMAIN_NEST)+per_nest_pmu_arr[pmu_index]=pmu_ptr;pp=of_find_property(parent,"name",NULL);if(!pp){
@@ -407,7 +415,10 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)gotofree_pmu;}/* Save the name to register it later */-sprintf(buf,"nest_%s",(char*)pp->value);+if(pmu_ptr->domain==IMC_DOMAIN_NEST)+sprintf(buf,"nest_%s",(char*)pp->value);+else+sprintf(buf,"%s_imc",(char*)pp->value);pmu_ptr->pmu.name=(char*)buf;/*
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:41
This patch adds the PMU functions required for event initialization,
read, update, add, del etc. for thread IMC PMU. Thread IMC PMUs are used
for per-task monitoring.
For each CPU, a page of memory is allocated and is kept static i.e.,
these pages will exist till the machine shuts down. The base address of
this page is assigned to the ldbar of that cpu. As soon as we do that,
the thread IMC counters start running for that cpu and the data of these
counters are assigned to the page allocated. But we use this for
per-task monitoring. Whenever we start monitoring a task, the event is
added is onto the task. At that point, we read the initial value of the
event. Whenever, we stop monitoring the task, the final value is taken
and the difference is the event data.
Now, a task can move to a different cpu. Suppose a task X is moving from
cpu A to cpu B. When the task is scheduled out of A, we get an
event_del for A, and hence, the event data is updated. And, we stop
updating the X's event data. As soon as X moves on to B, event_add is
called for B, and we again update the event_data. And this is how it
keeps on updating the event data even when the task is scheduled on to
different cpus.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 5 +
arch/powerpc/perf/imc-pmu.c | 209 +++++++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/opal-imc.c | 3 +
3 files changed, 216 insertions(+), 1 deletion(-)
@@ -38,6 +38,9 @@ static u64 per_core_pdbar_add[IMC_MAX_CHIPS][IMC_MAX_CORES];staticcpumask_tcore_imc_cpumask;structimc_pmu*core_imc_pmu;+/* Maintains base address for all the cpus */+staticu64per_cpu_add[NR_CPUS];+/* Needed for sanity check */externu64nest_max_offset;externu64core_max_offset;
@@ -480,6 +483,56 @@ static int core_imc_event_init(struct perf_event *event)return0;}+staticintthread_imc_event_init(structperf_event*event)+{+structtask_struct*target;++if(event->attr.type!=event->pmu->type)+return-ENOENT;++/* Sampling not supported */+if(event->hw.sample_period)+return-EINVAL;++event->hw.idx=-1;++/* Sanity check for config (event offset) */+if(event->attr.config>thread_max_offset)+return-EINVAL;++target=event->hw.target;++if(!target)+return-EINVAL;++event->pmu->task_ctx_nr=perf_sw_context;+return0;+}++staticvoidthread_imc_read_counter(structperf_event*event)+{+u64*addr,data;+intcpu_id=smp_processor_id();++addr=(u64*)(per_cpu_add[cpu_id]+event->attr.config);+data=__be64_to_cpu(READ_ONCE(*addr));+local64_set(&event->hw.prev_count,data);+}++staticvoidthread_imc_perf_event_update(structperf_event*event)+{+u64counter_prev,counter_new,final_count,*addr;+intcpu_id=smp_processor_id();++addr=(u64*)(per_cpu_add[cpu_id]+event->attr.config);+counter_prev=local64_read(&event->hw.prev_count);+counter_new=__be64_to_cpu(READ_ONCE(*addr));+final_count=counter_new-counter_prev;++local64_set(&event->hw.prev_count,counter_new);+local64_add(final_count,&event->count);+}+staticvoidimc_read_counter(structperf_event*event){u64*addr,data;
@@ -720,6 +773,84 @@ static int core_imc_event_add(struct perf_event *event, int flags)}+staticvoidthread_imc_event_start(structperf_event*event,intflags)+{+intrc;++/*+*Corepmuunitsareenabledonlywhenitisused.+*Seeifthisistriggeredforthefirsttime.+*Ifyes,takethemutexlockandenablethecorecounters.+*Ifnot,justincrementthecountincore_events.+*/+if(atomic_inc_return(&core_events)==1){+mutex_lock(&imc_core_reserve);+rc=core_imc_control(IMC_COUNTER_ENABLE);+mutex_unlock(&imc_core_reserve);+if(rc)+pr_err("IMC: Unbale to start the counters\n");+}+thread_imc_read_counter(event);+}++staticvoidthread_imc_event_stop(structperf_event*event,intflags)+{+intrc;++thread_imc_perf_event_update(event);+/*+*SeeifweneedtodisabletheIMCPMU.+*Ifnoeventsarecurrentlyinuse,thenwehavetotakea+*mutextoensurethatwedon'tracewithanothertaskdoing+*enableordisablethecorecounters.+*/+if(atomic_dec_return(&core_events)==0){+mutex_lock(&imc_core_reserve);+rc=core_imc_control(IMC_COUNTER_DISABLE);+mutex_unlock(&imc_core_reserve);+if(rc)+pr_err("IMC: Disable counters failed\n");++}+}++staticvoidthread_imc_event_del(structperf_event*event,intflags)+{+thread_imc_perf_event_update(event);+}++staticintthread_imc_event_add(structperf_event*event,intflags)+{+thread_imc_event_start(event,flags);++return0;+}++staticvoidthread_imc_pmu_start_txn(structpmu*pmu,+unsignedinttxn_flags)+{+if(txn_flags&~PERF_PMU_TXN_ADD)+return;+perf_pmu_disable(pmu);+}++staticvoidthread_imc_pmu_cancel_txn(structpmu*pmu)+{+perf_pmu_enable(pmu);+}++staticintthread_imc_pmu_commit_txn(structpmu*pmu)+{+perf_pmu_enable(pmu);+return0;+}++staticvoidthread_imc_pmu_sched_task(structperf_event_context*ctx,+boolsched_in)+{+return;+}+/* update_pmu_ops : Populate the appropriate operations for "pmu" */staticintupdate_pmu_ops(structimc_pmu*pmu){
@@ -745,7 +876,26 @@ static int update_pmu_ops(struct imc_pmu *pmu)pmu->pmu.read=imc_perf_event_update;pmu->attr_groups[IMC_FORMAT_ATTR]=&imc_format_group;pmu->pmu.attr_groups=pmu->attr_groups;-+if(pmu->domain==IMC_DOMAIN_THREAD){+pmu->pmu.event_init=thread_imc_event_init;+pmu->pmu.start=thread_imc_event_start;+pmu->pmu.add=thread_imc_event_add;+pmu->pmu.del=thread_imc_event_del;+pmu->pmu.stop=thread_imc_event_stop;+pmu->pmu.read=thread_imc_perf_event_update;+pmu->pmu.start_txn=thread_imc_pmu_start_txn;+pmu->pmu.cancel_txn=thread_imc_pmu_cancel_txn;+pmu->pmu.commit_txn=thread_imc_pmu_commit_txn;+pmu->pmu.sched_task=thread_imc_pmu_sched_task;++/*+*Sincethread_imcdoesnothaveanyCPUMASKattr,+*thismaydropthe"events"attralltogether.+*SoswaptheIMC_EVENT_ATTRslotwithIMC_CPUMASK_ATTR.+*/+pmu->attr_groups[IMC_CPUMASK_ATTR]=pmu->attr_groups[IMC_EVENT_ATTR];+pmu->attr_groups[IMC_EVENT_ATTR]=NULL;+}return0;}
@@ -806,6 +956,56 @@ static int update_events_in_group(struct imc_events *events,return0;}+staticvoidthread_imc_ldbar_disable(void*dummy)+{+/* LDBAR spr is a per-thread */+mtspr(SPRN_LDBAR,0);+}++voidthread_imc_disable(void)+{+on_each_cpu(thread_imc_ldbar_disable,NULL,1);+}++staticvoidcleanup_thread_imc_memory(void*dummy)+{+intcpu_id=smp_processor_id();+u64addr=per_cpu_add[cpu_id];++/* Only if the address is non-zero, shall we free it */+if(addr)+free_pages(addr,0);+}++staticvoidcleanup_all_thread_imc_memory(void)+{+on_each_cpu(cleanup_thread_imc_memory,NULL,1);+}++/*+*Allocatesapageofmemoryforeachoftheonlinecpus,and,writesthe+*physicalbaseaddressofthatpagetotheLDBARforthatcpu.Thisstarts+*thethreadIMCcounters.+*/+staticvoidthread_imc_mem_alloc(void*dummy)+{+u64ldbar_addr,ldbar_value;+intcpu_id=smp_processor_id();+intphys_id=topology_physical_package_id(smp_processor_id());++per_cpu_add[cpu_id]=(u64)alloc_pages_exact_nid(phys_id,+(size_t)IMC_THREAD_COUNTER_MEM,GFP_KERNEL|__GFP_ZERO);+ldbar_addr=(u64)virt_to_phys((void*)per_cpu_add[cpu_id]);+ldbar_value=(ldbar_addr&(u64)THREAD_IMC_LDBAR_MASK)|+(u64)THREAD_IMC_ENABLE;+mtspr(SPRN_LDBAR,ldbar_value);+}++voidthread_imc_cpu_init(void)+{+on_each_cpu(thread_imc_mem_alloc,NULL,1);+}+/**init_imc_pmu:SetupandregistertheIMCpmudevice.*
@@ -833,6 +1033,9 @@ int __init init_imc_pmu(struct imc_events *events, int idx,if(ret)returnret;break;+caseIMC_DOMAIN_THREAD:+thread_imc_cpu_init();+break;default:return-1;/* Unknown domain */}
@@ -865,5 +1068,9 @@ int __init init_imc_pmu(struct imc_events *events, int idx,if(pmu_ptr->domain==IMC_DOMAIN_CORE)cleanup_all_core_imc_memory();+/* For thread_imc, we have allocated memory, we need to free it */+if(pmu_ptr->domain==IMC_DOMAIN_THREAD)+cleanup_all_thread_imc_memory();+returnret;}
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:21:45
This patch adds support for thread IMC on cpuhotplug.
When a cpu goes offline, the LDBAR for that cpu is disabled, and when it comes
back online the previous ldbar value is written back to the LDBAR for that cpu.
To register the hotplug functions for thread_imc, a new state
CPUHP_AP_PERF_POWERPC_THREADIMC_ONLINE is added to the list of existing
states.
Reviewed-by: Gautham R. Shenoy <redacted>
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/perf/imc-pmu.c | 32 +++++++++++++++++++++++++++-----
include/linux/cpuhotplug.h | 1 +
2 files changed, 28 insertions(+), 5 deletions(-)
From: Anju T Sudhakar <hidden> Date: 2017-05-04 14:22:33
Patch adds support for detection of thread IMC events. It adds a new
domain IMC_DOMAIN_THREAD and it is determined with the help of the
compatibility string "ibm,imc-counters-thread" based on the IMC device
tree.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 2 ++
arch/powerpc/perf/imc-pmu.c | 1 +
arch/powerpc/platforms/powernv/opal-imc.c | 18 +++++++++++++++++-
3 files changed, 20 insertions(+), 1 deletion(-)
From: Daniel Axtens <hidden> Date: 2017-05-08 14:12:58
Hi all,
I've had a look at the API as it was a big thing I didn't like in the
earlier version.
I am much happier with this one.
Some comments:
- I'm no longer subscribed to skiboot but I've had a look at the
patches on that side:
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
* in start/stop, should there be some sort of write barrier to make
sure the cb->imc_chip_command actually gets written out to memory
at the time we expect?
The rest of my comments are in line.
Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any
online CPU) from each chip for nest PMUs is designated to read counters.
On CPU hotplug, dying CPU is checked to see whether it is one of the
designated cpus, if yes, next online cpu from the same chip (for nest
units) is designated as new cpu to read counters. For this purpose, we
introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_ONLINE.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 4 +
arch/powerpc/include/asm/opal-api.h | 12 +-
arch/powerpc/include/asm/opal.h | 4 +
arch/powerpc/perf/imc-pmu.c | 248 ++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/opal-wrappers.S | 3 +
include/linux/cpuhotplug.h | 1 +
Who owns this? get_maintainer.pl doesn't give me anything helpful
here... Do we need an Ack from anyone?
@@ -18,6 +18,11 @@structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];structimc_pmu*per_nest_pmu_arr[IMC_MAX_PMUS];+staticcpumask_tnest_imc_cpumask;++staticatomic_tnest_events;+/* Used to avoid races in calling enable/disable nest-pmu units*/
You need a space here between s and * ----------------------------^
@@ -33,6 +38,160 @@ static struct attribute_group imc_format_group = { .attrs = imc_format_attrs, };+/* Get the cpumask printed to a buffer "buf" */+static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,+ struct device_attribute *attr,+ char *buf)+{+ cpumask_t *active_mask;++ active_mask = &nest_imc_cpumask;+ return cpumap_print_to_pagebuf(true, buf, active_mask);+}++static DEVICE_ATTR(cpumask, S_IRUGO, imc_pmu_cpumask_get_attr, NULL);++static struct attribute *imc_pmu_cpumask_attrs[] = {+ &dev_attr_cpumask.attr,+ NULL,+};++static struct attribute_group imc_pmu_cpumask_attr_group = {+ .attrs = imc_pmu_cpumask_attrs,+};++/*+ * nest_init : Initializes the nest imc engine for the current chip.+ * by default the nest engine is disabled.+ */+static void nest_init(int *cpu_opal_rc)+{+ int rc;++ /*+ * OPAL figures out which CPU to start based on the CPU that is+ * currently running when we call into OPAL+ */+ rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
Why isn't this the init call? If this is correct, a comment explaning it
would be helpful.
+ if (rc)
+ cpu_opal_rc[smp_processor_id()] = 1;
+}
+
+static int nest_imc_control(int operation)
+{
+ int *cpus_opal_rc, cpu;
+
+ /*
+ * Memory for OPAL call return value.
+ */
+ cpus_opal_rc = kzalloc((sizeof(int) * nr_cpu_ids), GFP_KERNEL);
+ if (!cpus_opal_rc)
+ return -ENOMEM;
+ switch (operation) {
+
+ case IMC_COUNTER_ENABLE:
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_imc_start,
+ (void *)cpus_opal_rc, 1);
+ break;
+ case IMC_COUNTER_DISABLE:
+ /* Disable the counters */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
+ break;
+ default: return -EINVAL;
+
+ }
+
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
+ if (cpus_opal_rc[cpu])
+ return -ENODEV;
+ }
+ return 0;
+}
Two things:
- It doesn't look like you're freeing cpus_opal_rc anywhere - have I
missed it?
- Would it be better to split this function into two: so instead of
passing in `operation`, you just have a nest_imc_enable and
nest_imc_disable? All the call sites I can see call this with a
constant parameter anyway. Perhaps it could even be refactored into
nest_imc_event_start/stop and this method could be removed
entirely...
(I haven't checked if you use this in future patches or if it gets
expanded and makes sense to keep the function this way.)
quoted hunk
+
static void imc_event_start(struct perf_event *event, int flags)
{
/*
@@ -129,19 +333,44 @@ static void imc_event_stop(struct perf_event *event, int flags) imc_perf_event_update(event); }-/*- * The wrapper function is provided here, since we will have reserve- * and release lock for imc_event_start() in the following patch.- * Same in case of imc_event_stop().- */ static void nest_imc_event_start(struct perf_event *event, int flags) {+ int rc;++ /*+ * Nest pmu units are enabled only when it is used.+ * See if this is triggered for the first time.+ * If yes, take the mutex lock and enable the nest counters.+ * If not, just increment the count in nest_events.+ */+ if (atomic_inc_return(&nest_events) == 1) {+ mutex_lock(&imc_nest_reserve);+ rc = nest_imc_control(IMC_COUNTER_ENABLE);+ mutex_unlock(&imc_nest_reserve);+ if (rc)+ pr_err("IMC: Unbale to start the counters\n");
Spelling: s/Unbale/Unable/ ----------^
+ }
imc_event_start(event, flags);
}
Overall I'm much happer with this now, good work :)
Regards,
Daniel
On Monday 08 May 2017 07:42 PM, Daniel Axtens wrote:
Hi all,
I've had a look at the API as it was a big thing I didn't like in the
earlier version.
I am much happier with this one.
Thanks to mpe for suggesting this. :)
Some comments:
- I'm no longer subscribed to skiboot but I've had a look at the
patches on that side:
Thanks alot for the review comments.
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
So, init call will return OPAL_PARAMETER for the unsupported
domains (core and nest are supported). And if the init operation
fails for any reason, it would return OPAL_HARDWARE. And this is
documented.
* in start/stop, should there be some sort of write barrier to make
sure the cb->imc_chip_command actually gets written out to memory
at the time we expect?
In the current implementation we make the opal call in the
*_event_stop and *_event_start function. But we wanted to
move opal call to the corresponding *_event_init(), so this
avoid a opal call on each _event_start and _event_stop to
this pmu. With this change, we may not need the barrier.
Maddy
The rest of my comments are in line.
quoted
Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any
online CPU) from each chip for nest PMUs is designated to read counters.
On CPU hotplug, dying CPU is checked to see whether it is one of the
designated cpus, if yes, next online cpu from the same chip (for nest
units) is designated as new cpu to read counters. For this purpose, we
introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_ONLINE.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 4 +
arch/powerpc/include/asm/opal-api.h | 12 +-
arch/powerpc/include/asm/opal.h | 4 +
arch/powerpc/perf/imc-pmu.c | 248 ++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/opal-wrappers.S | 3 +
include/linux/cpuhotplug.h | 1 +
Who owns this? get_maintainer.pl doesn't give me anything helpful
here... Do we need an Ack from anyone?
@@ -18,6 +18,11 @@structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];structimc_pmu*per_nest_pmu_arr[IMC_MAX_PMUS];+staticcpumask_tnest_imc_cpumask;++staticatomic_tnest_events;+/* Used to avoid races in calling enable/disable nest-pmu units*/
You need a space here between s and * ----------------------------^
@@ -33,6 +38,160 @@ static struct attribute_group imc_format_group = { .attrs = imc_format_attrs, };+/* Get the cpumask printed to a buffer "buf" */+static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,+ struct device_attribute *attr,+ char *buf)+{+ cpumask_t *active_mask;++ active_mask = &nest_imc_cpumask;+ return cpumap_print_to_pagebuf(true, buf, active_mask);+}++static DEVICE_ATTR(cpumask, S_IRUGO, imc_pmu_cpumask_get_attr, NULL);++static struct attribute *imc_pmu_cpumask_attrs[] = {+ &dev_attr_cpumask.attr,+ NULL,+};++static struct attribute_group imc_pmu_cpumask_attr_group = {+ .attrs = imc_pmu_cpumask_attrs,+};++/*+ * nest_init : Initializes the nest imc engine for the current chip.+ * by default the nest engine is disabled.+ */+static void nest_init(int *cpu_opal_rc)+{+ int rc;++ /*+ * OPAL figures out which CPU to start based on the CPU that is+ * currently running when we call into OPAL+ */+ rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
Why isn't this the init call? If this is correct, a comment explaning it
would be helpful.
quoted
+ if (rc)
+ cpu_opal_rc[smp_processor_id()] = 1;
+}
+
quoted
+static int nest_imc_control(int operation)
+{
+ int *cpus_opal_rc, cpu;
+
+ /*
+ * Memory for OPAL call return value.
+ */
+ cpus_opal_rc = kzalloc((sizeof(int) * nr_cpu_ids), GFP_KERNEL);
+ if (!cpus_opal_rc)
+ return -ENOMEM;
+ switch (operation) {
+
+ case IMC_COUNTER_ENABLE:
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_imc_start,
+ (void *)cpus_opal_rc, 1);
+ break;
+ case IMC_COUNTER_DISABLE:
+ /* Disable the counters */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
+ break;
+ default: return -EINVAL;
+
+ }
+
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
+ if (cpus_opal_rc[cpu])
+ return -ENODEV;
+ }
+ return 0;
+}
Two things:
- It doesn't look like you're freeing cpus_opal_rc anywhere - have I
missed it?
- Would it be better to split this function into two: so instead of
passing in `operation`, you just have a nest_imc_enable and
nest_imc_disable? All the call sites I can see call this with a
constant parameter anyway. Perhaps it could even be refactored into
nest_imc_event_start/stop and this method could be removed
entirely...
(I haven't checked if you use this in future patches or if it gets
expanded and makes sense to keep the function this way.)
quoted
+
static void imc_event_start(struct perf_event *event, int flags)
{
/*
@@ -129,19 +333,44 @@ static void imc_event_stop(struct perf_event *event, int flags) imc_perf_event_update(event); }-/*- * The wrapper function is provided here, since we will have reserve- * and release lock for imc_event_start() in the following patch.- * Same in case of imc_event_stop().- */ static void nest_imc_event_start(struct perf_event *event, int flags) {+ int rc;++ /*+ * Nest pmu units are enabled only when it is used.+ * See if this is triggered for the first time.+ * If yes, take the mutex lock and enable the nest counters.+ * If not, just increment the count in nest_events.+ */+ if (atomic_inc_return(&nest_events) == 1) {+ mutex_lock(&imc_nest_reserve);+ rc = nest_imc_control(IMC_COUNTER_ENABLE);+ mutex_unlock(&imc_nest_reserve);+ if (rc)+ pr_err("IMC: Unbale to start the counters\n");
Spelling: s/Unbale/Unable/ ----------^
quoted
+ }
imc_event_start(event, flags);
}
Overall I'm much happer with this now, good work :)
Regards,
Daniel
From: Anju T Sudhakar <hidden> Date: 2017-05-09 10:55:18
Hi Daniel,
On Monday 08 May 2017 07:42 PM, Daniel Axtens wrote:
Hi all,
I've had a look at the API as it was a big thing I didn't like in the
earlier version.
I am much happier with this one.
Some comments:
- I'm no longer subscribed to skiboot but I've had a look at the
patches on that side:
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
* in start/stop, should there be some sort of write barrier to make
sure the cb->imc_chip_command actually gets written out to memory
at the time we expect?
The rest of my comments are in line.
quoted
Adds cpumask attribute to be used by each IMC pmu. Only one cpu (any
online CPU) from each chip for nest PMUs is designated to read counters.
On CPU hotplug, dying CPU is checked to see whether it is one of the
designated cpus, if yes, next online cpu from the same chip (for nest
units) is designated as new cpu to read counters. For this purpose, we
introduce a new state : CPUHP_AP_PERF_POWERPC_NEST_ONLINE.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/include/asm/imc-pmu.h | 4 +
arch/powerpc/include/asm/opal-api.h | 12 +-
arch/powerpc/include/asm/opal.h | 4 +
arch/powerpc/perf/imc-pmu.c | 248 ++++++++++++++++++++++++-
arch/powerpc/platforms/powernv/opal-wrappers.S | 3 +
include/linux/cpuhotplug.h | 1 +
Who owns this? get_maintainer.pl doesn't give me anything helpful
here... Do we need an Ack from anyone?
@@ -18,6 +18,11 @@structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];structimc_pmu*per_nest_pmu_arr[IMC_MAX_PMUS];+staticcpumask_tnest_imc_cpumask;++staticatomic_tnest_events;+/* Used to avoid races in calling enable/disable nest-pmu units*/
You need a space here between s and * ----------------------------^
@@ -33,6 +38,160 @@ static struct attribute_group imc_format_group = { .attrs = imc_format_attrs, };+/* Get the cpumask printed to a buffer "buf" */+static ssize_t imc_pmu_cpumask_get_attr(struct device *dev,+ struct device_attribute *attr,+ char *buf)+{+ cpumask_t *active_mask;++ active_mask = &nest_imc_cpumask;+ return cpumap_print_to_pagebuf(true, buf, active_mask);+}++static DEVICE_ATTR(cpumask, S_IRUGO, imc_pmu_cpumask_get_attr, NULL);++static struct attribute *imc_pmu_cpumask_attrs[] = {+ &dev_attr_cpumask.attr,+ NULL,+};++static struct attribute_group imc_pmu_cpumask_attr_group = {+ .attrs = imc_pmu_cpumask_attrs,+};++/*+ * nest_init : Initializes the nest imc engine for the current chip.+ * by default the nest engine is disabled.+ */+static void nest_init(int *cpu_opal_rc)+{+ int rc;++ /*+ * OPAL figures out which CPU to start based on the CPU that is+ * currently running when we call into OPAL+ */+ rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
Why isn't this the init call? If this is correct, a comment explaning it
would be helpful.
Basically init call is meant for any hardware initialization, that we need.
Here in case of nest, we are disabling the counters initially.
We enable them only during perf init.
I will document this here.
quoted
+ if (rc)
+ cpu_opal_rc[smp_processor_id()] = 1;
+}
+
quoted
+static int nest_imc_control(int operation)
+{
+ int *cpus_opal_rc, cpu;
+
+ /*
+ * Memory for OPAL call return value.
+ */
+ cpus_opal_rc = kzalloc((sizeof(int) * nr_cpu_ids), GFP_KERNEL);
+ if (!cpus_opal_rc)
+ return -ENOMEM;
+ switch (operation) {
+
+ case IMC_COUNTER_ENABLE:
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_imc_start,
+ (void *)cpus_opal_rc, 1);
+ break;
+ case IMC_COUNTER_DISABLE:
+ /* Disable the counters */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
+ break;
+ default: return -EINVAL;
+
+ }
+
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
+ if (cpus_opal_rc[cpu])
+ return -ENODEV;
+ }
+ return 0;
+}
Two things:
- It doesn't look like you're freeing cpus_opal_rc anywhere - have I
missed it?
yes. we have to free cpus_opal_rc here. Will fix this.
- Would it be better to split this function into two: so instead of
passing in `operation`, you just have a nest_imc_enable and
nest_imc_disable? All the call sites I can see call this with a
constant parameter anyway. Perhaps it could even be refactored into
nest_imc_event_start/stop and this method could be removed
entirely...
(I haven't checked if you use this in future patches or if it gets
expanded and makes sense to keep the function this way.)
We can avoid some code duplication if we wrap nest-imc enable/disable into
a single function. That is why I preferred this way. I can give proper
documentation for
this to avoid further confusion.
quoted
+
static void imc_event_start(struct perf_event *event, int flags)
{
/*
@@ -129,19 +333,44 @@ static void imc_event_stop(struct perf_event *event, int flags) imc_perf_event_update(event); }-/*- * The wrapper function is provided here, since we will have reserve- * and release lock for imc_event_start() in the following patch.- * Same in case of imc_event_stop().- */ static void nest_imc_event_start(struct perf_event *event, int flags) {+ int rc;++ /*+ * Nest pmu units are enabled only when it is used.+ * See if this is triggered for the first time.+ * If yes, take the mutex lock and enable the nest counters.+ * If not, just increment the count in nest_events.+ */+ if (atomic_inc_return(&nest_events) == 1) {+ mutex_lock(&imc_nest_reserve);+ rc = nest_imc_control(IMC_COUNTER_ENABLE);+ mutex_unlock(&imc_nest_reserve);+ if (rc)+ pr_err("IMC: Unbale to start the counters\n");
Spelling: s/Unbale/Unable/ ----------^
Will correct here. Thanks for mentioning. :-)
quoted
+ }
imc_event_start(event, flags);
}
Overall I'm much happer with this now, good work :)
Regards,
Daniel
From: Thomas Gleixner <hidden> Date: 2017-05-10 12:09:58
On Thu, 4 May 2017, Anju T Sudhakar wrote:
+/*
+ * nest_init : Initializes the nest imc engine for the current chip.
+ * by default the nest engine is disabled.
+ */
+static void nest_init(int *cpu_opal_rc)
+{
+ int rc;
+
+ /*
+ * OPAL figures out which CPU to start based on the CPU that is
+ * currently running when we call into OPAL
I have no idea what that comment tries to tell me and how it is related to
the init function or the invoked opal_imc_counters_stop() function.
+ */
+ rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
+ if (rc)
+ cpu_opal_rc[smp_processor_id()] = 1;
+}
+
+static void nest_change_cpu_context(int old_cpu, int new_cpu)
+{
+ int i;
+
+ for (i = 0;
+ (per_nest_pmu_arr[i] != NULL) && (i < IMC_MAX_PMUS); i++)
+ perf_pmu_migrate_context(&per_nest_pmu_arr[i]->pmu,
+ old_cpu, new_cpu);
Bah, this is horrible to read.
struct imc_pmu **pn = per_nest_pmu_arr;
int i;
for (i = 0; *pn && i < IMC_MAX_PMUS; i++, pn++)
perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
Hmm?
+}
+
+static int ppc_nest_imc_cpu_online(unsigned int cpu)
+{
+ int nid;
+ const struct cpumask *l_cpumask;
+ struct cpumask tmp_mask;
You should not allocate cpumask on stack unconditionally. Either make that
cpumask_var_t and use zalloc/free_cpumask_var() or simply make it
static struct cpumask tmp_mask;
That's fine, because this is serialized by the hotplug code already.
+
+ /* Find the cpumask of this node */
+ nid = cpu_to_node(cpu);
+ l_cpumask = cpumask_of_node(nid);
+
+ /*
+ * If any of the cpu from this node is already present in the mask,
+ * just return, if not, then set this cpu in the mask.
+ */
+ if (!cpumask_and(&tmp_mask, l_cpumask, &nest_imc_cpumask)) {
+ cpumask_set_cpu(cpu, &nest_imc_cpumask);
+ nest_change_cpu_context(-1, cpu);
+ return 0;
+ }
+
+ return 0;
+}
+
+static int ppc_nest_imc_cpu_offline(unsigned int cpu)
+{
+ int nid, target = -1;
+ const struct cpumask *l_cpumask;
+
+ /*
+ * Check in the designated list for this cpu. Dont bother
+ * if not one of them.
+ */
+ if (!cpumask_test_and_clear_cpu(cpu, &nest_imc_cpumask))
+ return 0;
+
+ /*
+ * Now that this cpu is one of the designated,
+ * find a next cpu a) which is online and b) in same chip.
+ */
+ nid = cpu_to_node(cpu);
+ l_cpumask = cpumask_of_node(nid);
+ target = cpumask_next(cpu, l_cpumask);
+
+ /*
+ * Update the cpumask with the target cpu and
+ * migrate the context if needed
+ */
+ if (target >= 0 && target <= nr_cpu_ids) {
+ cpumask_set_cpu(target, &nest_imc_cpumask);
+ nest_change_cpu_context(cpu, target);
+ }
What disables the perf context if this was the last CPU on the node?
+ return 0;
+}
+
+static int nest_pmu_cpumask_init(void)
+{
+ const struct cpumask *l_cpumask;
+ int cpu, nid;
+ int *cpus_opal_rc;
+
+ if (!cpumask_empty(&nest_imc_cpumask))
+ return 0;
What's that for? Paranoia engineering?
+
+ /*
+ * Memory for OPAL call return value.
+ */
+ cpus_opal_rc = kzalloc((sizeof(int) * nr_cpu_ids), GFP_KERNEL);
+ if (!cpus_opal_rc)
+ goto fail;
+
+ /*
+ * Nest PMUs are per-chip counters. So designate a cpu
+ * from each chip for counter collection.
+ */
+ for_each_online_node(nid) {
+ l_cpumask = cpumask_of_node(nid);
+
+ /* designate first online cpu in this node */
+ cpu = cpumask_first(l_cpumask);
+ cpumask_set_cpu(cpu, &nest_imc_cpumask);
+ }
This is all unprotected against CPU hotplug.
+
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
What does this check on nodes which are not yet online and become online
later?
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
But this whole function is completely overengineered. If you make that
nest_init() part of the online function then this works also for nodes
which come online later and it simplifies to:
static int ppc_nest_imc_cpu_online(unsigned int cpu)
{
const struct cpumask *l_cpumask;
static struct cpumask tmp_mask;
int res;
/* Get the cpumask of this node */
l_cpumask = cpumask_of_node(cpu_to_node(cpu));
/*
* If this is not the first online CPU on this node, then IMC is
* initialized already.
*/
if (cpumask_and(&tmp_mask, l_cpumask, &nest_imc_cpumask))
return 0;
/*
* If this fails, IMC is not usable.
*
* FIXME: Add a understandable comment what this actually does
* and why it can fail.
*/
res = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
if (res)
return res;
/* Make this CPU the designated target for counter collection */
cpumask_set_cpu(cpu, &nest_imc_cpumask);
nest_change_cpu_context(-1, cpu);
return 0;
}
static int nest_pmu_cpumask_init(void)
{
return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_NEST_ONLINE,
"perf/powerpc/imc:online",
ppc_nest_imc_cpu_online,
ppc_nest_imc_cpu_offline);
}
Hmm?
This function leaks cpus_opal_rc on each invocation. Great stuff!
+ switch (operation) {
+
+ case IMC_COUNTER_ENABLE:
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_imc_start,
How is that supposed to work? This is called from interrupt disabled
context. on_each_cpu_mask() must not be called from there. And in the worst
case this is called not only from interrupt disabled context but from a smp
function call .....
Aside of that. What's the point of these type casts? If your function does
not have the signature of a smp function call function, then you should fix
that. If it has, then the type cast is just crap.
+ (void *)cpus_opal_rc, 1);
Ditto for this. Casting to (void *) is pointless.
+ break;
+ case IMC_COUNTER_DISABLE:
+ /* Disable the counters */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
+ break;
+ default: return -EINVAL;
+
+ }
+
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
+ if (cpus_opal_rc[cpu])
+ return -ENODEV;
So this just checks whether the disable/enable was successful, but what's
the consequence? Counters stay half enabled or disabled depending on which
of the CPUs failed.
This whole result array dance is just useless as you are solely printing
stuff at the call site. So I assume those calls are not supposed to fail,
so you can do the print in the function itself and get rid of all this
cpus_opal_rc hackery. Though that's the least of your worries, see above.
@@ -129,19 +333,44 @@ static void imc_event_stop(struct perf_event *event, int flags) imc_perf_event_update(event); }-/*- * The wrapper function is provided here, since we will have reserve- * and release lock for imc_event_start() in the following patch.- * Same in case of imc_event_stop().- */ static void nest_imc_event_start(struct perf_event *event, int flags) {+ int rc;++ /*+ * Nest pmu units are enabled only when it is used.+ * See if this is triggered for the first time.+ * If yes, take the mutex lock and enable the nest counters.+ * If not, just increment the count in nest_events.+ */+ if (atomic_inc_return(&nest_events) == 1) {+ mutex_lock(&imc_nest_reserve);
How is that supposed to work? pmu->start() and pmu->stop() are called with
interrupts disabled. Locking a mutex there is a NONO.
+ rc = nest_imc_control(IMC_COUNTER_ENABLE);
+ mutex_unlock(&imc_nest_reserve);
+ if (rc)
+ pr_err("IMC: Unbale to start the counters\n");
+ }
imc_event_start(event, flags);
}
static void nest_imc_event_stop(struct perf_event *event, int flags)
{
+ int rc;
+
imc_event_stop(event, flags);
+ /*
+ * See if we need to disable the nest PMU.
+ * If no events are currently in use, then we have to take a
+ * mutex to ensure that we don't race with another task doing
+ * enable or disable the nest counters.
+ */
+ if (atomic_dec_return(&nest_events) == 0) {
+ mutex_lock(&imc_nest_reserve);
From: Stephen Rothwell <hidden> Date: 2017-05-10 23:40:27
Hi,
On Wed, 10 May 2017 14:09:53 +0200 (CEST) Thomas Gleixner [off-list ref] wrote:
quoted
+static void nest_change_cpu_context(int old_cpu, int new_cpu)
+{
+ int i;
+
+ for (i = 0;
+ (per_nest_pmu_arr[i] != NULL) && (i < IMC_MAX_PMUS); i++)
+ perf_pmu_migrate_context(&per_nest_pmu_arr[i]->pmu,
+ old_cpu, new_cpu);
Bah, this is horrible to read.
struct imc_pmu **pn = per_nest_pmu_arr;
int i;
for (i = 0; *pn && i < IMC_MAX_PMUS; i++, pn++)
perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
(Just a bit of bike shedding ...)
Or even (since "i" is not used any more):
struct imc_pmu **pn;
for (pn = per_nest_pmu_arr;
pn < &per_nest_pmu_arr[IMC_MAX_PMUS] && *pn;
pn++)
perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
--
Cheers,
Stephen Rothwell
From: Stewart Smith <hidden> Date: 2017-05-11 07:49:44
Anju T Sudhakar [off-list ref] writes:
quoted hunk
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).
Patch also adds a CONFIG_HV_PERF_IMC_CTRS for the IMC driver.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/platforms/powernv/Kconfig | 10 +++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-imc.c | 140 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal.c | 18 ++++
4 files changed, 169 insertions(+)
create mode 100644 arch/powerpc/platforms/powernv/opal-imc.c
@@ -0,0 +1,140 @@+/*+*OPALIMCinterfacedetectiondriver+*SupportedonPOWERNVplatform+*+*Copyright(C)2017MadhavanSrinivasan,IBMCorporation.+*(C)2017AnjuTSudhakar,IBMCorporation.+*(C)2017HemantKShaw,IBMCorporation.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/+#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>++structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];++/*+*imc_pmu_setup:SetuptheIMCPMUs(childrenof"parent").+*/+staticvoid__initimc_pmu_setup(structdevice_node*parent)+{+if(!parent)+return;+}++staticintopal_imc_counters_probe(structplatform_device*pdev)+{+structdevice_node*imc_dev,*dn,*rm_node=NULL;+structperchip_nest_info*pcni;+u32pages,nest_offset,nest_size,chip_id;+inti=0;+const__be32*addrp;+u64reg_addr,reg_size;++if(!pdev||!pdev->dev.of_node)+return-ENODEV;++/*+*Checkwhetherthisiskdumpkernel.Ifyes,justreturn.+*/+if(is_kdump_kernel())+return-ENODEV;++imc_dev=pdev->dev.of_node;++/*+*NestcounterdataaresavedinareservedmemorycalledHOMER.+*"imc-nest-offset"identifiesthecounterdatalocationwithinHOMER.+*size:sizeoftheentirenest-countersregion+*/+if(of_property_read_u32(imc_dev,"imc-nest-offset",&nest_offset))+gotoerr;++if(of_property_read_u32(imc_dev,"imc-nest-size",&nest_size))+gotoerr;++/* Sanity check */+if((nest_size/PAGE_SIZE)>IMC_NEST_MAX_PAGES)+gotoerr;++/* Find the "HOMER region" for each chip */+rm_node=of_find_node_by_path("/reserved-memory");+if(!rm_node)+gotoerr;++/*+*Weneedtolookforthe"ibm,homer-image"nodeinthe+*"/reserved-memory"node.+*/+for(dn=of_find_node_by_name(rm_node,"ibm,homer-image");dn;+dn=of_find_node_by_name(dn,"ibm,homer-image")){++/* Get the chip id to which the above homer region belongs to */+if(of_property_read_u32(dn,"ibm,chip-id",&chip_id))+gotoerr;
So, I was thinking on this (and should probably comment on the firmware
side as well).
I'd prefer an OPAL interface where instead of looking up where
ibm,homer-image is, we provide the kernel with a base address and then
have offsets into it.
That way, we don't tie the kernel code to counters that are only in the
HOMER region.
--
Stewart Smith
OPAL Architect, IBM.
From: Thomas Gleixner <hidden> Date: 2017-05-11 08:39:21
On Thu, 11 May 2017, Stephen Rothwell wrote:
Hi,
On Wed, 10 May 2017 14:09:53 +0200 (CEST) Thomas Gleixner [off-list ref] wrote:
quoted
quoted
+static void nest_change_cpu_context(int old_cpu, int new_cpu)
+{
+ int i;
+
+ for (i = 0;
+ (per_nest_pmu_arr[i] != NULL) && (i < IMC_MAX_PMUS); i++)
+ perf_pmu_migrate_context(&per_nest_pmu_arr[i]->pmu,
+ old_cpu, new_cpu);
Bah, this is horrible to read.
struct imc_pmu **pn = per_nest_pmu_arr;
int i;
for (i = 0; *pn && i < IMC_MAX_PMUS; i++, pn++)
perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
(Just a bit of bike shedding ...)
Or even (since "i" is not used any more):
struct imc_pmu **pn;
for (pn = per_nest_pmu_arr;
pn < &per_nest_pmu_arr[IMC_MAX_PMUS] && *pn;
pn++)
perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
Which is equally unreadable as the original code I complained about. Is that
a corporate preference?
Thanks,
tglx
From: Stewart Smith <hidden> Date: 2017-05-12 02:18:39
Madhavan Srinivasan [off-list ref] writes:
quoted
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
So, init call will return OPAL_PARAMETER for the unsupported
domains (core and nest are supported). And if the init operation
fails for any reason, it would return OPAL_HARDWARE. And this is
documented.
(I'll comment on the skiboot one too), but I think that if the class
exists but init is a no-op, then OPAL_IMC_COUNTERS_INIT should return
OPAL_SUCCESS and just do nothing. This future proofs everything, and the
API is that one *must* call _INIT before start.
--
Stewart Smith
OPAL Architect, IBM.
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-05-12 03:33:37
Stewart Smith [off-list ref] writes:
Madhavan Srinivasan [off-list ref] writes:
quoted
quoted
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
So, init call will return OPAL_PARAMETER for the unsupported
domains (core and nest are supported). And if the init operation
fails for any reason, it would return OPAL_HARDWARE. And this is
documented.
(I'll comment on the skiboot one too), but I think that if the class
exists but init is a no-op, then OPAL_IMC_COUNTERS_INIT should return
OPAL_SUCCESS and just do nothing. This future proofs everything, and the
API is that one *must* call _INIT before start.
Yes, 100%.
That's what I described in my replies to a previous version, if it
doesn't do that we need to fix it.
cheers
On Friday 12 May 2017 07:48 AM, Stewart Smith wrote:
Madhavan Srinivasan [off-list ref] writes:
quoted
quoted
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
So, init call will return OPAL_PARAMETER for the unsupported
domains (core and nest are supported). And if the init operation
fails for any reason, it would return OPAL_HARDWARE. And this is
documented.
(I'll comment on the skiboot one too), but I think that if the class
exists but init is a no-op, then OPAL_IMC_COUNTERS_INIT should return
OPAL_SUCCESS and just do nothing. This future proofs everything, and the
API is that one *must* call _INIT before start.
Hi stewart,
Yes. mpe did mention this in his review. And i have made the same in the v11
of the opal patchset. Currently we return OPAL_SUCCESS from _INIT
incase of type "Nest". Additionally i have also added a message to be
printed
but i guess we can get away with that.
Maddy
On Friday 12 May 2017 09:03 AM, Michael Ellerman wrote:
Stewart Smith [off-list ref] writes:
quoted
Madhavan Srinivasan [off-list ref] writes:
quoted
quoted
* in patch 9 should opal_imc_counters_init return something other
than OPAL_SUCCESS in the case on invalid arguments? Maybe
OPAL_PARAMETER? (I think you fix this in a later patch anyway?)
So, init call will return OPAL_PARAMETER for the unsupported
domains (core and nest are supported). And if the init operation
fails for any reason, it would return OPAL_HARDWARE. And this is
documented.
(I'll comment on the skiboot one too), but I think that if the class
exists but init is a no-op, then OPAL_IMC_COUNTERS_INIT should return
OPAL_SUCCESS and just do nothing. This future proofs everything, and the
API is that one *must* call _INIT before start.
Yes, 100%.
That's what I described in my replies to a previous version, if it
doesn't do that we need to fix it.
Hi mpe,
Yes, as you suggested in the opal v11 patchset, we return OPAL_SUCCESS
from _INIT for type "Nest". Have also added a prerror message logging
for debug, but can get away with it or make it as a prlog.
Maddy
On Thursday 11 May 2017 01:19 PM, Stewart Smith wrote:
Anju T Sudhakar [off-list ref] writes:
quoted
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).
Patch also adds a CONFIG_HV_PERF_IMC_CTRS for the IMC driver.
Signed-off-by: Anju T Sudhakar <redacted>
Signed-off-by: Hemant Kumar <redacted>
Signed-off-by: Madhavan Srinivasan <redacted>
---
arch/powerpc/platforms/powernv/Kconfig | 10 +++
arch/powerpc/platforms/powernv/Makefile | 1 +
arch/powerpc/platforms/powernv/opal-imc.c | 140 ++++++++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal.c | 18 ++++
4 files changed, 169 insertions(+)
create mode 100644 arch/powerpc/platforms/powernv/opal-imc.c
@@ -0,0 +1,140 @@+/*+*OPALIMCinterfacedetectiondriver+*SupportedonPOWERNVplatform+*+*Copyright(C)2017MadhavanSrinivasan,IBMCorporation.+*(C)2017AnjuTSudhakar,IBMCorporation.+*(C)2017HemantKShaw,IBMCorporation.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*/+#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>++structperchip_nest_infonest_perchip_info[IMC_MAX_CHIPS];++/*+*imc_pmu_setup:SetuptheIMCPMUs(childrenof"parent").+*/+staticvoid__initimc_pmu_setup(structdevice_node*parent)+{+if(!parent)+return;+}++staticintopal_imc_counters_probe(structplatform_device*pdev)+{+structdevice_node*imc_dev,*dn,*rm_node=NULL;+structperchip_nest_info*pcni;+u32pages,nest_offset,nest_size,chip_id;+inti=0;+const__be32*addrp;+u64reg_addr,reg_size;++if(!pdev||!pdev->dev.of_node)+return-ENODEV;++/*+*Checkwhetherthisiskdumpkernel.Ifyes,justreturn.+*/+if(is_kdump_kernel())+return-ENODEV;++imc_dev=pdev->dev.of_node;++/*+*NestcounterdataaresavedinareservedmemorycalledHOMER.+*"imc-nest-offset"identifiesthecounterdatalocationwithinHOMER.+*size:sizeoftheentirenest-countersregion+*/+if(of_property_read_u32(imc_dev,"imc-nest-offset",&nest_offset))+gotoerr;++if(of_property_read_u32(imc_dev,"imc-nest-size",&nest_size))+gotoerr;++/* Sanity check */+if((nest_size/PAGE_SIZE)>IMC_NEST_MAX_PAGES)+gotoerr;++/* Find the "HOMER region" for each chip */+rm_node=of_find_node_by_path("/reserved-memory");+if(!rm_node)+gotoerr;++/*+*Weneedtolookforthe"ibm,homer-image"nodeinthe+*"/reserved-memory"node.+*/+for(dn=of_find_node_by_name(rm_node,"ibm,homer-image");dn;+dn=of_find_node_by_name(dn,"ibm,homer-image")){++/* Get the chip id to which the above homer region belongs to */+if(of_property_read_u32(dn,"ibm,chip-id",&chip_id))+gotoerr;
So, I was thinking on this (and should probably comment on the firmware
side as well).
I'd prefer an OPAL interface where instead of looking up where
ibm,homer-image is, we provide the kernel with a base address and then
have offsets into it.
That way, we don't tie the kernel code to counters that are only in the
HOMER region.
Yes. This make sense. Adding something like this to IMC node
will be fine?
chip@<id> {
base_addr = < addr >;
ibm,chip-id = < id>;
};
Maddy
Sorry for delayed response.
On Wednesday 10 May 2017 05:39 PM, Thomas Gleixner wrote:
On Thu, 4 May 2017, Anju T Sudhakar wrote:
quoted
+/*
+ * nest_init : Initializes the nest imc engine for the current chip.
+ * by default the nest engine is disabled.
+ */
+static void nest_init(int *cpu_opal_rc)
+{
+ int rc;
+
+ /*
+ * OPAL figures out which CPU to start based on the CPU that is
+ * currently running when we call into OPAL
I have no idea what that comment tries to tell me and how it is related to
the init function or the invoked opal_imc_counters_stop() function.
Yep will fix the comment.
quoted
+ */
+ rc = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
+ if (rc)
+ cpu_opal_rc[smp_processor_id()] = 1;
+}
+
+static void nest_change_cpu_context(int old_cpu, int new_cpu)
+{
+ int i;
+
+ for (i = 0;
+ (per_nest_pmu_arr[i] != NULL) && (i < IMC_MAX_PMUS); i++)
+ perf_pmu_migrate_context(&per_nest_pmu_arr[i]->pmu,
+ old_cpu, new_cpu);
Bah, this is horrible to read.
struct imc_pmu **pn = per_nest_pmu_arr;
int i;
for (i = 0; *pn && i < IMC_MAX_PMUS; i++, pn++)
perf_pmu_migrate_context(&(*pn)->pmu, old_cpu, new_cpu);
Hmm?
Yes this is better. Will update the code.
quoted
+}
+
+static int ppc_nest_imc_cpu_online(unsigned int cpu)
+{
+ int nid;
+ const struct cpumask *l_cpumask;
+ struct cpumask tmp_mask;
You should not allocate cpumask on stack unconditionally. Either make that
cpumask_var_t and use zalloc/free_cpumask_var() or simply make it
static struct cpumask tmp_mask;
That's fine, because this is serialized by the hotplug code already.
ok will fix it as suggested.
quoted
+
+ /* Find the cpumask of this node */
+ nid = cpu_to_node(cpu);
+ l_cpumask = cpumask_of_node(nid);
+
+ /*
+ * If any of the cpu from this node is already present in the mask,
+ * just return, if not, then set this cpu in the mask.
+ */
+ if (!cpumask_and(&tmp_mask, l_cpumask, &nest_imc_cpumask)) {
+ cpumask_set_cpu(cpu, &nest_imc_cpumask);
+ nest_change_cpu_context(-1, cpu);
+ return 0;
+ }
+
+ return 0;
+}
+
+static int ppc_nest_imc_cpu_offline(unsigned int cpu)
+{
+ int nid, target = -1;
+ const struct cpumask *l_cpumask;
+
+ /*
+ * Check in the designated list for this cpu. Dont bother
+ * if not one of them.
+ */
+ if (!cpumask_test_and_clear_cpu(cpu, &nest_imc_cpumask))
+ return 0;
+
+ /*
+ * Now that this cpu is one of the designated,
+ * find a next cpu a) which is online and b) in same chip.
+ */
+ nid = cpu_to_node(cpu);
+ l_cpumask = cpumask_of_node(nid);
+ target = cpumask_next(cpu, l_cpumask);
+
+ /*
+ * Update the cpumask with the target cpu and
+ * migrate the context if needed
+ */
+ if (target >= 0 && target <= nr_cpu_ids) {
+ cpumask_set_cpu(target, &nest_imc_cpumask);
+ nest_change_cpu_context(cpu, target);
+ }
What disables the perf context if this was the last CPU on the node?
My bad. i did not understand this. Is this regarding the updates
of the "flags" in the perf_event and hw_perf_event structs?
quoted
+ return 0;
+}
+
+static int nest_pmu_cpumask_init(void)
+{
+ const struct cpumask *l_cpumask;
+ int cpu, nid;
+ int *cpus_opal_rc;
+
+ if (!cpumask_empty(&nest_imc_cpumask))
+ return 0;
What's that for? Paranoia engineering?
No. The idea here is to generate the cpu_mask attribute
field only for the first "nest" pmu and use the same
for other "nest" units.
quoted
+
+ /*
+ * Memory for OPAL call return value.
+ */
+ cpus_opal_rc = kzalloc((sizeof(int) * nr_cpu_ids), GFP_KERNEL);
+ if (!cpus_opal_rc)
+ goto fail;
+
+ /*
+ * Nest PMUs are per-chip counters. So designate a cpu
+ * from each chip for counter collection.
+ */
+ for_each_online_node(nid) {
+ l_cpumask = cpumask_of_node(nid);
+
+ /* designate first online cpu in this node */
+ cpu = cpumask_first(l_cpumask);
+ cpumask_set_cpu(cpu, &nest_imc_cpumask);
+ }
This is all unprotected against CPU hotplug.
quoted
+
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
What does this check on nodes which are not yet online and become online
later?
Idea here is, not to have the nest engines running always. Enable/start
them only when needed. So at init stage of the pmu setup, disable them.
That said, opal api call to disable is needed for a new node
that comes online at later stage. Nice catch, Thanks
quoted
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
But this whole function is completely overengineered. If you make that
nest_init() part of the online function then this works also for nodes
which come online later and it simplifies to:
static int ppc_nest_imc_cpu_online(unsigned int cpu)
{
const struct cpumask *l_cpumask;
static struct cpumask tmp_mask;
int res;
/* Get the cpumask of this node */
l_cpumask = cpumask_of_node(cpu_to_node(cpu));
/*
* If this is not the first online CPU on this node, then IMC is
* initialized already.
*/
if (cpumask_and(&tmp_mask, l_cpumask, &nest_imc_cpumask))
return 0;
/*
* If this fails, IMC is not usable.
*
* FIXME: Add a understandable comment what this actually does
* and why it can fail.
*/
res = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
if (res)
return res;
/* Make this CPU the designated target for counter collection */
cpumask_set_cpu(cpu, &nest_imc_cpumask);
nest_change_cpu_context(-1, cpu);
return 0;
}
static int nest_pmu_cpumask_init(void)
{
return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_NEST_ONLINE,
"perf/powerpc/imc:online",
ppc_nest_imc_cpu_online,
ppc_nest_imc_cpu_offline);
}
Hmm?
Yes this make sense. But we need to first designate a cpu in each
chip at init setup and use opal api to disable the engine in the same.
So probably, after cpuhp_setup_state, can we do that?
This function leaks cpus_opal_rc on each invocation. Great stuff!
Yep. Have fixed it and daniel also pointed out the
same in this review comments.
quoted
+ switch (operation) {
+
+ case IMC_COUNTER_ENABLE:
+ /* Initialize Nest PMUs in each node using designated cpus */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_imc_start,
How is that supposed to work? This is called from interrupt disabled
context. on_each_cpu_mask() must not be called from there. And in the worst
case this is called not only from interrupt disabled context but from a smp
function call .....
Indent here is to make sure nest engines running/enabled only
when needed and disabled rest of the time. So we added a
reserved/release logic to enable/diable the nest engine in the recent
version of the patchset. And my bad, could have done better
incase of testing this logic.
That said, we did hit "WARN_ON_ONCE" in smp_call_* with this version
of the patchset which you were spot on. So have moved the
reserved/release logic to *event_init(). Something like this, to the
latest internal version of the patchset.
@@ -74,7 +305,20 @@ static int nest_imc_event_init(struct perf_event *event) */ event->hw.event_base = pcni->vbase[config/PAGE_SIZE] + (config & ~PAGE_MASK);-+ /*+ * Nest pmu units are enabled only when it is used.+ * See if this is triggered for the first time.+ * If yes, take the mutex lock and enable the nest counters.+ * If not, just increment the count in nest_events.+ */+ if (atomic_inc_return(&nest_events) == 1) {+ mutex_lock(&imc_nest_reserve);+ rc = nest_imc_control(IMC_COUNTER_ENABLE);+ mutex_unlock(&imc_nest_reserve);+ if (rc) {+ pr_err("IMC: Unable to start the counters\n");+ return -EBUSY;+ }+ }+ event->destroy = nest_imc_counters_release; return 0; }
Aside of that. What's the point of these type casts? If your function does
not have the signature of a smp function call function, then you should fix
that. If it has, then the type cast is just crap.
yes. Will fix the casting part.
quoted
+ (void *)cpus_opal_rc, 1);
Ditto for this. Casting to (void *) is pointless.
quoted
+ break;
+ case IMC_COUNTER_DISABLE:
+ /* Disable the counters */
+ on_each_cpu_mask(&nest_imc_cpumask, (smp_call_func_t)nest_init,
+ (void *)cpus_opal_rc, 1);
+ break;
+ default: return -EINVAL;
+
+ }
+
+ /* Check return value array for any OPAL call failure */
+ for_each_cpu(cpu, &nest_imc_cpumask) {
+ if (cpus_opal_rc[cpu])
+ return -ENODEV;
So this just checks whether the disable/enable was successful, but what's
the consequence? Counters stay half enabled or disabled depending on which
of the CPUs failed.
This whole result array dance is just useless as you are solely printing
stuff at the call site. So I assume those calls are not supposed to fail,
so you can do the print in the function itself and get rid of all this
cpus_opal_rc hackery. Though that's the least of your worries, see above.
Yes. As I mentioned before, have moved the reserve/release
logic to the *event_init(). And apart from printing debug message
on the opal call failure, also return with -EBUSY in the *event_init()
incase of opal call failure.
@@ -129,19 +333,44 @@ static void imc_event_stop(struct perf_event *event, int flags) imc_perf_event_update(event); }-/*- * The wrapper function is provided here, since we will have reserve- * and release lock for imc_event_start() in the following patch.- * Same in case of imc_event_stop().- */ static void nest_imc_event_start(struct perf_event *event, int flags) {+ int rc;++ /*+ * Nest pmu units are enabled only when it is used.+ * See if this is triggered for the first time.+ * If yes, take the mutex lock and enable the nest counters.+ * If not, just increment the count in nest_events.+ */+ if (atomic_inc_return(&nest_events) == 1) {+ mutex_lock(&imc_nest_reserve);
How is that supposed to work? pmu->start() and pmu->stop() are called with
interrupts disabled. Locking a mutex there is a NONO.
quoted
+ rc = nest_imc_control(IMC_COUNTER_ENABLE);
+ mutex_unlock(&imc_nest_reserve);
+ if (rc)
+ pr_err("IMC: Unbale to start the counters\n");
+ }
imc_event_start(event, flags);
}
static void nest_imc_event_stop(struct perf_event *event, int flags)
{
+ int rc;
+
imc_event_stop(event, flags);
+ /*
+ * See if we need to disable the nest PMU.
+ * If no events are currently in use, then we have to take a
+ * mutex to ensure that we don't race with another task doing
+ * enable or disable the nest counters.
+ */
+ if (atomic_dec_return(&nest_events) == 0) {
+ mutex_lock(&imc_nest_reserve);
I have no idea how that survived any form of testing ....
yes could have done better incase of testing the reserve/release
logic it but that said, have identified and fixed the issues in the
latest internal version of the patchset with more test runs and will
post it out soon.
Thanks for your review comments.
Maddy
From: Thomas Gleixner <hidden> Date: 2017-05-15 11:06:15
On Mon, 15 May 2017, Madhavan Srinivasan wrote:
On Wednesday 10 May 2017 05:39 PM, Thomas Gleixner wrote:
quoted
On Thu, 4 May 2017, Anju T Sudhakar wrote:
quoted
+ /*
+ * Update the cpumask with the target cpu and
+ * migrate the context if needed
+ */
+ if (target >= 0 && target <= nr_cpu_ids) {
+ cpumask_set_cpu(target, &nest_imc_cpumask);
+ nest_change_cpu_context(cpu, target);
+ }
What disables the perf context if this was the last CPU on the node?
My bad. i did not understand this. Is this regarding the updates
of the "flags" in the perf_event and hw_perf_event structs?
Sorry, there is nothing to understand. It was me misreading it.
quoted
quoted
+static int nest_pmu_cpumask_init(void)
+{
+ const struct cpumask *l_cpumask;
+ int cpu, nid;
+ int *cpus_opal_rc;
+
+ if (!cpumask_empty(&nest_imc_cpumask))
+ return 0;
What's that for? Paranoia engineering?
No. The idea here is to generate the cpu_mask attribute
field only for the first "nest" pmu and use the same
for other "nest" units.
Why is nest_pmu_cpumask_init() called more than once? If it is then you
should have a proper flag marking it initialiazed rather than making a
completely obscure check for a cpu mask. That check could be empty for the
second invocation when the first invocation fails.
quoted
But this whole function is completely overengineered. If you make that
nest_init() part of the online function then this works also for nodes
which come online later and it simplifies to:
static int ppc_nest_imc_cpu_online(unsigned int cpu)
{
const struct cpumask *l_cpumask;
static struct cpumask tmp_mask;
int res;
/* Get the cpumask of this node */
l_cpumask = cpumask_of_node(cpu_to_node(cpu));
/*
* If this is not the first online CPU on this node, then IMC is
* initialized already.
*/
if (cpumask_and(&tmp_mask, l_cpumask, &nest_imc_cpumask))
return 0;
/*
* If this fails, IMC is not usable.
*
* FIXME: Add a understandable comment what this actually does
* and why it can fail.
*/
res = opal_imc_counters_stop(OPAL_IMC_COUNTERS_NEST);
if (res)
return res;
/* Make this CPU the designated target for counter collection */
cpumask_set_cpu(cpu, &nest_imc_cpumask);
nest_change_cpu_context(-1, cpu);
return 0;
}
static int nest_pmu_cpumask_init(void)
{
return cpuhp_setup_state(CPUHP_AP_PERF_POWERPC_NEST_ONLINE,
"perf/powerpc/imc:online",
ppc_nest_imc_cpu_online,
ppc_nest_imc_cpu_offline);
}
Hmm?
Yes this make sense. But we need to first designate a cpu in each
chip at init setup and use opal api to disable the engine in the same.
So probably, after cpuhp_setup_state, can we do that?
Errm. That's what ppc_nest_imc_cpu_online() does.
It checks whether this is the first online cpu on a node and if yes, it
calls opal_imc_counters_stop() and sets that cpu in nest_imc_cpumask.
cpuhp_setup_state() invokes the callback on each online CPU. Seo evrything
is set up proper after that.
Thanks,
tglx
Any reason for this not to be in the device tree?
This is the size of memory that Linux needs to give OPAL, so the size of
that should probably come from OPAL rather than Linux.
Otherwise, if in the future, we had a counter with an offset greater
than 8192 in, we'd have no way to detect that and we'd fail silently by
overwriting memory.
--
Stewart Smith
OPAL Architect, IBM.