Re: [PATCH v6 0/3] arm64: topology: Handle AMU FIE setup on CPU hotplug

[Beata Michalska <beata.michalska@arm.com>]

On Tue, Dec 02, 2025 at 11:05:25AM +0800, zhenglifeng (A) wrote:
> On 2025/12/1 23:27, Beata Michalska wrote:
> > Hi,
> > 
> > Apologies for the delay in reviewing this - currently in progress....
> > Out of curiosity: what's the cpufreq driver used for testing this series ?
> 
> I used cppc_cpufreq for testing this. But with some modifications in
> processor_driver.c, or you'll find that the driver will fail to load with
> maxcpus set. The modification below is only a temporary solution. I'm still
> working on that.
>
Right, so overall the implementation looks good - thanks for that.
There are two issues though with the cppc cpufreq driver.

One: as you have already noticed - it fails to register when
cpumask_present != cpumask_online.

Second: it will mix the sources of the freq scale if not all CPUs within the
policy have AMUs enabled/valid. This is due to the fact that at the time of
registering the driver and initializing the FIE support policy->cpus ==
policy->related_cpus. Assuming scenario when there are two CPUs within the
policy, one being offline and missing valid AMU counters,
the topology_set_scale_freq_source from cppc cpufreq driver will register
the tick handler for both CPUs, whereas AMU support will (rightly so) register
only for the firs one. When the second CPU comes online, the mismatch will be
detected and the arch callback will get cleared for the first CPU, but the
second one will remain unchanged.

That said, I do not think any of those issues is a blocker for this series.
But both would need fixing.

---
BR
Beata


> ---
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 5d824435b26b..2f286a1b0b02 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -33,6 +33,7 @@ MODULE_AUTHOR("Paul Diefenbaugh");
>  MODULE_DESCRIPTION("ACPI Processor Driver");
>  MODULE_LICENSE("GPL");
>  
> +static int acpi_processor_start(struct device *dev);
>  static int acpi_processor_stop(struct device *dev);
>  
>  static const struct acpi_device_id processor_device_ids[] = {
> @@ -46,6 +47,7 @@ static struct device_driver acpi_processor_driver = {
>  	.name = "processor",
>  	.bus = &cpu_subsys,
>  	.acpi_match_table = processor_device_ids,
> +	.probe = acpi_processor_start,
>  	.remove = acpi_processor_stop,
>  };
>  
> @@ -191,6 +193,21 @@ static int __acpi_processor_start(struct acpi_device *device)
>  	return result;
>  }
>  
> +static int acpi_processor_start(struct device *dev)
> +{
> +	struct acpi_device *device = ACPI_COMPANION(dev);
> +	int ret;
> +
> +	if (!device)
> +		return -ENODEV;
> +
> +	/* Protect against concurrent CPU hotplug operations */
> +	cpu_hotplug_disable();
> +	ret = __acpi_processor_start(device);
> +	cpu_hotplug_enable();
> +	return ret;
> +}
> +
>  static int acpi_processor_stop(struct device *dev)
>  {
>  	struct acpi_device *device = ACPI_COMPANION(dev);
> @@ -264,9 +281,9 @@ static int __init acpi_processor_driver_init(void)
>  
>  	acpi_processor_register_idle_driver();
>  
> -	result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> -				   "acpi/cpu-drv:online",
> -				   acpi_soft_cpu_online, NULL);
> +	result = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> +					   "acpi/cpu-drv:online",
> +					   acpi_soft_cpu_online, NULL);
>  	if (result < 0)
>  		goto err;
>  	hp_online = result;
> 
> > 
> > ---
> > BR
> > Beata
> > On Wed, Nov 19, 2025 at 04:13:53PM +0800, Lifeng Zheng wrote:
> >> Solve a problem that causes CPUs Setup AMU FIE failed in a corner case,
> >> even though they're eligible.
> >>
> >>
> > 
>