From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:38
v6:
Added cpu affinity sysfs entry
Converted pmu_types array, to linked list
Restrict use of the armv8_pmu_probe_table to ACPI systems
Rename MADT parsing routines in smp.c
Convert sysfs PMU name to use index rather than partnum
Remove pr_devel statements
Other Minor cleanups
Add Partial Ack-by Will Deacon
v5:
Remove list of CPU types for ACPI systems. We now match a generic
event list, and use the PMCIED[01] to select events which exist on
the given PMU. This avoids the need to update the kernel every time
a new CPU is released.
Update the maintainers list to include the new file.
v4:
Correct build issues with ARM (!ARM64) kernels.
Add ThunderX to list of PMU types.
v3:
Enable ARM performance monitoring units on ACPI/arm64 machines.
This patch expands and reworks the patches published by Mark Salter
in order to clean up a few of the previous review comments, as well as
add support for newer CPUs and big/little configurations.
Jeremy Linton (9):
arm64: pmu: Probe default hw/cache counters
arm64: pmu: Hoist pmu platform device name
arm64: Rename the common MADT parse routine
arm: arm64: Add routine to determine cpuid of other cpus
arm: arm64: pmu: Assign platform PMU CPU affinity
arm64: pmu: Provide cpumask attribute for PMU
arm64: pmu: Add routines for detecting differing PMU types in the
system
arm64: pmu: Enable multiple PMUs in an ACPI system
MAINTAINERS: Tweak ARM PMU maintainers
Mark Salter (2):
arm64: pmu: add fallback probe table
arm64: pmu: Add support for probing with ACPI
MAINTAINERS | 3 +-
arch/arm/include/asm/cputype.h | 6 +-
arch/arm64/include/asm/cputype.h | 4 +
arch/arm64/kernel/perf_event.c | 79 ++++++++++++++-
arch/arm64/kernel/smp.c | 18 ++--
drivers/perf/Kconfig | 4 +
drivers/perf/Makefile | 1 +
drivers/perf/arm_pmu.c | 60 +++++++++---
drivers/perf/arm_pmu_acpi.c | 207 +++++++++++++++++++++++++++++++++++++++
include/linux/perf/arm_pmu.h | 12 +++
10 files changed, 370 insertions(+), 24 deletions(-)
create mode 100644 drivers/perf/arm_pmu_acpi.c
--
2.5.5
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:39
From: Mark Salter <redacted>
In preparation for ACPI support, add a pmu_probe_info table to
the arm_pmu_device_probe() call. This table gets used when
probing in the absence of a devicetree node for PMU.
Signed-off-by: Mark Salter <redacted>
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm64/kernel/perf_event.c | 13 ++++++++++++-
drivers/perf/arm_pmu.c | 2 +-
include/linux/perf/arm_pmu.h | 3 +++
3 files changed, 16 insertions(+), 2 deletions(-)
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:40
ARMv8 machines can identify the micro/arch defined counters
that are available on a machine. Add all these counters to the
default armv8 perf map. At run-time disable the counters which
are not available on the given PMU.
Signed-off-by: Jeremy Linton <redacted>
Acked-by: Will Deacon <redacted>
---
arch/arm64/kernel/perf_event.c | 45 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 41 insertions(+), 4 deletions(-)
@@ -906,9 +925,22 @@ static void armv8pmu_reset(void *info)staticintarmv8_pmuv3_map_event(structperf_event*event){-returnarmpmu_map_event(event,&armv8_pmuv3_perf_map,-&armv8_pmuv3_perf_cache_map,-ARMV8_PMU_EVTYPE_EVENT);+inthw_event_id;+structarm_pmu*armpmu=to_arm_pmu(event->pmu);++hw_event_id=armpmu_map_event(event,&armv8_pmuv3_perf_map,+&armv8_pmuv3_perf_cache_map,+ARMV8_PMU_EVTYPE_EVENT);+if(hw_event_id<0)+returnhw_event_id;++/* disable micro/arch events not supported by this PMU */+if((hw_event_id<ARMV8_PMUV3_MAX_COMMON_EVENTS)&&+!test_bit(hw_event_id,armpmu->pmceid_bitmap)){+return-EOPNOTSUPP;+}++returnhw_event_id;}staticintarmv8_a53_map_event(structperf_event*event)
@@ -1045,8 +1077,13 @@ static const struct of_device_id armv8_pmu_of_device_ids[] = {{},};+/*+*NonDTsystemshavetheirmicro/archeventsprobedatrun-time.+*Afairlycompletelistofgenericeventsareprovidedandonesthat+*aren'tsupportedbythecurrentPMUaredisabled.+*/staticconststructpmu_probe_infoarmv8_pmu_probe_table[]={-PMU_PROBE(0,0,armv8_pmuv3_init),/* if all else fails... */+PMU_PROBE(0,0,armv8_pmuv3_init),/* enable all defined counters */{/* sentinel value */}};
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:41
Move the PMU name into a common header file so it may
be referenced by other users.
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm64/kernel/perf_event.c | 2 +-
include/linux/perf/arm_pmu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:42
The MADT parser in smp.c is now being used to parse
out NUMA, PMU and ACPI parking protocol information as
well as the GIC information for which it was originally
created. Rename it to avoid a misleading name.
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm64/kernel/smp.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
@@ -660,7 +661,7 @@ void __init smp_init_cpus(void)*weneedforSMPinit*/acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,-acpi_parse_gic_cpu_interface,0);+acpi_parse_madt_common,0);if(cpu_count>NR_CPUS)pr_warn("no. of cores (%d) greater than configured maximum of %d - clipping\n",
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:43
From: Mark Salter <redacted>
In the case of ACPI, the PMU IRQ information is contained in the
MADT table. Also, since the PMU does not exist as a device in the
ACPI DSDT table, it is necessary to create a platform device so
that the appropriate driver probing is triggered.
Signed-off-by: Mark Salter <redacted>
Signed-off-by: Jeremy Linton <redacted>
---
NOTE: Much of the code in pmu_acpi_init() is replaced in patches 0009 and
0010 this set. The later version of the patch cleans up most of the
possible style/error handling issues that have been pointed out with
this version.
arch/arm64/kernel/smp.c | 5 +++
drivers/perf/Kconfig | 4 ++
drivers/perf/Makefile | 1 +
drivers/perf/arm_pmu_acpi.c | 100 +++++++++++++++++++++++++++++++++++++++++++
include/linux/perf/arm_pmu.h | 7 +++
5 files changed, 117 insertions(+)
create mode 100644 drivers/perf/arm_pmu_acpi.c
@@ -0,0 +1,100 @@+/*+*PMUsupport+*+*Copyright(C)2015RedHatInc.+*Author:MarkSalter<msalter@redhat.com>+*+*ThisworkislicensedunderthetermsoftheGNUGPL,version2.See+*theCOPYINGfileinthetop-leveldirectory.+*+*/++#include<linux/perf/arm_pmu.h>+#include<linux/platform_device.h>+#include<linux/acpi.h>+#include<linux/irq.h>+#include<linux/irqdesc.h>++structpmu_irq{+intgsi;+inttrigger;+};++staticstructpmu_irqpmu_irqs[NR_CPUS]__initdata;++/*+*Calledfromacpi_map_gic_cpu_interface()'sMADTparsingduringboot.+*ThisroutinesavesofftheGSI'sandtheirtriggerstateforusewhenweare+*readytobuildthePMUplatformdevice.+*/+void__initarm_pmu_parse_acpi(intcpu,structacpi_madt_generic_interrupt*gic)+{+pmu_irqs[cpu].gsi=gic->performance_interrupt;+if(gic->flags&ACPI_MADT_PERFORMANCE_IRQ_MODE)+pmu_irqs[cpu].trigger=ACPI_EDGE_SENSITIVE;+else+pmu_irqs[cpu].trigger=ACPI_LEVEL_SENSITIVE;+}++staticint__initpmu_acpi_init(void)+{+structplatform_device*pdev;+structpmu_irq*pirq=pmu_irqs;+structresource*res,*r;+interr=-ENOMEM;+inti,count,irq;++if(acpi_disabled)+return0;++/* Must have irq for boot cpu, at least */+if(pirq->gsi==0)+return-EINVAL;++irq=acpi_register_gsi(NULL,pirq->gsi,pirq->trigger,+ACPI_ACTIVE_HIGH);++if(irq_is_percpu(irq))+count=1;+else+for(i=1,count=1;i<NR_CPUS;i++)+if(pmu_irqs[i].gsi)+++count;++pdev=platform_device_alloc(ARMV8_PMU_PDEV_NAME,-1);+if(!pdev)+gotoerr_free_gsi;++res=kcalloc(count,sizeof(*res),GFP_KERNEL);+if(!res)+gotoerr_free_device;++for(i=0,r=res;i<count;i++,pirq++,r++){+if(i)+irq=acpi_register_gsi(NULL,pirq->gsi,pirq->trigger,+ACPI_ACTIVE_HIGH);+r->start=r->end=irq;+r->flags=IORESOURCE_IRQ;+if(pirq->trigger==ACPI_EDGE_SENSITIVE)+r->flags|=IORESOURCE_IRQ_HIGHEDGE;+else+r->flags|=IORESOURCE_IRQ_HIGHLEVEL;+}++err=platform_device_add_resources(pdev,res,count);+if(!err)+err=platform_device_add(pdev);+kfree(res);+if(!err)+return0;++err_free_device:+platform_device_put(pdev);++err_free_gsi:+for(i=0;i<count;i++)+acpi_unregister_gsi(pmu_irqs[i].gsi);++returnerr;+}+arch_initcall(pmu_acpi_init);
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:44
It is helpful if we can read the cpuid/midr of other CPUs
in the system independent of arm/arm64.
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm/include/asm/cputype.h | 6 +++++-
arch/arm64/include/asm/cputype.h | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:45
On systems with multiple PMU types the PMU to CPU affinity
needs to be detected and set. The CPU to interrupt affinity
should also be set.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/perf/arm_pmu.c | 53 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 43 insertions(+), 10 deletions(-)
@@ -872,25 +874,57 @@ static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)}/*-*CPUPMUidentificationandprobing.+*CPUPMUidentificationandprobing.Itspossibletohave+*multipleCPUtypesinanARMmachine.Assurethatweare+*pickingtherightPMUtypesbasedontheCPUinquestion*/-staticintprobe_current_pmu(structarm_pmu*pmu,-conststructpmu_probe_info*info)+staticintprobe_plat_pmu(structarm_pmu*pmu,+conststructpmu_probe_info*info,+unsignedintpmuid){-intcpu=get_cpu();-unsignedintcpuid=read_cpuid_id();intret=-ENODEV;+intcpu;+intaff_ctr=0;+staticintduplicate_pmus;+structplatform_device*pdev=pmu->plat_device;+intirq=platform_get_irq(pdev,0);++if(irq>=0&&!irq_is_percpu(irq)){+pmu->irq_affinity=kcalloc(pdev->num_resources,sizeof(int),+GFP_KERNEL);+if(!pmu->irq_affinity)+return-ENOMEM;+}++for_each_possible_cpu(cpu){+unsignedintcpuid=read_specific_cpuid(cpu);-pr_info("probing PMU on CPU %d\n",cpu);+if(cpuid==pmuid){+cpumask_set_cpu(cpu,&pmu->supported_cpus);+if(pmu->irq_affinity){+pmu->irq_affinity[aff_ctr]=cpu;+aff_ctr++;+}+}+}+/* find the type of PMU given the CPU */for(;info->init!=NULL;info++){-if((cpuid&info->mask)!=info->cpuid)+if((pmuid&info->mask)!=info->cpuid)continue;ret=info->init(pmu);+if((!info->cpuid)&&(duplicate_pmus)){+pmu->name=kasprintf(GFP_KERNEL,"%s_%d",+pmu->name,duplicate_pmus);+if(!pmu->name){+kfree(pmu->irq_affinity);+ret=-ENOMEM;+}+}+duplicate_pmus++;break;}-put_cpu();returnret;}
@@ -1010,8 +1044,7 @@ int arm_pmu_device_probe(struct platform_device *pdev,if(!ret)ret=init_fn(pmu);}elseif(probe_table){-cpumask_setall(&pmu->supported_cpus);-ret=probe_current_pmu(pmu,probe_table);+ret=probe_plat_pmu(pmu,probe_table,read_cpuid_id());}if(ret){
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:46
With heterogeneous PMUs its helpful to know which PMUs are bound
to each CPU. Provide that information with a cpumask sysfs entry
similar to other PMUs.
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm64/kernel/perf_event.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:47
In preparation for enabling heterogeneous PMUs on ACPI systems
add routines that detect this and group the resulting PMUs and
interrupts.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 134 insertions(+), 3 deletions(-)
@@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)pmu_irqs[cpu].trigger=ACPI_LEVEL_SENSITIVE;}+/* Count number and type of CPU cores in the system. */+void__initarm_pmu_acpi_determine_cpu_types(structlist_head*pmus)+{+inti;++for_each_possible_cpu(i){+structcpuinfo_arm64*cinfo=per_cpu_ptr(&cpu_data,i);+u32partnum=MIDR_PARTNUM(cinfo->reg_midr);+structpmu_types*pmu;++list_for_each_entry(pmu,pmus,list){+if(pmu->cpu_type==partnum){+pmu->cpu_count++;+break;+}+}++/* we didn't find the CPU type, add an entry to identify it */+if(&pmu->list==pmus){+pmu=kcalloc(1,sizeof(structpmu_types),GFP_KERNEL);+if(!pmu){+pr_warn("Unable to allocate pmu_types\n");+}else{+pmu->cpu_type=partnum;+pmu->cpu_count++;+list_add_tail(&pmu->list,pmus);+}+}+}+}++/*+*RegistersthegroupofPMUinterfaceswhichcorrespondtothe'last_cpu_id'.+*Thisgrouputilizes'count'resourcesinthe'res'.+*/+int__initarm_pmu_acpi_register_pmu(intcount,structresource*res,+intlast_cpu_id)+{+inti;+interr=-ENOMEM;+boolfree_gsi=false;+structplatform_device*pdev;++if(count){+pdev=platform_device_alloc(ARMV8_PMU_PDEV_NAME,last_cpu_id);+if(pdev){+err=platform_device_add_resources(pdev,res,count);+if(!err){+err=platform_device_add(pdev);+if(err){+pr_warn("Unable to register PMU device\n");+free_gsi=true;+}+}else{+pr_warn("Unable to add resources to device\n");+free_gsi=true;+platform_device_put(pdev);+}+}else{+pr_warn("Unable to allocate platform device\n");+free_gsi=true;+}+}++/* unmark (and possibly unregister) registered GSIs */+for_each_possible_cpu(i){+if(pmu_irqs[i].registered){+if(free_gsi)+acpi_unregister_gsi(pmu_irqs[i].gsi);+pmu_irqs[i].registered=false;+}+}++returnerr;+}++/*+*Forthegivencpu/pmutype,walkallknownGSIs,registerthem,andadd+*themtotheresourcestructure.ReturnthenumberofGSI'scontained+*intheresstructure,andtheidofthelastCPU/PMUweadded.+*/+int__initarm_pmu_acpi_gsi_res(structpmu_types*pmus,+structresource*res,int*last_cpu_id)+{+inti,count;+intirq;++pr_info("Setting up %d PMUs for CPU type %X\n",pmus->cpu_count,+pmus->cpu_type);+/* lets group all the PMU's from similar CPU's together */+count=0;+for_each_possible_cpu(i){+structcpuinfo_arm64*cinfo=per_cpu_ptr(&cpu_data,i);++if(pmus->cpu_type==MIDR_PARTNUM(cinfo->reg_midr)){+if(pmu_irqs[i].gsi==0)+continue;++irq=acpi_register_gsi(NULL,pmu_irqs[i].gsi,+pmu_irqs[i].trigger,+ACPI_ACTIVE_HIGH);++res[count].start=res[count].end=irq;+res[count].flags=IORESOURCE_IRQ;++if(pmu_irqs[i].trigger==ACPI_EDGE_SENSITIVE)+res[count].flags|=IORESOURCE_IRQ_HIGHEDGE;+else+res[count].flags|=IORESOURCE_IRQ_HIGHLEVEL;++pmu_irqs[i].registered=true;+count++;+(*last_cpu_id)=cinfo->reg_midr;+}+}+returncount;+}+staticint__initpmu_acpi_init(void){structplatform_device*pdev;
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:48
Its possible that an ACPI system has multiple CPU types in it
with differing PMU counters. Use the newly provided acpi_pmu routines
to detect that case, and instantiate more than one set of counters.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/perf/arm_pmu.c | 7 +++-
drivers/perf/arm_pmu_acpi.c | 82 ++++++++++++++++-----------------------------
2 files changed, 35 insertions(+), 54 deletions(-)
@@ -1044,7 +1044,12 @@ int arm_pmu_device_probe(struct platform_device *pdev,if(!ret)ret=init_fn(pmu);}elseif(probe_table){-ret=probe_plat_pmu(pmu,probe_table,read_cpuid_id());+if(acpi_disabled){+/* use the current cpu. */+ret=probe_plat_pmu(pmu,probe_table,+read_cpuid_id());+}else+ret=probe_plat_pmu(pmu,probe_table,pdev->id);}if(ret){
@@ -50,7 +50,7 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic)}/* Count number and type of CPU cores in the system. */-void__initarm_pmu_acpi_determine_cpu_types(structlist_head*pmus)+staticvoid__initarm_pmu_acpi_determine_cpu_types(structlist_head*pmus){inti;
@@ -169,63 +169,39 @@ int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,staticint__initpmu_acpi_init(void){-structplatform_device*pdev;-structpmu_irq*pirq=pmu_irqs;-structresource*res,*r;+structresource*res;interr=-ENOMEM;-inti,count,irq;+intcount,cpu_id;+structpmu_types*pmu,*safe_temp;+LIST_HEAD(pmus);if(acpi_disabled)return0;-/* Must have irq for boot cpu, at least */-if(pirq->gsi==0)-return-EINVAL;--irq=acpi_register_gsi(NULL,pirq->gsi,pirq->trigger,-ACPI_ACTIVE_HIGH);--if(irq_is_percpu(irq))-count=1;-else-for(i=1,count=1;i<NR_CPUS;i++)-if(pmu_irqs[i].gsi)-++count;--pdev=platform_device_alloc(ARMV8_PMU_PDEV_NAME,-1);-if(!pdev)-gotoerr_free_gsi;--res=kcalloc(count,sizeof(*res),GFP_KERNEL);-if(!res)-gotoerr_free_device;--for(i=0,r=res;i<count;i++,pirq++,r++){-if(i)-irq=acpi_register_gsi(NULL,pirq->gsi,pirq->trigger,-ACPI_ACTIVE_HIGH);-r->start=r->end=irq;-r->flags=IORESOURCE_IRQ;-if(pirq->trigger==ACPI_EDGE_SENSITIVE)-r->flags|=IORESOURCE_IRQ_HIGHEDGE;-else-r->flags|=IORESOURCE_IRQ_HIGHLEVEL;+arm_pmu_acpi_determine_cpu_types(&pmus);++list_for_each_entry_safe(pmu,safe_temp,&pmus,list){+res=kcalloc(pmu->cpu_count,+sizeof(structresource),GFP_KERNEL);++/* for a given PMU type collect all the GSIs. */+if(res){+count=arm_pmu_acpi_gsi_res(pmu,res,+&cpu_id);+/*+*registerthissetofinterrupts+*withanewPMUdevice+*/+err=arm_pmu_acpi_register_pmu(count,res,cpu_id);+kfree(res);+}else+pr_warn("PMU unable to allocate interrupt resource space\n");++list_del(&pmu->list);+kfree(pmu);}-err=platform_device_add_resources(pdev,res,count);-if(!err)-err=platform_device_add(pdev);-kfree(res);-if(!err)-return0;--err_free_device:-platform_device_put(pdev);--err_free_gsi:-for(i=0;i<count;i++)-acpi_unregister_gsi(pmu_irqs[i].gsi);-returnerr;}+arch_initcall(pmu_acpi_init);
From: Jeremy Linton <hidden> Date: 2016-06-21 17:11:49
Update the ARM PMU file list, and add the arm mailing list.
Signed-off-by: Jeremy Linton <redacted>
---
MAINTAINERS | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
In preparation for enabling heterogeneous PMUs on ACPI systems
add routines that detect this and group the resulting PMUs and
interrupts.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 134 insertions(+), 3 deletions(-)
@@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic) pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE; }+/* Count number and type of CPU cores in the system. */+void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)+{+ int i;++ for_each_possible_cpu(i) {+ struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);+ u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);+ struct pmu_types *pmu;++ list_for_each_entry(pmu, pmus, list) {+ if (pmu->cpu_type == partnum) {+ pmu->cpu_count++;+ break;+ }+ }++ /* we didn't find the CPU type, add an entry to identify it */+ if (&pmu->list == pmus) {+ pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);
Use kzalloc here.
+ if (!pmu) {
+ pr_warn("Unable to allocate pmu_types\n");
Bail out with error if the memory can't be allocated. Otherwise, we risk
silently failing to register a PMU type.
+ } else {
+ pmu->cpu_type = partnum;
+ pmu->cpu_count++;
+ list_add_tail(&pmu->list, pmus);
+ }
+ }
+ }
+}
+
+/*
+ * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
+ * This group utilizes 'count' resources in the 'res'.
+ */
+int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
+ int last_cpu_id)
+{
With the addition of the irq resources to struct pmu_types, you can just pass
the pmu structure here.
+ int i;
+ int err = -ENOMEM;
+ bool free_gsi = false;
+ struct platform_device *pdev;
+
+ if (count) {
if (!count)
goto out;
That should help reduce the nesting below. Others might have a different
opinion, but I think it's ok to use goto when it helps make the code
more readable.
Similarly, some of the code below can be simplified as well.
+ return err;
+}
+
+/*
+ * For the given cpu/pmu type, walk all known GSIs, register them, and add
+ * them to the resource structure. Return the number of GSI's contained
+ * in the res structure, and the id of the last CPU/PMU we added.
+ */
+int __init arm_pmu_acpi_gsi_res(struct pmu_types *pmus,
+ struct resource *res, int *last_cpu_id)
With struct resource as part of the pmu_types structure you can drop the
last two arguments and allocate the resources in this function.
+{
+ int i, count;
+ int irq;
+
+ pr_info("Setting up %d PMUs for CPU type %X\n", pmus->cpu_count,
+ pmus->cpu_type);
Please drop this pr_info.
+ /* lets group all the PMU's from similar CPU's together */
+ count = 0;
+ for_each_possible_cpu(i) {
+ struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);
+
+ if (pmus->cpu_type == MIDR_PARTNUM(cinfo->reg_midr)) {
You can invert the condition check here and reduce nesting.
Hi Jeremy,
One typo below.
Jeremy Linton [off-list ref] writes:
quoted hunk
On systems with multiple PMU types the PMU to CPU affinity
needs to be detected and set. The CPU to interrupt affinity
should also be set.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/perf/arm_pmu.c | 53 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 43 insertions(+), 10 deletions(-)
From: Jeremy Linton <hidden> Date: 2016-07-01 14:54:41
On 07/01/2016 08:58 AM, Punit Agrawal wrote:
Jeremy Linton [off-list ref] writes:
quoted
In preparation for enabling heterogeneous PMUs on ACPI systems
add routines that detect this and group the resulting PMUs and
interrupts.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/perf/arm_pmu_acpi.c | 137 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 134 insertions(+), 3 deletions(-)
You can stash the associated resources in the above structure. That
should simplify some code below.
How is that? One structure is per cpu, the other is per pmu type in the
system, they are actually completely independent and intertwining them
will only server to obfuscate the code.
@@ -36,6 +49,124 @@ void __init arm_pmu_parse_acpi(int cpu, struct acpi_madt_generic_interrupt *gic) pmu_irqs[cpu].trigger = ACPI_LEVEL_SENSITIVE; }+/* Count number and type of CPU cores in the system. */+void __init arm_pmu_acpi_determine_cpu_types(struct list_head *pmus)+{+ int i;++ for_each_possible_cpu(i) {+ struct cpuinfo_arm64 *cinfo = per_cpu_ptr(&cpu_data, i);+ u32 partnum = MIDR_PARTNUM(cinfo->reg_midr);+ struct pmu_types *pmu;++ list_for_each_entry(pmu, pmus, list) {+ if (pmu->cpu_type == partnum) {+ pmu->cpu_count++;+ break;+ }+ }++ /* we didn't find the CPU type, add an entry to identify it */+ if (&pmu->list == pmus) {+ pmu = kcalloc(1, sizeof(struct pmu_types), GFP_KERNEL);
Use kzalloc here.
Ok fair point.
quoted
+ if (!pmu) {
+ pr_warn("Unable to allocate pmu_types\n");
Bail out with error if the memory can't be allocated. Otherwise, we risk
silently failing to register a PMU type.
? Its not silent, it fails to allocate the space complains about it, and
therefor this pmu type is not created. In a system with a single CPU
this basically cancels the whole operation. If there is more than one
pmu, the remaining PMUs continue to have a chance of being created,
although if the memory allocation fails (this is pretty early boot code)
there is a high probability there is something seriously wrong with the
system.
quoted
+ } else {
+ pmu->cpu_type = partnum;
+ pmu->cpu_count++;
+ list_add_tail(&pmu->list, pmus);
+ }
+ }
+ }
+}
+
+/*
+ * Registers the group of PMU interfaces which correspond to the 'last_cpu_id'.
+ * This group utilizes 'count' resources in the 'res'.
+ */
+int __init arm_pmu_acpi_register_pmu(int count, struct resource *res,
+ int last_cpu_id)
+{
With the addition of the irq resources to struct pmu_types, you can just pass
the pmu structure here.
Thats a point, but the lifetimes of the structures are different and
outside of their shared use in this single function never really
interact. I prefer not unnecessarily intertwine independent data
structures simply to reduce parameter counts for a single function.
Especially since it complicates cleanup because the validity of the
resource structure will have to be tracked relative to its successful
registration.
From: Jeremy Linton <hidden> Date: 2016-07-01 15:28:05
On 07/01/2016 08:58 AM, Punit Agrawal wrote:
(trimming)
quoted
+ if (!pmu) {
+ pr_warn("Unable to allocate pmu_types\n");
Bail out with error if the memory can't be allocated. Otherwise, we risk
silently failing to register a PMU type.
Reading this again, I think I misunderstood what you were saying.. Aka,
this should be pr_error() not that the error handing is failing to catch
the failure.
Anyway, I will promote this.
From: Will Deacon <hidden> Date: 2016-07-06 16:30:02
On Tue, Jun 21, 2016 at 12:11:44PM -0500, Jeremy Linton wrote:
quoted hunk
It is helpful if we can read the cpuid/midr of other CPUs
in the system independent of arm/arm64.
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm/include/asm/cputype.h | 6 +++++-
arch/arm64/include/asm/cputype.h | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
From: Will Deacon <hidden> Date: 2016-07-06 16:45:09
On Tue, Jun 21, 2016 at 12:11:43PM -0500, Jeremy Linton wrote:
From: Mark Salter <redacted>
In the case of ACPI, the PMU IRQ information is contained in the
MADT table. Also, since the PMU does not exist as a device in the
ACPI DSDT table, it is necessary to create a platform device so
that the appropriate driver probing is triggered.
Signed-off-by: Mark Salter <redacted>
Signed-off-by: Jeremy Linton <redacted>
---
NOTE: Much of the code in pmu_acpi_init() is replaced in patches 0009 and
0010 this set. The later version of the patch cleans up most of the
possible style/error handling issues that have been pointed out with
this version.
I agree with Punit that it would be a lot easier to review this series
if you could fold in the changes to pmu_acpi_init so the sum of the
changes can be viewed in one patch.
Will
From: Jeremy Linton <hidden> Date: 2016-07-07 00:34:23
On 07/06/2016 11:30 AM, Will Deacon wrote:
On Tue, Jun 21, 2016 at 12:11:44PM -0500, Jeremy Linton wrote:
quoted
It is helpful if we can read the cpuid/midr of other CPUs
in the system independent of arm/arm64.
Signed-off-by: Jeremy Linton <redacted>
---
arch/arm/include/asm/cputype.h | 6 +++++-
arch/arm64/include/asm/cputype.h | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
@@ -180,7 +182,7 @@ static inline unsigned int __attribute_const__ read_cpuid_implementor(void)*/staticinlineunsignedint__attribute_const__read_cpuid_part(void){-returnread_cpuid_id()&ARM_CPU_PART_MASK;+returnARM_PARTNUM(read_cpuid_id());
I don't understand why you need to make this change.
The short answer is that the ARM_PARTNUM stuff is left over from v4 (?)
of the patch, where it seemed a good idea to create a macro that was
arm/arm64 independent for use in arm_pmu.c. Somewhere along there I
reverted the ARM_PARTNUM to MIDR_PARTNUM in the arm_pmu_acpi.c but
didn't drop that portion from this patch. Partially because it seems
like a good idea. OTOH, your right probably doesn't belong here without
the large cleanup which would form their own patch set.
Hi Jeremy,
Apologies for the late reply on this.
On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:
With heterogeneous PMUs its helpful to know which PMUs are bound
to each CPU. Provide that information with a cpumask sysfs entry
similar to other PMUs.
Have you tested trying to stat on a particular PMU? e.g.
$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
I found that the presence of a cpumask file would cause (at least some
versions) of perf-stat to hang, and was holding off adding a cpumask
until we had a solution to that.
See [1,2] for more details on that.
From: Jeremy Linton <hidden> Date: 2016-07-11 15:05:39
Hi,
On 07/07/2016 11:21 AM, Mark Rutland wrote:
Hi Jeremy,
Apologies for the late reply on this.
On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:
quoted
With heterogeneous PMUs its helpful to know which PMUs are bound
to each CPU. Provide that information with a cpumask sysfs entry
similar to other PMUs.
Have you tested trying to stat on a particular PMU? e.g.
$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
I found that the presence of a cpumask file would cause (at least some
versions) of perf-stat to hang, and was holding off adding a cpumask
until we had a solution to that.
Nice!
I guess that is more proof that any tiny change can break things..
Another "fix" is to make the cpumap_print_to_pagebuf's first parameter
false which apparently keeps perf from understanding the cpu mask and
forces the user to use numactl, which is ugly because numactrl doesn't
understand the mask in that format either.
I guess fixing perf is probably the best bet here...
This should be generic across the arm-pmu code, and so should live under
drivers/perf/.
Which is where i had it initially, but (IIRC) getting to work there
required some core pmu changes that seemed a little ugly. I guess I
didn't like the idea of reallocating the per pmu attr group or tweaking
the pmu core code.. So I just moved it into perf_event where it fit more
naturally.
On Mon, Jul 11, 2016 at 10:05:39AM -0500, Jeremy Linton wrote:
Hi,
On 07/07/2016 11:21 AM, Mark Rutland wrote:
quoted
Hi Jeremy,
Apologies for the late reply on this.
On Tue, Jun 21, 2016 at 12:11:46PM -0500, Jeremy Linton wrote:
quoted
With heterogeneous PMUs its helpful to know which PMUs are bound
to each CPU. Provide that information with a cpumask sysfs entry
similar to other PMUs.
Have you tested trying to stat on a particular PMU? e.g.
$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
I found that the presence of a cpumask file would cause (at least some
versions) of perf-stat to hang, and was holding off adding a cpumask
until we had a solution to that.
Nice!
I guess that is more proof that any tiny change can break things..
Another "fix" is to make the cpumap_print_to_pagebuf's first
parameter false which apparently keeps perf from understanding the
cpu mask and forces the user to use numactl, which is ugly because
numactrl doesn't understand the mask in that format either.
If we have a cpumask, it should be in keeping with the usual style.
I guess fixing perf is probably the best bet here...
Indeed.
The major issue is that it's not clear to me how to avoid breaking
existing binaries when adding the cpumask. I suspect that we have to
expose it under a different name. :/
This should be generic across the arm-pmu code, and so should live under
drivers/perf/.
Which is where i had it initially, but (IIRC) getting to work there
required some core pmu changes that seemed a little ugly.
That was my experience from local refactoring, too.
I guess I didn't like the idea of reallocating the per pmu attr group
or tweaking the pmu core code.. So I just moved it into perf_event
where it fit more naturally.
I don't think we need to allocate the attr group per pmu (we get the PMU
pointer from the dev, so the attr group can be shared).
I think we can share the logic, the cpumask attr, and the cpumask
attr_group in drivers/perf/arm_pmu.c, exposing a common
arm_pmu_cpumask_attr_group pointer via the arm_pmu header.
Then all each driver needs to wire up is a single pointer in its
attr_groups array, which isn't so bad.
Thanks,
Mark.
From: Will Deacon <hidden> Date: 2016-07-11 16:14:02
On Mon, Jul 11, 2016 at 04:58:35PM +0100, Mark Rutland wrote:
On Mon, Jul 11, 2016 at 10:05:39AM -0500, Jeremy Linton wrote:
quoted
On 07/07/2016 11:21 AM, Mark Rutland wrote:
quoted
Have you tested trying to stat on a particular PMU? e.g.
$ perf stat -e armv8_cortex_a53/cpu_cycles/ ls
I found that the presence of a cpumask file would cause (at least some
versions) of perf-stat to hang, and was holding off adding a cpumask
until we had a solution to that.
I guess that is more proof that any tiny change can break things..
Another "fix" is to make the cpumap_print_to_pagebuf's first
parameter false which apparently keeps perf from understanding the
cpu mask and forces the user to use numactl, which is ugly because
numactrl doesn't understand the mask in that format either.
If we have a cpumask, it should be in keeping with the usual style.
quoted
I guess fixing perf is probably the best bet here...
Indeed.
The major issue is that it's not clear to me how to avoid breaking
existing binaries when adding the cpumask. I suspect that we have to
expose it under a different name. :/
Well, we need to understand exactly why the cpumask breaks those older
perf builds before we do that. Is it expecting some behaviour that we
don't honour, or does it break on other architectures providing a
cpumask too?
quoted
quoted
See [1,2] for more details on that.
That makes it sound specific to big/little. Did perf used to work on
those systems without a cpumask? I understand that it might not have
hung, but did it actually provide meaningful data?
I'm not against renaming the cpumask if that's our only option, but I'd
like to understand (and document) how we arrive at that, if we actually
do.
Will