Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Besar Wicaksono <bwicaksono@nvidia.com>
To:     Will Deacon <will@kernel.org>
CC:     "suzuki.poulose@arm.com" <suzuki.poulose@arm.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>,
        "catalin.marinas@arm.com" <catalin.marinas@arm.com>,
        "mark.rutland@arm.com" <mark.rutland@arm.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
        "sudeep.holla@arm.com" <sudeep.holla@arm.com>,
        "thanu.rangarajan@arm.com" <thanu.rangarajan@arm.com>,
        "Michael.Williams@arm.com" <Michael.Williams@arm.com>,
        Thierry Reding <treding@nvidia.com>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Vikram Sethi <vsethi@nvidia.com>,
        "mathieu.poirier@linaro.org" <mathieu.poirier@linaro.org>,
        "mike.leach@linaro.org" <mike.leach@linaro.org>,
        "leo.yan@linaro.org" <leo.yan@linaro.org>
Subject: RE: [PATCH v4 1/2] perf: arm_cspmu: Add support for ARM CoreSight PMU
 driver
Thread-Topic: [PATCH v4 1/2] perf: arm_cspmu: Add support for ARM CoreSight
 PMU driver
Thread-Index: AQHYsAscx+1Do35BjUm/Kr4IwLOFta3rtSeAgActf4A=
Date:   Tue, 27 Sep 2022 03:59:22 +0000
Message-ID: <SJ0PR12MB56763459EE6CE776680E0762A0559@SJ0PR12MB5676.namprd12.prod.outlook.com>
References: <20220814182351.8861-1-bwicaksono@nvidia.com>
 <20220814182351.8861-2-bwicaksono@nvidia.com>
 <20220922135257.GD12095@willie-the-truck>
In-Reply-To: <20220922135257.GD12095@willie-the-truck>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?xJVOso1THm7oVLaVIWeJebbGXI0D1QCJ2U/8RDRHKGKdyN6mVJLn+Lc8vDWs?=
 =?us-ascii?Q?3Ke+pUWPmCVKuwci/9811SjKO/oynaRb+SYacCdMQJXwLmQ1JslMbk1zO5oh?=
 =?us-ascii?Q?qVXAwd+R4Hn+6vGDRWwti0aIcn9CA62RherCdsIEqv+MmjpoAnP7fJjk2ZX7?=
 =?us-ascii?Q?xRfEiv1ILAKWIxOSqa9c3kF9/HFWMvInEvxPoPQKtw1hxyPW1eITF+bRa4i3?=
 =?us-ascii?Q?Om6gCPHX+ontNFe+hjhz/RNoQbUPbU9d4ItKQgU28saHeA0gIPwVJ+OGydyB?=
 =?us-ascii?Q?uegiaBGSC53vQ8xR6djmRLvtJRLiIMBOtBiExXHzOdQaJxxYtjxLGZ2Q4kFT?=
 =?us-ascii?Q?LhyAKuGcLS58+99tFbsdcgsb5kMiyHomHyJYYSamrm0jizjyOvNm+/Ix8YJ4?=
 =?us-ascii?Q?9Tcux6WC4zwTdNmHG3yg7cpwjzyjYTJWwfXuZZDrX0ru8AFzJ47O64KL8t5N?=
 =?us-ascii?Q?v9flxt6X8UKU4cizaIkue3Rv1Nm/etTk/lWrtspgE330VsiAlG9ws8e8YGC6?=
 =?us-ascii?Q?7gwCrhw/Iuja8UB3vxuAwwtPN3s7p/FSsdeMfvFEnFZ1AM6HjYJe/VmO/Mck?=
 =?us-ascii?Q?BaVKyexI40PjH84VV4m7+Cvwt9BwhK6fo9eJl8y0tYzHGFew0AwKx8bOJ7MP?=
 =?us-ascii?Q?HnVGq2df0Pav6PVL/CksXI3ZMzSVGct76p/Y4KmDdHpnGZNFw1bT6NmOGwoS?=
 =?us-ascii?Q?YuXVseMrRD+jcXKW9k5hKPP8WaLfcwISX+arGLr7lju0jCggMIwpERj8c7H0?=
 =?us-ascii?Q?ZglZYqbCOVs6Kxxn5L37IObuj22GQ9iVbf7ZEstRO53qThVq3H4EFSpOViZY?=
 =?us-ascii?Q?oPNkIlsZrgJRcHfVInJxN9MbpemTJ+eHSTBLZRWUKiOrDClBZz3YrkzpHE9x?=
 =?us-ascii?Q?JJhdkbwUoV6YZHrm5oKodfUDoQJgiyCe87XbqMM860O5KaNNNa9Mtq0nD1yY?=
 =?us-ascii?Q?xb0OST8Zja6jTPhU4ZxGDELicwU+BvH24WlfiNc/DAQVOqyvSvG/ABSst+Of?=
 =?us-ascii?Q?IZsWXt7PvZdcTk5ZiPZ/6SbG5AqFXgOu0FrpLg/jmLj9xn/BOLxXx+zfxjSN?=
 =?us-ascii?Q?19vmhaLxRjY+Jj4D0KJI+0BH2bEXADUwspSxv9tt5iBWsgVrHIhLnKybqQj+?=
 =?us-ascii?Q?K29RstlALbLIWB+5W8zL3iCsOBI9wqtQgPQtHYUnKKy65Wi8Mrm+RRyUlY7l?=
 =?us-ascii?Q?YOW6UfMJDgqnikbn05ODyGnBtNkR2iLVObd44R1PNIPZJU9Pj0YqpYEBSg+p?=
 =?us-ascii?Q?CJN2s5HmfVSMoMcugxDwNXGhoxRmQWMoKowbUSsJacV+5n4dCmMGAz9PrzQE?=
 =?us-ascii?Q?9qMqK1TSdSp9D8ZnVaBSGjVAhSFLbMLuZ5VYYoDtIdE2HRn5iJqDhdDIcepS?=
 =?us-ascii?Q?u09kQfUiL5hW/fiKbtOWLnb/YaO5XdTVVxD1J4hmIF7DXCAMkDjJLidwG5VK?=
 =?us-ascii?Q?9lTZIqz3vZ9sAqPnb5m9RIXhug7Eo7RJw3N0kAhc28XJ/UPRCA5ZolgriIuW?=
 =?us-ascii?Q?ZtOHDK/n9uDxbElWlwU8ZUFsa1qGqeUDM+6MjffrCGujZGD8vNJS19/6IDp5?=
 =?us-ascii?Q?8Bf69OqfCSdI/UWdEb6E4NHSBZaH26ceirQb4J2N?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR12MB5676.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 760677a5-2992-4f54-c9e4-08daa03ca991
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Sep 2022 03:59:22.9108
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: qTOwcfIbzt2Ox9rJe8tWpGGoiAllSoNQr/X5efIwYlrD6ucZISnLDD0/gfxAYtWsVWHZcazIG8dGQ54CqWXLgA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5076
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        SPF_HELO_PASS,SPF_NONE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Will,

Thanks for the comment. Please see my response inline.

> -----Original Message-----
> From: Will Deacon <will@kernel.org>
> Sent: Thursday, September 22, 2022 8:53 AM
> To: Besar Wicaksono <bwicaksono@nvidia.com>
> Cc: suzuki.poulose@arm.com; robin.murphy@arm.com;
> catalin.marinas@arm.com; mark.rutland@arm.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> tegra@vger.kernel.org; sudeep.holla@arm.com;
> thanu.rangarajan@arm.com; Michael.Williams@arm.com; Thierry Reding
> <treding@nvidia.com>; Jonathan Hunter <jonathanh@nvidia.com>; Vikram
> Sethi <vsethi@nvidia.com>; mathieu.poirier@linaro.org;
> mike.leach@linaro.org; leo.yan@linaro.org
> Subject: Re: [PATCH v4 1/2] perf: arm_cspmu: Add support for ARM
> CoreSight PMU driver
>=20
> External email: Use caution opening links or attachments
>=20
>=20
> On Sun, Aug 14, 2022 at 01:23:50PM -0500, 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
> >
> > Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
> > ---
> >  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 | 1262
> ++++++++++++++++++++++++++++
> >  drivers/perf/arm_cspmu/arm_cspmu.h |  151 ++++
> >  7 files changed, 1436 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
>=20
> [...]
>=20
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> b/drivers/perf/arm_cspmu/arm_cspmu.c
> > new file mode 100644
> > index 000000000000..410876f86eb0
> > --- /dev/null
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>=20
> [...]
>=20
> > +/*
> > + * Read 64-bit register as a pair of 32-bit registers using hi-lo-hi s=
equence.
> > + */
> > +static u64 read_reg64_hilohi(const void __iomem *addr)
> > +{
> > +     u32 val_lo, val_hi;
> > +     u64 val;
> > +
> > +     /* Use high-low-high sequence to avoid tearing */
> > +     do {
> > +             val_hi =3D readl(addr + 4);
> > +             val_lo =3D readl(addr);
> > +     } while (val_hi !=3D readl(addr + 4));
>=20
> Hmm, we probably want a timeout or something in here so we don't lock
> up the CPU if the device goes wonky.
>=20

This function is used to read the counter register. The perf driver APIs
(read, stop) that use this function do not return an error code. I am
not sure if we can just break the loop and return 0. Any suggestions ?
Is triggering a panic acceptable ?

> With that, how about adding this a helper to
> include/linux/io-64-nonatomic-*o.h so other folks can reuse it?
>=20
> > +/* Check if PMU supports 64-bit single copy atomic. */
> > +static inline bool supports_64bit_atomics(const struct arm_cspmu
> *cspmu)
> > +{
> > +     return CHECK_APMT_FLAG(cspmu->apmt_node->flags, ATOMIC,
> SUPP);
> > +}
>=20
> Is this just there because the architecture permits it, or are folks
> actually hanging these things off 32-bit MMIO buses on arm64 SoCs?
>=20

Yes, the PMU spec permits a system that needs to break 64-bit access into
a pair of 32-bit accesses.

> > +static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)
> > +{
> > +     int irq, ret;
> > +     struct device *dev;
> > +     struct platform_device *pdev;
> > +     struct acpi_apmt_node *apmt_node;
> > +
> > +     dev =3D cspmu->dev;
> > +     pdev =3D to_platform_device(dev);
> > +     apmt_node =3D cspmu->apmt_node;
> > +
> > +     /* Skip IRQ request if the PMU does not support overflow interrup=
t. */
> > +     if (apmt_node->ovflw_irq =3D=3D 0)
> > +             return 0;
>=20
> Set PERF_PMU_CAP_NO_INTERRUPT?
>=20

Thanks, I will apply it on the next version.

> Will