[RFC PATCH 2/2] {topost} drivers/perf: hisi: add driver for HNS3 PMU

John Garry john.garry at huawei.com
Thu Mar 25 15:53:42 GMT 2021


On 22/03/2021 01:49, Guangbin Huang wrote:
> HNS3 PMU End Point device supports to collect performance statistics
> of bandwidth, latency, packet rate, interrupt rate in HiSilicon SoC
> NIC.
> 

I tried to apply this patch as an experiment, but it does not apply 
cleanly on v5.12-rc4 or v5.11. What is the baseline?

> NIC of each IO DIE has one PMU device for it. Driver registers each
> PMU device to perf, and exports information of supported events,
> filter mode of each event, identifier and so on via sysfs.
> 
> Each PMU device has its own control, counter and interrupt registers,
> and supports up to 8 events by hardware.
> 
> Filter options contains:
> event        - select event
> subevent     - select subevent
> 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 at huawei.com>
> ---
>   MAINTAINERS                           |    5 +
>   drivers/perf/pci/Kconfig              |   10 +
>   drivers/perf/pci/hisilicon/Makefile   |    1 +
>   drivers/perf/pci/hisilicon/hns3_pmu.c | 1937 +++++++++++++++++++++++++++++++++
>   include/linux/cpuhotplug.h            |    1 +
>   5 files changed, 1954 insertions(+)
>   create mode 100644 drivers/perf/pci/hisilicon/hns3_pmu.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3353de0..9a94395 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8023,6 +8023,11 @@ W:	http://www.hisilicon.com
>   F:	Documentation/admin-guide/perf/hisi-pmu.rst
>   F:	drivers/perf/hisilicon
>   
> +HISILICON HNS3 PMU DRIVER
> +M:	Guangbin Huang <huangguangbin2 at huawei.com>
> +S:	Supported
> +F:	drivers/perf/pci/hisilicon/hns3_pmu.c

You seemed to miss the rst doc

> +
>   HISILICON QM AND ZIP Controller DRIVER
>   M:	Zhou Wang <wangzhou1 at hisilicon.com>
>   L:	linux-crypto at vger.kernel.org
> diff --git a/drivers/perf/pci/Kconfig b/drivers/perf/pci/Kconfig
> index 6fd7978..ff10054 100644
> --- a/drivers/perf/pci/Kconfig
> +++ b/drivers/perf/pci/Kconfig
> @@ -13,4 +13,14 @@ config HISI_PCIE_PMU
>   	  IEP devices.
>   	  Adds the PCIe PMU into perf events system for monitoring latency,
>   	  bandwidth etc.
> +
> +config HNS3_PMU
> +	tristate "HNS3 PERF PMU"
> +	depends on ARM64 && PCI

COMPILE_TEST?

> +	default m

I don't think that this should be added

> +	help
> +	  Provide support for HNS3 performance monitoring unit (PMU) IEP
> +	  devices.
> +	  Adds the HNS3 PMU into perf events system for monitoring latency,
> +	  bandwidth etc.
>   endmenu
> diff --git a/drivers/perf/pci/hisilicon/Makefile b/drivers/perf/pci/hisilicon/Makefile
> index 37813b6..e6b89d8 100644
> --- a/drivers/perf/pci/hisilicon/Makefile
> +++ b/drivers/perf/pci/hisilicon/Makefile
> @@ -1,2 +1,3 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   obj-$(CONFIG_HISI_PCIE_PMU) += hisi_pcie_pmu.o
> +obj-$(CONFIG_HNS3_PMU) += hns3_pmu.o
> diff --git a/drivers/perf/pci/hisilicon/hns3_pmu.c b/drivers/perf/pci/hisilicon/hns3_pmu.c
> new file mode 100644
> index 0000000..f840549
> --- /dev/null
> +++ b/drivers/perf/pci/hisilicon/hns3_pmu.c
> @@ -0,0 +1,1937 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * This driver adds support for HNS3 PMU IEP device. Related perf events are

iEP

> + * bandwidth, latency, packet rate, interrupt rate etc.
> + *
> + * Copyright (C) 2021 HiSilicon Limited
> + */
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/bug.h>
> +#include <linux/cpuhotplug.h>
> +#include <linux/cpumask.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/perf_event.h>
> +#include <linux/smp.h>
> +
> +#define BAR_ID					2
> +
> +#define HNS3_PMU_DEVICE_ID			0xA22B

personally I don't see a point in declaring a macro for a value which is 
only used once. And it's not a magic number.

> +#define HNS3_IRQ_VECTOR				0
> +#define HNS3_IRQ_VECS				1
> +
> +/* max event counters supported */
> +#define HNS3_PMU_MAX_COUNTERS			8
> +
> +/* registers offset address */
> +#define HNS3_PMU_REG_GLOBAL_CTRL		0x0000
> +#define HNS3_PMU_REG_CLOCK_FREQ			0x0020
> +#define HNS3_PMU_REG_BDF			0x0FE0
> +#define HNS3_PMU_REG_VERSION			0x0FE4
> +#define HNS3_PMU_REG_DEVICE_ID			0x0FE8
> +
> +#define HNS3_PMU_REG_EVENT_OFFSET		0x1000
> +#define HNS3_PMU_REG_EVENT_SIZE			0x1000
> +#define HNS3_PMU_REG_EVENT_CTRL_LOW		0x00
> +#define HNS3_PMU_REG_EVENT_CTRL_HIGH		0x04
> +#define HNS3_PMU_REG_EVENT_INTR_STATUS		0x08
> +#define HNS3_PMU_REG_EVENT_INTR_MASK		0x0C
> +#define HNS3_PMU_REG_EVENT_COUNTER		0x10
> +#define HNS3_PMU_REG_EVENT_EXT_COUNTER		0x18
> +#define HNS3_PMU_REG_EVENT_QID_CTRL		0x28
> +#define HNS3_PMU_REG_EVENT_QID_PARA		0x2C
> +
> +#define HNS3_PMU_FILTER_SUPPORT_GLOBAL		BIT(0)
> +#define HNS3_PMU_FILTER_SUPPORT_PORT		BIT(1)
> +#define HNS3_PMU_FILTER_SUPPORT_PORT_TC		BIT(2)
> +#define HNS3_PMU_FILTER_SUPPORT_FUNC		BIT(3)
> +#define HNS3_PMU_FILTER_SUPPORT_FUNC_QUEUE	BIT(4)
> +#define HNS3_PMU_FILTER_SUPPORT_FUNC_INTR	BIT(5)
> +
> +#define HNS3_PMU_FILTER_ALL_TC			0xF
> +#define HNS3_PMU_FILTER_ALL_QUEUE		0xFFFF
> +
> +#define HNS3_PMU_CTRL_SUBEVENT_S		4
> +#define HNS3_PMU_CTRL_FILTER_MODE_S		24
> +
> +#define HNS3_PMU_GLOBAL_START			BIT(0)
> +
> +#define HNS3_PMU_EVENT_STATUS_RESET		BIT(11)
> +#define HNS3_PMU_EVENT_EN			BIT(12)
> +#define HNS3_PMU_EVENT_OVERFLOW_RESTART		BIT(15)
> +
> +#define HNS3_PMU_QID_PARA_FUNC_S		0
> +#define HNS3_PMU_QID_PARA_QUEUE_S		16
> +
> +#define HNS3_PMU_QID_CTRL_REQ_ENABLE		BIT(0)
> +#define HNS3_PMU_QID_CTRL_DONE			BIT(1)
> +#define HNS3_PMU_QID_CTRL_MISS			BIT(2)
> +
> +#define HNS3_PMU_INTR_MASK_OVERFLOW		BIT(1)
> +
> +#define HNS3_PMU_FREQ_1K_HZ			1000
> +#define HNS3_PMU_FREQ_1G_HZ			1000000000

really?


> +
> +#define HNS3_PMU_INVALID_CPU_ID			(-1)
> +
> +enum hns3_pmu_event_type {
> +	HNS3_PMU_EVENT_TYPE_BANDWIDTH,
> +	HNS3_PMU_EVENT_TYPE_PACKET_RATE,
> +	HNS3_PMU_EVENT_TYPE_LATENCY,
> +	HNS3_PMU_EVENT_TYPE_INTR_RATE,
> +};
> +
> +enum hns3_pmu_bandwidth_subevent_code {
> +	HNS3_PMU_SUBEVT_BW_IGU_SSU,
> +	HNS3_PMU_SUBEVT_BW_SSU_EGU,
> +	HNS3_PMU_SUBEVT_BW_SSU_RPU,
> +	HNS3_PMU_SUBEVT_BW_SSU_ROCE,

is it intended that sometimes we have _BW_XXX_SSU and sometimes _BW_SSU_YYY?

> +	HNS3_PMU_SUBEVT_BW_ROCE_SSU,
> +	HNS3_PMU_SUBEVT_BW_TPU_SSU,
> +	HNS3_PMU_SUBEVT_BW_RPU_RCBRX,
> +	HNS3_PMU_SUBEVT_BW_RCBTX_TPU,
> +	HNS3_PMU_SUBEVT_BW_RCBTX_TXSCH,
> +	HNS3_PMU_SUBEVT_BW_WR_FBD,
> +	HNS3_PMU_SUBEVT_BW_WR_EBD,
> +	HNS3_PMU_SUBEVT_BW_RD_FBD,
> +	HNS3_PMU_SUBEVT_BW_RD_EBD,
> +	HNS3_PMU_SUBEVT_BW_RD_PAY_M0,
> +	HNS3_PMU_SUBEVT_BW_RD_PAY_M1,
> +	HNS3_PMU_SUBEVT_BW_WR_PAY_M0,
> +	HNS3_PMU_SUBEVT_BW_WR_PAY_M1,
> +};
> +
> +enum hns3_pmu_packet_rate_subevent_code {
> +	HNS3_PMU_SUBEVT_PPS_IGU_SSU,
> +	HNS3_PMU_SUBEVT_PPS_SSU_EGU,
> +	HNS3_PMU_SUBEVT_PPS_SSU_RPU,
> +	HNS3_PMU_SUBEVT_PPS_SSU_ROCE,
> +	HNS3_PMU_SUBEVT_PPS_ROCE_SSU,
> +	HNS3_PMU_SUBEVT_PPS_TPU_SSU,
> +	HNS3_PMU_SUBEVT_PPS_RPU_RCBRX,
> +	HNS3_PMU_SUBEVT_PPS_RCBTX_TPU,
> +	HNS3_PMU_SUBEVT_PPS_RCBTX_SCH,
> +	HNS3_PMU_SUBEVT_PPS_WR_FBD,
> +	HNS3_PMU_SUBEVT_PPS_WR_EBD,
> +	HNS3_PMU_SUBEVT_PPS_RD_FBD,
> +	HNS3_PMU_SUBEVT_PPS_RD_EBD,
> +	HNS3_PMU_SUBEVT_PPS_RD_PAY_M0,
> +	HNS3_PMU_SUBEVT_PPS_RD_PAY_M1,
> +	HNS3_PMU_SUBEVT_PPS_WR_PAY_M0,
> +	HNS3_PMU_SUBEVT_PPS_WR_PAY_M1,
> +	HNS3_PMU_SUBEVT_PPS_TX_PRE,
> +	HNS3_PMU_SUBEVT_PPS_RX_PRE,
> +};
> +
> +enum hns3_pmu_latency_subevent_code {
> +	HNS3_PMU_SUBEVT_DLY_RX_INTR,
> +	HNS3_PMU_SUBEVT_DLY_RX_INTR_ROH,
> +	HNS3_PMU_SUBEVT_DLY_TX_PUSH,
> +	HNS3_PMU_SUBEVT_DLY_TX_PUSH_ROH,
> +	HNS3_PMU_SUBEVT_DLY_TX,
> +	HNS3_PMU_SUBEVT_DLY_TX_ROH,
> +	HNS3_PMU_SUBEVT_DLY_SSU_TX_NIC,
> +	HNS3_PMU_SUBEVT_DLY_SSU_TX_ROCE,
> +	HNS3_PMU_SUBEVT_DLY_SSU_RX_NIC,
> +	HNS3_PMU_SUBEVT_DLY_SSU_RX_ROCE,
> +	HNS3_PMU_SUBEVT_DLY_SSU_T2R_NIC,
> +	HNS3_PMU_SUBEVT_DLY_SSU_T2R_ROCE,
> +	HNS3_PMU_SUBEVT_DLY_SSU_R2T_NIC,
> +	HNS3_PMU_SUBEVT_DLY_SSU_R2T_ROCE,
> +	HNS3_PMU_SUBEVT_DLY_RPU,
> +	HNS3_PMU_SUBEVT_DLY_TPU,
> +	HNS3_PMU_SUBEVT_DLY_RPE,
> +	HNS3_PMU_SUBEVT_DLY_TPE,
> +	HNS3_PMU_SUBEVT_DLY_TPE_PUSH,
> +	HNS3_PMU_SUBEVT_DLY_WR_FBD,
> +	HNS3_PMU_SUBEVT_DLY_WR_EBD,
> +	HNS3_PMU_SUBEVT_DLY_RD_FBD,
> +	HNS3_PMU_SUBEVT_DLY_RD_EBD,
> +	HNS3_PMU_SUBEVT_DLY_RD_PAY_M0,
> +	HNS3_PMU_SUBEVT_DLY_RD_PAY_M1,
> +	HNS3_PMU_SUBEVT_DLY_WR_PAY_M0,
> +	HNS3_PMU_SUBEVT_DLY_WR_PAY_M1,
> +	HNS3_PMU_SUBEVT_DLY_INTR_MERGE,
> +	HNS3_PMU_SUBEVT_DLY_MSIX_WRITE,
> +};
> +
> +enum hns3_pmu_interrupt_rate_subevent_code {
> +	HNS3_PMU_SUBEVT_INTR_MSIX_NIC,
> +	HNS3_PMU_SUBEVT_INTR_MSIX_ROH,
> +	HNS3_PMU_SUBEVT_INTR_MERGE,
> +};
> +
> +/* filter mode supported by each bandwidth subevent code */
> +#define HNS3_PMU_FILTER_BW_IGU_SSU		0x07
> +#define HNS3_PMU_FILTER_BW_SSU_EGU		0x07
> +#define HNS3_PMU_FILTER_BW_SSU_RPU		0x1F
> +#define HNS3_PMU_FILTER_BW_SSU_ROCE		0x0F
> +#define HNS3_PMU_FILTER_BW_ROCE_SSU		0x0F
> +#define HNS3_PMU_FILTER_BW_TPU_SSU		0x1F
> +#define HNS3_PMU_FILTER_BW_RPU_RCBRX		0x11
> +#define HNS3_PMU_FILTER_BW_RCBTX_TPU		0x1F
> +#define HNS3_PMU_FILTER_BW_RCBTX_TXSCH		0x1B
> +#define HNS3_PMU_FILTER_BW_WR_FBD		0x1B
> +#define HNS3_PMU_FILTER_BW_WR_EBD		0x1B
> +#define HNS3_PMU_FILTER_BW_RD_FBD		0x1B
> +#define HNS3_PMU_FILTER_BW_RD_EBD		0x1B
> +#define HNS3_PMU_FILTER_BW_RD_PAY_M0		0x01
> +#define HNS3_PMU_FILTER_BW_RD_PAY_M1		0x01
> +#define HNS3_PMU_FILTER_BW_WR_PAY_M0		0x01
> +#define HNS3_PMU_FILTER_BW_WR_PAY_M1		0x01
> +
> +/* filter mode supported by each packet rate subevent code */
> +#define HNS3_PMU_FILTER_PPS_IGU_SSU		0x07
> +#define HNS3_PMU_FILTER_PPS_SSU_EGU		0x07
> +#define HNS3_PMU_FILTER_PPS_SSU_RPU		0x1F
> +#define HNS3_PMU_FILTER_PPS_SSU_ROCE		0x0F
> +#define HNS3_PMU_FILTER_PPS_ROCE_SSU		0x0F
> +#define HNS3_PMU_FILTER_PPS_TPU_SSU		0x1F
> +#define HNS3_PMU_FILTER_PPS_RPU_RCBRX		0x11
> +#define HNS3_PMU_FILTER_PPS_RCBTX_TPU		0x1F
> +#define HNS3_PMU_FILTER_PPS_RCBTX_SCH		0x1B
> +#define HNS3_PMU_FILTER_PPS_WR_FBD		0x1B
> +#define HNS3_PMU_FILTER_PPS_WR_EBD		0x1B
> +#define HNS3_PMU_FILTER_PPS_RD_FBD		0x1B
> +#define HNS3_PMU_FILTER_PPS_RD_EBD		0x1B
> +#define HNS3_PMU_FILTER_PPS_RD_PAY_M0		0x01
> +#define HNS3_PMU_FILTER_PPS_RD_PAY_M1		0x01
> +#define HNS3_PMU_FILTER_PPS_WR_PAY_M0		0x01
> +#define HNS3_PMU_FILTER_PPS_WR_PAY_M1		0x01
> +#define HNS3_PMU_FILTER_PPS_TX_PRE		0x21
> +#define HNS3_PMU_FILTER_PPS_RX_PRE		0x21
> +
> +/* filter mode supported by each latency subevent code */
> +#define HNS3_PMU_FILTER_DLY_RX_INTR		0x01
> +#define HNS3_PMU_FILTER_DLY_RX_INTR_ROH		0x01
> +#define HNS3_PMU_FILTER_DLY_TX_PUSH		0x01
> +#define HNS3_PMU_FILTER_DLY_TX_PUSH_ROH		0x01
> +#define HNS3_PMU_FILTER_DLY_TX			0x01
> +#define HNS3_PMU_FILTER_DLY_TX_ROH		0x01
> +#define HNS3_PMU_FILTER_DLY_SSU_TX_NIC		0x0F
> +#define HNS3_PMU_FILTER_DLY_SSU_TX_ROCE		0x0F
> +#define HNS3_PMU_FILTER_DLY_SSU_RX_NIC		0x07
> +#define HNS3_PMU_FILTER_DLY_SSU_RX_ROCE		0x07
> +#define HNS3_PMU_FILTER_DLY_SSU_T2R_NIC		0x1F
> +#define HNS3_PMU_FILTER_DLY_SSU_T2R_ROCE	0x0F
> +#define HNS3_PMU_FILTER_DLY_SSU_R2T_NIC		0x07
> +#define HNS3_PMU_FILTER_DLY_SSU_R2T_ROCE	0x07
> +#define HNS3_PMU_FILTER_DLY_RPU			0x11
> +#define HNS3_PMU_FILTER_DLY_TPU			0x1F
> +#define HNS3_PMU_FILTER_DLY_RPE			0x1B
> +#define HNS3_PMU_FILTER_DLY_TPE			0x1B
> +#define HNS3_PMU_FILTER_DLY_TPE_PUSH		0x1B
> +#define HNS3_PMU_FILTER_DLY_WR_FBD		0x1B
> +#define HNS3_PMU_FILTER_DLY_WR_EBD		0x11
> +#define HNS3_PMU_FILTER_DLY_RD_FBD		0x1B
> +#define HNS3_PMU_FILTER_DLY_RD_EBD		0x1B
> +#define HNS3_PMU_FILTER_DLY_RD_PAY_M0		0x01
> +#define HNS3_PMU_FILTER_DLY_RD_PAY_M1		0x01
> +#define HNS3_PMU_FILTER_DLY_WR_PAY_M0		0x01
> +#define HNS3_PMU_FILTER_DLY_WR_PAY_M1		0x01
> +#define HNS3_PMU_FILTER_DLY_INTR_MERGE		0x01
> +#define HNS3_PMU_FILTER_DLY_MSIX_WRITE		0x01
> +
> +/* filter mode supported by each interrupt rate subevent code */
> +#define HNS3_PMU_FILTER_INTR_MSIX_NIC		0x01
> +#define HNS3_PMU_FILTER_INTR_MSIX_ROH		0x01
> +#define HNS3_PMU_FILTER_INTR_MERGE		0x01
> +
> +/* hardware filter mode */

I don't think that this comment helps much

> +enum hns3_pmu_hw_filter_mode {
> +	HNS3_PMU_HW_FILTER_GLOBAL,
> +	HNS3_PMU_HW_FILTER_PORT,
> +	HNS3_PMU_HW_FILTER_PORT_TC,
> +	HNS3_PMU_HW_FILTER_FUNC,
> +	HNS3_PMU_HW_FILTER_FUNC_QUEUE,
> +	HNS3_PMU_HW_FILTER_FUNC_INTR,
> +};
> +
> +struct hns3_pmu_event_attr {
> +	u8 event;
> +	u8 subevent;
> +	u16 filter_support;
> +};
> +
> +struct hns3_pmu {
> +	struct perf_event *hw_events[HNS3_PMU_MAX_COUNTERS];
> +	struct hlist_node node;
> +	struct pci_dev *pdev;
> +	struct pmu pmu;
> +	void __iomem *base;
> +	cpumask_t associated_cpus;
> +	int domain_id;
> +	int irq;
> +	int on_cpu;
> +	u32 identifier;
> +	u32 hw_clk;
> +	/* maximum and minimun bdf allowed by PMU */
> +	u16 bdf_min;
> +	u16 bdf_max;
> +};
> +
> +#define to_hns3_pmu(p)  (container_of((p), struct hns3_pmu, pmu))
> +#define attr_to_dattr(a) (container_of((a), struct device_attribute, attr))
> +#define dattr_to_eattr(d) (container_of((d), struct dev_ext_attribute, attr))
> +
> +#define GET_PCI_DEVFN(bdf)  ((bdf) & 0xFF)
> +
> +#define FILTER_CONDITION_PORT(port) ((1 << (port)) & 0xFF)
> +#define FILTER_CONDITION_PORT_TC(port, tc) (((port) << 3) | ((tc) & 0x07))
> +#define FILTER_CONDITION_FUNC_INTR(func, intr) (((intr) << 8) | (func))
> +
> +#define BYTES_TO_BITS(bytes)		((bytes) * 8)
> +
> +#define HNS3_PMU_FILTER_ATTR(_name, _config, _start, _end)            \
> +	static inline u64 hns3_get_##_name(struct perf_event *event)  \
> +	{                                                             \
> +		return FIELD_GET(GENMASK_ULL(_end, _start),           \
> +				 event->attr._config);                \
> +	}
> +
> +HNS3_PMU_FILTER_ATTR(event, config, 0, 7);
> +HNS3_PMU_FILTER_ATTR(subevent, config, 8, 15);
> +HNS3_PMU_FILTER_ATTR(port, config1, 0, 3);
> +HNS3_PMU_FILTER_ATTR(tc, config1, 4, 7);
> +HNS3_PMU_FILTER_ATTR(func, config1, 8, 23);
> +HNS3_PMU_FILTER_ATTR(queue, config1, 24, 39);
> +HNS3_PMU_FILTER_ATTR(intr, config1, 40, 51);
> +HNS3_PMU_FILTER_ATTR(global, config1, 52, 52);
> +
> +/* construct each bandwidth event */
> +#define HNS3_PMU_EVENT_CONSTRUCT_BW_IGU_SSU                               \
> +	(&(struct hns3_pmu_event_attr) {HNS3_PMU_EVENT_TYPE_BANDWIDTH,    \
> +					HNS3_PMU_SUBEVT_BW_IGU_SSU,       \
> +					HNS3_PMU_FILTER_BW_IGU_SSU})

How about add:
#define BW_EVENT(name)\
(&(struct hns3_pmu_event_attr) {HNS3_PMU_EVENT_TYPE_BANDWIDTH,\
	HNS3_PMU_SUBEVT_BW_#name,       \
	HNS3_PMU_FILTER_BW_#name})

#define HNS3_PMU_EVENT_CONSTRUCT_BW_SSU_EGU BW_EVENT(SSU_EGU)

As above, I couldn't apply the patch to experiment.

> +static ssize_t hns3_pmu_sysfs_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct dev_ext_attribute *eattr;
> +
> +	eattr = container_of(attr, struct dev_ext_attribute, attr);
> +
> +	return sprintf(buf, "%s\n", (char *)eattr->var);
> +}
> +
> +static ssize_t hns3_pmu_event_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct hns3_pmu_event_attr *event;
> +	struct dev_ext_attribute *eattr;
> +
> +	eattr = container_of(attr, struct dev_ext_attribute, attr);
> +	event = (struct hns3_pmu_event_attr *)eattr->var;
> +
> +	return sprintf(buf, "event=%#x, subevent=%#x\n", event->event,
> +		       event->subevent);

see comment on rst file about a single value per sysfs file

> +}
> +
> +static ssize_t hns3_pmu_filter_mode_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct hns3_pmu_event_attr *event;
> +	struct dev_ext_attribute *eattr;
> +	int ret;
> +
> +	eattr = container_of(attr, struct dev_ext_attribute, attr);
> +	event = (struct hns3_pmu_event_attr *)eattr->var;
> +
> +	ret = sprintf(buf, "filter mode supported: ");
> +	if (event->filter_support & HNS3_PMU_FILTER_SUPPORT_GLOBAL)
> +		ret += sprintf(buf + ret, "global/");
> +	if (event->filter_support & HNS3_PMU_FILTER_SUPPORT_PORT)
> +		ret += sprintf(buf + ret, "port/");
> +	if (event->filter_support & HNS3_PMU_FILTER_SUPPORT_PORT_TC)
> +		ret += sprintf(buf + ret, "port-tc/");
> +	if (event->filter_support & HNS3_PMU_FILTER_SUPPORT_FUNC)
> +		ret += sprintf(buf + ret, "func/");
> +	if (event->filter_support & HNS3_PMU_FILTER_SUPPORT_FUNC_QUEUE)
> +		ret += sprintf(buf + ret, "func-queue/");
> +	if (event->filter_support & HNS3_PMU_FILTER_SUPPORT_FUNC_INTR)
> +		ret += sprintf(buf + ret, "func-intr/");
> +
> +	ret += sprintf(buf + ret, "\n");
> +

as above

> +	return ret;
> +}
> +
> +#define HNS3_PMU_ATTR(_name, _func, _config)				\
> +	(&((struct dev_ext_attribute[]) {				\
> +		{ __ATTR(_name, 0444, _func, NULL), (void *)_config }	\
> +	})[0].attr.attr)
> +
> +#define HNS3_PMU_FORMAT_ATTR(_name, _format) \
> +	HNS3_PMU_ATTR(_name, hns3_pmu_sysfs_show, (void *)_format)
> +#define HNS3_PMU_EVENT_ATTR(_name, _event) \
> +	HNS3_PMU_ATTR(_name, hns3_pmu_event_show, (void *)_event)
> +
> +static u8 hns3_pmu_hw_filter_modes[] = {
> +	HNS3_PMU_HW_FILTER_GLOBAL,
> +	HNS3_PMU_HW_FILTER_PORT,
> +	HNS3_PMU_HW_FILTER_PORT_TC,
> +	HNS3_PMU_HW_FILTER_FUNC,
> +	HNS3_PMU_HW_FILTER_FUNC_QUEUE,
> +	HNS3_PMU_HW_FILTER_FUNC_INTR
> +};
> +
> +#define HNS3_PMU_SET_HW_FILTER(_hwc, _mode) \
> +	((_hwc)->addr_filters = (void *)&hns3_pmu_hw_filter_modes[(_mode)])
> +
> +static ssize_t identifier_show(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct hns3_pmu *hns3_pmu;
> +
> +	hns3_pmu = to_hns3_pmu(dev_get_drvdata(dev));
> +
> +	return sprintf(buf, "%#x\n", hns3_pmu->identifier);

generally we prefix 0x to hex numbers to avoid doubt

> +}
> +static DEVICE_ATTR_RO(identifier);
> +
> +static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct hns3_pmu *hns3_pmu;
> +
> +	hns3_pmu = to_hns3_pmu(dev_get_drvdata(dev));
> +
> +	return sprintf(buf, "%d\n", hns3_pmu->on_cpu);
> +}
> +static DEVICE_ATTR_RO(cpumask);
> +
> +static struct attribute *hns3_pmu_events_attr[] = {
> +	/* bandwidth events */
> +	HNS3_PMU_EVENT_ATTR(bw_igu_ssu,
> +			    HNS3_PMU_EVENT_CONSTRUCT_BW_IGU_SSU),
> +	HNS3_PMU_EVENT_ATTR(bw_ssu_egu,
> +			    HNS3_PMU_EVENT_CONSTRUCT_BW_SSU_EGU),
> +	HNS3_PMU_EVENT_ATTR(bw_ssu_rpu,

... so many events. I'd say perf list is big...

     HNS3_PMU_EVENT_CONSTRUCT_INTR_MERGE),
> +	NULL
> +};
> +
> +static struct attribute_group hns3_pmu_events_group = {
> +	.name = "events",
> +	.attrs = hns3_pmu_events_attr,
> +};
> +
> +static struct attribute_group hns3_pmu_filter_mode_group = {
> +	.name = "filtermode",
> +};
> +
> +static struct attribute *hns3_pmu_format_attr[] = {
> +	HNS3_PMU_FORMAT_ATTR(event, "config:0-7"),
> +	HNS3_PMU_FORMAT_ATTR(subevent, "config:8-15"),
> +	HNS3_PMU_FORMAT_ATTR(port, "config1:0-3"),
> +	HNS3_PMU_FORMAT_ATTR(tc, "config1:4-7"),
> +	HNS3_PMU_FORMAT_ATTR(func, "config1:8-23"),
> +	HNS3_PMU_FORMAT_ATTR(queue, "config1:24-39"),
> +	HNS3_PMU_FORMAT_ATTR(intr, "config1:40-51"),
> +	HNS3_PMU_FORMAT_ATTR(global, "config1:52-52"),
> +	NULL
> +};
> +
> +static struct attribute_group hns3_pmu_format_group = {
> +	.name = "format",
> +	.attrs = hns3_pmu_format_attr,
> +};
> +
> +static struct attribute *hns3_pmu_cpumask_attrs[] = {
> +	&dev_attr_cpumask.attr,
> +	NULL
> +};
> +
> +static struct attribute_group hns3_pmu_cpumask_attr_group = {
> +	.attrs = hns3_pmu_cpumask_attrs,
> +};
> +
> +static struct attribute *hns3_pmu_identifier_attrs[] = {
> +	&dev_attr_identifier.attr,
> +	NULL
> +};
> +
> +static struct attribute_group hns3_pmu_identifier_attr_group = {
> +	.attrs = hns3_pmu_identifier_attrs,
> +};
> +
> +static const struct attribute_group *hns3_pmu_attr_groups[] = {
> +	&hns3_pmu_events_group,
> +	&hns3_pmu_filter_mode_group,
> +	&hns3_pmu_format_group,
> +	&hns3_pmu_cpumask_attr_group,
> +	&hns3_pmu_identifier_attr_group,
> +	NULL
> +};
> +
> +static int hns3_pmu_init_filter_mode_group(void)
> +{
> +	u32 group_size = ARRAY_SIZE(hns3_pmu_events_attr);
> +	struct dev_ext_attribute *eattr_filter;
> +	struct dev_ext_attribute *eattr_event;
> +	struct device_attribute *dattr_event;
> +	struct attribute **attr_filter;
> +	struct attribute *attr_event;
> +	u32 i;
> +
> +	attr_filter = kcalloc(group_size, sizeof(struct attribute *),
> +			      GFP_KERNEL);
> +	if (!attr_filter)
> +		return -ENOMEM;

do we really need dynamic memory for all of this? I seem to recall 
asking this all before...

> +
> +	eattr_filter = kcalloc(group_size - 1, sizeof(struct dev_ext_attribute),
> +			       GFP_KERNEL);
> +	if (!eattr_filter) {
> +		kfree(attr_filter);
> +		return -ENOMEM;
> +	}
> +
> +	for (i = 0; i < group_size - 1; i++) {
> +		attr_event = hns3_pmu_events_attr[i];
> +		dattr_event = attr_to_dattr(attr_event);
> +		eattr_event = dattr_to_eattr(dattr_event);
> +
> +		memcpy(&eattr_filter[i], eattr_event,
> +		       sizeof(struct dev_ext_attribute));
> +		eattr_filter[i].attr.show = hns3_pmu_filter_mode_show;
> +
> +		attr_filter[i] = &eattr_filter[i].attr.attr;
> +	}
> +
> +	hns3_pmu_filter_mode_group.attrs = attr_filter;
> +
> +	return 0;
> +}
> +
> +static void hns3_pmu_uinit_filter_mode_group(void)

Any function which accepts no argument and returns nothing is 
suspicious. See comment about dynamic memory allocation earlier.

> +{
> +	struct dev_ext_attribute *eattr;
> +	struct device_attribute *dattr;
> +	struct attribute **attrs;
> +
> +	attrs = hns3_pmu_filter_mode_group.attrs;
> +	dattr = attr_to_dattr(attrs[0]);
> +	eattr = dattr_to_eattr(dattr);
> +
> +	kfree(eattr);
> +	kfree(attrs);
> +
> +	hns3_pmu_filter_mode_group.attrs = NULL;
> +}
> +
> +static u32 hns3_pmu_get_offset(u32 offset, u32 idx)
> +{
> +	return offset + HNS3_PMU_REG_EVENT_OFFSET +
> +	       HNS3_PMU_REG_EVENT_SIZE * idx;
> +}
> +
> +static u32 hns3_pmu_readl(struct hns3_pmu *hns3_pmu, u32 reg_offset, u32 idx)
> +{
> +	u32 offset = hns3_pmu_get_offset(reg_offset, idx);
> +
> +	return readl(hns3_pmu->base + offset);
> +}
> +
> +static void hns3_pmu_writel(struct hns3_pmu *hns3_pmu, u32 reg_offset, u32 idx,
> +			    u32 val)
> +{
> +	u32 offset = hns3_pmu_get_offset(reg_offset, idx);
> +
> +	writel(val, hns3_pmu->base + offset);
> +}
> +
> +static u64 hns3_pmu_readq(struct hns3_pmu *hns3_pmu, u32 reg_offset, u32 idx)
> +{
> +	u32 offset = hns3_pmu_get_offset(reg_offset, idx);
> +
> +	return readq(hns3_pmu->base + offset);
> +}
> +
> +static void hns3_pmu_writeq(struct hns3_pmu *hns3_pmu, u32 reg_offset, u32 idx,
> +			    u64 val)
> +{
> +	u32 offset = hns3_pmu_get_offset(reg_offset, idx);
> +
> +	writeq(val, hns3_pmu->base + offset);
> +}
> +
> +static int hns3_pmu_get_event_idx(struct hns3_pmu *hns3_pmu)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < HNS3_PMU_MAX_COUNTERS; idx++)
> +		if (!hns3_pmu->hw_events[idx])
> +			return idx;
> +
> +	return -EBUSY;
> +}
> +
> +static bool hns3_pmu_valid_bdf(struct hns3_pmu *hns3_pmu, u16 bdf)
> +{
> +	struct pci_dev *pci_dev;
> +	u8 bus, devfn;
> +
> +	if (bdf < hns3_pmu->bdf_min || bdf > hns3_pmu->bdf_max) {
> +		pci_err(hns3_pmu->pdev, "Invalid EP device: %#x!\n", bdf);
> +		return false;
> +	}
> +
> +	bus = PCI_BUS_NUM(bdf);
> +	devfn = GET_PCI_DEVFN(bdf);
> +	pci_dev = pci_get_domain_bus_and_slot(hns3_pmu->domain_id, bus, devfn);
> +	if (!pci_dev) {
> +		pci_err(hns3_pmu->pdev, "Nonexistent EP device: %#x!\n", bdf);
> +		return false;
> +	}
> +
> +	pci_dev_put(pci_dev);
> +	return true;
> +}
> +
> +static void hns3_pmu_set_qid_para(struct hns3_pmu *hns3_pmu, u32 idx, u16 bdf,
> +				  u16 queue)
> +{
> +	u32 val;
> +
> +	val = GET_PCI_DEVFN(bdf);
> +	val |= (u32)queue << HNS3_PMU_QID_PARA_QUEUE_S;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_QID_PARA, idx, val);
> +}
> +
> +static bool hns3_pmu_qid_req_start(struct hns3_pmu *hns3_pmu, u32 idx)
> +{
> +#define HNS3_PMU_QID_REQ_WAIT_TIMES			5
> +
> +	bool queue_id_valid = false;
> +	u32 cnt = 0;
> +	u32 val;
> +
> +	/* enable queue id request */
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_QID_CTRL, idx,
> +			HNS3_PMU_QID_CTRL_REQ_ENABLE);
> +
> +	while (cnt < HNS3_PMU_QID_REQ_WAIT_TIMES) {
> +		val = hns3_pmu_readl(hns3_pmu,
> +				     HNS3_PMU_REG_EVENT_QID_CTRL, idx);
> +		if (val & HNS3_PMU_QID_CTRL_DONE) {
> +			queue_id_valid = (val & HNS3_PMU_QID_CTRL_MISS) == 0;
> +			break;
> +		}
> +
> +		cnt++;
> +		/* sleep at least 1ms to wait for hardware checks qid done */
> +		usleep_range(1000, 2000);

readl_poll_timeout()?

> +	}
> +
> +	/* disable qid request and clear status */
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_QID_CTRL, idx, 0);
> +
> +	return queue_id_valid;
> +}
> +
> +static bool hns3_pmu_valid_queue(struct hns3_pmu *hns3_pmu, u32 idx, u16 bdf,
> +				 u16 queue)
> +{
> +	hns3_pmu_set_qid_para(hns3_pmu, idx, bdf, queue);
> +
> +	return hns3_pmu_qid_req_start(hns3_pmu, idx);
> +}
> +
> +static struct hns3_pmu_event_attr *hns3_pmu_get_pmu_event(u8 event, u8 subevent)
> +{
> +	struct hns3_pmu_event_attr *pmu_event;
> +	struct dev_ext_attribute *eattr;
> +	struct device_attribute *dattr;
> +	struct attribute *attr;
> +	u32 i;
> +
> +	for (i = 0; i < ARRAY_SIZE(hns3_pmu_events_attr) - 1; i++) {
> +		attr = hns3_pmu_events_attr[i];
> +		dattr = container_of(attr, struct device_attribute, attr);
> +		eattr = container_of(dattr, struct dev_ext_attribute, attr);
> +		pmu_event = (struct hns3_pmu_event_attr *)eattr->var;
> +
> +		if (event == pmu_event->event &&
> +		    subevent == pmu_event->subevent)
> +			return pmu_event;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int hns3_pmu_set_func_mode(struct perf_event *event,
> +				  struct hns3_pmu *hns3_pmu)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	u16 func_bdf = hns3_get_func(event);
> +
> +	if (!hns3_pmu_valid_bdf(hns3_pmu, func_bdf))
> +		return -ENOENT;
> +
> +	HNS3_PMU_SET_HW_FILTER(hwc, HNS3_PMU_HW_FILTER_FUNC);
> +
> +	return 0;
> +}
> +
> +static int hns3_pmu_set_func_queue_mode(struct perf_event *event,
> +					struct hns3_pmu *hns3_pmu)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	u16 queue_id = hns3_get_queue(event);
> +	u16 func_bdf = hns3_get_func(event);
> +
> +	if (!hns3_pmu_valid_bdf(hns3_pmu, func_bdf))
> +		return -ENOENT;
> +
> +	if (!hns3_pmu_valid_queue(hns3_pmu, hwc->idx, func_bdf, queue_id)) {
> +		pci_err(hns3_pmu->pdev, "Invalid queue: %u\n", queue_id);
> +		return -ENOENT;
> +	}
> +
> +	HNS3_PMU_SET_HW_FILTER(hwc, HNS3_PMU_HW_FILTER_FUNC_QUEUE);
> +
> +	return 0;
> +}
> +
> +static bool hns3_pmu_check_global_mode(struct perf_event *event,
> +				       struct hns3_pmu_event_attr *pmu_event)
> +{
> +	u8 global = hns3_get_global(event);
> +
> +	return pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_GLOBAL &&
> +	       global;
> +}
> +
> +static bool hns3_pmu_check_func_mode(struct perf_event *event,
> +				     struct hns3_pmu_event_attr *pmu_event)

what are you checking? "check" in a function name is too vague

"is_supported" or "is_enabled" are better terms


> +{
> +	u16 queue_id = hns3_get_queue(event);
> +	u16 func_bdf = hns3_get_func(event);
> +
> +	return pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_FUNC &&
> +	       queue_id == HNS3_PMU_FILTER_ALL_QUEUE && func_bdf;

logical AND on u16 looks strange (func_bdf is what I am referring to)

> +}
> +
> +static bool
> +hns3_pmu_check_func_queue_mode(struct perf_event *event,
> +			       struct hns3_pmu_event_attr *pmu_event)
> +{
> +	u16 queue_id = hns3_get_queue(event);
> +	u16 func_bdf = hns3_get_func(event);
> +
> +	return pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_FUNC_QUEUE &&
> +	       queue_id != HNS3_PMU_FILTER_ALL_QUEUE && func_bdf;
> +}
> +
> +static bool hns3_pmu_check_port_mode(struct perf_event *event,
> +				     struct hns3_pmu_event_attr *pmu_event)
> +{
> +	u8 tc_id = hns3_get_tc(event);
> +
> +	return pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_PORT &&
> +	       tc_id == HNS3_PMU_FILTER_ALL_TC;
> +}
> +
> +static bool hns3_pmu_check_port_tc_mode(struct perf_event *event,
> +					struct hns3_pmu_event_attr *pmu_event)
> +{
> +	u8 tc_id = hns3_get_tc(event);
> +
> +	return pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_PORT_TC &&
> +	       tc_id != HNS3_PMU_FILTER_ALL_TC;
> +}
> +
> +static bool hns3_pmu_check_func_intr_mode(struct perf_event *event,
> +					  struct hns3_pmu *hns3_pmu,
> +					  struct hns3_pmu_event_attr *pmu_event)
> +{
> +	u16 func_bdf = hns3_get_func(event);
> +
> +	return pmu_event->filter_support & HNS3_PMU_FILTER_SUPPORT_FUNC_INTR &&
> +	       hns3_pmu_valid_bdf(hns3_pmu, func_bdf);
> +}
> +
> +static int hns3_pmu_check_config(struct perf_event *event,
> +				 struct hns3_pmu *hns3_pmu)

please explain what this function is supposed to be doing. It is quite a 
strange style, to say the least.

> +{
> +	struct hns3_pmu_event_attr *pmu_event;
> +	struct hw_perf_event *hwc = &event->hw;
> +	u8 subevent_id = hns3_get_subevent(event);
> +	u8 event_id = hns3_get_event(event);
> +
> +	pmu_event = hns3_pmu_get_pmu_event(event_id, subevent_id);
> +	if (!pmu_event) {
> +		pci_err(hns3_pmu->pdev, "Invalid pmu event\n");
> +		return -ENOENT;
> +	}
> +
> +	if (hns3_pmu_check_global_mode(event, pmu_event)) {
> +		HNS3_PMU_SET_HW_FILTER(hwc, HNS3_PMU_HW_FILTER_GLOBAL);
> +		return 0;
> +	}
> +
> +	if (hns3_pmu_check_func_mode(event, pmu_event))
> +		return hns3_pmu_set_func_mode(event, hns3_pmu);
> +
> +	if (hns3_pmu_check_func_queue_mode(event, pmu_event))
> +		return hns3_pmu_set_func_queue_mode(event, hns3_pmu);
> +
> +	if (hns3_pmu_check_port_mode(event, pmu_event)) {
> +		HNS3_PMU_SET_HW_FILTER(hwc, HNS3_PMU_HW_FILTER_PORT);
> +		return 0;
> +	}
> +
> +	if (hns3_pmu_check_port_tc_mode(event, pmu_event)) {
> +		HNS3_PMU_SET_HW_FILTER(hwc, HNS3_PMU_HW_FILTER_PORT_TC);
> +		return 0;
> +	}
> +
> +	if (hns3_pmu_check_func_intr_mode(event, hns3_pmu, pmu_event)) {
> +		HNS3_PMU_SET_HW_FILTER(hwc, HNS3_PMU_HW_FILTER_FUNC_INTR);
> +		return 0;
> +	}
> +
> +	return -ENOENT;
> +}
> +
> +static bool hns3_pmu_validate_event_group(struct perf_event *event)
> +{
> +	struct perf_event *leader = event->group_leader;
> +	struct perf_event *sibling;
> +	int counters = 1;
> +
> +	if (!is_software_event(leader)) {
> +		/*
> +		 * We must NOT create groups containing mixed PMUs, although
> +		 * software events are acceptable
> +		 */
> +		if (leader->pmu != event->pmu)
> +			return false;
> +
> +		/* Increase counter for the leader */
> +		if (leader != event)
> +			counters++;
> +	}
> +
> +	for_each_sibling_event(sibling, event->group_leader) {
> +		if (is_software_event(sibling))
> +			continue;
> +
> +		if (sibling->pmu != event->pmu)
> +			return false;
> +
> +		/* Increase counter for each sibling */
> +		counters++;
> +	}
> +
> +	return counters <= HNS3_PMU_MAX_COUNTERS;
> +}
> +
> +static u32 hns3_pmu_get_filter_condition(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	u16 func_bdf = hns3_get_func(event);
> +	u16 intr_id = hns3_get_intr(event);
> +	u8 port_id = hns3_get_port(event);
> +	u8 tc_id = hns3_get_tc(event);
> +	u8 filter_mode;
> +	u32 filter = 0;
> +
> +	filter_mode = *(u8 *)hwc->addr_filters;
> +	switch (filter_mode) {
> +	case HNS3_PMU_HW_FILTER_PORT:
> +		filter = FILTER_CONDITION_PORT(port_id);
> +		break;
> +	case HNS3_PMU_HW_FILTER_PORT_TC:
> +		filter = FILTER_CONDITION_PORT_TC(port_id, tc_id);
> +		break;
> +	case HNS3_PMU_HW_FILTER_FUNC:
> +	case HNS3_PMU_HW_FILTER_FUNC_QUEUE:
> +		filter = GET_PCI_DEVFN(func_bdf);
> +		break;
> +	case HNS3_PMU_HW_FILTER_FUNC_INTR:
> +		filter = FILTER_CONDITION_FUNC_INTR(GET_PCI_DEVFN(func_bdf),
> +						    intr_id);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return filter;
> +}
> +
> +static void hns3_pmu_config_filter(struct perf_event *event)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	u8 filter_mode = *(u8 *)hwc->addr_filters;
> +	u8 subevent_id = hns3_get_subevent(event);
> +	u16 queue_id = hns3_get_queue(event);
> +	u8 event_id = hns3_get_event(event);
> +	u16 func_bdf = hns3_get_func(event);
> +	u32 idx = hwc->idx;
> +	u32 val;
> +
> +	val = event_id;
> +	val |= subevent_id << HNS3_PMU_CTRL_SUBEVENT_S;
> +	val |= filter_mode << HNS3_PMU_CTRL_FILTER_MODE_S;
> +	val |= HNS3_PMU_EVENT_OVERFLOW_RESTART;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx, val);
> +
> +	val = hns3_pmu_get_filter_condition(event);
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_HIGH, idx, val);
> +
> +	if (filter_mode == HNS3_PMU_HW_FILTER_FUNC_QUEUE)
> +		hns3_pmu_set_qid_para(hns3_pmu, idx, func_bdf, queue_id);
> +}
> +
> +static void hns3_pmu_enable_counter(struct hns3_pmu *hns3_pmu,
> +				    struct hw_perf_event *hwc)
> +{
> +	u32 idx = hwc->idx;
> +	u32 val;
> +
> +	val = hns3_pmu_readl(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx);
> +	val |= HNS3_PMU_EVENT_EN;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx, val);
> +}
> +
> +static void hns3_pmu_disable_counter(struct hns3_pmu *hns3_pmu,
> +				     struct hw_perf_event *hwc)
> +{
> +	u32 idx = hwc->idx;
> +	u32 val;
> +
> +	val = hns3_pmu_readl(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx);
> +	val &= ~HNS3_PMU_EVENT_EN;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx, val);
> +}
> +
> +static void hns3_pmu_enable_intr(struct hns3_pmu *hns3_pmu,
> +				 struct hw_perf_event *hwc)
> +{
> +	u32 idx = hwc->idx;
> +	u32 val;
> +
> +	val = hns3_pmu_readl(hns3_pmu, HNS3_PMU_REG_EVENT_INTR_MASK, idx);
> +	val &= ~HNS3_PMU_INTR_MASK_OVERFLOW;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_INTR_MASK, idx, val);
> +}
> +
> +static void hns3_pmu_disable_intr(struct hns3_pmu *hns3_pmu,
> +				  struct hw_perf_event *hwc)
> +{
> +	u32 idx = hwc->idx;
> +	u32 val;
> +
> +	val = hns3_pmu_readl(hns3_pmu, HNS3_PMU_REG_EVENT_INTR_MASK, idx);
> +	val |= HNS3_PMU_INTR_MASK_OVERFLOW;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_INTR_MASK, idx, val);
> +}
> +
> +static void hns3_pmu_clear_intr_status(struct hns3_pmu *hns3_pmu, u32 idx)
> +{
> +	u32 val;
> +
> +	val = hns3_pmu_readl(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx);
> +	val |= HNS3_PMU_EVENT_STATUS_RESET;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx, val);
> +
> +	val = hns3_pmu_readl(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx);
> +	val &= ~HNS3_PMU_EVENT_STATUS_RESET;
> +	hns3_pmu_writel(hns3_pmu, HNS3_PMU_REG_EVENT_CTRL_LOW, idx, val);
> +}
> +
> +static void hns3_pmu_read_counter(struct perf_event *event, u64 *cnt,
> +				  u64 *cnt_ext)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	u32 idx = event->hw.idx;
> +
> +	*cnt = hns3_pmu_readq(hns3_pmu, HNS3_PMU_REG_EVENT_COUNTER, idx);
> +	*cnt_ext = hns3_pmu_readq(hns3_pmu, HNS3_PMU_REG_EVENT_EXT_COUNTER,
> +				  idx);
> +}
> +
> +static void hns3_pmu_write_counter(struct perf_event *event, u64 cnt,
> +				   u64 cnt_ext)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	u32 idx = event->hw.idx;
> +
> +	hns3_pmu_writeq(hns3_pmu, HNS3_PMU_REG_EVENT_COUNTER, idx, cnt);
> +	hns3_pmu_writeq(hns3_pmu, HNS3_PMU_REG_EVENT_EXT_COUNTER, idx, cnt_ext);
> +}
> +
> +static void hns3_pmu_init_counter(struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hw_perf_event_extra *hwc_ext;
> +
> +	hwc_ext = &hwc->extra_reg;
> +	local64_set(&hwc->prev_count, 0);
> +	hwc_ext->config = 0;
> +	hns3_pmu_write_counter(event, 0, 0);
> +}
> +
> +static u64 hns3_pmu_tick_to_ms(u32 hw_clk, u64 ticks)

what hw_clk is this?

> +{
> +	u64 time_ms = ticks / (hw_clk / HNS3_PMU_FREQ_1K_HZ);
> +
> +	if (!time_ms)
> +		time_ms = 1;
> +
> +	return time_ms;
> +}
> +
> +static u64 hns3_pmu_calculate(u8 event_type, u32 hw_clk, u64 val, u64 val_ext)

calculate what? The arguments are totally cryptic.

> +{
> +	u64 result;
> +
> +	if (!hw_clk || !val || !val_ext)
> +		return 0;
> +
> +	switch (event_type) {
> +	/* val: byte number, val_ext: timer ticks number */
> +	case HNS3_PMU_EVENT_TYPE_BANDWIDTH:
> +		/* process data to set unit of bandwidth as "KBits/s" */
> +		result = val / hns3_pmu_tick_to_ms(hw_clk, val_ext);
> +		result = BYTES_TO_BITS(result);
> +		break;
> +
> +	/* val: packet number, val_ext: timer ticks number */
> +	case HNS3_PMU_EVENT_TYPE_PACKET_RATE:
> +	/* val: interrupt number, val_ext: timer ticks number */
> +	case HNS3_PMU_EVENT_TYPE_INTR_RATE:
> +		/* process data to set unit as "pps" */
> +		result = val * HNS3_PMU_FREQ_1K_HZ /
> +			 hns3_pmu_tick_to_ms(hw_clk, val_ext);
> +		break;
> +
> +	/* val: timer ticks number, val_ext: packet number */
> +	case HNS3_PMU_EVENT_TYPE_LATENCY:
> +		/* process data to set unit of latency as "ns" */
> +		result = val / val_ext;
> +		result = result * HNS3_PMU_FREQ_1G_HZ / hw_clk;
> +		break;
> +
> +	default:
> +		result = val / val_ext;
> +		break;
> +	}
> +
> +	return result;
> +}
> +
> +static int hns3_pmu_event_init(struct perf_event *event)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	int idx;
> +	int ret;
> +
> +	if (event->attr.type != event->pmu->type)
> +		return -ENOENT;
> +
> +	/* Sampling is not supported */
> +	if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
> +		return -EOPNOTSUPP;
> +
> +	if (hns3_pmu->on_cpu < 0) {
> +		pci_err(hns3_pmu->pdev, "PMU has no online CPU.\n");
> +		return -EINVAL;
> +	}
> +
> +	idx = hns3_pmu_get_event_idx(hns3_pmu);
> +	if (idx < 0) {
> +		pci_err(hns3_pmu->pdev, "Up to %u events are supported!\n",
> +			HNS3_PMU_MAX_COUNTERS);
> +		return -EBUSY;
> +	}
> +
> +	hwc->idx = idx;
> +	event->cpu = hns3_pmu->on_cpu;
> +
> +	ret = hns3_pmu_check_config(event, hns3_pmu);
> +	if (ret) {
> +		pci_err(hns3_pmu->pdev, "Invalid filter, ret = %d.\n", ret);
> +		return ret;
> +	}
> +
> +	if (!hns3_pmu_validate_event_group(event)) {
> +		pci_err(hns3_pmu->pdev, "Invalid event group.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void hns3_pmu_read(struct perf_event *event)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	struct hw_perf_event_extra *hwc_ext;
> +	u64 new_cnt_ext, prev_cnt_ext;
> +	u64 new_cnt, prev_cnt, delta;
> +
> +	hwc_ext = &hwc->extra_reg;
> +	do {
> +		prev_cnt = local64_read(&hwc->prev_count);
> +		prev_cnt_ext = hwc_ext->config;
> +		hns3_pmu_read_counter(event, &new_cnt, &new_cnt_ext);
> +	} while (local64_cmpxchg(&hwc->prev_count, prev_cnt, new_cnt) !=
> +		 prev_cnt);
> +
> +	hwc_ext->config = new_cnt_ext;
> +
> +	delta = hns3_pmu_calculate(hns3_get_event(event), hns3_pmu->hw_clk,
> +				   new_cnt - prev_cnt,
> +				   new_cnt_ext - prev_cnt_ext);
> +
> +	local64_add(delta, &event->count);
> +}
> +
> +static void hns3_pmu_start(struct perf_event *event, int flags)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> +		return;
> +
> +	WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> +	hwc->state = 0;
> +
> +	hns3_pmu_config_filter(event);
> +	hns3_pmu_init_counter(event);
> +	hns3_pmu_enable_intr(hns3_pmu, hwc);
> +	hns3_pmu_enable_counter(hns3_pmu, hwc);
> +
> +	perf_event_update_userpage(event);
> +}
> +
> +static void hns3_pmu_stop(struct perf_event *event, int flags)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	hns3_pmu_disable_counter(hns3_pmu, hwc);
> +	hns3_pmu_disable_intr(hns3_pmu, hwc);
> +
> +	WARN_ON_ONCE(hwc->state & PERF_HES_STOPPED);
> +	hwc->state |= PERF_HES_STOPPED;
> +
> +	if (hwc->state & PERF_HES_UPTODATE)
> +		return;
> +
> +	/* Read hardware counter and update the perf counter statistics */
> +	hns3_pmu_read(event);
> +	hwc->state |= PERF_HES_UPTODATE;
> +}
> +
> +static int hns3_pmu_add(struct perf_event *event, int flags)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +	int idx;
> +
> +	hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE;
> +
> +	/* Get an available counter index for counting */
> +	idx = hns3_pmu_get_event_idx(hns3_pmu);
> +	if (idx < 0)
> +		return idx;
> +
> +	hwc->idx = idx;
> +	hns3_pmu->hw_events[idx] = event;
> +
> +	if (flags & PERF_EF_START)
> +		hns3_pmu_start(event, PERF_EF_RELOAD);
> +
> +	return 0;
> +}
> +
> +static void hns3_pmu_del(struct perf_event *event, int flags)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(event->pmu);
> +	struct hw_perf_event *hwc = &event->hw;
> +
> +	hns3_pmu_stop(event, PERF_EF_UPDATE);
> +	hns3_pmu->hw_events[hwc->idx] = NULL;
> +	perf_event_update_userpage(event);
> +}
> +
> +static void hns3_pmu_enable(struct pmu *pmu)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(pmu);
> +	u32 val;
> +
> +	val = readl(hns3_pmu->base + HNS3_PMU_REG_GLOBAL_CTRL);
> +	val |= HNS3_PMU_GLOBAL_START;
> +	writel(val, hns3_pmu->base + HNS3_PMU_REG_GLOBAL_CTRL);
> +}
> +
> +static void hns3_pmu_disable(struct pmu *pmu)
> +{
> +	struct hns3_pmu *hns3_pmu = to_hns3_pmu(pmu);
> +	u32 val;
> +
> +	val = readl(hns3_pmu->base + HNS3_PMU_REG_GLOBAL_CTRL);
> +	val &= ~HNS3_PMU_GLOBAL_START;
> +	writel(val, hns3_pmu->base + HNS3_PMU_REG_GLOBAL_CTRL);
> +}
> +
> +static int hns3_pmu_alloc(struct pci_dev *pdev, struct hns3_pmu *hns3_pmu)
> +{
> +	u16 device_id;
> +	char *name;
> +	u32 val;
> +
> +	hns3_pmu->base = pci_ioremap_bar(pdev, BAR_ID);
> +	if (!hns3_pmu->base) {
> +		pci_err(pdev, "ioremap failed for hns3_pmu resource\n");
> +		pci_release_regions(pdev);
> +		return -ENOMEM;
> +	}
> +
> +	hns3_pmu->hw_clk = readl(hns3_pmu->base + HNS3_PMU_REG_CLOCK_FREQ);
> +
> +	val = readl(hns3_pmu->base + HNS3_PMU_REG_BDF);
> +	hns3_pmu->bdf_min = val & 0xffff;
> +	hns3_pmu->bdf_max = val >> 16;
> +
> +	val = readl(hns3_pmu->base + HNS3_PMU_REG_DEVICE_ID);
> +	device_id = val & 0xffff;
> +	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "hns3_pmu_%u", device_id);

you don't check the return value

> +
> +	hns3_pmu->pdev = pdev;
> +	hns3_pmu->domain_id = pdev->bus->domain_nr;
> +	hns3_pmu->on_cpu = HNS3_PMU_INVALID_CPU_ID;
> +	hns3_pmu->identifier = readl(hns3_pmu->base + HNS3_PMU_REG_VERSION);
> +	hns3_pmu->pmu = (struct pmu) {
> +		.name		= name,
> +		.module		= THIS_MODULE,
> +		.event_init	= hns3_pmu_event_init,
> +		.pmu_enable	= hns3_pmu_enable,
> +		.pmu_disable	= hns3_pmu_disable,
> +		.add		= hns3_pmu_add,
> +		.del		= hns3_pmu_del,
> +		.start		= hns3_pmu_start,
> +		.stop		= hns3_pmu_stop,
> +		.read		= hns3_pmu_read,
> +		.task_ctx_nr	= perf_invalid_context,
> +		.attr_groups	= hns3_pmu_attr_groups,
> +		.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
> +	};
> +
> +	return 0;
> +}
> +
> +static irqreturn_t hns3_pmu_irq(int irq, void *data)
> +{
> +	struct hns3_pmu *hns3_pmu = data;
> +	u32 intr_status, idx;
> +
> +	for (idx = 0; idx < HNS3_PMU_MAX_COUNTERS; idx++) {
> +		intr_status = hns3_pmu_readl(hns3_pmu,
> +					     HNS3_PMU_REG_EVENT_INTR_STATUS,
> +					     idx);
> +
> +		/*
> +		 * As each counter will restart from 0 when it is overflowed,
> +		 * extra processing is no need, just clear interrupt status.
> +		 */
> +		if (intr_status)
> +			hns3_pmu_clear_intr_status(hns3_pmu, idx);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int hns3_pmu_associate_cpu(struct hns3_pmu *hns3_pmu, unsigned int cpu)
> +{
> +	struct device *dev = &hns3_pmu->pdev->dev;
> +	const struct cpumask *mask;
> +
> +	/*
> +	 * Check whether this CPU is associated with this HNS3 PMU
> +	 * according to the NUMA node.
> +	 */
> +	mask = (dev_to_node(dev) == NUMA_NO_NODE) ? cpu_online_mask :
> +		cpumask_of_node(dev_to_node(dev));
> +
> +	return cpumask_test_cpu(cpu, mask);
> +}
> +
> +static int hns3_pmu_online_cpu(unsigned int cpu, struct hlist_node *node)
> +{

I think someone may be factoring this out of all the hisi PMUs..

And here I really don't know why we do this with online/offline mechanism.

Can't we do for_each_possible_cpu()?

We do offline/online for hisi_uncore_pmu() as we need finer grain detail 
if cluster/super cluster.

> +	struct hns3_pmu *hns3_pmu;
> +
> +	hns3_pmu = hlist_entry_safe(node, struct hns3_pmu, node);
> +	if (!hns3_pmu)
> +		return -ENODEV;
> +
> +	if (!hns3_pmu_associate_cpu(hns3_pmu, cpu))
> +		return 0;
> +
> +	cpumask_set_cpu(cpu, &hns3_pmu->associated_cpus);
> +	if (hns3_pmu->on_cpu != HNS3_PMU_INVALID_CPU_ID)
> +		return 0;
> +
> +	/* Use this CPU in cpumask for event counting */
> +	hns3_pmu->on_cpu = cpu;
> +
> +	/* Overflow interrupt also should use the same CPU */
> +	irq_set_affinity_hint(hns3_pmu->irq, cpumask_of(cpu));
> +
> +	return 0;
> +}
> +
> +static int hns3_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> +	struct hns3_pmu *hns3_pmu;
> +	cpumask_t pmu_online_cpus;
> +	unsigned int target;
> +
> +	hns3_pmu = hlist_entry_safe(node, struct hns3_pmu, node);
> +	if (!hns3_pmu)
> +		return -ENODEV;
> +
> +	if (!cpumask_test_and_clear_cpu(cpu, &hns3_pmu->associated_cpus))
> +		return 0;
> +
> +	if (hns3_pmu->on_cpu != cpu)
> +		return 0;
> +
> +	/* Give up ownership of the PMU */
> +	hns3_pmu->on_cpu = HNS3_PMU_INVALID_CPU_ID;
> +
> +	/* Choose a new CPU to migrate ownership of the PMU */
> +	cpumask_and(&pmu_online_cpus, &hns3_pmu->associated_cpus,
> +		    cpu_online_mask);
> +	target = cpumask_any_but(&pmu_online_cpus, cpu);
> +	if (target >= nr_cpu_ids)
> +		return 0;
> +
> +	hns3_pmu->on_cpu = target;
> +	perf_pmu_migrate_context(&hns3_pmu->pmu, cpu, target);
> +	irq_set_affinity_hint(hns3_pmu->irq, cpumask_of(target));
> +
> +	return 0;
> +}
> +
> +static int hns3_pmu_irq_register(struct pci_dev *pdev,
> +				 struct hns3_pmu *hns3_pmu)
> +{
> +	int irq, ret;
> +
> +	ret = pci_alloc_irq_vectors(pdev, HNS3_IRQ_VECS, HNS3_IRQ_VECS,

just use 1 directly here

> +				    PCI_IRQ_MSI);
> +	if (ret < 0) {
> +		pci_err(pdev, "failed to enable MSI vectors, ret = %d.\n", ret);
> +		return ret;
> +	}
> +
> +	irq = pci_irq_vector(pdev, HNS3_IRQ_VECTOR);
> +	ret = request_irq(irq, hns3_pmu_irq, IRQF_SHARED,
> +			  hns3_pmu->pmu.name, hns3_pmu);

don't we have managed versions of all these now?

> +	if (ret) {
> +		pci_err(pdev, "failed to register irq, ret = %d.\n", ret);
> +		pci_free_irq_vectors(pdev);
> +		return ret;
> +	}
> +
> +	hns3_pmu->irq = irq;
> +
> +	return 0;
> +}
> +
> +static int hns3_pmu_init(struct pci_dev *pdev, struct hns3_pmu *hns3_pmu)
> +{
> +	int ret;
> +
> +	ret = hns3_pmu_alloc(pdev, hns3_pmu);
> +	if (ret)
> +		return ret;
> +
> +	ret = hns3_pmu_irq_register(pdev, hns3_pmu);
> +	if (ret)
> +		goto err_set_pmu_fail;
> +
> +	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE,
> +				       &hns3_pmu->node);
> +	if (ret) {
> +		pci_err(pdev, "failed to register hotplug, ret = %d.\n", ret);
> +		goto err_irq_unregister;
> +	}
> +
> +	ret = perf_pmu_register(&hns3_pmu->pmu, hns3_pmu->pmu.name,
> +				HNS3_PMU_INVALID_CPU_ID);
> +	if (ret) {
> +		pci_err(pdev, "failed to register HNS3 PMU, ret = %d.\n", ret);
> +		goto err_hotplug_unregister;
> +	}
> +
> +	return ret;
> +
> +err_hotplug_unregister:
> +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE,
> +				    &hns3_pmu->node);
> +
> +err_irq_unregister:
> +	irq_set_affinity_hint(hns3_pmu->irq, NULL);
> +	free_irq(hns3_pmu->irq, hns3_pmu);
> +
> +err_set_pmu_fail:
> +	iounmap(hns3_pmu->base);
> +
> +	return ret;
> +}
> +
> +static void hns3_pmu_uninit(struct pci_dev *pdev, struct hns3_pmu *hns3_pmu)
> +{
> +	perf_pmu_unregister(&hns3_pmu->pmu);
> +	irq_set_affinity_hint(hns3_pmu->irq, NULL);
> +	free_irq(hns3_pmu->irq, hns3_pmu);
> +	pci_free_irq_vectors(pdev);
> +	cpuhp_state_remove_instance(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE,
> +				    &hns3_pmu->node);
> +	iounmap(hns3_pmu->base);
> +}
> +
> +static int hns3_pmu_init_dev(struct pci_dev *pdev)
> +{
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		pci_err(pdev, "failed to enable pci device, ret = %d.\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pci_request_mem_regions(pdev, "hns3_pmu");
> +	if (ret < 0) {
> +		pci_err(pdev, "failed to request pci mem regions, ret = %d.\n",
> +			ret);
> +		pci_disable_device(pdev);
> +		return ret;
> +	}
> +
> +	pci_set_master(pdev);
> +
> +	return 0;
> +}
> +
> +static void hns3_pmu_uninit_dev(struct pci_dev *pdev)
> +{
> +	pci_clear_master(pdev);
> +	pci_release_mem_regions(pdev);
> +	pci_disable_device(pdev);
> +}
> +
> +static int hns3_pmu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct hns3_pmu *hns3_pmu;
> +	int ret;
> +
> +	hns3_pmu = devm_kzalloc(&pdev->dev, sizeof(*hns3_pmu), GFP_KERNEL);
> +	if (!hns3_pmu)
> +		return -ENOMEM;
> +
> +	ret = hns3_pmu_init_dev(pdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = hns3_pmu_init(pdev, hns3_pmu);
> +	if (ret) {
> +		hns3_pmu_uninit_dev(pdev);
> +		return ret;
> +	}
> +
> +	pci_set_drvdata(pdev, hns3_pmu);
> +
> +	return ret;
> +}
> +
> +static void hns3_pmu_remove(struct pci_dev *pdev)
> +{
> +	hns3_pmu_uninit(pdev, pci_get_drvdata(pdev));
> +	hns3_pmu_uninit_dev(pdev);
> +	pci_set_drvdata(pdev, NULL);
> +}
> +
> +static const struct pci_device_id hns3_pmu_ids[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_HUAWEI, HNS3_PMU_DEVICE_ID) },
> +	{ 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, hns3_pmu_ids);
> +
> +static struct pci_driver hns3_pmu_driver = {
> +	.name = "hns3_pmu",
> +	.id_table = hns3_pmu_ids,
> +	.probe = hns3_pmu_probe,
> +	.remove = hns3_pmu_remove,
> +};
> +
> +static int __init hns3_pmu_module_init(void)
> +{
> +	int ret;
> +
> +	ret = hns3_pmu_init_filter_mode_group();
> +	if (ret)
> +		return ret;
> +
> +	ret = cpuhp_setup_state_multi(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE,
> +				      "AP_PERF_ARM_HNS3_PMU_ONLINE",
> +				      hns3_pmu_online_cpu,
> +				      hns3_pmu_offline_cpu);
> +	if (ret) {
> +		pr_err("failed to setup HNS3 PMU hotplug, ret = %d.\n", ret);
> +		goto err_init;
> +	}
> +
> +	ret = pci_register_driver(&hns3_pmu_driver);
> +	if (ret) {
> +		pr_err("failed to register pci driver, ret = %d.\n", ret);
> +		goto err_hp;
> +	}
> +
> +	return 0;
> +
> +err_hp:
> +	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE);
> +err_init:
> +	hns3_pmu_uinit_filter_mode_group();
> +	return ret;
> +}
> +module_init(hns3_pmu_module_init);
> +
> +static void __exit hns3_pmu_module_exit(void)
> +{
> +	pci_unregister_driver(&hns3_pmu_driver);
> +	cpuhp_remove_multi_state(CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE);
> +	hns3_pmu_uinit_filter_mode_group();
> +}
> +module_exit(hns3_pmu_module_exit);
> +
> +MODULE_DESCRIPTION("HNS3 PMU driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 113f51f..50c50e6 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -174,6 +174,7 @@ enum cpuhp_state {
>   	CPUHP_AP_PERF_ARM_HISI_DDRC_ONLINE,
>   	CPUHP_AP_PERF_ARM_HISI_HHA_ONLINE,
>   	CPUHP_AP_PERF_ARM_HISI_L3_ONLINE,
> +	CPUHP_AP_PERF_ARM_HNS3_PMU_ONLINE,
>   	CPUHP_AP_PERF_ARM_L2X0_ONLINE,
>   	CPUHP_AP_PERF_ARM_QCOM_L2_ONLINE,
>   	CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
> 




More information about the linux-arm-kernel mailing list