Re: [PATCH v2] perf vendor events arm64: Revise core JSON events for eMAG

[William Cohen <wcohen@redhat.com>]

On 9/9/18 8:26 PM, Sean V Kelley wrote:
> Split the PMU events into meaningful functional groups.  Update core
> pmu events based on supported ARMv8 recommended IMPLEMENTATION DEFINED
> events.
> 
> The JSON files are updated with reference to a PMU table shared here:
> 
> https://github.com/AmpereComputing/ampere-centos-kernel/blob/amp-centos-7.5-kernel/Documentation/arm64/eMAG-ARM-CoreImpDefined.pdf
> 
> --
> Changes in V2:
> - Provided documentation for changes - John, William
> - Broke up into meaningful groups - William
> --
> 
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: William Cohen <wcohen@redhat.com>
> Cc: John Garry <john.garry@huawei.com>
> Cc: linux-arm-kernel@lists.infradead.org
> 
> Signed-off-by: Sean V Kelley <seanvk.dev@oregontracks.org>
> ---
>  .../arch/arm64/ampere/emag/branch.json        |  23 +++
>  .../arch/arm64/ampere/emag/bus.json           |  26 +++
>  .../arch/arm64/ampere/emag/cache.json         | 191 ++++++++++++++++++
>  .../arch/arm64/ampere/emag/clock.json         |  20 ++
>  .../arch/arm64/ampere/emag/core-imp-def.json  |  32 ---
>  .../arch/arm64/ampere/emag/counter.json       |   8 +
>  .../arch/arm64/ampere/emag/exception.json     |  50 +++++
>  .../arch/arm64/ampere/emag/instruction.json   |  89 ++++++++
>  .../arch/arm64/ampere/emag/intrinsic.json     |  14 ++
>  .../arch/arm64/ampere/emag/memory.json        |  29 +++
>  .../arch/arm64/ampere/emag/pipeline.json      |  50 +++++
>  11 files changed, 500 insertions(+), 32 deletions(-)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/branch.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/bus.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/cache.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/clock.json
>  delete mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/core-imp-def.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/counter.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/exception.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/instruction.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/intrinsic.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/memory.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/ampere/emag/pipeline.json

Hi,

The patch looks okay for most part. It would be good to factor out the events 0x00-0x38 from "Table D5-8 PMU common architectural and microarchitectural event numbers" in https://static.docs.arm.com/ddi0487/ca/DDI0487C_a_armv8_arm.pdf and have them in armv8-recommended.json so events like BR_MIS_PRED and BR_PRED (and anything else in the range of 0x000-0x038) just end up be ArchStdEvent like the other predefined events.  Maybe do the factoring out as a follow on patch to this patch.

The following counter.json wouldn't be that useful.  Perf doesn't allow layout of what goes in which PMU register. It should be removed:

> diff --git a/tools/perf/pmu-events/arch/arm64/ampere/emag/counter.json b/tools/perf/pmu-events/arch/arm64/ampere/emag/counter.json
> new file mode 100644
> index 000000000000..6561ed58f13f
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/ampere/emag/counter.json
> @@ -0,0 +1,8 @@
> +[
> +    {
> +        "PublicDescription": "For odd-numbered counters, increments the count by one for each overflow of the preceding even-numbered counter. For even-numbered counters there is no increment.",
> +        "EventCode": "0x1e",
> +        "EventName": "CHAIN",
> +        "BriefDescription": "Chain counter"
> +    },
> +]



-Will Cohen