pmu: armv7_a9_pmu_init() fails with -ENXIO

pmu: armv7_a9_pmu_init() fails with -ENXIO

[Mason]

Hello,

On my (dual-core) system, armv7_a9_pmu_init() fails with -ENXIO
(I'm running v4.2)

[    0.090148] cpu=4 nr_cpu_ids=2
[    0.090164] armv7_a9_pmu_init: ret=-6
[    0.090171] hw perfevents: failed to probe PMU!
[    0.090175] hw perfevents: failed to register PMU devices!

armv7_a9_pmu_init() eventually calls generic_exec_single()
which fails this test:

	if ((unsigned)cpu >= nr_cpu_ids || !cpu_online(cpu))

cpu = 4 looks fishy, doesn't it?
<grasping@straws> I'm wondering if commit 0e3038d18adce or
commit cc88116da0d18 might be related at all?

The value 4 comes from smp_call_function_any()

  /* Any online will do: smp_call_function_single handles nr_cpu_ids. */
  cpu = cpumask_any_and(mask, cpu_online_mask);

mask = &arm_pmu->supported_cpus
p arm_pmu->supported_cpus
$3 = {bits = {0}}

/**
 * cpumask_next_and - get the next cpu in *src1p & *src2p
 * @n: the cpu prior to the place to search (ie. return will be > @n)
 * @src1p: the first cpumask pointer
 * @src2p: the second cpumask pointer
 *
 * Returns >= nr_cpu_ids if no further cpus set in both.
 */

Shouldn't cpu_pmu->supported_cpus be cpu_present_mask or cpu_possible_mask?
Shouldn't armv7pmu_init() set arm_pmu->supported_cpus?

Regards.

pmu: armv7_a9_pmu_init() fails with -ENXIO

[Mason]

On 21/10/2015 15:01, Mason wrote:

> On my (dual-core) system, armv7_a9_pmu_init() fails with -ENXIO
> (I'm running v4.2)
> 
> [    0.090148] cpu=4 nr_cpu_ids=2
> [    0.090164] armv7_a9_pmu_init: ret=-6
> [    0.090171] hw perfevents: failed to probe PMU!
> [    0.090175] hw perfevents: failed to register PMU devices!
> 
> armv7_a9_pmu_init() eventually calls generic_exec_single()
> which fails this test:
> 
> 	if ((unsigned)cpu >= nr_cpu_ids || !cpu_online(cpu))
> 
> cpu = 4 looks fishy, doesn't it?
> <grasping@straws> I'm wondering if commit 0e3038d18adce or
> commit cc88116da0d18 might be related at all?
> 
> The value 4 comes from smp_call_function_any()
> 
>   /* Any online will do: smp_call_function_single handles nr_cpu_ids. */
>   cpu = cpumask_any_and(mask, cpu_online_mask);
> 
> mask = &arm_pmu->supported_cpus
> p arm_pmu->supported_cpus
> $3 = {bits = {0}}
> 
> /**
>  * cpumask_next_and - get the next cpu in *src1p & *src2p
>  * @n: the cpu prior to the place to search (ie. return will be > @n)
>  * @src1p: the first cpumask pointer
>  * @src2p: the second cpumask pointer
>  *
>  * Returns >= nr_cpu_ids if no further cpus set in both.
>  */
> 
> Shouldn't cpu_pmu->supported_cpus be cpu_present_mask or cpu_possible_mask?
> Shouldn't armv7pmu_init() set arm_pmu->supported_cpus?

Looking more closely at Documentation/devicetree/bindings/arm/pmu.txt
I now see commit 71bbf038eaa44

- interrupt-affinity : Valid only when using SPIs, specifies a list of phandles
                       to CPU nodes corresponding directly to the affinity of
		       the SPIs listed in the interrupts property.

		       This property should be present when there is more than
		       a single SPI.

I currently have

	pmu {
		compatible = "arm,cortex-a9-pmu";
		interrupts =
			<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>,
			<GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
	};

<confused> I wrote the above node by sampling other platforms,
and they don't define any interrupt-affinity property, e.g.

exynos4415.dtsi:

		pmu {
			compatible = "arm,cortex-a9-pmu";
			interrupts = <0 18 0>, <0 19 0>, <0 20 0>, <0 21 0>;
		};

exynos5440.dtsi:

	arm-pmu {
		compatible = "arm,cortex-a15-pmu", "arm,cortex-a9-pmu";
		interrupts = <0 52 4>,
			     <0 53 4>,
			     <0 54 4>,
			     <0 55 4>;
	};

highbank.dts:

		pmu {
			compatible = "arm,cortex-a9-pmu";
			interrupts = <0 76 4  0 75 4  0 74 4  0 73 4>;
		};

omap4460.dtsi:

	pmu {
		compatible = "arm,cortex-a9-pmu";
		interrupts = <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
			     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
		ti,hwmods = "debugss";
	};


I also looked more closely at of_pmu_irq_cfg()

platform_get_irq(pdev, 0); returns -6
pdev->num_resources is 0 therefore...

	if (i == pdev->num_resources) {
		pmu->irq_affinity = irqs;
	} else {
		kfree(irqs);
		cpumask_setall(&pmu->supported_cpus);
	}

pmu->irq_affinity gets set to an invalid pointer (0x00000010)
and cpumask_setall() is not called.

Something is not working as expected, right?

Regards.

pmu: armv7_a9_pmu_init() fails with -ENXIO

[Sudeep Holla]

On 21/10/15 14:46, Mason wrote:

[...]

>
> I also looked more closely at of_pmu_irq_cfg()
>
> platform_get_irq(pdev, 0); returns -6

Won't it return here ? Though you must first check why is that returning
error.

         irq = platform_get_irq(pdev, 0);
         if (irq >= 0 && irq_is_percpu(irq))
                 return 0;

> pdev->num_resources is 0 therefore...
>
> 	if (i == pdev->num_resources) {
> 		pmu->irq_affinity = irqs;
> 	} else {
> 		kfree(irqs);
> 		cpumask_setall(&pmu->supported_cpus);
> 	}
>
> pmu->irq_affinity gets set to an invalid pointer (0x00000010)
> and cpumask_setall() is not called.
>
> Something is not working as expected, right?
>

So you won't execute this in that case.

Sorry if I missed something, I briefly checked v4.2
Code have been changed and even moved since then.

-- 
Regards,
Sudeep

pmu: armv7_a9_pmu_init() fails with -ENXIO

[Mason]

On 21/10/2015 16:27, Sudeep Holla wrote:

> On 21/10/15 14:46, Mason wrote:
> 
>> I also looked more closely at of_pmu_irq_cfg()
>>
>> platform_get_irq(pdev, 0); returns -6
> 
> Won't it return here ?
> Though you must first check why is that returning error.
> 
>          irq = platform_get_irq(pdev, 0);
>          if (irq >= 0 && irq_is_percpu(irq))
>                  return 0;

Thanks for prodding me in the right direction.
platform_get_irq() was failing because I hadn't properly defined
the node's interrupt-parent...

I've added
  interrupt-parent = <&gic>;
at the root of my DT, and things now work as expected.
(Haven't tested actually using the counters yet.)

[    0.090058] irq=212 irqs=e7603400 pdev->num_resources=2
[    0.090091] hw perfevents: Failed to parse /pmu/interrupt-affinity[0]
[    0.090103] armv7_a9_pmu_init: ret=0
[    0.090145] hw perfevents: enabled with armv7_cortex_a9 PMU driver, 7 counters available

Thanks again.

Regards.

pmu: armv7_a9_pmu_init() fails with -ENXIO

[Sudeep Holla]

On 21/10/15 16:44, Mason wrote:
> On 21/10/2015 16:27, Sudeep Holla wrote:
>
>> On 21/10/15 14:46, Mason wrote:
>>
>>> I also looked more closely at of_pmu_irq_cfg()
>>>
>>> platform_get_irq(pdev, 0); returns -6
>>
>> Won't it return here ?
>> Though you must first check why is that returning error.
>>
>>           irq = platform_get_irq(pdev, 0);
>>           if (irq >= 0 && irq_is_percpu(irq))
>>                   return 0;
>
> Thanks for prodding me in the right direction.
> platform_get_irq() was failing because I hadn't properly defined
> the node's interrupt-parent...
>

Yes that's what I guessed seeing of_irq_get.

> I've added
>    interrupt-parent = <&gic>;
> at the root of my DT, and things now work as expected.
> (Haven't tested actually using the counters yet.)
>
> [    0.090058] irq=212 irqs=e7603400 pdev->num_resources=2
> [    0.090091] hw perfevents: Failed to parse /pmu/interrupt-affinity[0]

Better to add this property. It assigns the cpu affinity in logical
order which might break if your logical and physical cpu ordering
differs.(e.g. you boot on CPU#1 instead of CPU#0)

> [    0.090103] armv7_a9_pmu_init: ret=0
> [    0.090145] hw perfevents: enabled with armv7_cortex_a9 PMU driver, 7 counters available
>

Cool

-- 
Regards,
Sudeep