[Skiboot] [PATCH v8 0/7] OPAL support for Nest instrumentation

[Madhavan Srinivasan]

On Wednesday 16 March 2016 11:06 AM, Michael Neuling wrote:
> Maddy,
>
>>                 chip at 7fdb20000 {
>>                         ibm,chip-id = <0x0>;
>>                         ranges = <0x0 0x7 0xfdb20000 0x30000>;
> Should this be a reg property?
>
>
>>                         #address-cells = <0x1>;
>>                         phandle = <0x100001e1>;
>>                         #size-cells = <0x1>;
>>                         linux,phandle = <0x100001e1>;
>>
>>                         mcs {
>>                                 compatible = "ibm,nest-counters
>> -chip";
>>                                 ranges;
>>                                 #address-cells = <0x1>;
>>                                 phandle = <0x100001e2>;
>>                                 #size-cells = <0x1>;
>>                                 linux,phandle = <0x100001e2>;
>>
>>                                 write_mc at 38 {
>>                                         id = <0x0>;
>>                                         reg = <0x38 0x8>;
>>                                         unit = "MiB";
>>                                         scale = "1.2207e-4";
> As I mentioned elsewhere, I'm not keen on these bindings.

Mikey,

Considering the MCS unit, the raw counter value (say X)
may not be of much use. That said, mcs counter give
us no. of 128byte blocks of reads or writes. Now, user level
tool must know how to convert the raw value into some
metric like GiB or MiB. Hence, have added the "scale"
and "unit" binding for the events. With this, kernel
pmu driver creates scale and unit files in the sysfs.

x86 also does something similiar, meaning consider the commit

 433678bdc6ed3 perf/rapl: Fix sysfs_show() initialization for RAPL PMU

Where they have hardcoded "scaling" and "unit" factor to
get a metric out of the counter data from their
Uncore RAPL PMU unit.

Hardcoding these values in the kernel PMU driver may work,
but, then having the unit and scale in device tree will
allow the kernel drive to be independent of the microcode
and platform firmware versions. Kernel PMU driver will be
clean and extensible for new nest units in P8 and
P9 generations.

> 1.2204e-4 seems to just be 1/8192.  MiB is just a number also.  I think
> you are combining these to give a final scale number which in this case
> (I think is just 128).
>
>   1.2204e-4 * 2**20 = 127.96821504
>
> I'd prefer you just put that in the device tree.

Now, for the scaling factor value, if we have "128"
then the unit has to be in "Bytes". Since, these
memory bandwidth counters are per chip level,
preferred representing them in "MiB" instead of "Bytes".

In that case scaling factor value comes out as

 (128/(1024x1024))  = "1.2207e-4"

Regardring the "reg" property question,

IIUC, these devices counters are memory mapped on the
CPU domain, having these as "ranges" is right and
individual event offsets are represented as "regs".

Maddy

> Mikey