| Message ID | 1621581944-17381-1-git-send-email-zhangshaokun@hisilicon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: perf: Add more support on caps under sysfs | expand |
On 2021-05-21 08:25, Shaokun Zhang wrote: > Armv8.7 has introduced BUS_SLOTS and BUS_WIDTH in PMMIR_EL1 register, > add two entries in caps for bus_slots and bus_width under sysfs. It > will return the true slots and width if the information is available, > otherwise it will return 0. > > Cc: Will Deacon <will@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com> > --- > arch/arm64/include/asm/perf_event.h | 5 +++ > arch/arm64/kernel/perf_event.c | 64 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 69 insertions(+) > > diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h > index 60731f602d3e..4ef6f19331f9 100644 > --- a/arch/arm64/include/asm/perf_event.h > +++ b/arch/arm64/include/asm/perf_event.h > @@ -239,6 +239,11 @@ > /* PMMIR_EL1.SLOTS mask */ > #define ARMV8_PMU_SLOTS_MASK 0xff > > +#define ARMV8_PMU_BUS_SLOTS_SHIFT 8 > +#define ARMV8_PMU_BUS_SLOTS_MASK 0xff > +#define ARMV8_PMU_BUS_WIDTH_SHIFT 16 > +#define ARMV8_PMU_BUS_WIDTH_MASK 0xf > + > #ifdef CONFIG_PERF_EVENTS > struct pt_regs; > extern unsigned long perf_instruction_pointer(struct pt_regs *regs); > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c > index f594957e29bd..f0847e4f48a9 100644 > --- a/arch/arm64/kernel/perf_event.c > +++ b/arch/arm64/kernel/perf_event.c > @@ -317,8 +317,72 @@ static ssize_t slots_show(struct device *dev, struct device_attribute *attr, > > static DEVICE_ATTR_RO(slots); > > +static ssize_t bus_slots_show(struct device *dev, struct device_attribute *attr, > + char *page) > +{ > + struct pmu *pmu = dev_get_drvdata(dev); > + struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu); > + u32 bus_slots = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_SLOTS_SHIFT) > + & ARMV8_PMU_BUS_SLOTS_MASK; > + > + return snprintf(page, PAGE_SIZE, "0x%08x\n", bus_slots); > +} > + > +static DEVICE_ATTR_RO(bus_slots); > + > +static ssize_t bus_width_show(struct device *dev, struct device_attribute *attr, > + char *page) > +{ > + struct pmu *pmu = dev_get_drvdata(dev); > + struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu); > + u32 bus_width = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_WIDTH_SHIFT) > + & ARMV8_PMU_BUS_WIDTH_MASK; > + u32 val; > + > + switch (bus_width) { > + case 3: > + val = 4; > + break; > + case 4: > + val = 8; > + break; > + case 5: > + val = 16; > + break; > + case 6: > + val = 32; > + break; > + case 7: > + val = 64; > + break; > + case 8: > + val = 128; > + break; > + case 9: > + val = 256; > + break; > + case 10: > + val = 512; > + break; > + case 11: > + val = 1024; > + break; > + case 12: > + val = 2048; > + break; Nit: the encoding is explicitly defined as "log2(number of bytes) plus one", so this might be neater as simply: val = 1 << (bus_width - 1); I'm not really sure whether interpreting reserved values as 0 is any better or worse than interpreting them as implied :/ > + default: > + val = 0; > + } If the information is not available, wouldn't it make sense to just hide the attribute(s) entirely? Userspace should already have to cope with older kernels, so from that point of view it seems easier to only have two states of "not present" and "present and valid", without having to also consider a third "present but invalid" state. Robin. > + > + return snprintf(page, PAGE_SIZE, "0x%08x\n", val); > +} > + > +static DEVICE_ATTR_RO(bus_width); > + > static struct attribute *armv8_pmuv3_caps_attrs[] = { > &dev_attr_slots.attr, > + &dev_attr_bus_slots.attr, > + &dev_attr_bus_width.attr, > NULL, > }; > >
Hi Robin, On 2021/5/25 1:18, Robin Murphy wrote: > On 2021-05-21 08:25, Shaokun Zhang wrote: >> Armv8.7 has introduced BUS_SLOTS and BUS_WIDTH in PMMIR_EL1 register, >> add two entries in caps for bus_slots and bus_width under sysfs. It >> will return the true slots and width if the information is available, >> otherwise it will return 0. >> >> Cc: Will Deacon <will@kernel.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com> >> --- >> arch/arm64/include/asm/perf_event.h | 5 +++ >> arch/arm64/kernel/perf_event.c | 64 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 69 insertions(+) >> >> diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h >> index 60731f602d3e..4ef6f19331f9 100644 >> --- a/arch/arm64/include/asm/perf_event.h >> +++ b/arch/arm64/include/asm/perf_event.h >> @@ -239,6 +239,11 @@ >> /* PMMIR_EL1.SLOTS mask */ >> #define ARMV8_PMU_SLOTS_MASK 0xff >> +#define ARMV8_PMU_BUS_SLOTS_SHIFT 8 >> +#define ARMV8_PMU_BUS_SLOTS_MASK 0xff >> +#define ARMV8_PMU_BUS_WIDTH_SHIFT 16 >> +#define ARMV8_PMU_BUS_WIDTH_MASK 0xf >> + >> #ifdef CONFIG_PERF_EVENTS >> struct pt_regs; >> extern unsigned long perf_instruction_pointer(struct pt_regs *regs); >> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c >> index f594957e29bd..f0847e4f48a9 100644 >> --- a/arch/arm64/kernel/perf_event.c >> +++ b/arch/arm64/kernel/perf_event.c >> @@ -317,8 +317,72 @@ static ssize_t slots_show(struct device *dev, struct device_attribute *attr, >> static DEVICE_ATTR_RO(slots); >> +static ssize_t bus_slots_show(struct device *dev, struct device_attribute *attr, >> + char *page) >> +{ >> + struct pmu *pmu = dev_get_drvdata(dev); >> + struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu); >> + u32 bus_slots = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_SLOTS_SHIFT) >> + & ARMV8_PMU_BUS_SLOTS_MASK; >> + >> + return snprintf(page, PAGE_SIZE, "0x%08x\n", bus_slots); >> +} >> + >> +static DEVICE_ATTR_RO(bus_slots); >> + >> +static ssize_t bus_width_show(struct device *dev, struct device_attribute *attr, >> + char *page) >> +{ >> + struct pmu *pmu = dev_get_drvdata(dev); >> + struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu); >> + u32 bus_width = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_WIDTH_SHIFT) >> + & ARMV8_PMU_BUS_WIDTH_MASK; >> + u32 val; >> + >> + switch (bus_width) { >> + case 3: >> + val = 4; >> + break; >> + case 4: >> + val = 8; >> + break; >> + case 5: >> + val = 16; >> + break; >> + case 6: >> + val = 32; >> + break; >> + case 7: >> + val = 64; >> + break; >> + case 8: >> + val = 128; >> + break; >> + case 9: >> + val = 256; >> + break; >> + case 10: >> + val = 512; >> + break; >> + case 11: >> + val = 1024; >> + break; >> + case 12: >> + val = 2048; >> + break; > > Nit: the encoding is explicitly defined as "log2(number of bytes) plus one", so this might be neater > as simply: > > val = 1 << (bus_width - 1); > Good catch, I will follow it in next version. > I'm not really sure whether interpreting reserved values as 0 is any better or worse than > interpreting them as implied :/ To be honest, I'm also not sure about it and I checked it in the document that For BUS_WIDTH and BUS_SLOTS, if it is not from Armv8.7, it shall be Reserved, RAZ and I returned 0 including other values from Armv8.7. > >> + default: >> + val = 0; >> + } > > If the information is not available, wouldn't it make sense to just hide the attribute(s) entirely? > Userspace should already have to cope with older kernels, so from that point of view it seems easier > to only have two states of "not present" and "present and valid", without having to also consider a > third "present but invalid" state. When talked about SLOTS field [1], Will suggested to get rid of visible hook and return zero if the CPUs without it. [1]https://www.spinics.net/lists/arm-kernel/msg824289.html Thanks, Shaokun > > Robin. > >> + >> + return snprintf(page, PAGE_SIZE, "0x%08x\n", val); >> +} >> + >> +static DEVICE_ATTR_RO(bus_width); >> + >> static struct attribute *armv8_pmuv3_caps_attrs[] = { >> &dev_attr_slots.attr, >> + &dev_attr_bus_slots.attr, >> + &dev_attr_bus_width.attr, >> NULL, >> }; >> > .
diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h index 60731f602d3e..4ef6f19331f9 100644 --- a/arch/arm64/include/asm/perf_event.h +++ b/arch/arm64/include/asm/perf_event.h @@ -239,6 +239,11 @@ /* PMMIR_EL1.SLOTS mask */ #define ARMV8_PMU_SLOTS_MASK 0xff +#define ARMV8_PMU_BUS_SLOTS_SHIFT 8 +#define ARMV8_PMU_BUS_SLOTS_MASK 0xff +#define ARMV8_PMU_BUS_WIDTH_SHIFT 16 +#define ARMV8_PMU_BUS_WIDTH_MASK 0xf + #ifdef CONFIG_PERF_EVENTS struct pt_regs; extern unsigned long perf_instruction_pointer(struct pt_regs *regs); diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c index f594957e29bd..f0847e4f48a9 100644 --- a/arch/arm64/kernel/perf_event.c +++ b/arch/arm64/kernel/perf_event.c @@ -317,8 +317,72 @@ static ssize_t slots_show(struct device *dev, struct device_attribute *attr, static DEVICE_ATTR_RO(slots); +static ssize_t bus_slots_show(struct device *dev, struct device_attribute *attr, + char *page) +{ + struct pmu *pmu = dev_get_drvdata(dev); + struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu); + u32 bus_slots = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_SLOTS_SHIFT) + & ARMV8_PMU_BUS_SLOTS_MASK; + + return snprintf(page, PAGE_SIZE, "0x%08x\n", bus_slots); +} + +static DEVICE_ATTR_RO(bus_slots); + +static ssize_t bus_width_show(struct device *dev, struct device_attribute *attr, + char *page) +{ + struct pmu *pmu = dev_get_drvdata(dev); + struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu); + u32 bus_width = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_WIDTH_SHIFT) + & ARMV8_PMU_BUS_WIDTH_MASK; + u32 val; + + switch (bus_width) { + case 3: + val = 4; + break; + case 4: + val = 8; + break; + case 5: + val = 16; + break; + case 6: + val = 32; + break; + case 7: + val = 64; + break; + case 8: + val = 128; + break; + case 9: + val = 256; + break; + case 10: + val = 512; + break; + case 11: + val = 1024; + break; + case 12: + val = 2048; + break; + default: + val = 0; + } + + return snprintf(page, PAGE_SIZE, "0x%08x\n", val); +} + +static DEVICE_ATTR_RO(bus_width); + static struct attribute *armv8_pmuv3_caps_attrs[] = { &dev_attr_slots.attr, + &dev_attr_bus_slots.attr, + &dev_attr_bus_width.attr, NULL, };
Armv8.7 has introduced BUS_SLOTS and BUS_WIDTH in PMMIR_EL1 register, add two entries in caps for bus_slots and bus_width under sysfs. It will return the true slots and width if the information is available, otherwise it will return 0. Cc: Will Deacon <will@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com> --- arch/arm64/include/asm/perf_event.h | 5 +++ arch/arm64/kernel/perf_event.c | 64 +++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+)