RE: [EXTERNAL] Re: [PATCH v4 1/3] perf: marvell: Add MPAM partid filtering to CN10K TAD PMU

[Geethasowjanya Akula]

>-----Original Message-----
>From: Ben Horgan <ben.horgan@xxxxxxx>
>Sent: Thursday, June 25, 2026 7:23 PM
>To: Geethasowjanya Akula <gakula@xxxxxxxxxxx>; linux-perf-
>users@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
>kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
>Cc: mark.rutland@xxxxxxx; will@xxxxxxxxxx; krzk+dt@xxxxxxxxxx;
>james.morse@xxxxxxx
>Subject: [EXTERNAL] Re: [PATCH v4 1/3] perf: marvell: Add MPAM partid
>filtering to CN10K TAD PMU
>Hi Geetha,
>
>+CC James
>
>On 6/18/26 16:36, Geetha sowjanya wrote:
>> From: Tanmay Jagdale <tanmay@xxxxxxxxxxx>
>>
>> The TAD PMU exposes counters that can be filtered by MPAM partition id
>> for a subset of allocation and hit events.
>>
>> Add a 9-bit partid format attribute (config1) and route counter
>> programming through variant-specific ops so CN10K keeps MPAM-capable
>> programming while Odyssey keeps the reduced event set without advertising
>partid in sysfs.
>>
>> Probe no longer mutates the platform_device MMIO resource (walk a
>> local map_start), rejects tad-cnt / page sizes of zero, validates the
>> memory window against tad-cnt, and registers the perf PMU before
>> hotplug with correct unwind.
>>
>> Example:
>>   perf stat -e tad/tad_alloc_any,partid=0x12,partid_en=1/ -- <program>
>
>Where is the user expected to get the PARTID from? The MPAM driver
>considers the PARTID as an internal only value.
>
>resctrl does support a 'debug' mount option which will show the CLOSID
>associated with a control group. Whilst the CLOSID is often the PARTID, it is
>really a set of PARTIDs. When the cdp mount option is used, CLOSID maps to 2
>PARTIDs and if we use PARTID narrowing to give us more monitors, as in
>proposed in [1], then the set of PARTIDs may be bigger.
>Furthermore, if the PARTID narrowing scheme is made dynamic the size of the
>PARTID set may change when control or monitoring groups are created or
>deleted.
>
>It seems that a way to map from a resctrl control group to the set of PARTIDs is
>required and a mechanism to tie this to lifetime of the resctrl mount.
>
>Perhaps some helpers along the lines of:
>
>int resctrl_mount_generation(void)
>int mpam_rdtgrp_to_partid_is_static(int mount_gen) int
>resctrl_rdtgrp_generation(char *name) int
>mpam_rdtgrp_to_partid_count(char *name, int rdt_gen) int
>mpam_rdtgrp_to_partid_array(char *name, int rdt_gen, int* partids)
>
>The rdtgrp generation is to an attempt to avoid having to use a debug interface
>in anger and cope with renaming of control groups in resctrl.
>This does seem a bit unwieldly so hopefully there is better way to do this.
>
>Sorry to throw a spanner in the works.
On …, … wrote:
> Where is the user expected to get the PARTID from? The MPAM driver
> considers the PARTID as an internal only value.
> …
> Perhaps some helpers along the lines of:
> int resctrl_mount_generation(void)
> …
Hi Ben,

Thank you for the detailed feedback — the concern you raise is valid, particularly when 
viewed from the perspective of resctrl-managed deployments.

However, to clarify the intent of this patch: the exposure of partid in the TAD PMU is deliberately
a low-level, hardware-facing interface, and is not intended to integrate with or mirror the
abstractions provided by resctrl. It is mainly meant for platform bring-up and low-level
performance/debug users, who already have explicit knowledge of the MPAM configuration,
typically provisioned by firmware or other privileged software layers (e.g. EL3/EL2).
In such environments, PARTIDs are known out-of-band, so the expectation is that the
user supplying partid is already aware of the MPAM IDs programmed on the system.

A proper “profile this resctrl group” path would require MPAM–resctrl support (e.g. something along the lines of the helpers you suggest) 
to resolve a group to its PARTID set. This is indeed important, but it constitutes a separate design discussion that is outside the scope of this driver patch.

We will clarify this in the commit message and avoid implying that users normally obtain PARTIDs from resctrl today.


Thanks,
Geetha
>
>Thanks,
>
>Ben
>
>>
>> Signed-off-by: Tanmay Jagdale <tanmay@xxxxxxxxxxx>
>> Signed-off-by: Geetha sowjanya <gakula@xxxxxxxxxxx>
>> ---
>>
>> Changelog (since v3)
>> --------------------
>> - Restore cpuhp_state_add_instance_nocalls before perf_pmu_register in
>probe
>>   so users cannot attach events before the hotplug instance exists; unwind
>>   removes the hotplug instance if perf registration fails.
>> - Add perf_ready: tad_pmu_offline_cpu skips perf_pmu_migrate_context
>until after
>>   successful perf_pmu_register, so a CPU offline between hotplug add and
>perf
>>   register does not touch perf core state for an unregistered PMU.
>>
>> Changelog (since v2)
>> --------------------
>> - Validate the eventId using an appropriate mask to ensure
>>   it is restricted to 8 bits.
>>
>> Changelog (since v1)
>> --------------------
>> - Fix config1 filter enable to use bit 9 consistently with the PMU format
>>   string (partid_en) and reject reserved bits with GENMASK(9, 0).
>> - Register perf_pmu_register before cpuhp_state_add_instance_nocalls and
>>   unregister on hotplug failure.
>>
>>  drivers/perf/marvell_cn10k_tad_pmu.c | 220
>> +++++++++++++++++++++------
>>  1 file changed, 171 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c
>> b/drivers/perf/marvell_cn10k_tad_pmu.c
>> index 51ccb0befa05..340be3776fe7 100644
>> --- a/drivers/perf/marvell_cn10k_tad_pmu.c
>> +++ b/drivers/perf/marvell_cn10k_tad_pmu.c
>> @@ -7,6 +7,8 @@
>>  #define pr_fmt(fmt) "tad_pmu: " fmt
>>
>>  #include <linux/io.h>
>> +#include <linux/bits.h>
>> +#include <linux/compiler.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/cpuhotplug.h>
>> @@ -14,12 +16,20 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/acpi.h>
>>
>> -#define TAD_PFC_OFFSET		0x800
>> -#define TAD_PFC(counter)	(TAD_PFC_OFFSET | (counter << 3))
>>  #define TAD_PRF_OFFSET		0x900
>> -#define TAD_PRF(counter)	(TAD_PRF_OFFSET | (counter << 3))
>> +#define TAD_PFC_OFFSET		0x800
>> +#define TAD_PFC(base, counter)	((base) | ((u64)(counter) << 3))
>> +#define TAD_PRF(base, counter)	((base) | ((u64)(counter) << 3))
>>  #define TAD_PRF_CNTSEL_MASK	0xFF
>> +#define TAD_PRF_MATCH_PARTID	BIT(8)
>> +#define TAD_PRF_PARTID_NS	BIT(10)
>> +/*
>> + * config1: bits 0..8 MPAM partition id (including 0); bit 9 requests
>> + * filtering for MPAM-capable events. All-zero config1 means no filter.
>> + */
>> +#define TAD_PARTID_FILTER_EN	BIT(9)
>>  #define TAD_MAX_COUNTERS	8
>> +#define TAD_EVENT_SEL_MASK	GENMASK(7, 0)
>>
>>  #define to_tad_pmu(p) (container_of(p, struct tad_pmu, pmu))
>>
>> @@ -27,30 +37,94 @@ struct tad_region {
>>  	void __iomem	*base;
>>  };
>>
>> +enum mrvl_tad_pmu_version {
>> +	TAD_PMU_V1 = 1,
>> +	TAD_PMU_V2,
>> +};
>> +
>> +struct tad_pmu_data {
>> +	int id;
>> +	u64 tad_prf_offset;
>> +	u64 tad_pfc_offset;
>> +};
>> +
>>  struct tad_pmu {
>>  	struct pmu pmu;
>>  	struct tad_region *regions;
>>  	u32 region_cnt;
>>  	unsigned int cpu;
>> +	/* Set after successful perf_pmu_register(); gates offline migration. */
>> +	bool perf_ready;
>> +	const struct tad_pmu_ops *ops;
>> +	const struct tad_pmu_data *pdata;
>>  	struct hlist_node node;
>>  	struct perf_event *events[TAD_MAX_COUNTERS];
>>  	DECLARE_BITMAP(counters_map, TAD_MAX_COUNTERS);  };
>>
>> -enum mrvl_tad_pmu_version {
>> -	TAD_PMU_V1 = 1,
>> -	TAD_PMU_V2,
>> -};
>> -
>> -struct tad_pmu_data {
>> -	int id;
>> +struct tad_pmu_ops {
>> +	void (*start_counter)(struct tad_pmu *pmu, struct perf_event
>> +*event);
>>  };
>>
>>  static int tad_pmu_cpuhp_state;
>>
>> +static void tad_pmu_start_counter(struct tad_pmu *pmu,
>> +				  struct perf_event *event)
>> +{
>> +	const struct tad_pmu_data *pdata = pmu->pdata;
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	u32 event_idx = (u32)(event->attr.config & TAD_EVENT_SEL_MASK);
>> +	u32 counter_idx = hwc->idx;
>> +	u64 partid_filter = 0;
>> +	u64 reg_val;
>> +	u64 cfg1 = event->attr.config1;
>> +	bool use_mpam = cfg1 & TAD_PARTID_FILTER_EN;
>> +	u32 partid = (u32)(cfg1 & GENMASK(8, 0));
>> +	int i;
>> +
>> +	for (i = 0; i < pmu->region_cnt; i++)
>> +		writeq_relaxed(0, pmu->regions[i].base +
>> +			       TAD_PFC(pdata->tad_pfc_offset, counter_idx));
>> +
>> +	if (use_mpam && event_idx > 0x19 && event_idx < 0x21) {
>> +		partid_filter = TAD_PRF_MATCH_PARTID |
>TAD_PRF_PARTID_NS |
>> +				((u64)partid << 11);
>> +	}
>> +
>> +
>> +	for (i = 0; i < pmu->region_cnt; i++) {
>> +		reg_val = event_idx & 0xFF;
>> +		reg_val |= partid_filter;
>> +		writeq_relaxed(reg_val, pmu->regions[i].base +
>> +			       TAD_PRF(pdata->tad_prf_offset, counter_idx));
>> +	}
>> +}
>> +
>> +static void tad_pmu_v2_start_counter(struct tad_pmu *pmu,
>> +				     struct perf_event *event)
>> +{
>> +	const struct tad_pmu_data *pdata = pmu->pdata;
>> +	struct hw_perf_event *hwc = &event->hw;
>> +	u32 event_idx = (u32)(event->attr.config & TAD_EVENT_SEL_MASK);
>> +	u32 counter_idx = hwc->idx;
>> +	u64 reg_val;
>> +	int i;
>> +
>> +	for (i = 0; i < pmu->region_cnt; i++)
>> +		writeq_relaxed(0, pmu->regions[i].base +
>> +			       TAD_PFC(pdata->tad_pfc_offset, counter_idx));
>> +
>> +	for (i = 0; i < pmu->region_cnt; i++) {
>> +		reg_val = event_idx & 0xFF;
>> +		writeq_relaxed(reg_val, pmu->regions[i].base +
>> +			       TAD_PRF(pdata->tad_prf_offset, counter_idx));
>> +	}
>> +}
>> +
>>  static void tad_pmu_event_counter_read(struct perf_event *event)  {
>>  	struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
>> +	const struct tad_pmu_data *pdata = tad_pmu->pdata;
>>  	struct hw_perf_event *hwc = &event->hw;
>>  	u32 counter_idx = hwc->idx;
>>  	u64 prev, new;
>> @@ -60,7 +134,7 @@ static void tad_pmu_event_counter_read(struct
>perf_event *event)
>>  		prev = local64_read(&hwc->prev_count);
>>  		for (i = 0, new = 0; i < tad_pmu->region_cnt; i++)
>>  			new += readq(tad_pmu->regions[i].base +
>> -				     TAD_PFC(counter_idx));
>> +				     TAD_PFC(pdata->tad_pfc_offset,
>counter_idx));
>>  	} while (local64_cmpxchg(&hwc->prev_count, prev, new) != prev);
>>
>>  	local64_add(new - prev, &event->count); @@ -69,16 +143,14 @@
>static
>> void tad_pmu_event_counter_read(struct perf_event *event)  static void
>> tad_pmu_event_counter_stop(struct perf_event *event, int flags)  {
>>  	struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
>> +	const struct tad_pmu_data *pdata = tad_pmu->pdata;
>>  	struct hw_perf_event *hwc = &event->hw;
>>  	u32 counter_idx = hwc->idx;
>>  	int i;
>>
>> -	/* TAD()_PFC() stop counting on the write
>> -	 * which sets TAD()_PRF()[CNTSEL] == 0
>> -	 */
>>  	for (i = 0; i < tad_pmu->region_cnt; i++) {
>>  		writeq_relaxed(0, tad_pmu->regions[i].base +
>> -			       TAD_PRF(counter_idx));
>> +			       TAD_PRF(pdata->tad_prf_offset, counter_idx));
>>  	}
>>
>>  	tad_pmu_event_counter_read(event);
>> @@ -89,26 +161,10 @@ static void tad_pmu_event_counter_start(struct
>> perf_event *event, int flags)  {
>>  	struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
>>  	struct hw_perf_event *hwc = &event->hw;
>> -	u32 event_idx = event->attr.config;
>> -	u32 counter_idx = hwc->idx;
>> -	u64 reg_val;
>> -	int i;
>>
>>  	hwc->state = 0;
>>
>> -	/* Typically TAD_PFC() are zeroed to start counting */
>> -	for (i = 0; i < tad_pmu->region_cnt; i++)
>> -		writeq_relaxed(0, tad_pmu->regions[i].base +
>> -			       TAD_PFC(counter_idx));
>> -
>> -	/* TAD()_PFC() start counting on the write
>> -	 * which sets TAD()_PRF()[CNTSEL] != 0
>> -	 */
>> -	for (i = 0; i < tad_pmu->region_cnt; i++) {
>> -		reg_val = event_idx & 0xFF;
>> -		writeq_relaxed(reg_val,	tad_pmu->regions[i].base +
>> -			       TAD_PRF(counter_idx));
>> -	}
>> +	tad_pmu->ops->start_counter(tad_pmu, event);
>>  }
>>
>>  static void tad_pmu_event_counter_del(struct perf_event *event, int
>> flags) @@ -128,7 +184,6 @@ static int tad_pmu_event_counter_add(struct
>perf_event *event, int flags)
>>  	struct hw_perf_event *hwc = &event->hw;
>>  	int idx;
>>
>> -	/* Get a free counter for this event */
>>  	idx = find_first_zero_bit(tad_pmu->counters_map,
>TAD_MAX_COUNTERS);
>>  	if (idx == TAD_MAX_COUNTERS)
>>  		return -EAGAIN;
>> @@ -148,6 +203,9 @@ static int tad_pmu_event_counter_add(struct
>> perf_event *event, int flags)  static int tad_pmu_event_init(struct
>> perf_event *event)  {
>>  	struct tad_pmu *tad_pmu = to_tad_pmu(event->pmu);
>> +	const struct tad_pmu_data *pdata = tad_pmu->pdata;
>> +	u32 event_idx = (u32)(event->attr.config & TAD_EVENT_SEL_MASK);
>> +	u64 cfg1 = event->attr.config1;
>>
>>  	if (event->attr.type != event->pmu->type)
>>  		return -ENOENT;
>> @@ -158,6 +216,23 @@ static int tad_pmu_event_init(struct perf_event
>*event)
>>  	if (event->state != PERF_EVENT_STATE_OFF)
>>  		return -EINVAL;
>>
>> +	if (event->attr.config & ~TAD_EVENT_SEL_MASK)
>> +		return -EINVAL;
>> +
>> +	if (pdata->id == TAD_PMU_V2) {
>> +		if (cfg1)
>> +			return -EINVAL;
>> +	} else {
>> +		if ((cfg1 & GENMASK(8, 0)) && !(cfg1 &
>TAD_PARTID_FILTER_EN))
>> +			return -EINVAL;
>> +		if (cfg1 & TAD_PARTID_FILTER_EN) {
>> +			if (event_idx <= 0x19 || event_idx >= 0x21)
>> +				return -EINVAL;
>> +		}
>> +		if (cfg1 & ~GENMASK(9, 0))
>> +			return -EINVAL;
>> +	}
>> +
>>  	event->cpu = tad_pmu->cpu;
>>  	event->hw.idx = -1;
>>  	event->hw.config_base = event->attr.config; @@ -232,7 +307,7 @@
>> static struct attribute *ody_tad_pmu_event_attrs[] = {
>>  	TAD_PMU_EVENT_ATTR(tad_hit_ltg, 0x1e),
>>  	TAD_PMU_EVENT_ATTR(tad_hit_any, 0x1f),
>>  	TAD_PMU_EVENT_ATTR(tad_tag_rd, 0x20),
>> -	TAD_PMU_EVENT_ATTR(tad_tot_cycle, 0xFF),
>> +	TAD_PMU_EVENT_ATTR(tad_tot_cycle, 0xff),
>>  	NULL
>>  };
>>
>> @@ -242,9 +317,13 @@ static const struct attribute_group
>> ody_tad_pmu_events_attr_group = {  };
>>
>>  PMU_FORMAT_ATTR(event, "config:0-7");
>> +PMU_FORMAT_ATTR(partid, "config1:0-8"); PMU_FORMAT_ATTR(partid_en,
>> +"config1:9-9");
>>
>>  static struct attribute *tad_pmu_format_attrs[] = {
>>  	&format_attr_event.attr,
>> +	&format_attr_partid.attr,
>> +	&format_attr_partid_en.attr,
>>  	NULL
>>  };
>>
>> @@ -253,6 +332,16 @@ static struct attribute_group
>tad_pmu_format_attr_group = {
>>  	.attrs = tad_pmu_format_attrs,
>>  };
>>
>> +static struct attribute *ody_tad_pmu_format_attrs[] = {
>> +	&format_attr_event.attr,
>> +	NULL
>> +};
>> +
>> +static struct attribute_group ody_tad_pmu_format_attr_group = {
>> +	.name = "format",
>> +	.attrs = ody_tad_pmu_format_attrs,
>> +};
>> +
>>  static ssize_t tad_pmu_cpumask_show(struct device *dev,
>>  				struct device_attribute *attr, char *buf)  { @@
>-281,16 +370,25
>> @@ static const struct attribute_group *tad_pmu_attr_groups[] = {
>>
>>  static const struct attribute_group *ody_tad_pmu_attr_groups[] = {
>>  	&ody_tad_pmu_events_attr_group,
>> -	&tad_pmu_format_attr_group,
>> +	&ody_tad_pmu_format_attr_group,
>>  	&tad_pmu_cpumask_attr_group,
>>  	NULL
>>  };
>>
>> +static const struct tad_pmu_ops tad_pmu_ops = {
>> +	.start_counter = tad_pmu_start_counter, };
>> +
>> +static const struct tad_pmu_ops tad_pmu_v2_ops = {
>> +	.start_counter = tad_pmu_v2_start_counter, };
>> +
>>  static int tad_pmu_probe(struct platform_device *pdev)  {
>>  	const struct tad_pmu_data *dev_data;
>>  	struct device *dev = &pdev->dev;
>>  	struct tad_region *regions;
>> +	resource_size_t map_start;
>>  	struct tad_pmu *tad_pmu;
>>  	struct resource *res;
>>  	u32 tad_pmu_page_size;
>> @@ -298,7 +396,6 @@ static int tad_pmu_probe(struct platform_device
>*pdev)
>>  	u32 tad_cnt;
>>  	int version;
>>  	int i, ret;
>> -	char *name;
>>
>>  	tad_pmu = devm_kzalloc(&pdev->dev, sizeof(*tad_pmu),
>GFP_KERNEL);
>>  	if (!tad_pmu)
>> @@ -312,6 +409,7 @@ static int tad_pmu_probe(struct platform_device
>*pdev)
>>  		return -ENODEV;
>>  	}
>>  	version = dev_data->id;
>> +	tad_pmu->pdata = dev_data;
>>
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>  	if (!res) {
>> @@ -338,22 +436,31 @@ static int tad_pmu_probe(struct platform_device
>*pdev)
>>  		dev_err(&pdev->dev, "Can't find tad-cnt property\n");
>>  		return ret;
>>  	}
>> +	if (!tad_cnt || !tad_page_size || !tad_pmu_page_size) {
>> +		dev_err(&pdev->dev, "Invalid tad-cnt or page size\n");
>> +		return -EINVAL;
>> +	}
>>
>>  	regions = devm_kcalloc(&pdev->dev, tad_cnt,
>>  			       sizeof(*regions), GFP_KERNEL);
>>  	if (!regions)
>>  		return -ENOMEM;
>>
>> -	/* ioremap the distributed TAD pmu regions */
>> -	for (i = 0; i < tad_cnt && res->start < res->end; i++) {
>> -		regions[i].base = devm_ioremap(&pdev->dev,
>> -					       res->start,
>> +	map_start = res->start;
>> +	for (i = 0; i < tad_cnt; i++) {
>> +		if (map_start > res->end ||
>> +		    tad_pmu_page_size > (resource_size_t)(res->end -
>map_start + 1)) {
>> +			dev_err(&pdev->dev, "TAD PMU mem window too
>small for tad-cnt=%u\n",
>> +				tad_cnt);
>> +			return -EINVAL;
>> +		}
>> +		regions[i].base = devm_ioremap(&pdev->dev, map_start,
>>  					       tad_pmu_page_size);
>>  		if (!regions[i].base) {
>>  			dev_err(&pdev->dev, "TAD%d ioremap fail\n", i);
>>  			return -ENOMEM;
>>  		}
>> -		res->start += tad_page_size;
>> +		map_start += tad_page_size;
>>  	}
>>
>>  	tad_pmu->regions = regions;
>> @@ -374,14 +481,16 @@ static int tad_pmu_probe(struct platform_device
>*pdev)
>>  		.read		= tad_pmu_event_counter_read,
>>  	};
>>
>> -	if (version == TAD_PMU_V1)
>> +	if (version == TAD_PMU_V1) {
>>  		tad_pmu->pmu.attr_groups = tad_pmu_attr_groups;
>> -	else
>> +		tad_pmu->ops		 = &tad_pmu_ops;
>> +	} else {
>>  		tad_pmu->pmu.attr_groups = ody_tad_pmu_attr_groups;
>> +		tad_pmu->ops		 = &tad_pmu_v2_ops;
>> +	}
>>
>>  	tad_pmu->cpu = raw_smp_processor_id();
>>
>> -	/* Register pmu instance for cpu hotplug */
>>  	ret = cpuhp_state_add_instance_nocalls(tad_pmu_cpuhp_state,
>>  					       &tad_pmu->node);
>>  	if (ret) {
>> @@ -389,19 +498,24 @@ static int tad_pmu_probe(struct platform_device
>*pdev)
>>  		return ret;
>>  	}
>>
>> -	name = "tad";
>> -	ret = perf_pmu_register(&tad_pmu->pmu, name, -1);
>> -	if (ret)
>> +	ret = perf_pmu_register(&tad_pmu->pmu, "tad", -1);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Error %d registering perf PMU\n", ret);
>>  		cpuhp_state_remove_instance_nocalls(tad_pmu_cpuhp_state,
>>  						    &tad_pmu->node);
>> +		return ret;
>> +	}
>>
>> -	return ret;
>> +	WRITE_ONCE(tad_pmu->perf_ready, true);
>> +
>> +	return 0;
>>  }
>>
>>  static void tad_pmu_remove(struct platform_device *pdev)  {
>>  	struct tad_pmu *pmu = platform_get_drvdata(pdev);
>>
>> +	WRITE_ONCE(pmu->perf_ready, false);
>>  	cpuhp_state_remove_instance_nocalls(tad_pmu_cpuhp_state,
>>  						&pmu->node);
>>  	perf_pmu_unregister(&pmu->pmu);
>> @@ -410,12 +524,17 @@ static void tad_pmu_remove(struct
>> platform_device *pdev)  #if defined(CONFIG_OF) || defined(CONFIG_ACPI)
>> static const struct tad_pmu_data tad_pmu_data = {
>>  	.id   = TAD_PMU_V1,
>> +	.tad_prf_offset = TAD_PRF_OFFSET,
>> +	.tad_pfc_offset = TAD_PFC_OFFSET,
>>  };
>> +
>>  #endif
>>
>>  #ifdef CONFIG_ACPI
>>  static const struct tad_pmu_data tad_pmu_v2_data = {
>>  	.id   = TAD_PMU_V2,
>> +	.tad_prf_offset = TAD_PRF_OFFSET,
>> +	.tad_pfc_offset = TAD_PFC_OFFSET,
>>  };
>>  #endif
>>
>> @@ -451,6 +570,9 @@ static int tad_pmu_offline_cpu(unsigned int cpu,
>struct hlist_node *node)
>>  	struct tad_pmu *pmu = hlist_entry_safe(node, struct tad_pmu, node);
>>  	unsigned int target;
>>
>> +	if (!READ_ONCE(pmu->perf_ready))
>> +		return 0;
>> +
>>  	if (cpu != pmu->cpu)
>>  		return 0;
>>
>> @@ -491,6 +613,6 @@ static void __exit tad_pmu_exit(void)
>> module_init(tad_pmu_init);  module_exit(tad_pmu_exit);
>>
>> -MODULE_DESCRIPTION("Marvell CN10K LLC-TAD Perf driver");
>> +MODULE_DESCRIPTION("Marvell CN10K LLC-TAD perf driver");
>>  MODULE_AUTHOR("Bhaskara Budiredla <bbudiredla@xxxxxxxxxxx>");
>> MODULE_LICENSE("GPL v2");