Hi Suzuki, > -----Original Message----- > From: Besar Wicaksono > Sent: Thursday, September 29, 2022 9:06 AM > To: Suzuki K Poulose <suzuki.poulose@xxxxxxx>; robin.murphy@xxxxxxx; > catalin.marinas@xxxxxxx; will@xxxxxxxxxx; mark.rutland@xxxxxxx > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > linux-tegra@xxxxxxxxxxxxxxx; sudeep.holla@xxxxxxx; > thanu.rangarajan@xxxxxxx; Michael.Williams@xxxxxxx; Thierry Reding > <treding@xxxxxxxxxx>; Jonathan Hunter <jonathanh@xxxxxxxxxx>; Vikram > Sethi <vsethi@xxxxxxxxxx>; mathieu.poirier@xxxxxxxxxx; > mike.leach@xxxxxxxxxx; leo.yan@xxxxxxxxxx > Subject: RE: [PATCH v5 1/2] perf: arm_cspmu: Add support for ARM > CoreSight PMU driver > > Hi Suzuki, > > > -----Original Message----- > > From: Suzuki K Poulose <suzuki.poulose@xxxxxxx> > > Sent: Thursday, September 29, 2022 3:29 AM > > To: Besar Wicaksono <bwicaksono@xxxxxxxxxx>; robin.murphy@xxxxxxx; > > catalin.marinas@xxxxxxx; will@xxxxxxxxxx; mark.rutland@xxxxxxx > > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > linux-tegra@xxxxxxxxxxxxxxx; sudeep.holla@xxxxxxx; > > thanu.rangarajan@xxxxxxx; Michael.Williams@xxxxxxx; Thierry Reding > > <treding@xxxxxxxxxx>; Jonathan Hunter <jonathanh@xxxxxxxxxx>; Vikram > > Sethi <vsethi@xxxxxxxxxx>; mathieu.poirier@xxxxxxxxxx; > > mike.leach@xxxxxxxxxx; leo.yan@xxxxxxxxxx > > Subject: Re: [PATCH v5 1/2] perf: arm_cspmu: Add support for ARM > > CoreSight PMU driver > > > > External email: Use caution opening links or attachments > > > > > > Hi Besar > > > > On 28/09/2022 21:18, Besar Wicaksono wrote: > > > Add support for ARM CoreSight PMU driver framework and interfaces. > > > The driver provides generic implementation to operate uncore PMU > based > > > on ARM CoreSight PMU architecture. The driver also provides interface > > > to get vendor/implementation specific information, for example event > > > attributes and formating. > > > > > > The specification used in this implementation can be found below: > > > * ACPI Arm Performance Monitoring Unit table: > > > https://developer.arm.com/documentation/den0117/latest > > > * ARM Coresight PMU architecture: > > > https://developer.arm.com/documentation/ihi0091/latest > > > > > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx> > > > Signed-off-by: Besar Wicaksono <bwicaksono@xxxxxxxxxx> > > > > Apologies for this late comment. Please find it below. > > > > > --- > > > arch/arm64/configs/defconfig | 1 + > > > drivers/perf/Kconfig | 2 + > > > drivers/perf/Makefile | 1 + > > > drivers/perf/arm_cspmu/Kconfig | 13 + > > > drivers/perf/arm_cspmu/Makefile | 6 + > > > drivers/perf/arm_cspmu/arm_cspmu.c | 1276 > > ++++++++++++++++++++++++++++ > > > drivers/perf/arm_cspmu/arm_cspmu.h | 151 ++++ > > > 7 files changed, 1450 insertions(+) > > > create mode 100644 drivers/perf/arm_cspmu/Kconfig > > > create mode 100644 drivers/perf/arm_cspmu/Makefile > > > create mode 100644 drivers/perf/arm_cspmu/arm_cspmu.c > > > create mode 100644 drivers/perf/arm_cspmu/arm_cspmu.h > > > > > > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig > > > index 7d1105343bc2..ee31c9159a5b 100644 > > > --- a/arch/arm64/configs/defconfig > > > +++ b/arch/arm64/configs/defconfig > > > @@ -1212,6 +1212,7 @@ CONFIG_PHY_UNIPHIER_USB3=y > > > CONFIG_PHY_TEGRA_XUSB=y > > > CONFIG_PHY_AM654_SERDES=m > > > CONFIG_PHY_J721E_WIZ=m > > > +CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU=y > > > CONFIG_ARM_SMMU_V3_PMU=m > > > CONFIG_FSL_IMX8_DDR_PMU=m > > > CONFIG_QCOM_L2_PMU=y > > > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig > > > index 1e2d69453771..c94d3601eb48 100644 > > > --- a/drivers/perf/Kconfig > > > +++ b/drivers/perf/Kconfig > > > @@ -192,4 +192,6 @@ config MARVELL_CN10K_DDR_PMU > > > Enable perf support for Marvell DDR Performance monitoring > > > event on CN10K platform. > > > > > > +source "drivers/perf/arm_cspmu/Kconfig" > > > + > > > endmenu > > > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile > > > index 57a279c61df5..3bc9323f0965 100644 > > > --- a/drivers/perf/Makefile > > > +++ b/drivers/perf/Makefile > > > @@ -20,3 +20,4 @@ obj-$(CONFIG_ARM_DMC620_PMU) += > > arm_dmc620_pmu.o > > > obj-$(CONFIG_MARVELL_CN10K_TAD_PMU) += > > marvell_cn10k_tad_pmu.o > > > obj-$(CONFIG_MARVELL_CN10K_DDR_PMU) += > > marvell_cn10k_ddr_pmu.o > > > obj-$(CONFIG_APPLE_M1_CPU_PMU) += apple_m1_cpu_pmu.o > > > +obj-$(CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU) += > > arm_cspmu/ > > > diff --git a/drivers/perf/arm_cspmu/Kconfig > > b/drivers/perf/arm_cspmu/Kconfig > > > new file mode 100644 > > > index 000000000000..c2c56ecafccb > > > --- /dev/null > > > +++ b/drivers/perf/arm_cspmu/Kconfig > > > @@ -0,0 +1,13 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > +# > > > +# Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. > > > + > > > +config ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU > > > + tristate "ARM Coresight Architecture PMU" > > > + depends on ACPI > > > + depends on ACPI_APMT || COMPILE_TEST > > > + help > > > + Provides support for performance monitoring unit (PMU) devices > > > + based on ARM CoreSight PMU architecture. Note that this PMU > > > + architecture does not have relationship with the ARM CoreSight > > > + Self-Hosted Tracing. > > > diff --git a/drivers/perf/arm_cspmu/Makefile > > b/drivers/perf/arm_cspmu/Makefile > > > new file mode 100644 > > > index 000000000000..cdc3455f74d8 > > > --- /dev/null > > > +++ b/drivers/perf/arm_cspmu/Makefile > > > @@ -0,0 +1,6 @@ > > > +# Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. > > > +# > > > +# SPDX-License-Identifier: GPL-2.0 > > > + > > > +obj-$(CONFIG_ARM_CORESIGHT_PMU_ARCH_SYSTEM_PMU) += \ > > > + arm_cspmu.o > > > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c > > b/drivers/perf/arm_cspmu/arm_cspmu.c > > > new file mode 100644 > > > index 000000000000..0fa5f29bf1c6 > > > --- /dev/null > > > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c > > > @@ -0,0 +1,1276 @@ > > > > > > > +/* Default event list. */ > > > +static struct attribute *arm_cspmu_event_attrs[] = { > > > + ARM_CSPMU_EVENT_ATTR(cycles, > > ARM_CSPMU_EVT_CYCLES_DEFAULT), > > > + NULL, > > > +}; > > > + > > > +static struct attribute ** > > > +arm_cspmu_get_event_attrs(const struct arm_cspmu *cspmu) > > > +{ > > > + return arm_cspmu_event_attrs; > > > +} > > > > This would make all the "PMU" instances (which don't implement the > > callback) share a non-const array. Could we instead return a copy of > > the attrs to avoid drivers messing up with the array ? > > The array could be allocated via devm_ on the specific device, thus > > it gets cleaned up on the device tear down ? > > > > ... > > > > > +static struct attribute ** > > > +arm_cspmu_get_format_attrs(const struct arm_cspmu *cspmu) > > > +{ > > > + return arm_cspmu_format_attrs; > > > +} > > > + > > > > Same as above > > I was expecting the vendor specific impl to be aware and make its own > copy if it needs to make modifications to the array. But I can return a copy > to be on the safe side. We can revisit again if it is unnecessary. > > I will send the updated patch shortly. Here is the link to the latest patch: https://lore.kernel.org/linux-arm-kernel/20220930023443.9463-2-bwicaksono@xxxxxxxxxx/ Thanks, Besar
