diff mbox series

[v3,1/3] arm64: perf: Add support caps in sysfs

Message ID 1592487344-30555-1-git-send-email-zhangshaokun@hisilicon.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] arm64: perf: Add support caps in sysfs | expand

Commit Message

Shaokun Zhang June 18, 2020, 1:35 p.m. UTC
ARMv8.4-PMU introduces the PMMIR_EL1 registers and some new PMU events,
like STALL_SLOT etc, are related to it. Let's add a caps directory to
/sys/bus/event_source/devices/armv8_pmuv3_0/ and support slots from
PMMIR_EL1 registers in this entry. The user programs can get the slots
from sysfs directly.

Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
ChangeLog in v3:
    * Fix one typo in patch3

ChangeLog in v2:
    * Add caps entry in sysfs
    * Fix the PMU events typos
    * Add one new patch to correct event ID in sysfs

 arch/arm64/include/asm/sysreg.h |  2 +
 arch/arm64/kernel/perf_event.c  | 87 +++++++++++++++++++++++++++++++----------
 include/linux/perf/arm_pmu.h    |  1 +
 3 files changed, 69 insertions(+), 21 deletions(-)

Comments

Shaokun Zhang July 7, 2020, 1:33 p.m. UTC | #1
Hi Will/Mark,

Sorry for my noise, any more comments please?

Thanks,
Shaokun

在 2020/6/18 21:35, Shaokun Zhang 写道:
> ARMv8.4-PMU introduces the PMMIR_EL1 registers and some new PMU events,
> like STALL_SLOT etc, are related to it. Let's add a caps directory to
> /sys/bus/event_source/devices/armv8_pmuv3_0/ and support slots from
> PMMIR_EL1 registers in this entry. The user programs can get the slots
> from sysfs directly.
> 
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
> ChangeLog in v3:
>     * Fix one typo in patch3
> 
> ChangeLog in v2:
>     * Add caps entry in sysfs
>     * Fix the PMU events typos
>     * Add one new patch to correct event ID in sysfs
> 
>  arch/arm64/include/asm/sysreg.h |  2 +
>  arch/arm64/kernel/perf_event.c  | 87 +++++++++++++++++++++++++++++++----------
>  include/linux/perf/arm_pmu.h    |  1 +
>  3 files changed, 69 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 463175f80341..56c45a9207c7 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -321,6 +321,8 @@
>  #define SYS_PMINTENSET_EL1		sys_reg(3, 0, 9, 14, 1)
>  #define SYS_PMINTENCLR_EL1		sys_reg(3, 0, 9, 14, 2)
>  
> +#define SYS_PMMIR_EL1			sys_reg(3, 0, 9, 14, 6)
> +
>  #define SYS_MAIR_EL1			sys_reg(3, 0, 10, 2, 0)
>  #define SYS_AMAIR_EL1			sys_reg(3, 0, 10, 3, 0)
>  
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 4d7879484cec..5f2ac87e4b91 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -277,6 +277,51 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
>  	.attrs = armv8_pmuv3_format_attrs,
>  };
>  
> +static inline int armv8pmu_get_pmu_version(void)
> +{
> +	int pmuver;
> +	u64 dfr0;
> +
> +	dfr0 = read_sysreg(id_aa64dfr0_el1);
> +	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> +			ID_AA64DFR0_PMUVER_SHIFT);
> +
> +	return pmuver;
> +}
> +
> +static umode_t
> +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
> +			      int unused)
> +{
> +	int pmuver = armv8pmu_get_pmu_version();
> +
> +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
> +		return attr->mode;
> +
> +	return 0;
> +}
> +
> +static ssize_t slots_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	int slots = read_sysreg_s(SYS_PMMIR_EL1) & 0xFF;
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", slots);
> +}
> +
> +static DEVICE_ATTR_RO(slots);
> +
> +static struct attribute *armv8_pmuv3_caps_attrs[] = {
> +	&dev_attr_slots.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group armv8_pmuv3_caps_attr_group = {
> +	.name = "caps",
> +	.attrs = armv8_pmuv3_caps_attrs,
> +	.is_visible = armv8pmu_caps_attr_is_visible,
> +};
> +
>  /*
>   * Perf Events' indices
>   */
> @@ -940,14 +985,11 @@ static void __armv8pmu_probe_pmu(void *info)
>  {
>  	struct armv8pmu_probe_info *probe = info;
>  	struct arm_pmu *cpu_pmu = probe->pmu;
> -	u64 dfr0;
>  	u64 pmceid_raw[2];
>  	u32 pmceid[2];
>  	int pmuver;
>  
> -	dfr0 = read_sysreg(id_aa64dfr0_el1);
> -	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> -			ID_AA64DFR0_PMUVER_SHIFT);
> +	pmuver = armv8pmu_get_pmu_version();
>  	if (pmuver == 0xf || pmuver == 0)
>  		return;
>  
> @@ -994,7 +1036,8 @@ static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
>  static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  			  int (*map_event)(struct perf_event *event),
>  			  const struct attribute_group *events,
> -			  const struct attribute_group *format)
> +			  const struct attribute_group *format,
> +			  const struct attribute_group *caps)
>  {
>  	int ret = armv8pmu_probe_pmu(cpu_pmu);
>  	if (ret)
> @@ -1019,6 +1062,8 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  			events : &armv8_pmuv3_events_attr_group;
>  	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] = format ?
>  			format : &armv8_pmuv3_format_attr_group;
> +	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_CAPS] = caps ?
> +			caps : &armv8_pmuv3_caps_attr_group;
>  
>  	return 0;
>  }
> @@ -1026,97 +1071,97 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  static int armv8_pmuv3_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_pmuv3",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a34_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a34",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a35_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a35",
> -			      armv8_a53_map_event, NULL, NULL);
> +			      armv8_a53_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a53_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a53",
> -			      armv8_a53_map_event, NULL, NULL);
> +			      armv8_a53_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a55_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a55",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a57_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a57",
> -			      armv8_a57_map_event, NULL, NULL);
> +			      armv8_a57_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a65_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a65",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a72_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a72",
> -			      armv8_a57_map_event, NULL, NULL);
> +			      armv8_a57_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a73_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a73",
> -			      armv8_a73_map_event, NULL, NULL);
> +			      armv8_a73_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a75_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a75",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a76_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a76",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_a77_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a77",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_e1_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_neoverse_e1",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_n1_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_neoverse_n1",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_cavium_thunder",
> -			      armv8_thunder_map_event, NULL, NULL);
> +			      armv8_thunder_map_event, NULL, NULL, NULL);
>  }
>  
>  static int armv8_vulcan_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_brcm_vulcan",
> -			      armv8_vulcan_map_event, NULL, NULL);
> +			      armv8_vulcan_map_event, NULL, NULL, NULL);
>  }
>  
>  static const struct of_device_id armv8_pmu_of_device_ids[] = {
> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
> index 5b616dde9a4c..1e129b57d51a 100644
> --- a/include/linux/perf/arm_pmu.h
> +++ b/include/linux/perf/arm_pmu.h
> @@ -73,6 +73,7 @@ enum armpmu_attr_groups {
>  	ARMPMU_ATTR_GROUP_COMMON,
>  	ARMPMU_ATTR_GROUP_EVENTS,
>  	ARMPMU_ATTR_GROUP_FORMATS,
> +	ARMPMU_ATTR_GROUP_CAPS,
>  	ARMPMU_NR_ATTR_GROUPS
>  };
>  
>
Will Deacon July 20, 2020, 10:15 a.m. UTC | #2
On Thu, Jun 18, 2020 at 09:35:42PM +0800, Shaokun Zhang wrote:
> ARMv8.4-PMU introduces the PMMIR_EL1 registers and some new PMU events,
> like STALL_SLOT etc, are related to it. Let's add a caps directory to
> /sys/bus/event_source/devices/armv8_pmuv3_0/ and support slots from
> PMMIR_EL1 registers in this entry. The user programs can get the slots
> from sysfs directly.
> 
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
> ChangeLog in v3:
>     * Fix one typo in patch3
> 
> ChangeLog in v2:
>     * Add caps entry in sysfs
>     * Fix the PMU events typos
>     * Add one new patch to correct event ID in sysfs
> 
>  arch/arm64/include/asm/sysreg.h |  2 +
>  arch/arm64/kernel/perf_event.c  | 87 +++++++++++++++++++++++++++++++----------
>  include/linux/perf/arm_pmu.h    |  1 +
>  3 files changed, 69 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 463175f80341..56c45a9207c7 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -321,6 +321,8 @@
>  #define SYS_PMINTENSET_EL1		sys_reg(3, 0, 9, 14, 1)
>  #define SYS_PMINTENCLR_EL1		sys_reg(3, 0, 9, 14, 2)
>  
> +#define SYS_PMMIR_EL1			sys_reg(3, 0, 9, 14, 6)
> +
>  #define SYS_MAIR_EL1			sys_reg(3, 0, 10, 2, 0)
>  #define SYS_AMAIR_EL1			sys_reg(3, 0, 10, 3, 0)
>  
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 4d7879484cec..5f2ac87e4b91 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -277,6 +277,51 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
>  	.attrs = armv8_pmuv3_format_attrs,
>  };
>  
> +static inline int armv8pmu_get_pmu_version(void)
> +{

No need for 'inline' here.

> +	int pmuver;
> +	u64 dfr0;
> +
> +	dfr0 = read_sysreg(id_aa64dfr0_el1);
> +	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> +			ID_AA64DFR0_PMUVER_SHIFT);
> +
> +	return pmuver;
> +}
> +
> +static umode_t
> +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
> +			      int unused)
> +{
> +	int pmuver = armv8pmu_get_pmu_version();
> +
> +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
> +		return attr->mode;

Is this sufficient? I'm a bit confused by the text in the Arm ARM that says:

  | If ARMv8.4-PMU is implemented:
  | * If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether
  |   the PMMIR System registers are implemented.
  | * If STALL_SLOT is implemented, then the PMMIR System registers are
  |   implemented.

whereas the register description for PMMIR_EL1 says:

  | This register is present only when ARMv8.4-PMU is implemented.

Mark -- please could you clarify whether or not we need to check STALL_SLOT
as well as the PMUVer?

> +
> +	return 0;
> +}
> +
> +static ssize_t slots_show(struct device *dev, struct device_attribute *attr,
> +			  char *buf)
> +{
> +	int slots = read_sysreg_s(SYS_PMMIR_EL1) & 0xFF;
> +
> +	return snprintf(buf, PAGE_SIZE, "%d\n", slots);
> +}
> +
> +static DEVICE_ATTR_RO(slots);
> +
> +static struct attribute *armv8_pmuv3_caps_attrs[] = {
> +	&dev_attr_slots.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group armv8_pmuv3_caps_attr_group = {
> +	.name = "caps",
> +	.attrs = armv8_pmuv3_caps_attrs,
> +	.is_visible = armv8pmu_caps_attr_is_visible,
> +};
> +
>  /*
>   * Perf Events' indices
>   */
> @@ -940,14 +985,11 @@ static void __armv8pmu_probe_pmu(void *info)
>  {
>  	struct armv8pmu_probe_info *probe = info;
>  	struct arm_pmu *cpu_pmu = probe->pmu;
> -	u64 dfr0;
>  	u64 pmceid_raw[2];
>  	u32 pmceid[2];
>  	int pmuver;
>  
> -	dfr0 = read_sysreg(id_aa64dfr0_el1);
> -	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
> -			ID_AA64DFR0_PMUVER_SHIFT);
> +	pmuver = armv8pmu_get_pmu_version();
>  	if (pmuver == 0xf || pmuver == 0)
>  		return;
>  
> @@ -994,7 +1036,8 @@ static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
>  static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  			  int (*map_event)(struct perf_event *event),
>  			  const struct attribute_group *events,
> -			  const struct attribute_group *format)
> +			  const struct attribute_group *format,
> +			  const struct attribute_group *caps)
>  {
>  	int ret = armv8pmu_probe_pmu(cpu_pmu);
>  	if (ret)
> @@ -1019,6 +1062,8 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  			events : &armv8_pmuv3_events_attr_group;
>  	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] = format ?
>  			format : &armv8_pmuv3_format_attr_group;
> +	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_CAPS] = caps ?
> +			caps : &armv8_pmuv3_caps_attr_group;
>  
>  	return 0;
>  }
> @@ -1026,97 +1071,97 @@ static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
>  static int armv8_pmuv3_init(struct arm_pmu *cpu_pmu)
>  {
>  	return armv8_pmu_init(cpu_pmu, "armv8_pmuv3",
> -			      armv8_pmuv3_map_event, NULL, NULL);
> +			      armv8_pmuv3_map_event, NULL, NULL, NULL);

Maybe we should add:

static int armv8_pmu_init_nogroups(struct arm_pmu *cpu_pmu, char *name,
				   int (*map_event)(struct perf_event *event))
{
	return armv8_pmu_init(cpu_pmu, name, map_event, NULL, NULL, NULL);
}

and then update all these CPU initialisers to use that instead?

Will
Mark Rutland July 20, 2020, 10:50 a.m. UTC | #3
On Mon, Jul 20, 2020 at 11:15:19AM +0100, Will Deacon wrote:
> On Thu, Jun 18, 2020 at 09:35:42PM +0800, Shaokun Zhang wrote:

> > +static umode_t
> > +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
> > +			      int unused)
> > +{
> > +	int pmuver = armv8pmu_get_pmu_version();
> > +
> > +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
> > +		return attr->mode;
> 
> Is this sufficient? I'm a bit confused by the text in the Arm ARM that says:
> 
>   | If ARMv8.4-PMU is implemented:
>   | * If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether
>   |   the PMMIR System registers are implemented.
>   | * If STALL_SLOT is implemented, then the PMMIR System registers are
>   |   implemented.
> 
> whereas the register description for PMMIR_EL1 says:
> 
>   | This register is present only when ARMv8.4-PMU is implemented.

I think this is trying to say that when ARMv8.4-PMU is not implemented,
PMMIR definitely isn't implemented (i.e. the the presence of PMMIR_EL1
implies the presence of ARMv8.4-PMU).

> Mark -- please could you clarify whether or not we need to check STALL_SLOT
> as well as the PMUVer?

Given the explciit wording that it's IMP DEF, I suspect that we need to
check both.

I'll go chase this up.

Mark.
Will Deacon July 20, 2020, 10:54 a.m. UTC | #4
On Mon, Jul 20, 2020 at 11:50:19AM +0100, Mark Rutland wrote:
> On Mon, Jul 20, 2020 at 11:15:19AM +0100, Will Deacon wrote:
> > On Thu, Jun 18, 2020 at 09:35:42PM +0800, Shaokun Zhang wrote:
> 
> > > +static umode_t
> > > +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
> > > +			      int unused)
> > > +{
> > > +	int pmuver = armv8pmu_get_pmu_version();
> > > +
> > > +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
> > > +		return attr->mode;
> > 
> > Is this sufficient? I'm a bit confused by the text in the Arm ARM that says:
> > 
> >   | If ARMv8.4-PMU is implemented:
> >   | * If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether
> >   |   the PMMIR System registers are implemented.
> >   | * If STALL_SLOT is implemented, then the PMMIR System registers are
> >   |   implemented.
> > 
> > whereas the register description for PMMIR_EL1 says:
> > 
> >   | This register is present only when ARMv8.4-PMU is implemented.
> 
> I think this is trying to say that when ARMv8.4-PMU is not implemented,
> PMMIR definitely isn't implemented (i.e. the the presence of PMMIR_EL1
> implies the presence of ARMv8.4-PMU).
> 
> > Mark -- please could you clarify whether or not we need to check STALL_SLOT
> > as well as the PMUVer?
> 
> Given the explciit wording that it's IMP DEF, I suspect that we need to
> check both.
> 
> I'll go chase this up.

Thanks. In the meantime, Shaokun, can you send updated versions of the first
two patches, please? We'll play it safe and check STALL_SLOT as well pending
Mark's findings.

Will
Shaokun Zhang July 20, 2020, 1:11 p.m. UTC | #5
Hi Will,

在 2020/7/20 18:54, Will Deacon 写道:
> On Mon, Jul 20, 2020 at 11:50:19AM +0100, Mark Rutland wrote:
>> On Mon, Jul 20, 2020 at 11:15:19AM +0100, Will Deacon wrote:
>>> On Thu, Jun 18, 2020 at 09:35:42PM +0800, Shaokun Zhang wrote:
>>
>>>> +static umode_t
>>>> +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
>>>> +			      int unused)
>>>> +{
>>>> +	int pmuver = armv8pmu_get_pmu_version();
>>>> +
>>>> +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
>>>> +		return attr->mode;
>>>
>>> Is this sufficient? I'm a bit confused by the text in the Arm ARM that says:
>>>
>>>   | If ARMv8.4-PMU is implemented:
>>>   | * If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether
>>>   |   the PMMIR System registers are implemented.
>>>   | * If STALL_SLOT is implemented, then the PMMIR System registers are
>>>   |   implemented.
>>>
>>> whereas the register description for PMMIR_EL1 says:
>>>
>>>   | This register is present only when ARMv8.4-PMU is implemented.
>>
>> I think this is trying to say that when ARMv8.4-PMU is not implemented,
>> PMMIR definitely isn't implemented (i.e. the the presence of PMMIR_EL1
>> implies the presence of ARMv8.4-PMU).
>>
>>> Mark -- please could you clarify whether or not we need to check STALL_SLOT
>>> as well as the PMUVer?
>>
>> Given the explciit wording that it's IMP DEF, I suspect that we need to
>> check both.
>>
>> I'll go chase this up.
> 
> Thanks. In the meantime, Shaokun, can you send updated versions of the first
> two patches, please? We'll play it safe and check STALL_SLOT as well pending

Ok, I will submit them soon addressed your other comments together.

Thanks,
Shaokun

> Mark's findings.
> 
> Will
> 
> .
>
Shaokun Zhang July 21, 2020, 8:05 a.m. UTC | #6
Hi Will/Mark,

在 2020/7/20 18:54, Will Deacon 写道:
> On Mon, Jul 20, 2020 at 11:50:19AM +0100, Mark Rutland wrote:
>> On Mon, Jul 20, 2020 at 11:15:19AM +0100, Will Deacon wrote:
>>> On Thu, Jun 18, 2020 at 09:35:42PM +0800, Shaokun Zhang wrote:
>>
>>>> +static umode_t
>>>> +armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
>>>> +			      int unused)
>>>> +{
>>>> +	int pmuver = armv8pmu_get_pmu_version();
>>>> +
>>>> +	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
>>>> +		return attr->mode;
>>>
>>> Is this sufficient? I'm a bit confused by the text in the Arm ARM that says:
>>>
>>>   | If ARMv8.4-PMU is implemented:
>>>   | * If STALL_SLOT is not implemented, it is IMPLEMENTATION DEFINED whether
>>>   |   the PMMIR System registers are implemented.
>>>   | * If STALL_SLOT is implemented, then the PMMIR System registers are
>>>   |   implemented.
>>>
>>> whereas the register description for PMMIR_EL1 says:
>>>
>>>   | This register is present only when ARMv8.4-PMU is implemented.
>>
>> I think this is trying to say that when ARMv8.4-PMU is not implemented,
>> PMMIR definitely isn't implemented (i.e. the the presence of PMMIR_EL1
>> implies the presence of ARMv8.4-PMU).
>>
>>> Mark -- please could you clarify whether or not we need to check STALL_SLOT
>>> as well as the PMUVer?
>>
>> Given the explciit wording that it's IMP DEF, I suspect that we need to
>> check both.
>>
>> I'll go chase this up.
> 
> Thanks. In the meantime, Shaokun, can you send updated versions of the first
> two patches, please? We'll play it safe and check STALL_SLOT as well pending

In Arm ARM for PMMIR_EL1:

SLOTS, bits [7:0]
	Operation width. The largest value by which the STALL_SLOT event might increment by in a
	single cycle. If the STALL_SLOT event is not implemented, this field might read as zero.

And it's IMP DEF, even if the STALL_SLOT event is not implemented, and it can take
the SLOTS value and we can display it in sysfs if I follow it correctly.

Thanks,
Shaokun

> Mark's findings.
> 
> Will
> 
> .
>
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 463175f80341..56c45a9207c7 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -321,6 +321,8 @@ 
 #define SYS_PMINTENSET_EL1		sys_reg(3, 0, 9, 14, 1)
 #define SYS_PMINTENCLR_EL1		sys_reg(3, 0, 9, 14, 2)
 
+#define SYS_PMMIR_EL1			sys_reg(3, 0, 9, 14, 6)
+
 #define SYS_MAIR_EL1			sys_reg(3, 0, 10, 2, 0)
 #define SYS_AMAIR_EL1			sys_reg(3, 0, 10, 3, 0)
 
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 4d7879484cec..5f2ac87e4b91 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -277,6 +277,51 @@  static struct attribute_group armv8_pmuv3_format_attr_group = {
 	.attrs = armv8_pmuv3_format_attrs,
 };
 
+static inline int armv8pmu_get_pmu_version(void)
+{
+	int pmuver;
+	u64 dfr0;
+
+	dfr0 = read_sysreg(id_aa64dfr0_el1);
+	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
+			ID_AA64DFR0_PMUVER_SHIFT);
+
+	return pmuver;
+}
+
+static umode_t
+armv8pmu_caps_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+			      int unused)
+{
+	int pmuver = armv8pmu_get_pmu_version();
+
+	if (pmuver >= ID_AA64DFR0_PMUVER_8_4)
+		return attr->mode;
+
+	return 0;
+}
+
+static ssize_t slots_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	int slots = read_sysreg_s(SYS_PMMIR_EL1) & 0xFF;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", slots);
+}
+
+static DEVICE_ATTR_RO(slots);
+
+static struct attribute *armv8_pmuv3_caps_attrs[] = {
+	&dev_attr_slots.attr,
+	NULL,
+};
+
+static struct attribute_group armv8_pmuv3_caps_attr_group = {
+	.name = "caps",
+	.attrs = armv8_pmuv3_caps_attrs,
+	.is_visible = armv8pmu_caps_attr_is_visible,
+};
+
 /*
  * Perf Events' indices
  */
@@ -940,14 +985,11 @@  static void __armv8pmu_probe_pmu(void *info)
 {
 	struct armv8pmu_probe_info *probe = info;
 	struct arm_pmu *cpu_pmu = probe->pmu;
-	u64 dfr0;
 	u64 pmceid_raw[2];
 	u32 pmceid[2];
 	int pmuver;
 
-	dfr0 = read_sysreg(id_aa64dfr0_el1);
-	pmuver = cpuid_feature_extract_unsigned_field(dfr0,
-			ID_AA64DFR0_PMUVER_SHIFT);
+	pmuver = armv8pmu_get_pmu_version();
 	if (pmuver == 0xf || pmuver == 0)
 		return;
 
@@ -994,7 +1036,8 @@  static int armv8pmu_probe_pmu(struct arm_pmu *cpu_pmu)
 static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
 			  int (*map_event)(struct perf_event *event),
 			  const struct attribute_group *events,
-			  const struct attribute_group *format)
+			  const struct attribute_group *format,
+			  const struct attribute_group *caps)
 {
 	int ret = armv8pmu_probe_pmu(cpu_pmu);
 	if (ret)
@@ -1019,6 +1062,8 @@  static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
 			events : &armv8_pmuv3_events_attr_group;
 	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_FORMATS] = format ?
 			format : &armv8_pmuv3_format_attr_group;
+	cpu_pmu->attr_groups[ARMPMU_ATTR_GROUP_CAPS] = caps ?
+			caps : &armv8_pmuv3_caps_attr_group;
 
 	return 0;
 }
@@ -1026,97 +1071,97 @@  static int armv8_pmu_init(struct arm_pmu *cpu_pmu, char *name,
 static int armv8_pmuv3_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_pmuv3",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a34_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a34",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a35_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a35",
-			      armv8_a53_map_event, NULL, NULL);
+			      armv8_a53_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a53_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a53",
-			      armv8_a53_map_event, NULL, NULL);
+			      armv8_a53_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a55_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a55",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a57_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a57",
-			      armv8_a57_map_event, NULL, NULL);
+			      armv8_a57_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a65_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a65",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a72_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a72",
-			      armv8_a57_map_event, NULL, NULL);
+			      armv8_a57_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a73_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a73",
-			      armv8_a73_map_event, NULL, NULL);
+			      armv8_a73_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a75_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a75",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a76_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a76",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_a77_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cortex_a77",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_e1_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_neoverse_e1",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_n1_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_neoverse_n1",
-			      armv8_pmuv3_map_event, NULL, NULL);
+			      armv8_pmuv3_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_thunder_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_cavium_thunder",
-			      armv8_thunder_map_event, NULL, NULL);
+			      armv8_thunder_map_event, NULL, NULL, NULL);
 }
 
 static int armv8_vulcan_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return armv8_pmu_init(cpu_pmu, "armv8_brcm_vulcan",
-			      armv8_vulcan_map_event, NULL, NULL);
+			      armv8_vulcan_map_event, NULL, NULL, NULL);
 }
 
 static const struct of_device_id armv8_pmu_of_device_ids[] = {
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 5b616dde9a4c..1e129b57d51a 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -73,6 +73,7 @@  enum armpmu_attr_groups {
 	ARMPMU_ATTR_GROUP_COMMON,
 	ARMPMU_ATTR_GROUP_EVENTS,
 	ARMPMU_ATTR_GROUP_FORMATS,
+	ARMPMU_ATTR_GROUP_CAPS,
 	ARMPMU_NR_ATTR_GROUPS
 };