[PATCH v2 0/5] Add JSON metrics for arm CMN and Yitian710 DDR

[PATCH v2 0/5] Add JSON metrics for arm CMN and Yitian710 DDR

[Jing Zhang]

Changes since RFC:
- Refact arm-cmn PMU identifier.
- Not add arm-cmn PMU aliasing currently because it's Eventcode is
  difficult to define.
- Rename ali_drw PMU identifier and Unit name.
- Divide ali_drw PMU metric and aliasing into two patches.

Add an identifier sysfs file for the yitian710 SoC DDR and arm CMN to
allow userspace to identify the specific implementation of the device,
so that the perf tool can match the corresponding uncore events and
metrics through the identifier. Then added several general CMN700 metrics
and yitian710 soc DDR metrics.

$perf list:
...
ali_drw:
  hif_rmw
       [A Read-Modify-Write Op at HIF interface. 64B. Unit: ali_drw]
  hif_hi_pri_rd
       [A high priority Read at HIF interface. 64B. Unit: ali_drw]
  hif_rd
       [A Read Op at HIF interface. 64B. Unit: ali_drw]
  hif_rd_or_wr
       [A Write or Read Op at HIF interface. 64B. Unit: ali_drw]
  hif_wr
       [A Write Op at HIF interface. 64B. Unit: ali_drw]
...

$perf stat -M ddr_read_bandwidth.all ./test

Performance counter stats for 'system wide':

            38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
     1,000,957,941 ns   duration_time

       1.000957941 seconds time elapsed

Jing Zhang (5):
  driver/perf: Add identifier sysfs file for CMN
  perf vendor events: Add JSON metrics for CMN-700
  driver/perf: Add identifier sysfs file for Yitian 710 DDR
  perf jevents: Add support for Yitian 710 DDR PMU aliasing
  perf vendor events: Add JSON metrics for Yitian 710 DDR

 drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
 drivers/perf/arm-cmn.c                             |  49 +++
 .../arch/arm64/arm/cmn700/sys/metrics.json         |  74 ++++
 .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
 .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
 tools/perf/pmu-events/jevents.py                   |   2 +
 6 files changed, 545 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json

-- 
1.8.3.1

[PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[Jing Zhang]

To allow userspace to identify the specific implementation of the device,
add an "identifier" sysfs file.

The perf tool can match the arm CMN metric through the identifier.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 drivers/perf/arm-cmn.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
index c968986..f425610 100644
--- a/drivers/perf/arm-cmn.c
+++ b/drivers/perf/arm-cmn.c
@@ -334,6 +334,7 @@ struct arm_cmn {
 
 	struct pmu pmu;
 	struct dentry *debug;
+	const char *identifier;
 };
 
 #define to_cmn(p)	container_of(p, struct arm_cmn, pmu)
@@ -1168,10 +1169,41 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
 	.attrs = arm_cmn_cpumask_attrs,
 };
 
+static ssize_t arm_cmn_identifier_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
+
+	return sysfs_emit(buf, "%s\n", cmn->identifier);
+}
+
+static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
+		struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
+
+	if (cmn->identifier == NULL)
+		return 0;
+	return attr->mode;
+};
+
+static struct device_attribute arm_cmn_identifier_attr =
+__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
+
+static struct attribute *arm_cmn_identifier_attrs[] = {
+	&arm_cmn_identifier_attr.attr,
+	NULL
+};
+
+static struct attribute_group arm_cmn_identifier_attr_group = {
+	.attrs = arm_cmn_identifier_attrs,
+	.is_visible = arm_cmn_identifier_attr_visible
+};
+
 static const struct attribute_group *arm_cmn_attr_groups[] = {
 	&arm_cmn_event_attrs_group,
 	&arm_cmn_format_attrs_group,
 	&arm_cmn_cpumask_attr_group,
+	&arm_cmn_identifier_attr_group,
 	NULL
 };
 
@@ -2241,6 +2273,22 @@ static int arm_cmn600_of_probe(struct device_node *np)
 	return of_property_read_u32(np, "arm,root-node", &rootnode) ?: rootnode;
 }
 
+const char *arm_cmn_identifier(unsigned long model)
+{
+	switch (model) {
+	case CMN600:
+		return "cmn600";
+	case CMN650:
+		return "cmn650";
+	case CMN700:
+		return "cmn700";
+	case CI700:
+		return "ci700";
+	default:
+		return NULL;
+	}
+}
+
 static int arm_cmn_probe(struct platform_device *pdev)
 {
 	struct arm_cmn *cmn;
@@ -2254,6 +2302,7 @@ static int arm_cmn_probe(struct platform_device *pdev)
 
 	cmn->dev = &pdev->dev;
 	cmn->model = (unsigned long)device_get_match_data(cmn->dev);
+	cmn->identifier = arm_cmn_identifier(cmn->model);
 	platform_set_drvdata(pdev, cmn);
 
 	if (cmn->model == CMN600 && has_acpi_companion(cmn->dev)) {
-- 
1.8.3.1

Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[John Garry]

On 24/04/2023 10:44, Jing Zhang wrote:
> To allow userspace to identify the specific implementation of the device,
> add an "identifier" sysfs file.
> 
> The perf tool can match the arm CMN metric through the identifier.
> 
> Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com>
> ---
>   drivers/perf/arm-cmn.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index c968986..f425610 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c
> @@ -334,6 +334,7 @@ struct arm_cmn {
>   
>   	struct pmu pmu;
>   	struct dentry *debug;
> +	const char *identifier;
>   };
>   
>   #define to_cmn(p)	container_of(p, struct arm_cmn, pmu)
> @@ -1168,10 +1169,41 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
>   	.attrs = arm_cmn_cpumask_attrs,
>   };
>   
> +static ssize_t arm_cmn_identifier_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> +
> +	return sysfs_emit(buf, "%s\n", cmn->identifier);
> +}
> +
> +static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
> +		struct attribute *attr, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> +
> +	if (cmn->identifier == NULL)
> +		return 0;

nit: generally if (!val) is preferred

> +	return attr->mode;
> +};
> +
> +static struct device_attribute arm_cmn_identifier_attr =
> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
> +
> +static struct attribute *arm_cmn_identifier_attrs[] = {
> +	&arm_cmn_identifier_attr.attr,
> +	NULL
> +};
> +
> +static struct attribute_group arm_cmn_identifier_attr_group = {
> +	.attrs = arm_cmn_identifier_attrs,
> +	.is_visible = arm_cmn_identifier_attr_visible
> +};
> +
>   static const struct attribute_group *arm_cmn_attr_groups[] = {
>   	&arm_cmn_event_attrs_group,
>   	&arm_cmn_format_attrs_group,
>   	&arm_cmn_cpumask_attr_group,
> +	&arm_cmn_identifier_attr_group,
>   	NULL
>   };
>   
> @@ -2241,6 +2273,22 @@ static int arm_cmn600_of_probe(struct device_node *np)
>   	return of_property_read_u32(np, "arm,root-node", &rootnode) ?: rootnode;
>   }
>   
> +const char *arm_cmn_identifier(unsigned long model)
> +{
> +	switch (model) {
> +	case CMN600:
> +		return "cmn600";
> +	case CMN650:
> +		return "cmn650";
> +	case CMN700:
> +		return "cmn700";
> +	case CI700:
> +		return "ci700";
> +	default:
> +		return NULL;
> +	}

nit: I think that it would be nicer to have this per-model string stored 
statically in arm_cmn_acpi_match[].driver_data and 
arm_cmn_of_match[].data, so we have a straight lookup

> +}
> +
>   static int arm_cmn_probe(struct platform_device *pdev)
>   {
>   	struct arm_cmn *cmn;
> @@ -2254,6 +2302,7 @@ static int arm_cmn_probe(struct platform_device *pdev)
>   
>   	cmn->dev = &pdev->dev;
>   	cmn->model = (unsigned long)device_get_match_data(cmn->dev);
> +	cmn->identifier = arm_cmn_identifier(cmn->model);
>   	platform_set_drvdata(pdev, cmn);
>   

Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[Robin Murphy]

On 2023-05-01 13:38, John Garry wrote:
> On 24/04/2023 10:44, Jing Zhang wrote:
>> To allow userspace to identify the specific implementation of the device,
>> add an "identifier" sysfs file.
>>
>> The perf tool can match the arm CMN metric through the identifier.
>>
>> Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com>
>> ---
>>   drivers/perf/arm-cmn.c | 49 
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index c968986..f425610 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c
>> @@ -334,6 +334,7 @@ struct arm_cmn {
>>       struct pmu pmu;
>>       struct dentry *debug;
>> +    const char *identifier;
>>   };
>>   #define to_cmn(p)    container_of(p, struct arm_cmn, pmu)
>> @@ -1168,10 +1169,41 @@ static ssize_t arm_cmn_cpumask_show(struct 
>> device *dev,
>>       .attrs = arm_cmn_cpumask_attrs,
>>   };
>> +static ssize_t arm_cmn_identifier_show(struct device *dev,
>> +        struct device_attribute *attr, char *buf)
>> +{
>> +    struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>> +
>> +    return sysfs_emit(buf, "%s\n", cmn->identifier);
>> +}
>> +
>> +static umode_t arm_cmn_identifier_attr_visible(struct kobject *kobj,
>> +        struct attribute *attr, int n)
>> +{
>> +    struct device *dev = kobj_to_dev(kobj);
>> +    struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
>> +
>> +    if (cmn->identifier == NULL)
>> +        return 0;
> 
> nit: generally if (!val) is preferred

Although either way it can only be NULL in cases of memory corruption or 
developers making broken changes to the driver, neither of which are 
worth pretending to defend against.

>> +    return attr->mode;
>> +};
>> +
>> +static struct device_attribute arm_cmn_identifier_attr =
>> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
>> +
>> +static struct attribute *arm_cmn_identifier_attrs[] = {
>> +    &arm_cmn_identifier_attr.attr,
>> +    NULL
>> +};
>> +
>> +static struct attribute_group arm_cmn_identifier_attr_group = {
>> +    .attrs = arm_cmn_identifier_attrs,
>> +    .is_visible = arm_cmn_identifier_attr_visible
>> +};
>> +
>>   static const struct attribute_group *arm_cmn_attr_groups[] = {
>>       &arm_cmn_event_attrs_group,
>>       &arm_cmn_format_attrs_group,
>>       &arm_cmn_cpumask_attr_group,
>> +    &arm_cmn_identifier_attr_group,
>>       NULL
>>   };
>> @@ -2241,6 +2273,22 @@ static int arm_cmn600_of_probe(struct 
>> device_node *np)
>>       return of_property_read_u32(np, "arm,root-node", &rootnode) ?: 
>> rootnode;
>>   }
>> +const char *arm_cmn_identifier(unsigned long model)
>> +{
>> +    switch (model) {
>> +    case CMN600:
>> +        return "cmn600";
>> +    case CMN650:
>> +        return "cmn650";
>> +    case CMN700:
>> +        return "cmn700";
>> +    case CI700:
>> +        return "ci700";
>> +    default:
>> +        return NULL;
>> +    }
> 
> nit: I think that it would be nicer to have this per-model string stored 
> statically in arm_cmn_acpi_match[].driver_data and 
> arm_cmn_of_match[].data, so we have a straight lookup

Again, I'm not really convinced how useful this coarse per-model scheme 
is - for instance, in terms of many events, CMN-600 r3 is closer to 
CMN-650 than it is to CMN-600 r1, so what exactly would "CMN-600" mean 
to the user?

Thanks,
Robin.

>> +}
>> +
>>   static int arm_cmn_probe(struct platform_device *pdev)
>>   {
>>       struct arm_cmn *cmn;
>> @@ -2254,6 +2302,7 @@ static int arm_cmn_probe(struct platform_device 
>> *pdev)
>>       cmn->dev = &pdev->dev;
>>       cmn->model = (unsigned long)device_get_match_data(cmn->dev);
>> +    cmn->identifier = arm_cmn_identifier(cmn->model);
>>       platform_set_drvdata(pdev, cmn);
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[John Garry]

On 04/05/2023 10:47, Robin Murphy wrote:
>> nit: generally if (!val) is preferred

Hi Robin,

> 
> Although either way it can only be NULL in cases of memory corruption or 
> developers making broken changes to the driver, neither of which are 
> worth pretending to defend against.

If there was some broken code for setting this identifier, then we would 
just not show the identifier file, rather than show it containing "NULL" 
- that seems better. However, there may be other implications from that 
same broken code, so you maintainers and contributors please decide.

> 
>>> +    return attr->mode;
>>> +};
>>> +
>>> +static struct device_attribute arm_cmn_identifier_attr =
>>> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
>>> +
>>> +static struct attribute *arm_cmn_identifier_attrs[] = {
>>> +    &arm_cmn_identifier_attr.attr,
>>> +    NULL
>>> +};
>>> +
>>> +static struct attribute_group arm_cmn_identifier_attr_group = {
>>> +    .attrs = arm_cmn_identifier_attrs,
>>> +    .is_visible = arm_cmn_identifier_attr_visible
>>> +};
>>> +
>>>   static const struct attribute_group *arm_cmn_attr_groups[] = {
>>>       &arm_cmn_event_attrs_group,
>>>       &arm_cmn_format_attrs_group,
>>>       &arm_cmn_cpumask_attr_group,
>>> +    &arm_cmn_identifier_attr_group,
>>>       NULL
>>>   };
>>> @@ -2241,6 +2273,22 @@ static int arm_cmn600_of_probe(struct 
>>> device_node *np)
>>>       return of_property_read_u32(np, "arm,root-node", &rootnode) ?: 
>>> rootnode;
>>>   }
>>> +const char *arm_cmn_identifier(unsigned long model)
>>> +{
>>> +    switch (model) {
>>> +    case CMN600:
>>> +        return "cmn600";
>>> +    case CMN650:
>>> +        return "cmn650";
>>> +    case CMN700:
>>> +        return "cmn700";
>>> +    case CI700:
>>> +        return "ci700";
>>> +    default:
>>> +        return NULL;
>>> +    }
>>
>> nit: I think that it would be nicer to have this per-model string 
>> stored statically in arm_cmn_acpi_match[].driver_data and 
>> arm_cmn_of_match[].data, so we have a straight lookup
> 
> Again, I'm not really convinced how useful this coarse per-model scheme 
> is - for instance, in terms of many events, CMN-600 r3 is closer to 
> CMN-650 than it is to CMN-600 r1, so what exactly would "CMN-600" mean 
> to the user?

ok, I see, that's what I was asking about in the cmn-700 JSON review; 
and from what you say, it is not the case that we always have the same 
events for every revision. So we need a more fine-grained identifier.

For DT support, I suppose per-revision compat strings could be added, 
but I would not be sure what to do about ACPI.

BTW, My comment was more about coding style of case a, case b, case c, 
... case z, does not scale well.

Thanks,
John

Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[Robin Murphy]

On 2023-05-04 12:02, John Garry wrote:
> On 04/05/2023 10:47, Robin Murphy wrote:
>>> nit: generally if (!val) is preferred
> 
> Hi Robin,
> 
>>
>> Although either way it can only be NULL in cases of memory corruption 
>> or developers making broken changes to the driver, neither of which 
>> are worth pretending to defend against.
> 
> If there was some broken code for setting this identifier, then we would 
> just not show the identifier file, rather than show it containing "NULL" 
> - that seems better. However, there may be other implications from that 
> same broken code, so you maintainers and contributors please decide.

Yeah, from the usage point of view, if there should be an identifier at 
all then there should always be one, so it makes little sense to pretend 
to accommodate a case where there wouldn't be. And it would be trivially 
obvious to see in review if someone adds a new model enum without any 
necessary identifier updates at the same time (far more so than spotting 
whether all the subtle functional differences of the new model have been 
accounted for).

>>>> +    return attr->mode;
>>>> +};
>>>> +
>>>> +static struct device_attribute arm_cmn_identifier_attr =
>>>> +__ATTR(identifier, 0444, arm_cmn_identifier_show, NULL);
>>>> +
>>>> +static struct attribute *arm_cmn_identifier_attrs[] = {
>>>> +    &arm_cmn_identifier_attr.attr,
>>>> +    NULL
>>>> +};
>>>> +
>>>> +static struct attribute_group arm_cmn_identifier_attr_group = {
>>>> +    .attrs = arm_cmn_identifier_attrs,
>>>> +    .is_visible = arm_cmn_identifier_attr_visible
>>>> +};
>>>> +
>>>>   static const struct attribute_group *arm_cmn_attr_groups[] = {
>>>>       &arm_cmn_event_attrs_group,
>>>>       &arm_cmn_format_attrs_group,
>>>>       &arm_cmn_cpumask_attr_group,
>>>> +    &arm_cmn_identifier_attr_group,
>>>>       NULL
>>>>   };
>>>> @@ -2241,6 +2273,22 @@ static int arm_cmn600_of_probe(struct 
>>>> device_node *np)
>>>>       return of_property_read_u32(np, "arm,root-node", &rootnode) ?: 
>>>> rootnode;
>>>>   }
>>>> +const char *arm_cmn_identifier(unsigned long model)
>>>> +{
>>>> +    switch (model) {
>>>> +    case CMN600:
>>>> +        return "cmn600";
>>>> +    case CMN650:
>>>> +        return "cmn650";
>>>> +    case CMN700:
>>>> +        return "cmn700";
>>>> +    case CI700:
>>>> +        return "ci700";
>>>> +    default:
>>>> +        return NULL;
>>>> +    }
>>>
>>> nit: I think that it would be nicer to have this per-model string 
>>> stored statically in arm_cmn_acpi_match[].driver_data and 
>>> arm_cmn_of_match[].data, so we have a straight lookup
>>
>> Again, I'm not really convinced how useful this coarse per-model 
>> scheme is - for instance, in terms of many events, CMN-600 r3 is 
>> closer to CMN-650 than it is to CMN-600 r1, so what exactly would 
>> "CMN-600" mean to the user?
> 
> ok, I see, that's what I was asking about in the cmn-700 JSON review; 
> and from what you say, it is not the case that we always have the same 
> events for every revision. So we need a more fine-grained identifier.

Yes, it's mostly just a case of new events getting added as the 
microarchitecture evolves over the product's lifetime, but there has 
been at least one event ID which had a meaning in very early versions of 
CMN-600, was subsequently removed, and then got reused for a *different* 
event a couple of revisions after that. Thankfully I believe those 
earliest versions only ever existed on FPGA internally, and the TRMs 
were never made public, so upstream doesn't care about that specific case.

> For DT support, I suppose per-revision compat strings could be added, 
> but I would not be sure what to do about ACPI.

We know the version from the ID registers, that's no problem - it's 
already used to manage visibility of the sysfs event aliases. In 
principle we *should* have a model code in CFGM_PERIPH_ID_0 as well, and 
be able to compose an identifier exactly the same way as 
smmu_pmu_get_iidr() does in the SMMUv3 PMCG driver, but as I mentioned 
before I'm not entirely confident in the implementation: I do happen to 
know what codes have been nominally assigned for each product, but the 
TRMs claim otherwise :(

> BTW, My comment was more about coding style of case a, case b, case c, 
> ... case z, does not scale well.

Indeed, it's probably the nature of the switch statement that leads to 
the perceived need for a not-practically-meaningful "default" case in 
the first place.

Thanks,
Robin.

Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[John Garry]

On 04/05/2023 13:15, Robin Murphy wrote:
>>
>> ok, I see, that's what I was asking about in the cmn-700 JSON review; 
>> and from what you say, it is not the case that we always have the same 
>> events for every revision. So we need a more fine-grained identifier.
> 
> Yes, it's mostly just a case of new events getting added as the 
> microarchitecture evolves over the product's lifetime,

I might have mentioned this before: Currently userspace matches both the 
PMU sysfs name and this HW identifier for creating these event aliases. 
For fixed events - like those non-IMPDEF ones on the PMCG - userspace 
could just assume those standard ones are implemented and not need to 
check the HW identifier. Maybe this will help with scalability of JSONs 
for CMN PMU implementations. Or even HW identifier wildcard matching. 
However, as you might anticipate, we may need to add some sort of 
userspace support for times when HW implementations get messed up...

> but there has 
> been at least one event ID which had a meaning in very early versions of 
> CMN-600, was subsequently removed, and then got reused for a *different* 
> event a couple of revisions after that.
> Thankfully I believe those 
> earliest versions only ever existed on FPGA internally, and the TRMs 
> were never made public, so upstream doesn't care about that specific case.

ok, good

> 
>> For DT support, I suppose per-revision compat strings could be added, 
>> but I would not be sure what to do about ACPI.
> 
> We know the version from the ID registers, that's no problem - it's 
> already used to manage visibility of the sysfs event aliases. In 
> principle we *should* have a model code in CFGM_PERIPH_ID_0 as well, and 
> be able to compose an identifier exactly the same way as 
> smmu_pmu_get_iidr() does in the SMMUv3 PMCG driver, but as I mentioned 
> before I'm not entirely confident in the implementation: I do happen to 
> know what codes have been nominally assigned for each product, but the 
> TRMs claim otherwise 🙁

Thanks,
John

Re: [PATCH v2 1/5] driver/perf: Add identifier sysfs file for CMN

[Jing Zhang]

在 2023/5/5 下午8:24, John Garry 写道:
> On 04/05/2023 13:15, Robin Murphy wrote:
>>>
>>> ok, I see, that's what I was asking about in the cmn-700 JSON review; and from what you say, it is not the case that we always have the same events for every revision. So we need a more fine-grained identifier.
>>
>> Yes, it's mostly just a case of new events getting added as the microarchitecture evolves over the product's lifetime,
> 
> I might have mentioned this before: Currently userspace matches both the PMU sysfs name and this HW identifier for creating these event aliases. For fixed events - like those non-IMPDEF ones on the PMCG - userspace could just assume those standard ones are implemented and not need to check the HW identifier. Maybe this will help with scalability of JSONs for CMN PMU implementations. Or even HW identifier wildcard matching. However, as you might anticipate, we may need to add some sort of userspace support for times when HW implementations get messed up...
> 
>> but there has been at least one event ID which had a meaning in very early versions of CMN-600, was subsequently removed, and then got reused for a *different* event a couple of revisions after that.
>> Thankfully I believe those earliest versions only ever existed on FPGA internally, and the TRMs were never made public, so upstream doesn't care about that specific case.
> 
> ok, good
> 

Thanks John and Robin, perhaps using CMN's model and revision to form an identifier is currently the most feasible solution?
For example: CMN600_R1P0, CMN600_R1P1. The revision of CMN can be obtained from the revision register.

>>
>>> For DT support, I suppose per-revision compat strings could be added, but I would not be sure what to do about ACPI.
>>

Yes, I also have this doubt. On DT, char* can be passed in through the data field because it is of void* type, but the driver_data
field of ACPI is ulong. In addition, the revision is read out through the register, and I cannot get the revision in
arm_cmn_acpi/of_match[]. We still need to compose the identifier in the probe function. So, I wonder if it is better to use an
additional function to form the identifier instead of passing the identifier through arm_cmn_acpi/of_match[]?

>> We know the version from the ID registers, that's no problem - it's already used to manage visibility of the sysfs event aliases. In principle we *should* have a model code in CFGM_PERIPH_ID_0 as well, and be able to compose an identifier exactly the same way as smmu_pmu_get_iidr() does in the SMMUv3 PMCG driver, but as I mentioned before I'm not entirely confident in the implementation: I do happen to know what codes have been nominally assigned for each product, but the TRMs claim otherwise 🙁
> 
> Thanks,
> John


Thanks,
Jing

[PATCH v2 2/5] perf vendor events: Add JSON metrics for CMN-700

[Jing Zhang]

Add JSON metrics for CMN-700. Currently just add part of CMN-700 PMU
metrics which are general and compatible for any SoC.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 .../arch/arm64/arm/cmn700/sys/metrics.json         | 74 ++++++++++++++++++++++
 tools/perf/pmu-events/jevents.py                   |  1 +
 2 files changed, 75 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
new file mode 100644
index 0000000..1577d86
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
@@ -0,0 +1,74 @@
+[
+	{
+		"MetricName": "slc_miss_rate",
+		"BriefDescription": "The system level cache miss rate include.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "hnf_cache_miss / hnf_slc_sf_cache_access",
+		"ScaleUnit": "100%",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "hnf_message_retry_rate",
+		"BriefDescription": "HN-F message retry rate indicates whether a lack of credits is causing the bottlenecks.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "hnf_pocq_retry / hnf_pocq_reqs_recvd",
+		"ScaleUnit": "100%",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "sf_hit_rate",
+		"BriefDescription": "Snoop filter hit rate can be used to measure the Snoop Filter efficiency.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "hnf_sf_hit / hnf_slc_sf_cache_access",
+		"ScaleUnit": "100%",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "mc_message_retry_rate",
+		"BriefDescription": "The memory controller request retries rate indicates whether the memory controller is the bottleneck.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "hnf_mc_retries / hnf_mc_reqs",
+		"ScaleUnit": "100%",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "rni_actual_read_bandwidth.all",
+		"BriefDescription": "This event measure the actual bandwidth(MB/sec) that RN-I bridge sends to the interconnect.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "rnid_rxdat_flits * 32 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "rni_actual_write_bandwidth.all",
+		"BriefDescription": "This event measures the actual write bandwidth(MB/sec) at RN-I bridges.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "rnid_txdat_flits * 32 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "rni_retry_rate",
+		"BriefDescription": "RN-I bridge retry rate indicates whether the memory controller is the bottleneck.",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "rnid_txreq_flits_retried / rnid_txreq_flits_total",
+		"ScaleUnit": "100%",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	},
+	{
+		"MetricName": "sbsx_actual_write_bandwidth.all",
+		"BriefDescription": "sbsx actual write bandwidth(MB/sec).",
+		"MetricGroup": "cmn700",
+		"MetricExpr": "sbsx_txdat_flitv * 32 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "arm_cmn",
+		"Compat": "cmn700"
+	}
+]
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 2bcd07c..7cff2c6 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -256,6 +256,7 @@ class JsonEvent:
           'DFPMC': 'amd_df',
           'cpu_core': 'cpu_core',
           'cpu_atom': 'cpu_atom',
+          'arm_cmn': 'arm_cmn',
       }
       return table[unit] if unit in table else f'uncore_{unit.lower()}'
 
-- 
1.8.3.1

Re: [PATCH v2 2/5] perf vendor events: Add JSON metrics for CMN-700

[John Garry]

On 24/04/2023 10:44, Jing Zhang wrote:
> Add JSON metrics for CMN-700. Currently just add part of CMN-700 PMU
> metrics which are general and compatible for any SoC.
> 
> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> ---
>   .../arch/arm64/arm/cmn700/sys/metrics.json         | 74 ++++++++++++++++++++++
>   tools/perf/pmu-events/jevents.py                   |  1 +
>   2 files changed, 75 insertions(+)
>   create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
> 
> diff --git a/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
> new file mode 100644
> index 0000000..1577d86
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json

I suppose putting these here is ok. Are all these events a must for a 
CMN-700 implementation? Is there potential for impdef event in some 
implementations?

Previously we considered ARM PMCG events would be put in a per-SoC sys 
folder only (and not in perf/pmu-events/arch/arm64/arm)

> @@ -0,0 +1,74 @@
> +[
> +	{
> +		"MetricName": "slc_miss_rate",
> +		"BriefDescription": "The system level cache miss rate include.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "hnf_cache_miss / hnf_slc_sf_cache_access",
> +		"ScaleUnit": "100%",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "hnf_message_retry_rate",
> +		"BriefDescription": "HN-F message retry rate indicates whether a lack of credits is causing the bottlenecks.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "hnf_pocq_retry / hnf_pocq_reqs_recvd",
> +		"ScaleUnit": "100%",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "sf_hit_rate",
> +		"BriefDescription": "Snoop filter hit rate can be used to measure the Snoop Filter efficiency.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "hnf_sf_hit / hnf_slc_sf_cache_access",
> +		"ScaleUnit": "100%",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "mc_message_retry_rate",
> +		"BriefDescription": "The memory controller request retries rate indicates whether the memory controller is the bottleneck.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "hnf_mc_retries / hnf_mc_reqs",
> +		"ScaleUnit": "100%",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "rni_actual_read_bandwidth.all",
> +		"BriefDescription": "This event measure the actual bandwidth(MB/sec) that RN-I bridge sends to the interconnect.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "rnid_rxdat_flits * 32 / 1e6 / duration_time",
> +		"ScaleUnit": "1MB/s",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "rni_actual_write_bandwidth.all",
> +		"BriefDescription": "This event measures the actual write bandwidth(MB/sec) at RN-I bridges.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "rnid_txdat_flits * 32 / 1e6 / duration_time",
> +		"ScaleUnit": "1MB/s",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "rni_retry_rate",
> +		"BriefDescription": "RN-I bridge retry rate indicates whether the memory controller is the bottleneck.",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "rnid_txreq_flits_retried / rnid_txreq_flits_total",
> +		"ScaleUnit": "100%",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	},
> +	{
> +		"MetricName": "sbsx_actual_write_bandwidth.all",
> +		"BriefDescription": "sbsx actual write bandwidth(MB/sec).",
> +		"MetricGroup": "cmn700",
> +		"MetricExpr": "sbsx_txdat_flitv * 32 / 1e6 / duration_time",
> +		"ScaleUnit": "1MB/s",
> +		"Unit": "arm_cmn",
> +		"Compat": "cmn700"
> +	}
> +]
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 2bcd07c..7cff2c6 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -256,6 +256,7 @@ class JsonEvent:
>             'DFPMC': 'amd_df',
>             'cpu_core': 'cpu_core',
>             'cpu_atom': 'cpu_atom',
> +          'arm_cmn': 'arm_cmn',
>         }
>         return table[unit] if unit in table else f'uncore_{unit.lower()}'
>   

Re: [PATCH v2 2/5] perf vendor events: Add JSON metrics for CMN-700

[Jing Zhang]

在 2023/5/1 下午8:27, John Garry 写道:
> On 24/04/2023 10:44, Jing Zhang wrote:
>> Add JSON metrics for CMN-700. Currently just add part of CMN-700 PMU
>> metrics which are general and compatible for any SoC.
>>
>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
>> ---
>>   .../arch/arm64/arm/cmn700/sys/metrics.json         | 74 ++++++++++++++++++++++
>>   tools/perf/pmu-events/jevents.py                   |  1 +
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>>
>> diff --git a/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>> new file mode 100644
>> index 0000000..1577d86
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
> 
> I suppose putting these here is ok. Are all these events a must for a CMN-700 implementation? Is there potential for impdef event in some implementations?
> 

Yes, according it's driver code, all these events are a must for a CMN-700 implementation even in different revision. But if the revision is added
as an identifier, then these metrics need to be defined for each revision. This doesn't seem lean enough. So I also need to consider how to
match the common metrics of multiple rivisions. Could you provide some suggestions?

> Previously we considered ARM PMCG events would be put in a per-SoC sys folder only (and not in perf/pmu-events/arch/arm64/arm)
> 
>> @@ -0,0 +1,74 @@
>> +[
>> +    {
>> +        "MetricName": "slc_miss_rate",
>> +        "BriefDescription": "The system level cache miss rate include.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_cache_miss / hnf_slc_sf_cache_access",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "hnf_message_retry_rate",
>> +        "BriefDescription": "HN-F message retry rate indicates whether a lack of credits is causing the bottlenecks.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_pocq_retry / hnf_pocq_reqs_recvd",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "sf_hit_rate",
>> +        "BriefDescription": "Snoop filter hit rate can be used to measure the Snoop Filter efficiency.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_sf_hit / hnf_slc_sf_cache_access",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "mc_message_retry_rate",
>> +        "BriefDescription": "The memory controller request retries rate indicates whether the memory controller is the bottleneck.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "hnf_mc_retries / hnf_mc_reqs",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "rni_actual_read_bandwidth.all",
>> +        "BriefDescription": "This event measure the actual bandwidth(MB/sec) that RN-I bridge sends to the interconnect.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "rnid_rxdat_flits * 32 / 1e6 / duration_time",
>> +        "ScaleUnit": "1MB/s",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "rni_actual_write_bandwidth.all",
>> +        "BriefDescription": "This event measures the actual write bandwidth(MB/sec) at RN-I bridges.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "rnid_txdat_flits * 32 / 1e6 / duration_time",
>> +        "ScaleUnit": "1MB/s",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "rni_retry_rate",
>> +        "BriefDescription": "RN-I bridge retry rate indicates whether the memory controller is the bottleneck.",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "rnid_txreq_flits_retried / rnid_txreq_flits_total",
>> +        "ScaleUnit": "100%",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    },
>> +    {
>> +        "MetricName": "sbsx_actual_write_bandwidth.all",
>> +        "BriefDescription": "sbsx actual write bandwidth(MB/sec).",
>> +        "MetricGroup": "cmn700",
>> +        "MetricExpr": "sbsx_txdat_flitv * 32 / 1e6 / duration_time",
>> +        "ScaleUnit": "1MB/s",
>> +        "Unit": "arm_cmn",
>> +        "Compat": "cmn700"
>> +    }
>> +]
>> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
>> index 2bcd07c..7cff2c6 100755
>> --- a/tools/perf/pmu-events/jevents.py
>> +++ b/tools/perf/pmu-events/jevents.py
>> @@ -256,6 +256,7 @@ class JsonEvent:
>>             'DFPMC': 'amd_df',
>>             'cpu_core': 'cpu_core',
>>             'cpu_atom': 'cpu_atom',
>> +          'arm_cmn': 'arm_cmn',
>>         }
>>         return table[unit] if unit in table else f'uncore_{unit.lower()}'
>>   

[PATCH v2 3/5] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[Jing Zhang]

To allow userspace to identify the specific implementation of the device,
add an "identifier" sysfs file.

The perf tool can match the Yitian 710 DDR metric through the identifier.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
index a7689fe..fe075fd 100644
--- a/drivers/perf/alibaba_uncore_drw_pmu.c
+++ b/drivers/perf/alibaba_uncore_drw_pmu.c
@@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
 	.attrs = ali_drw_pmu_cpumask_attrs,
 };
 
+static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
+					struct device_attribute *attr,
+					char *page)
+{
+	return sysfs_emit(page, "%s\n", "ali_drw_pmu");
+}
+
+static umode_t ali_drw_pmu_identifier_attr_visible(struct kobject *kobj,
+						struct attribute *attr, int n)
+{
+	return attr->mode;
+}
+
+static struct device_attribute ali_drw_pmu_identifier_attr =
+	__ATTR(identifier, 0444, ali_drw_pmu_identifier_show, NULL);
+
+static struct attribute *ali_drw_pmu_identifier_attrs[] = {
+	&ali_drw_pmu_identifier_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ali_drw_pmu_identifier_attr_group = {
+	.attrs = ali_drw_pmu_identifier_attrs,
+	.is_visible = ali_drw_pmu_identifier_attr_visible
+};
+
 static const struct attribute_group *ali_drw_pmu_attr_groups[] = {
 	&ali_drw_pmu_events_attr_group,
 	&ali_drw_pmu_cpumask_attr_group,
 	&ali_drw_pmu_format_group,
+	&ali_drw_pmu_identifier_attr_group,
 	NULL,
 };
 
-- 
1.8.3.1

Re: [PATCH v2 3/5] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[John Garry]

On 24/04/2023 10:44, Jing Zhang wrote:
> To allow userspace to identify the specific implementation of the device,
> add an "identifier" sysfs file.
> 
> The perf tool can match the Yitian 710 DDR metric through the identifier.
> 
> Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com>
> ---
>   drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
> index a7689fe..fe075fd 100644
> --- a/drivers/perf/alibaba_uncore_drw_pmu.c
> +++ b/drivers/perf/alibaba_uncore_drw_pmu.c
> @@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
>   	.attrs = ali_drw_pmu_cpumask_attrs,
>   };
>   
> +static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *page)
> +{
> +	return sysfs_emit(page, "%s\n", "ali_drw_pmu");
> +}

Would there be anything more unique per implementation version? Having a 
constant like this means that all implementations will have same events 
from userspace PoV.

Thanks,
John

Re: [PATCH v2 3/5] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[Jing Zhang]

在 2023/5/1 下午8:31, John Garry 写道:
> On 24/04/2023 10:44, Jing Zhang wrote:
>> To allow userspace to identify the specific implementation of the device,
>> add an "identifier" sysfs file.
>>
>> The perf tool can match the Yitian 710 DDR metric through the identifier.
>>
>> Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com>
>> ---
>>   drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
>> index a7689fe..fe075fd 100644
>> --- a/drivers/perf/alibaba_uncore_drw_pmu.c
>> +++ b/drivers/perf/alibaba_uncore_drw_pmu.c
>> @@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
>>       .attrs = ali_drw_pmu_cpumask_attrs,
>>   };
>>   +static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
>> +                    struct device_attribute *attr,
>> +                    char *page)
>> +{
>> +    return sysfs_emit(page, "%s\n", "ali_drw_pmu");
>> +}
> 
> Would there be anything more unique per implementation version? Having a constant like this means that all implementations will have same events from userspace PoV.
> 

Currently there is only this version, and no new revision will be added in the future. We can consider all implementations
to have the same events.

Thanks,
Jing

Re: [PATCH v2 3/5] driver/perf: Add identifier sysfs file for Yitian 710 DDR

[Shuai Xue]

On 2023/4/24 17:44, Jing Zhang wrote:
> To allow userspace to identify the specific implementation of the device,
> add an "identifier" sysfs file.
> 
> The perf tool can match the Yitian 710 DDR metric through the identifier.
> 
> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>

LGTM, thanks.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Best Regards,
Shuai

> ---
>  drivers/perf/alibaba_uncore_drw_pmu.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
> index a7689fe..fe075fd 100644
> --- a/drivers/perf/alibaba_uncore_drw_pmu.c
> +++ b/drivers/perf/alibaba_uncore_drw_pmu.c
> @@ -236,10 +236,37 @@ static ssize_t ali_drw_pmu_cpumask_show(struct device *dev,
>  	.attrs = ali_drw_pmu_cpumask_attrs,
>  };
>  
> +static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *page)
> +{
> +	return sysfs_emit(page, "%s\n", "ali_drw_pmu");
> +}
> +
> +static umode_t ali_drw_pmu_identifier_attr_visible(struct kobject *kobj,
> +						struct attribute *attr, int n)
> +{
> +	return attr->mode;
> +}
> +
> +static struct device_attribute ali_drw_pmu_identifier_attr =
> +	__ATTR(identifier, 0444, ali_drw_pmu_identifier_show, NULL);
> +
> +static struct attribute *ali_drw_pmu_identifier_attrs[] = {
> +	&ali_drw_pmu_identifier_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ali_drw_pmu_identifier_attr_group = {
> +	.attrs = ali_drw_pmu_identifier_attrs,
> +	.is_visible = ali_drw_pmu_identifier_attr_visible
> +};
> +
>  static const struct attribute_group *ali_drw_pmu_attr_groups[] = {
>  	&ali_drw_pmu_events_attr_group,
>  	&ali_drw_pmu_cpumask_attr_group,
>  	&ali_drw_pmu_format_group,
> +	&ali_drw_pmu_identifier_attr_group,
>  	NULL,
>  };
>  

[PATCH v2 4/5] perf jevents: Add support for Yitian 710 DDR PMU aliasing

[Jing Zhang]

Add support for T-HEAD Yitian 710 SoC DDR PMU aliasing.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
 tools/perf/pmu-events/jevents.py                   |   1 +
 2 files changed, 374 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json

diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
new file mode 100644
index 0000000..f17f239
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
@@ -0,0 +1,373 @@
+[
+	{
+		"BriefDescription": "A Write or Read Op at HIF interface. 64B",
+		"ConfigCode": "0x0",
+		"EventName": "hif_rd_or_wr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Write Op at HIF interface. 64B",
+		"ConfigCode": "0x1",
+		"EventName": "hif_wr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Read Op at HIF interface. 64B",
+		"ConfigCode": "0x2",
+		"EventName": "hif_rd",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Read-Modify-Write Op at HIF interface. 64B",
+		"ConfigCode": "0x3",
+		"EventName": "hif_rmw",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A high priority Read at HIF interface. 64B",
+		"ConfigCode": "0x4",
+		"EventName": "hif_hi_pri_rd",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A write data cycle at DFI interface (to DRAM)",
+		"ConfigCode": "0x7",
+		"EventName": "dfi_wr_data_cycles",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A read data cycle at DFI interface (to DRAM).",
+		"ConfigCode": "0x8",
+		"EventName": "dfi_rd_data_cycles",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A high priority read becomes critical.",
+		"ConfigCode": "0x9",
+		"EventName": "hpr_xact_when_critical",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A low priority read becomes critical.",
+		"ConfigCode": "0xA",
+		"EventName": "lpr_xact_when_critical",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A write becomes critical.",
+		"ConfigCode": "0xB",
+		"EventName": "wr_xact_when_critical",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "An Activate (ACT) command to DRAM.",
+		"ConfigCode": "0xC",
+		"EventName": "op_is_activate",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Read or Write CAS command to DRAM.",
+		"ConfigCode": "0xD",
+		"EventName": "op_is_rd_or_wr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "An ACT command for read to DRAM.",
+		"ConfigCode": "0xE",
+		"EventName": "op_is_rd_activate",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Read CAS command to DRAM.",
+		"ConfigCode": "0xF",
+		"EventName": "op_is_rd",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Write CAS command to DRAM.",
+		"ConfigCode": "0x10",
+		"EventName": "op_is_wr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Masked Write command to DRAM.",
+		"ConfigCode": "0x11",
+		"EventName": "op_is_mwr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Precharge (PRE) command to DRAM.",
+		"ConfigCode": "0x12",
+		"EventName": "op_is_precharge",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A PRE required by read or write.",
+		"ConfigCode": "0x13",
+		"EventName": "precharge_for_rdwr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A PRE required by other conditions.",
+		"ConfigCode": "0x14",
+		"EventName": "precharge_for_other",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A read-write turnaround.",
+		"ConfigCode": "0x15",
+		"EventName": "rdwr_transitions",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A write combine (merge) in write data buffer.",
+		"ConfigCode": "0x16",
+		"EventName": "write_combine",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Write-After-Read hazard.",
+		"ConfigCode": "0x17",
+		"EventName": "war_hazard",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Read-After-Write hazard.",
+		"ConfigCode": "0x18",
+		"EventName": "raw_hazard",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A Write-After-Write hazard.",
+		"ConfigCode": "0x19",
+		"EventName": "waw_hazard",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank0 enters self-refresh (SRE).",
+		"ConfigCode": "0x1A",
+		"EventName": "op_is_enter_selfref_rk0",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank1 enters self-refresh (SRE).",
+		"ConfigCode": "0x1B",
+		"EventName": "op_is_enter_selfref_rk1",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank2 enters self-refresh (SRE).",
+		"ConfigCode": "0x1C",
+		"EventName": "op_is_enter_selfref_rk2",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank3 enters self-refresh (SRE).",
+		"ConfigCode": "0x1D",
+		"EventName": "op_is_enter_selfref_rk3",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank0 enters power-down (PDE).",
+		"ConfigCode": "0x1E",
+		"EventName": "op_is_enter_powerdown_rk0",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank1 enters power-down (PDE).",
+		"ConfigCode": "0x1F",
+		"EventName": "op_is_enter_powerdown_rk1",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank2 enters power-down (PDE).",
+		"ConfigCode": "0x20",
+		"EventName": "op_is_enter_powerdown_rk2",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "Rank3 enters power-down (PDE).",
+		"ConfigCode": "0x21",
+		"EventName": "op_is_enter_powerdown_rk3",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A cycle that Rank0 stays in self-refresh mode.",
+		"ConfigCode": "0x26",
+		"EventName": "selfref_mode_rk0",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A cycle that Rank1 stays in self-refresh mode.",
+		"ConfigCode": "0x27",
+		"EventName": "selfref_mode_rk1",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A cycle that Rank2 stays in self-refresh mode.",
+		"ConfigCode": "0x28",
+		"EventName": "selfref_mode_rk2",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A cycle that Rank3 stays in self-refresh mode.",
+		"ConfigCode": "0x29",
+		"EventName": "selfref_mode_rk3",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "An auto-refresh (REF) command to DRAM.",
+		"ConfigCode": "0x2A",
+		"EventName": "op_is_refresh",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A critical REF command to DRAM.",
+		"ConfigCode": "0x2B",
+		"EventName": "op_is_crit_ref",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "An MRR or MRW command to DRAM.",
+		"ConfigCode": "0x2D",
+		"EventName": "op_is_load_mode",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A ZQCal command to DRAM.",
+		"ConfigCode": "0x2E",
+		"EventName": "op_is_zqcl",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "At least one entry in read queue reaches the visible window limit.",
+		"ConfigCode": "0x30",
+		"EventName": "visible_window_limit_reached_rd",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "At least one entry in write queue reaches the visible window limit.",
+		"ConfigCode": "0x31",
+		"EventName": "visible_window_limit_reached_wr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A DQS Oscillator MPC command to DRAM.",
+		"ConfigCode": "0x34",
+		"EventName": "op_is_dqsosc_mpc",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A DQS Oscillator MRR command to DRAM.",
+		"ConfigCode": "0x35",
+		"EventName": "op_is_dqsosc_mrr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A TCR (Temperature Compensated Refresh) MRR command to DRAM.",
+		"ConfigCode": "0x36",
+		"EventName": "op_is_tcr_mrr",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A ZQCal Start command to DRAM.",
+		"ConfigCode": "0x37",
+		"EventName": "op_is_zqstart",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A ZQCal Latch command to DRAM.",
+		"ConfigCode": "0x38",
+		"EventName": "op_is_zqlatch",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A packet at CHI TXREQ interface (request).",
+		"ConfigCode": "0x39",
+		"EventName": "chi_txreq",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A packet at CHI TXDAT interface (read data).",
+		"ConfigCode": "0x3A",
+		"EventName": "chi_txdat",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A packet at CHI RXDAT interface (write data).",
+		"ConfigCode": "0x3B",
+		"EventName": "chi_rxdat",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A packet at CHI RXRSP interface.",
+		"ConfigCode": "0x3C",
+		"EventName": "chi_rxrsp",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "A violation detected in TZC.",
+		"ConfigCode": "0x3D",
+		"EventName": "tsz_vio",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"BriefDescription": "The ddr cycle.",
+		"ConfigCode": "0x80",
+		"EventName": "cycle",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	}
+]
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 7cff2c6..aff4051 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -257,6 +257,7 @@ class JsonEvent:
           'cpu_core': 'cpu_core',
           'cpu_atom': 'cpu_atom',
           'arm_cmn': 'arm_cmn',
+          'ali_drw': 'ali_drw',
       }
       return table[unit] if unit in table else f'uncore_{unit.lower()}'
 
-- 
1.8.3.1

Re: [PATCH v2 4/5] perf jevents: Add support for Yitian 710 DDR PMU aliasing

[John Garry]

On 24/04/2023 10:44, Jing Zhang wrote:
> Add support for T-HEAD Yitian 710 SoC DDR PMU aliasing.
> 
> Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com>


Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH v2 4/5] perf jevents: Add support for Yitian 710 DDR PMU aliasing

[Shuai Xue]

On 2023/4/24 17:44, Jing Zhang wrote:
> Add support for T-HEAD Yitian 710 SoC DDR PMU aliasing.
> 
> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> ---
>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>  tools/perf/pmu-events/jevents.py                   |   1 +
>  2 files changed, 374 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
> 
> diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
> new file mode 100644
> index 0000000..f17f239
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
> @@ -0,0 +1,373 @@
> +[
> +	{
> +		"BriefDescription": "A Write or Read Op at HIF interface. 64B",
> +		"ConfigCode": "0x0",
> +		"EventName": "hif_rd_or_wr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Write Op at HIF interface. 64B",
> +		"ConfigCode": "0x1",
> +		"EventName": "hif_wr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Read Op at HIF interface. 64B",
> +		"ConfigCode": "0x2",
> +		"EventName": "hif_rd",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Read-Modify-Write Op at HIF interface. 64B",
> +		"ConfigCode": "0x3",
> +		"EventName": "hif_rmw",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A high priority Read at HIF interface. 64B",
> +		"ConfigCode": "0x4",
> +		"EventName": "hif_hi_pri_rd",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A write data cycle at DFI interface (to DRAM)",

Add an ending period to BriefDescription as you do latter.

> +		"ConfigCode": "0x7",
> +		"EventName": "dfi_wr_data_cycles",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A read data cycle at DFI interface (to DRAM).",
> +		"ConfigCode": "0x8",
> +		"EventName": "dfi_rd_data_cycles",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A high priority read becomes critical.",
> +		"ConfigCode": "0x9",
> +		"EventName": "hpr_xact_when_critical",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A low priority read becomes critical.",
> +		"ConfigCode": "0xA",
> +		"EventName": "lpr_xact_when_critical",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A write becomes critical.",
> +		"ConfigCode": "0xB",
> +		"EventName": "wr_xact_when_critical",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "An Activate (ACT) command to DRAM.",
> +		"ConfigCode": "0xC",
> +		"EventName": "op_is_activate",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Read or Write CAS command to DRAM.",
> +		"ConfigCode": "0xD",
> +		"EventName": "op_is_rd_or_wr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "An ACT command for read to DRAM.",
> +		"ConfigCode": "0xE",
> +		"EventName": "op_is_rd_activate",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Read CAS command to DRAM.",
> +		"ConfigCode": "0xF",
> +		"EventName": "op_is_rd",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Write CAS command to DRAM.",
> +		"ConfigCode": "0x10",
> +		"EventName": "op_is_wr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Masked Write command to DRAM.",
> +		"ConfigCode": "0x11",
> +		"EventName": "op_is_mwr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Precharge (PRE) command to DRAM.",
> +		"ConfigCode": "0x12",
> +		"EventName": "op_is_precharge",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A PRE required by read or write.",

A non-abbreviation format is prefered here.

> +		"ConfigCode": "0x13",
> +		"EventName": "precharge_for_rdwr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A PRE required by other conditions.",
> +		"ConfigCode": "0x14",
> +		"EventName": "precharge_for_other",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A read-write turnaround.",
> +		"ConfigCode": "0x15",
> +		"EventName": "rdwr_transitions",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A write combine (merge) in write data buffer.",
> +		"ConfigCode": "0x16",
> +		"EventName": "write_combine",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Write-After-Read hazard.",
> +		"ConfigCode": "0x17",
> +		"EventName": "war_hazard",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Read-After-Write hazard.",
> +		"ConfigCode": "0x18",
> +		"EventName": "raw_hazard",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A Write-After-Write hazard.",
> +		"ConfigCode": "0x19",
> +		"EventName": "waw_hazard",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank0 enters self-refresh (SRE).",
> +		"ConfigCode": "0x1A",
> +		"EventName": "op_is_enter_selfref_rk0",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank1 enters self-refresh (SRE).",
> +		"ConfigCode": "0x1B",
> +		"EventName": "op_is_enter_selfref_rk1",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank2 enters self-refresh (SRE).",
> +		"ConfigCode": "0x1C",
> +		"EventName": "op_is_enter_selfref_rk2",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank3 enters self-refresh (SRE).",
> +		"ConfigCode": "0x1D",
> +		"EventName": "op_is_enter_selfref_rk3",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank0 enters power-down (PDE).",
> +		"ConfigCode": "0x1E",
> +		"EventName": "op_is_enter_powerdown_rk0",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank1 enters power-down (PDE).",
> +		"ConfigCode": "0x1F",
> +		"EventName": "op_is_enter_powerdown_rk1",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank2 enters power-down (PDE).",
> +		"ConfigCode": "0x20",
> +		"EventName": "op_is_enter_powerdown_rk2",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "Rank3 enters power-down (PDE).",
> +		"ConfigCode": "0x21",
> +		"EventName": "op_is_enter_powerdown_rk3",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A cycle that Rank0 stays in self-refresh mode.",
> +		"ConfigCode": "0x26",
> +		"EventName": "selfref_mode_rk0",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A cycle that Rank1 stays in self-refresh mode.",
> +		"ConfigCode": "0x27",
> +		"EventName": "selfref_mode_rk1",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A cycle that Rank2 stays in self-refresh mode.",
> +		"ConfigCode": "0x28",
> +		"EventName": "selfref_mode_rk2",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A cycle that Rank3 stays in self-refresh mode.",
> +		"ConfigCode": "0x29",
> +		"EventName": "selfref_mode_rk3",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "An auto-refresh (REF) command to DRAM.",
> +		"ConfigCode": "0x2A",
> +		"EventName": "op_is_refresh",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A critical REF command to DRAM.",
> +		"ConfigCode": "0x2B",
> +		"EventName": "op_is_crit_ref",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "An MRR or MRW command to DRAM.",
> +		"ConfigCode": "0x2D",
> +		"EventName": "op_is_load_mode",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A ZQCal command to DRAM.",
> +		"ConfigCode": "0x2E",
> +		"EventName": "op_is_zqcl",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "At least one entry in read queue reaches the visible window limit.",
> +		"ConfigCode": "0x30",
> +		"EventName": "visible_window_limit_reached_rd",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "At least one entry in write queue reaches the visible window limit.",
> +		"ConfigCode": "0x31",
> +		"EventName": "visible_window_limit_reached_wr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A DQS Oscillator MPC command to DRAM.",
> +		"ConfigCode": "0x34",
> +		"EventName": "op_is_dqsosc_mpc",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A DQS Oscillator MRR command to DRAM.",
> +		"ConfigCode": "0x35",
> +		"EventName": "op_is_dqsosc_mrr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A TCR (Temperature Compensated Refresh) MRR command to DRAM.",
> +		"ConfigCode": "0x36",
> +		"EventName": "op_is_tcr_mrr",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A ZQCal Start command to DRAM.",
> +		"ConfigCode": "0x37",
> +		"EventName": "op_is_zqstart",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A ZQCal Latch command to DRAM.",
> +		"ConfigCode": "0x38",
> +		"EventName": "op_is_zqlatch",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A packet at CHI TXREQ interface (request).",
> +		"ConfigCode": "0x39",
> +		"EventName": "chi_txreq",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A packet at CHI TXDAT interface (read data).",
> +		"ConfigCode": "0x3A",
> +		"EventName": "chi_txdat",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A packet at CHI RXDAT interface (write data).",
> +		"ConfigCode": "0x3B",
> +		"EventName": "chi_rxdat",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A packet at CHI RXRSP interface.",
> +		"ConfigCode": "0x3C",
> +		"EventName": "chi_rxrsp",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "A violation detected in TZC.",
> +		"ConfigCode": "0x3D",
> +		"EventName": "tsz_vio",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"BriefDescription": "The ddr cycle.",
> +		"ConfigCode": "0x80",
> +		"EventName": "cycle",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	}
> +]
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index 7cff2c6..aff4051 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -257,6 +257,7 @@ class JsonEvent:
>            'cpu_core': 'cpu_core',
>            'cpu_atom': 'cpu_atom',
>            'arm_cmn': 'arm_cmn',
> +          'ali_drw': 'ali_drw',
>        }
>        return table[unit] if unit in table else f'uncore_{unit.lower()}'
>  

Otherwise, LGTM.

Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>

Best Regards,
Shuai

Re: [PATCH v2 4/5] perf jevents: Add support for Yitian 710 DDR PMU aliasing

[Jing Zhang]

在 2023/5/4 下午3:18, Shuai Xue 写道:
> 
> 
> On 2023/4/24 17:44, Jing Zhang wrote:
>> Add support for T-HEAD Yitian 710 SoC DDR PMU aliasing.
>>
>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
>> ---
>>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>>  tools/perf/pmu-events/jevents.py                   |   1 +
>>  2 files changed, 374 insertions(+)
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>>
>> diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>> new file mode 100644
>> index 0000000..f17f239
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>> @@ -0,0 +1,373 @@
>> +[
>> +	{
>> +		"BriefDescription": "A Write or Read Op at HIF interface. 64B",
>> +		"ConfigCode": "0x0",
>> +		"EventName": "hif_rd_or_wr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Write Op at HIF interface. 64B",
>> +		"ConfigCode": "0x1",
>> +		"EventName": "hif_wr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Read Op at HIF interface. 64B",
>> +		"ConfigCode": "0x2",
>> +		"EventName": "hif_rd",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Read-Modify-Write Op at HIF interface. 64B",
>> +		"ConfigCode": "0x3",
>> +		"EventName": "hif_rmw",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A high priority Read at HIF interface. 64B",
>> +		"ConfigCode": "0x4",
>> +		"EventName": "hif_hi_pri_rd",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A write data cycle at DFI interface (to DRAM)",
> 
> Add an ending period to BriefDescription as you do latter.
> 

Ok.

>> +		"ConfigCode": "0x7",
>> +		"EventName": "dfi_wr_data_cycles",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A read data cycle at DFI interface (to DRAM).",
>> +		"ConfigCode": "0x8",
>> +		"EventName": "dfi_rd_data_cycles",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A high priority read becomes critical.",
>> +		"ConfigCode": "0x9",
>> +		"EventName": "hpr_xact_when_critical",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A low priority read becomes critical.",
>> +		"ConfigCode": "0xA",
>> +		"EventName": "lpr_xact_when_critical",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A write becomes critical.",
>> +		"ConfigCode": "0xB",
>> +		"EventName": "wr_xact_when_critical",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "An Activate (ACT) command to DRAM.",
>> +		"ConfigCode": "0xC",
>> +		"EventName": "op_is_activate",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Read or Write CAS command to DRAM.",
>> +		"ConfigCode": "0xD",
>> +		"EventName": "op_is_rd_or_wr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "An ACT command for read to DRAM.",
>> +		"ConfigCode": "0xE",
>> +		"EventName": "op_is_rd_activate",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Read CAS command to DRAM.",
>> +		"ConfigCode": "0xF",
>> +		"EventName": "op_is_rd",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Write CAS command to DRAM.",
>> +		"ConfigCode": "0x10",
>> +		"EventName": "op_is_wr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Masked Write command to DRAM.",
>> +		"ConfigCode": "0x11",
>> +		"EventName": "op_is_mwr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Precharge (PRE) command to DRAM.",
>> +		"ConfigCode": "0x12",
>> +		"EventName": "op_is_precharge",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A PRE required by read or write.",
> 
> A non-abbreviation format is prefered here.
> 

Ok.

>> +		"ConfigCode": "0x13",
>> +		"EventName": "precharge_for_rdwr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A PRE required by other conditions.",
>> +		"ConfigCode": "0x14",
>> +		"EventName": "precharge_for_other",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A read-write turnaround.",
>> +		"ConfigCode": "0x15",
>> +		"EventName": "rdwr_transitions",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A write combine (merge) in write data buffer.",
>> +		"ConfigCode": "0x16",
>> +		"EventName": "write_combine",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Write-After-Read hazard.",
>> +		"ConfigCode": "0x17",
>> +		"EventName": "war_hazard",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Read-After-Write hazard.",
>> +		"ConfigCode": "0x18",
>> +		"EventName": "raw_hazard",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A Write-After-Write hazard.",
>> +		"ConfigCode": "0x19",
>> +		"EventName": "waw_hazard",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank0 enters self-refresh (SRE).",
>> +		"ConfigCode": "0x1A",
>> +		"EventName": "op_is_enter_selfref_rk0",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank1 enters self-refresh (SRE).",
>> +		"ConfigCode": "0x1B",
>> +		"EventName": "op_is_enter_selfref_rk1",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank2 enters self-refresh (SRE).",
>> +		"ConfigCode": "0x1C",
>> +		"EventName": "op_is_enter_selfref_rk2",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank3 enters self-refresh (SRE).",
>> +		"ConfigCode": "0x1D",
>> +		"EventName": "op_is_enter_selfref_rk3",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank0 enters power-down (PDE).",
>> +		"ConfigCode": "0x1E",
>> +		"EventName": "op_is_enter_powerdown_rk0",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank1 enters power-down (PDE).",
>> +		"ConfigCode": "0x1F",
>> +		"EventName": "op_is_enter_powerdown_rk1",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank2 enters power-down (PDE).",
>> +		"ConfigCode": "0x20",
>> +		"EventName": "op_is_enter_powerdown_rk2",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "Rank3 enters power-down (PDE).",
>> +		"ConfigCode": "0x21",
>> +		"EventName": "op_is_enter_powerdown_rk3",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A cycle that Rank0 stays in self-refresh mode.",
>> +		"ConfigCode": "0x26",
>> +		"EventName": "selfref_mode_rk0",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A cycle that Rank1 stays in self-refresh mode.",
>> +		"ConfigCode": "0x27",
>> +		"EventName": "selfref_mode_rk1",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A cycle that Rank2 stays in self-refresh mode.",
>> +		"ConfigCode": "0x28",
>> +		"EventName": "selfref_mode_rk2",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A cycle that Rank3 stays in self-refresh mode.",
>> +		"ConfigCode": "0x29",
>> +		"EventName": "selfref_mode_rk3",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "An auto-refresh (REF) command to DRAM.",
>> +		"ConfigCode": "0x2A",
>> +		"EventName": "op_is_refresh",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A critical REF command to DRAM.",
>> +		"ConfigCode": "0x2B",
>> +		"EventName": "op_is_crit_ref",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "An MRR or MRW command to DRAM.",
>> +		"ConfigCode": "0x2D",
>> +		"EventName": "op_is_load_mode",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A ZQCal command to DRAM.",
>> +		"ConfigCode": "0x2E",
>> +		"EventName": "op_is_zqcl",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "At least one entry in read queue reaches the visible window limit.",
>> +		"ConfigCode": "0x30",
>> +		"EventName": "visible_window_limit_reached_rd",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "At least one entry in write queue reaches the visible window limit.",
>> +		"ConfigCode": "0x31",
>> +		"EventName": "visible_window_limit_reached_wr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A DQS Oscillator MPC command to DRAM.",
>> +		"ConfigCode": "0x34",
>> +		"EventName": "op_is_dqsosc_mpc",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A DQS Oscillator MRR command to DRAM.",
>> +		"ConfigCode": "0x35",
>> +		"EventName": "op_is_dqsosc_mrr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A TCR (Temperature Compensated Refresh) MRR command to DRAM.",
>> +		"ConfigCode": "0x36",
>> +		"EventName": "op_is_tcr_mrr",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A ZQCal Start command to DRAM.",
>> +		"ConfigCode": "0x37",
>> +		"EventName": "op_is_zqstart",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A ZQCal Latch command to DRAM.",
>> +		"ConfigCode": "0x38",
>> +		"EventName": "op_is_zqlatch",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A packet at CHI TXREQ interface (request).",
>> +		"ConfigCode": "0x39",
>> +		"EventName": "chi_txreq",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A packet at CHI TXDAT interface (read data).",
>> +		"ConfigCode": "0x3A",
>> +		"EventName": "chi_txdat",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A packet at CHI RXDAT interface (write data).",
>> +		"ConfigCode": "0x3B",
>> +		"EventName": "chi_rxdat",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A packet at CHI RXRSP interface.",
>> +		"ConfigCode": "0x3C",
>> +		"EventName": "chi_rxrsp",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "A violation detected in TZC.",
>> +		"ConfigCode": "0x3D",
>> +		"EventName": "tsz_vio",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"BriefDescription": "The ddr cycle.",
>> +		"ConfigCode": "0x80",
>> +		"EventName": "cycle",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	}
>> +]
>> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
>> index 7cff2c6..aff4051 100755
>> --- a/tools/perf/pmu-events/jevents.py
>> +++ b/tools/perf/pmu-events/jevents.py
>> @@ -257,6 +257,7 @@ class JsonEvent:
>>            'cpu_core': 'cpu_core',
>>            'cpu_atom': 'cpu_atom',
>>            'arm_cmn': 'arm_cmn',
>> +          'ali_drw': 'ali_drw',
>>        }
>>        return table[unit] if unit in table else f'uncore_{unit.lower()}'
>>  
> 
> Otherwise, LGTM.
> 
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> 
> Best Regards,
> Shuai

Thanks!

[PATCH v2 5/5] perf vendor events: Add JSON metrics for Yitian 710 DDR

[Jing Zhang]

Add JSON metrics for T-HEAD Yitian 710 SoC DDR.

Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
---
 .../arch/arm64/freescale/yitian710/sys/metrics.json  | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
new file mode 100644
index 0000000..1a92477
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
@@ -0,0 +1,20 @@
+[
+	{
+		"MetricName": "ddr_read_bandwidth.all",
+		"BriefDescription": "The ddr read bandwidth(MB/s).",
+		"MetricGroup": "ddr",
+		"MetricExpr": "hif_rd * 64 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	},
+	{
+		"MetricName": "ddr_write_bandwidth.all",
+		"BriefDescription": "The ddr write bandwidth(MB/s).",
+		"MetricGroup": "ddr",
+		"MetricExpr": "(hif_wr + hif_rmw) * 64 / 1e6 / duration_time",
+		"ScaleUnit": "1MB/s",
+		"Unit": "ali_drw",
+		"Compat": "ali_drw_pmu"
+	}
+]
-- 
1.8.3.1

Re: [PATCH v2 5/5] perf vendor events: Add JSON metrics for Yitian 710 DDR

[John Garry]

On 24/04/2023 10:44, Jing Zhang wrote:
> Add JSON metrics for T-HEAD Yitian 710 SoC DDR.
> 
> Signed-off-by: Jing Zhang<renyu.zj@linux.alibaba.com>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Re: [PATCH v2 5/5] perf vendor events: Add JSON metrics for Yitian 710 DDR

[Shuai Xue]

On 2023/4/24 17:44, Jing Zhang wrote:
> Add JSON metrics for T-HEAD Yitian 710 SoC DDR.
> 
> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>


Could you also update the Documentation/admin-guide/perf/alibaba_pmu.rst
with some example of how to use these metrics?

Best Regards,
Shuai


> ---
>  .../arch/arm64/freescale/yitian710/sys/metrics.json  | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
> 
> diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
> new file mode 100644
> index 0000000..1a92477
> --- /dev/null
> +++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
> @@ -0,0 +1,20 @@
> +[
> +	{
> +		"MetricName": "ddr_read_bandwidth.all",
> +		"BriefDescription": "The ddr read bandwidth(MB/s).",
> +		"MetricGroup": "ddr",
> +		"MetricExpr": "hif_rd * 64 / 1e6 / duration_time",
> +		"ScaleUnit": "1MB/s",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	},
> +	{
> +		"MetricName": "ddr_write_bandwidth.all",
> +		"BriefDescription": "The ddr write bandwidth(MB/s).",
> +		"MetricGroup": "ddr",
> +		"MetricExpr": "(hif_wr + hif_rmw) * 64 / 1e6 / duration_time",
> +		"ScaleUnit": "1MB/s",
> +		"Unit": "ali_drw",
> +		"Compat": "ali_drw_pmu"
> +	}
> +]

Re: [PATCH v2 5/5] perf vendor events: Add JSON metrics for Yitian 710 DDR

[Jing Zhang]

在 2023/5/4 下午3:31, Shuai Xue 写道:
> 
> 
> On 2023/4/24 17:44, Jing Zhang wrote:
>> Add JSON metrics for T-HEAD Yitian 710 SoC DDR.
>>
>> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> 
> 
> Could you also update the Documentation/admin-guide/perf/alibaba_pmu.rst
> with some example of how to use these metrics?
> 

Yes，will do.

Thanks,
Jing


> Best Regards,
> Shuai
> 
> 
>> ---
>>  .../arch/arm64/freescale/yitian710/sys/metrics.json  | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>>
>> diff --git a/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>> new file mode 100644
>> index 0000000..1a92477
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>> @@ -0,0 +1,20 @@
>> +[
>> +	{
>> +		"MetricName": "ddr_read_bandwidth.all",
>> +		"BriefDescription": "The ddr read bandwidth(MB/s).",
>> +		"MetricGroup": "ddr",
>> +		"MetricExpr": "hif_rd * 64 / 1e6 / duration_time",
>> +		"ScaleUnit": "1MB/s",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	},
>> +	{
>> +		"MetricName": "ddr_write_bandwidth.all",
>> +		"BriefDescription": "The ddr write bandwidth(MB/s).",
>> +		"MetricGroup": "ddr",
>> +		"MetricExpr": "(hif_wr + hif_rmw) * 64 / 1e6 / duration_time",
>> +		"ScaleUnit": "1MB/s",
>> +		"Unit": "ali_drw",
>> +		"Compat": "ali_drw_pmu"
>> +	}
>> +]

Re: [PATCH v2 0/5] Add JSON metrics for arm CMN and Yitian710 DDR

[Ian Rogers]

On Mon, Apr 24, 2023 at 2:44 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>
> Changes since RFC:
> - Refact arm-cmn PMU identifier.
> - Not add arm-cmn PMU aliasing currently because it's Eventcode is
>   difficult to define.
> - Rename ali_drw PMU identifier and Unit name.
> - Divide ali_drw PMU metric and aliasing into two patches.
>
> Add an identifier sysfs file for the yitian710 SoC DDR and arm CMN to
> allow userspace to identify the specific implementation of the device,
> so that the perf tool can match the corresponding uncore events and
> metrics through the identifier. Then added several general CMN700 metrics
> and yitian710 soc DDR metrics.
>
> $perf list:
> ...
> ali_drw:
>   hif_rmw
>        [A Read-Modify-Write Op at HIF interface. 64B. Unit: ali_drw]
>   hif_hi_pri_rd
>        [A high priority Read at HIF interface. 64B. Unit: ali_drw]
>   hif_rd
>        [A Read Op at HIF interface. 64B. Unit: ali_drw]
>   hif_rd_or_wr
>        [A Write or Read Op at HIF interface. 64B. Unit: ali_drw]
>   hif_wr
>        [A Write Op at HIF interface. 64B. Unit: ali_drw]
> ...
>
> $perf stat -M ddr_read_bandwidth.all ./test
>
> Performance counter stats for 'system wide':
>
>             38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
>      1,000,957,941 ns   duration_time
>
>        1.000957941 seconds time elapsed
>
> Jing Zhang (5):
>   driver/perf: Add identifier sysfs file for CMN
>   perf vendor events: Add JSON metrics for CMN-700
>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>   perf jevents: Add support for Yitian 710 DDR PMU aliasing
>   perf vendor events: Add JSON metrics for Yitian 710 DDR

The perf tool changes all make sense. John Garry is more of an expert
on the ARM part of this than me though.
Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

>  drivers/perf/alibaba_uncore_drw_pmu.c              |  27 ++
>  drivers/perf/arm-cmn.c                             |  49 +++
>  .../arch/arm64/arm/cmn700/sys/metrics.json         |  74 ++++
>  .../arm64/freescale/yitian710/sys/ali_drw.json     | 373 +++++++++++++++++++++
>  .../arm64/freescale/yitian710/sys/metrics.json     |  20 ++
>  tools/perf/pmu-events/jevents.py                   |   2 +
>  6 files changed, 545 insertions(+)
>  create mode 100644 tools/perf/pmu-events/arch/arm64/arm/cmn700/sys/metrics.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/ali_drw.json
>  create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/yitian710/sys/metrics.json
>
> --
> 1.8.3.1
>

Re: [PATCH v2 0/5] Add JSON metrics for arm CMN and Yitian710 DDR

[Jing Zhang]

在 2023/4/30 上午5:01, Ian Rogers 写道:
> On Mon, Apr 24, 2023 at 2:44 AM Jing Zhang <renyu.zj@linux.alibaba.com> wrote:
>>
>> Changes since RFC:
>> - Refact arm-cmn PMU identifier.
>> - Not add arm-cmn PMU aliasing currently because it's Eventcode is
>>   difficult to define.
>> - Rename ali_drw PMU identifier and Unit name.
>> - Divide ali_drw PMU metric and aliasing into two patches.
>>
>> Add an identifier sysfs file for the yitian710 SoC DDR and arm CMN to
>> allow userspace to identify the specific implementation of the device,
>> so that the perf tool can match the corresponding uncore events and
>> metrics through the identifier. Then added several general CMN700 metrics
>> and yitian710 soc DDR metrics.
>>
>> $perf list:
>> ...
>> ali_drw:
>>   hif_rmw
>>        [A Read-Modify-Write Op at HIF interface. 64B. Unit: ali_drw]
>>   hif_hi_pri_rd
>>        [A high priority Read at HIF interface. 64B. Unit: ali_drw]
>>   hif_rd
>>        [A Read Op at HIF interface. 64B. Unit: ali_drw]
>>   hif_rd_or_wr
>>        [A Write or Read Op at HIF interface. 64B. Unit: ali_drw]
>>   hif_wr
>>        [A Write Op at HIF interface. 64B. Unit: ali_drw]
>> ...
>>
>> $perf stat -M ddr_read_bandwidth.all ./test
>>
>> Performance counter stats for 'system wide':
>>
>>             38,150      hif_rd        #  2.4 MB/s  ddr_read_bandwidth.all
>>      1,000,957,941 ns   duration_time
>>
>>        1.000957941 seconds time elapsed
>>
>> Jing Zhang (5):
>>   driver/perf: Add identifier sysfs file for CMN
>>   perf vendor events: Add JSON metrics for CMN-700
>>   driver/perf: Add identifier sysfs file for Yitian 710 DDR
>>   perf jevents: Add support for Yitian 710 DDR PMU aliasing
>>   perf vendor events: Add JSON metrics for Yitian 710 DDR
> 
> The perf tool changes all make sense. John Garry is more of an expert
> on the ARM part of this than me though.
> Acked-by: Ian Rogers <irogers@google.com>
> 

Thanks Ian!