linux-kernel - RE: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <SJ0PR12MB567608B25A04CB5837BE5A57A03DA@SJ0PR12MB5676.namprd12.prod.outlook.com>
Date: Fri, 22 Aug 2025 01:28:59 +0000
From: Besar Wicaksono <bwicaksono@...dia.com>
To: Robin Murphy <robin.murphy@....com>, Ilkka Koskinen
	<ilkka@...amperecomputing.com>
CC: "will@...nel.org" <will@...nel.org>,
	"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>, "suzuki.poulose@....com"
	<suzuki.poulose@....com>, "mark.rutland@....com" <mark.rutland@....com>,
	Thierry Reding <treding@...dia.com>, Jon Hunter <jonathanh@...dia.com>,
	Vikram Sethi <vsethi@...dia.com>, Rich Wiley <rwiley@...dia.com>, Shanker
 Donthineni <sdonthineni@...dia.com>
Subject: RE: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node

Hi Robin,

Please see my comment inline.

> -----Original Message-----
> From: Robin Murphy <robin.murphy@....com>
> Sent: Thursday, August 21, 2025 11:05 AM
> To: Besar Wicaksono <bwicaksono@...dia.com>; Ilkka Koskinen
> <ilkka@...amperecomputing.com>
> Cc: will@...nel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; linux-tegra@...r.kernel.org;
> suzuki.poulose@....com; mark.rutland@....com; Thierry Reding
> <treding@...dia.com>; Jon Hunter <jonathanh@...dia.com>; Vikram Sethi
> <vsethi@...dia.com>; Rich Wiley <rwiley@...dia.com>; Shanker Donthineni
> <sdonthineni@...dia.com>
> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
> 
> External email: Use caution opening links or attachments
> 
> 
> On 2025-08-20 8:07 pm, Besar Wicaksono wrote:
> > Hi Ilkka,
> >
> > Thanks for the feedback. Please see my comment inline.
> >
> >> -----Original Message-----
> >> From: Ilkka Koskinen <ilkka@...amperecomputing.com>
> >> Sent: Tuesday, August 19, 2025 3:16 PM
> >> To: Besar Wicaksono <bwicaksono@...dia.com>
> >> Cc: will@...nel.org; linux-arm-kernel@...ts.infradead.org; linux-
> >> kernel@...r.kernel.org; linux-tegra@...r.kernel.org;
> >> suzuki.poulose@....com; robin.murphy@....com;
> >> ilkka@...amperecomputing.com; mark.rutland@....com; Thierry Reding
> >> <treding@...dia.com>; Jon Hunter <jonathanh@...dia.com>; Vikram Sethi
> >> <vsethi@...dia.com>; Rich Wiley <rwiley@...dia.com>; Shanker
> Donthineni
> >> <sdonthineni@...dia.com>
> >> Subject: Re: [PATCH 1/5] perf/arm_cspmu: Export arm_cspmu_apmt_node
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hi Ben,
> >>
> >> On Tue, 12 Aug 2025, Besar Wicaksono wrote:
> >>> Make arm_cspmu_apmt_node API accessible to vendor driver.
> >>
> >> I think I haven't seen the latest version of the spec. So, I'm curious,
> >> what kind of information the table has that the vendor drivers needs to
> >> have access to it?
> >>
> >
> > The vendor driver may need the node instance primary and secondary
> > fields to get additional properties of the PMU that is not covered
> > by the standard properties. For example, the PMU device entry in
> > APMT can be defined as ACPI node type. The node instance primary
> > and secondary will contain the HID and UID of an ACPI device object
> > that is associated with the PMU. This ACPI object can have more info
> > to supplement the standard props.
> 
> Rather than exposing the raw APMT data, maybe then cspmu should just
> encapsulate a method for retrieving the associated ACPI device, if any?

Thanks for the suggestion, this makes sense.

> I guess it could be a generalised "firmware device" notion - even though
> for DT that should mostly be cspmu->dev already - since that could then
> work neatly for generic device properties, but perhaps we don't expect
> many sub-drivers to support both ACPI and DT...
> 

How about making it explicitly return acpi_device object? i.e returns NULL
if not an ACPI node type. Vendor driver can choose (or not) to fallback to
cspmu->dev in case of DT.

Thanks,
Besar

> Thanks,
> Robin.
> 
> >>>
> >>> Signed-off-by: Besar Wicaksono <bwicaksono@...dia.com>
> >>> ---
> >>> drivers/perf/arm_cspmu/arm_cspmu.c | 3 ++-
> >>> drivers/perf/arm_cspmu/arm_cspmu.h | 4 ++++
> >>> 2 files changed, 6 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c
> >> b/drivers/perf/arm_cspmu/arm_cspmu.c
> >>> index efa9b229e701..e4b98cfa606c 100644
> >>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> >>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> >>> @@ -70,12 +70,13 @@ static void arm_cspmu_set_ev_filter(struct
> >> arm_cspmu *cspmu,
> >>> static void arm_cspmu_set_cc_filter(struct arm_cspmu *cspmu,
> >>>                                    const struct perf_event *event);
> >>>
> >>> -static struct acpi_apmt_node *arm_cspmu_apmt_node(struct device
> *dev)
> >>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev)
> >>> {
> >>>        struct acpi_apmt_node **ptr = dev_get_platdata(dev);
> >>>
> >>>        return ptr ? *ptr : NULL;
> >>> }
> >>> +EXPORT_SYMBOL_GPL(arm_cspmu_apmt_node);
> >>
> >> Rather than exporting the function, wouldn't it be better to move it to
> >> arm_cspmu.h instead?
> >
> > Sounds good to me. I will make the change on the next revision.
> >
> > Thanks,
> > Besar
> >
> >>
> >> Cheers, Ilkka
> >>
> >>>
> >>> /*
> >>>   * In CoreSight PMU architecture, all of the MMIO registers are 32-bit
> except
> >>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h
> >> b/drivers/perf/arm_cspmu/arm_cspmu.h
> >>> index 19684b76bd96..36c1dcce33d6 100644
> >>> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> >>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> >>> @@ -8,6 +8,7 @@
> >>> #ifndef __ARM_CSPMU_H__
> >>> #define __ARM_CSPMU_H__
> >>>
> >>> +#include <linux/acpi.h>
> >>> #include <linux/bitfield.h>
> >>> #include <linux/cpumask.h>
> >>> #include <linux/device.h>
> >>> @@ -222,4 +223,7 @@ int arm_cspmu_impl_register(const struct
> >> arm_cspmu_impl_match *impl_match);
> >>> /* Unregister vendor backend. */
> >>> void arm_cspmu_impl_unregister(const struct arm_cspmu_impl_match
> >> *impl_match);
> >>>
> >>> +/* Get ACPI APMT node. */
> >>> +struct acpi_apmt_node *arm_cspmu_apmt_node(struct device *dev);
> >>> +
> >>> #endif /* __ARM_CSPMU_H__ */
> >>> --
> >>> 2.47.0
> >>>
> >>>