lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <SJ0PR12MB5676FE048F8105D01C83D0EBA0579@SJ0PR12MB5676.namprd12.prod.outlook.com>
Date:   Thu, 29 Sep 2022 14:06:08 +0000
From:   Besar Wicaksono <bwicaksono@...dia.com>
To:     Suzuki K Poulose <suzuki.poulose@....com>,
        "robin.murphy@....com" <robin.murphy@....com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will@...nel.org" <will@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "sudeep.holla@....com" <sudeep.holla@....com>,
        "thanu.rangarajan@....com" <thanu.rangarajan@....com>,
        "Michael.Williams@....com" <Michael.Williams@....com>,
        Thierry Reding <treding@...dia.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Vikram Sethi <vsethi@...dia.com>,
        "mathieu.poirier@...aro.org" <mathieu.poirier@...aro.org>,
        "mike.leach@...aro.org" <mike.leach@...aro.org>,
        "leo.yan@...aro.org" <leo.yan@...aro.org>
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@....com>
> Sent: Thursday, September 29, 2022 3:29 AM
> To: Besar Wicaksono <bwicaksono@...dia.com>; robin.murphy@....com;
> catalin.marinas@....com; will@...nel.org; mark.rutland@....com
> Cc: linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> linux-tegra@...r.kernel.org; sudeep.holla@....com;
> thanu.rangarajan@....com; Michael.Williams@....com; Thierry Reding
> <treding@...dia.com>; Jonathan Hunter <jonathanh@...dia.com>; Vikram
> Sethi <vsethi@...dia.com>; mathieu.poirier@...aro.org;
> mike.leach@...aro.org; leo.yan@...aro.org
> 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@....com>
> > Signed-off-by: Besar Wicaksono <bwicaksono@...dia.com>
> 
> 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.

Thanks,
Besar

> 
> Suzuki

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ