Messages in this thread | | | Subject | Re: [RESEND PATCH V7 2/2] drivers/perf: hisi: add driver for HNS3 PMU | From | "huangguangbin (A)" <> | Date | Mon, 27 Jun 2022 22:31:54 +0800 |
| |
On 2022/6/27 18:52, Will Deacon wrote: > On Wed, May 25, 2022 at 08:52:11AM +0800, Guangbin Huang wrote: >> HNS3(HiSilicon Network System 3) PMU is RCiEP device in HiSilicon SoC NIC, >> supports collection of performance statistics such as bandwidth, latency, >> packet rate and interrupt rate. >> >> NIC of each SICL has one PMU device for it. Driver registers each PMU >> device to perf, and exports information of supported events, filter mode of >> each event, bdf range, hardware clock frequency, identifier and so on via >> sysfs. >> >> Each PMU device has its own registers of control, counters and interrupt, >> and it supports 8 hardware events, each hardward event has its own >> registers for configuration, counters and interrupt. >> >> Filter options contains: >> event - select event >> port - select physical port of nic >> tc - select tc(must be used with port) >> func - select PF/VF >> queue - select queue of PF/VF(must be used with func) >> intr - select interrupt number(must be used with func) >> global - select all functions of IO DIE >> >> Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com> >> Reviewed-by: John Garry <john.garry@huawei.com> >> Reviewed-by: Shaokun Zhang <zhangshaokun@hisilicon.com> >> --- >> MAINTAINERS | 6 + >> drivers/perf/hisilicon/Kconfig | 10 + >> drivers/perf/hisilicon/Makefile | 1 + >> drivers/perf/hisilicon/hns3_pmu.c | 1662 +++++++++++++++++++++++++++++ >> include/linux/cpuhotplug.h | 1 + >> 5 files changed, 1680 insertions(+) >> create mode 100644 drivers/perf/hisilicon/hns3_pmu.c > > This mostly looks good to me, but I have one niggling concern with the ABI: > >> +#define HNS3_PMU_FILTER_ATTR(_name, _config, _start, _end) \ >> + static inline u64 hns3_pmu_get_##_name(struct perf_event *event) \ >> + { \ >> + return FIELD_GET(GENMASK_ULL(_end, _start), \ >> + event->attr._config); \ >> + } >> + >> +HNS3_PMU_FILTER_ATTR(event, config, 0, 16); >> +HNS3_PMU_FILTER_ATTR(subevent, config, 0, 7); >> +HNS3_PMU_FILTER_ATTR(event_type, config, 8, 15); >> +HNS3_PMU_FILTER_ATTR(ext_counter_used, config, 16, 16); >> +HNS3_PMU_FILTER_ATTR(real_event, config, 0, 15); > > How does perf tool deal with overlapping fields like this? It seems like > quite a bad idea to allow things like "event=0xffff,subevent=0" when they > are no longer distinct and I don't _think_ any other drivers do this. > > Can you remove 'event' and 'real_event' for now, or are they needed? > > Will > . > Ok, I will modify this part.
| |