mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
@ 2026-01-20 11:32 Lifeng Zheng
  2026-01-27 14:42 ` Rafael J. Wysocki
  2026-08-26  7:59 ` Pengjie Zhang
  0 siblings, 2 replies; 10+ messages in thread
From: Lifeng Zheng @ 2026-01-20 11:32 UTC (permalink / raw)
  To: rafael, catalin.marinas, Jonathan.Cameron, lenb
  Cc: linux-acpi, linux-kernel, linuxarm, gshan, miguel.luis,
	guohanjun, zhanjie9, lihuisong, yubowen8, zhangpengjie2,
	wangzhi12, linhongye, zhenglifeng1

Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
will fail to register. Because it requires the domain information of all
possible CPUs to construct shared_cpu_map, which shows the CPUs that share
the same domain.

Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
same path for cold and hotplug") removes probe() of acpi_processor_driver
and makes acpi_cppc_processor_probe() only being called the first time CPU
goes online. This means that CPUs that haven't yet gone online will not
have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.

Add acpi_processor_start() back as the probe() callback of
acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
sure all _CPC tables will be parsed when acpi_processor_driver registered.

Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
 drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 65e779be64ff..c8b4daf580b0 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,
 };
 
@@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
 	if (!pr)
 		return -ENODEV;
 
-	result = acpi_cppc_processor_probe(pr);
-	if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
-		dev_dbg(&device->dev, "CPPC data invalid or not present\n");
-
 	if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
 		acpi_processor_power_init(pr);
 
@@ -192,6 +190,30 @@ 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);
+	struct acpi_processor *pr;
+	int result;
+
+	if (!device)
+		return -ENODEV;
+
+	pr = acpi_driver_data(device);
+	if (!pr)
+		return -ENODEV;
+
+	/* Protect against concurrent CPU hotplug operations */
+	cpu_hotplug_disable();
+	result = acpi_cppc_processor_probe(pr);
+	cpu_hotplug_enable();
+
+	if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
+		dev_dbg(&device->dev, "CPPC data invalid or not present\n");
+
+	return 0;
+}
+
 static int acpi_processor_stop(struct device *dev)
 {
 	struct acpi_device *device = ACPI_COMPANION(dev);
-- 
2.33.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-01-20 11:32 [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online Lifeng Zheng
@ 2026-01-27 14:42 ` Rafael J. Wysocki
  2026-01-27 16:58   ` Jonathan Cameron
  2026-08-26  7:59 ` Pengjie Zhang
  1 sibling, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2026-01-27 14:42 UTC (permalink / raw)
  To: Lifeng Zheng, jonathan.cameron
  Cc: catalin.marinas, linux-acpi, linux-kernel, linuxarm, gshan,
	miguel.luis, guohanjun, zhanjie9, lihuisong, yubowen8,
	zhangpengjie2, wangzhi12, linhongye

On Tue, Jan 20, 2026 at 12:33 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
>
> Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
> will fail to register. Because it requires the domain information of all
> possible CPUs to construct shared_cpu_map, which shows the CPUs that share
> the same domain.
>
> Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
> same path for cold and hotplug") removes probe() of acpi_processor_driver
> and makes acpi_cppc_processor_probe() only being called the first time CPU
> goes online. This means that CPUs that haven't yet gone online will not
> have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
>
> Add acpi_processor_start() back as the probe() callback of
> acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
> sure all _CPC tables will be parsed when acpi_processor_driver registered.
>
> Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>  drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 65e779be64ff..c8b4daf580b0 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,
>  };
>
> @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
>         if (!pr)
>                 return -ENODEV;
>
> -       result = acpi_cppc_processor_probe(pr);
> -       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> -               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> -
>         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>                 acpi_processor_power_init(pr);
>
> @@ -192,6 +190,30 @@ 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);
> +       struct acpi_processor *pr;
> +       int result;
> +
> +       if (!device)
> +               return -ENODEV;
> +
> +       pr = acpi_driver_data(device);
> +       if (!pr)
> +               return -ENODEV;
> +
> +       /* Protect against concurrent CPU hotplug operations */
> +       cpu_hotplug_disable();
> +       result = acpi_cppc_processor_probe(pr);
> +       cpu_hotplug_enable();

This means that CPPC will be initialized for vCPUs that are not
enabled on ARM if I'm not mistaken.

I'm not sure if it is valid to do so.

Jonathan?

> +
> +       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> +               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> +
> +       return 0;
> +}
> +
>  static int acpi_processor_stop(struct device *dev)
>  {
>         struct acpi_device *device = ACPI_COMPANION(dev);
> --
> 2.33.0
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-01-27 14:42 ` Rafael J. Wysocki
@ 2026-01-27 16:58   ` Jonathan Cameron
  2026-01-27 18:00     ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2026-01-27 16:58 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lifeng Zheng, catalin.marinas, linux-acpi, linux-kernel,
	linuxarm, gshan, miguel.luis, guohanjun, zhanjie9, lihuisong,
	yubowen8, zhangpengjie2, wangzhi12, linhongye, salil.mehta

On Tue, 27 Jan 2026 15:42:16 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> On Tue, Jan 20, 2026 at 12:33 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
> >
> > Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
> > will fail to register. Because it requires the domain information of all
> > possible CPUs to construct shared_cpu_map, which shows the CPUs that share
> > the same domain.
> >
> > Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
> > same path for cold and hotplug") removes probe() of acpi_processor_driver
> > and makes acpi_cppc_processor_probe() only being called the first time CPU
> > goes online. This means that CPUs that haven't yet gone online will not
> > have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
> >
> > Add acpi_processor_start() back as the probe() callback of
> > acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
> > sure all _CPC tables will be parsed when acpi_processor_driver registered.
> >
> > Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
> > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> > ---
> >  drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
> >  1 file changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> > index 65e779be64ff..c8b4daf580b0 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,
> >  };
> >
> > @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
> >         if (!pr)
> >                 return -ENODEV;
> >
> > -       result = acpi_cppc_processor_probe(pr);
> > -       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> > -               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> > -
> >         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
> >                 acpi_processor_power_init(pr);
> >
> > @@ -192,6 +190,30 @@ 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);
> > +       struct acpi_processor *pr;
> > +       int result;
> > +
> > +       if (!device)
> > +               return -ENODEV;
> > +
> > +       pr = acpi_driver_data(device);
> > +       if (!pr)
> > +               return -ENODEV;
> > +
> > +       /* Protect against concurrent CPU hotplug operations */
> > +       cpu_hotplug_disable();
> > +       result = acpi_cppc_processor_probe(pr);
> > +       cpu_hotplug_enable();  
> 
> This means that CPPC will be initialized for vCPUs that are not
> enabled on ARM if I'm not mistaken.

If we are just talking powered down at boot it used to do that
so I assume it was fine. The corner case is ones we are explicitly
saying are not onlineable yet but marked online capable and will
turn up later.

> 
> I'm not sure if it is valid to do so.

The conclusion of the following is I think this is fine but I'm not
entirely confident about it.

I'm struggling to figure out the right answer to this and
it's not easy to test. I vaguely recall having some nasty emulation
hacks to poke some x86 related _CPC stuff a while back.
I might be able to hack something up for this as well and try to
create pathological corner cases.

The short answer is CPPC + hotplug isn't a thing today in KVM + QEMU,
but that's not to say it never will be if someone virtualizes CPC for
a guest.  Let's consider that hypothetical virtualization / emulation.

So the questions:
1) Does simply making this acpi_cppc_processor_probe() result in any
   register accesses to the registers that might be found in _CPC or
   used via other ACPI methods?
2) Can we rely on a a VMM not do something nasty if those are accessed
   on CPUs that haven't been instantiated yet?  e.g. Bus error.
   A related useful question is: Can we assume these registers are
   accessible on offlined CPUs?  If they can be unsafe to access from
   CPUs that are temporary powered down / offline then I think we are fine because
   the CPPC code must guarantee not to access them. (I'm relying on this!)

For the particular case Lifeng has run into, I think the code that matters
(beyond instantiation of the infrastructure) is the creation of the
domain info in acpi_get_psd(). I think _PSD can only be static data so
shouldn't cause any register accesses to the powered down CPUs.

So 'probably' fine + we'll not really know unless we get CPU HP and
CPC.

Alternative much more complex change would be to separate the grabbing of
static data (done here) from setting up anything dynamic which would remain
in the hotplug handler.  If those registers haven't been discovered we definitely
can't access them from the cpu freq driver.

Thanks,

Jonathan




> 
> Jonathan?
> 
> > +
> > +       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> > +               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> > +
> > +       return 0;
> > +}
> > +
> >  static int acpi_processor_stop(struct device *dev)
> >  {
> >         struct acpi_device *device = ACPI_COMPANION(dev);
> > --
> > 2.33.0
> >
> >  
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-01-27 16:58   ` Jonathan Cameron
@ 2026-01-27 18:00     ` Rafael J. Wysocki
  2026-01-29 12:45       ` zhenglifeng (A)
  0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2026-01-27 18:00 UTC (permalink / raw)
  To: Jonathan Cameron, Lifeng Zheng
  Cc: catalin.marinas, linux-acpi, linux-kernel, linuxarm, gshan,
	miguel.luis, guohanjun, zhanjie9, lihuisong, yubowen8,
	zhangpengjie2, wangzhi12, linhongye, salil.mehta, Viresh Kumar,
	Linux PM

+linux-pm and Viresh

On Tue, Jan 27, 2026 at 5:58 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Tue, 27 Jan 2026 15:42:16 +0100
> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>
> > On Tue, Jan 20, 2026 at 12:33 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
> > >
> > > Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
> > > will fail to register. Because it requires the domain information of all
> > > possible CPUs to construct shared_cpu_map, which shows the CPUs that share
> > > the same domain.
> > >
> > > Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
> > > same path for cold and hotplug") removes probe() of acpi_processor_driver
> > > and makes acpi_cppc_processor_probe() only being called the first time CPU
> > > goes online. This means that CPUs that haven't yet gone online will not
> > > have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
> > >
> > > Add acpi_processor_start() back as the probe() callback of
> > > acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
> > > sure all _CPC tables will be parsed when acpi_processor_driver registered.
> > >
> > > Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
> > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> > > ---
> > >  drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
> > >  1 file changed, 26 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> > > index 65e779be64ff..c8b4daf580b0 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,
> > >  };
> > >
> > > @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
> > >         if (!pr)
> > >                 return -ENODEV;
> > >
> > > -       result = acpi_cppc_processor_probe(pr);
> > > -       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> > > -               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> > > -
> > >         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
> > >                 acpi_processor_power_init(pr);
> > >
> > > @@ -192,6 +190,30 @@ 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);
> > > +       struct acpi_processor *pr;
> > > +       int result;
> > > +
> > > +       if (!device)
> > > +               return -ENODEV;
> > > +
> > > +       pr = acpi_driver_data(device);
> > > +       if (!pr)
> > > +               return -ENODEV;
> > > +
> > > +       /* Protect against concurrent CPU hotplug operations */
> > > +       cpu_hotplug_disable();
> > > +       result = acpi_cppc_processor_probe(pr);
> > > +       cpu_hotplug_enable();
> >
> > This means that CPPC will be initialized for vCPUs that are not
> > enabled on ARM if I'm not mistaken.
>
> If we are just talking powered down at boot it used to do that
> so I assume it was fine. The corner case is ones we are explicitly
> saying are not onlineable yet but marked online capable and will
> turn up later.
>
> >
> > I'm not sure if it is valid to do so.
>
> The conclusion of the following is I think this is fine but I'm not
> entirely confident about it.
>
> I'm struggling to figure out the right answer to this and
> it's not easy to test. I vaguely recall having some nasty emulation
> hacks to poke some x86 related _CPC stuff a while back.
> I might be able to hack something up for this as well and try to
> create pathological corner cases.
>
> The short answer is CPPC + hotplug isn't a thing today in KVM + QEMU,
> but that's not to say it never will be if someone virtualizes CPC for
> a guest.  Let's consider that hypothetical virtualization / emulation.
>
> So the questions:
> 1) Does simply making this acpi_cppc_processor_probe() result in any
>    register accesses to the registers that might be found in _CPC or
>    used via other ACPI methods?
> 2) Can we rely on a a VMM not do something nasty if those are accessed
>    on CPUs that haven't been instantiated yet?  e.g. Bus error.
>    A related useful question is: Can we assume these registers are
>    accessible on offlined CPUs?  If they can be unsafe to access from
>    CPUs that are temporary powered down / offline then I think we are fine because
>    the CPPC code must guarantee not to access them. (I'm relying on this!)
>
> For the particular case Lifeng has run into, I think the code that matters
> (beyond instantiation of the infrastructure) is the creation of the
> domain info in acpi_get_psd(). I think _PSD can only be static data so
> shouldn't cause any register accesses to the powered down CPUs.
>
> So 'probably' fine + we'll not really know unless we get CPU HP and
> CPC.
>
> Alternative much more complex change would be to separate the grabbing of
> static data (done here) from setting up anything dynamic which would remain
> in the hotplug handler.  If those registers haven't been discovered we definitely
> can't access them from the cpu freq driver.

I'm thinking that maybe cppc_cpufreq should be updated instead.

I'm not really sure why it needs to collect information on offline
CPUs.  Surely, they don't matter until they are brought online.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-01-27 18:00     ` Rafael J. Wysocki
@ 2026-01-29 12:45       ` zhenglifeng (A)
  2026-02-06  9:57         ` zhenglifeng (A)
  0 siblings, 1 reply; 10+ messages in thread
From: zhenglifeng (A) @ 2026-01-29 12:45 UTC (permalink / raw)
  To: Rafael J. Wysocki, Jonathan Cameron
  Cc: catalin.marinas, linux-acpi, linux-kernel, linuxarm, gshan,
	miguel.luis, guohanjun, zhanjie9, lihuisong, yubowen8,
	zhangpengjie2, wangzhi12, linhongye, salil.mehta, Viresh Kumar,
	Linux PM



On 2026/1/28 2:00, Rafael J. Wysocki wrote:
> +linux-pm and Viresh
> 
> On Tue, Jan 27, 2026 at 5:58 PM Jonathan Cameron
> <jonathan.cameron@huawei.com> wrote:
>>
>> On Tue, 27 Jan 2026 15:42:16 +0100
>> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>>
>>> On Tue, Jan 20, 2026 at 12:33 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
>>>>
>>>> Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
>>>> will fail to register. Because it requires the domain information of all
>>>> possible CPUs to construct shared_cpu_map, which shows the CPUs that share
>>>> the same domain.
>>>>
>>>> Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
>>>> same path for cold and hotplug") removes probe() of acpi_processor_driver
>>>> and makes acpi_cppc_processor_probe() only being called the first time CPU
>>>> goes online. This means that CPUs that haven't yet gone online will not
>>>> have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
>>>>
>>>> Add acpi_processor_start() back as the probe() callback of
>>>> acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
>>>> sure all _CPC tables will be parsed when acpi_processor_driver registered.
>>>>
>>>> Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
>>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>>>> ---
>>>>  drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
>>>>  1 file changed, 26 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>>>> index 65e779be64ff..c8b4daf580b0 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,
>>>>  };
>>>>
>>>> @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
>>>>         if (!pr)
>>>>                 return -ENODEV;
>>>>
>>>> -       result = acpi_cppc_processor_probe(pr);
>>>> -       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
>>>> -               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
>>>> -
>>>>         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>>>                 acpi_processor_power_init(pr);
>>>>
>>>> @@ -192,6 +190,30 @@ 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);
>>>> +       struct acpi_processor *pr;
>>>> +       int result;
>>>> +
>>>> +       if (!device)
>>>> +               return -ENODEV;
>>>> +
>>>> +       pr = acpi_driver_data(device);
>>>> +       if (!pr)
>>>> +               return -ENODEV;
>>>> +
>>>> +       /* Protect against concurrent CPU hotplug operations */
>>>> +       cpu_hotplug_disable();
>>>> +       result = acpi_cppc_processor_probe(pr);
>>>> +       cpu_hotplug_enable();
>>>
>>> This means that CPPC will be initialized for vCPUs that are not
>>> enabled on ARM if I'm not mistaken.
>>
>> If we are just talking powered down at boot it used to do that
>> so I assume it was fine. The corner case is ones we are explicitly
>> saying are not onlineable yet but marked online capable and will
>> turn up later.
>>
>>>
>>> I'm not sure if it is valid to do so.
>>
>> The conclusion of the following is I think this is fine but I'm not
>> entirely confident about it.
>>
>> I'm struggling to figure out the right answer to this and
>> it's not easy to test. I vaguely recall having some nasty emulation
>> hacks to poke some x86 related _CPC stuff a while back.
>> I might be able to hack something up for this as well and try to
>> create pathological corner cases.
>>
>> The short answer is CPPC + hotplug isn't a thing today in KVM + QEMU,
>> but that's not to say it never will be if someone virtualizes CPC for
>> a guest.  Let's consider that hypothetical virtualization / emulation.
>>
>> So the questions:
>> 1) Does simply making this acpi_cppc_processor_probe() result in any
>>    register accesses to the registers that might be found in _CPC or
>>    used via other ACPI methods?
>> 2) Can we rely on a a VMM not do something nasty if those are accessed
>>    on CPUs that haven't been instantiated yet?  e.g. Bus error.
>>    A related useful question is: Can we assume these registers are
>>    accessible on offlined CPUs?  If they can be unsafe to access from
>>    CPUs that are temporary powered down / offline then I think we are fine because
>>    the CPPC code must guarantee not to access them. (I'm relying on this!)
>>
>> For the particular case Lifeng has run into, I think the code that matters
>> (beyond instantiation of the infrastructure) is the creation of the
>> domain info in acpi_get_psd(). I think _PSD can only be static data so
>> shouldn't cause any register accesses to the powered down CPUs.
>>
>> So 'probably' fine + we'll not really know unless we get CPU HP and
>> CPC.
>>
>> Alternative much more complex change would be to separate the grabbing of
>> static data (done here) from setting up anything dynamic which would remain
>> in the hotplug handler.  If those registers haven't been discovered we definitely
>> can't access them from the cpu freq driver.
> 
> I'm thinking that maybe cppc_cpufreq should be updated instead.
> 
> I'm not really sure why it needs to collect information on offline
> CPUs.  Surely, they don't matter until they are brought online.

This information is collected in order to generate related_cpus. Without
doing so, a new policy will be created when the second CPU in the same
domain comes online, instead of reusing the existing policy. And this will
make a mess.

I can't find a good way to solve this problem in cppc_cpufreq or cpufreq.

> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-01-29 12:45       ` zhenglifeng (A)
@ 2026-02-06  9:57         ` zhenglifeng (A)
  2026-04-02  8:36           ` zhenglifeng (A)
  0 siblings, 1 reply; 10+ messages in thread
From: zhenglifeng (A) @ 2026-02-06  9:57 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jonathan Cameron, catalin.marinas, linux-acpi, linux-kernel,
	linuxarm, gshan, miguel.luis, guohanjun, zhanjie9, lihuisong,
	yubowen8, zhangpengjie2, wangzhi12, linhongye, salil.mehta,
	Viresh Kumar, Linux PM

On 2026/1/29 20:45, zhenglifeng (A) wrote:
> 
> 
> On 2026/1/28 2:00, Rafael J. Wysocki wrote:
>> +linux-pm and Viresh
>>
>> On Tue, Jan 27, 2026 at 5:58 PM Jonathan Cameron
>> <jonathan.cameron@huawei.com> wrote:
>>>
>>> On Tue, 27 Jan 2026 15:42:16 +0100
>>> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>>>
>>>> On Tue, Jan 20, 2026 at 12:33 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
>>>>>
>>>>> Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
>>>>> will fail to register. Because it requires the domain information of all
>>>>> possible CPUs to construct shared_cpu_map, which shows the CPUs that share
>>>>> the same domain.
>>>>>
>>>>> Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
>>>>> same path for cold and hotplug") removes probe() of acpi_processor_driver
>>>>> and makes acpi_cppc_processor_probe() only being called the first time CPU
>>>>> goes online. This means that CPUs that haven't yet gone online will not
>>>>> have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
>>>>>
>>>>> Add acpi_processor_start() back as the probe() callback of
>>>>> acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
>>>>> sure all _CPC tables will be parsed when acpi_processor_driver registered.
>>>>>
>>>>> Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
>>>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>>>>> ---
>>>>>  drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
>>>>>  1 file changed, 26 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>>>>> index 65e779be64ff..c8b4daf580b0 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,
>>>>>  };
>>>>>
>>>>> @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
>>>>>         if (!pr)
>>>>>                 return -ENODEV;
>>>>>
>>>>> -       result = acpi_cppc_processor_probe(pr);
>>>>> -       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
>>>>> -               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
>>>>> -
>>>>>         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>>>>                 acpi_processor_power_init(pr);
>>>>>
>>>>> @@ -192,6 +190,30 @@ 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);
>>>>> +       struct acpi_processor *pr;
>>>>> +       int result;
>>>>> +
>>>>> +       if (!device)
>>>>> +               return -ENODEV;
>>>>> +
>>>>> +       pr = acpi_driver_data(device);
>>>>> +       if (!pr)
>>>>> +               return -ENODEV;
>>>>> +
>>>>> +       /* Protect against concurrent CPU hotplug operations */
>>>>> +       cpu_hotplug_disable();
>>>>> +       result = acpi_cppc_processor_probe(pr);
>>>>> +       cpu_hotplug_enable();
>>>>
>>>> This means that CPPC will be initialized for vCPUs that are not
>>>> enabled on ARM if I'm not mistaken.
>>>
>>> If we are just talking powered down at boot it used to do that
>>> so I assume it was fine. The corner case is ones we are explicitly
>>> saying are not onlineable yet but marked online capable and will
>>> turn up later.
>>>
>>>>
>>>> I'm not sure if it is valid to do so.
>>>
>>> The conclusion of the following is I think this is fine but I'm not
>>> entirely confident about it.
>>>
>>> I'm struggling to figure out the right answer to this and
>>> it's not easy to test. I vaguely recall having some nasty emulation
>>> hacks to poke some x86 related _CPC stuff a while back.
>>> I might be able to hack something up for this as well and try to
>>> create pathological corner cases.
>>>
>>> The short answer is CPPC + hotplug isn't a thing today in KVM + QEMU,
>>> but that's not to say it never will be if someone virtualizes CPC for
>>> a guest.  Let's consider that hypothetical virtualization / emulation.
>>>
>>> So the questions:
>>> 1) Does simply making this acpi_cppc_processor_probe() result in any
>>>    register accesses to the registers that might be found in _CPC or
>>>    used via other ACPI methods?
>>> 2) Can we rely on a a VMM not do something nasty if those are accessed
>>>    on CPUs that haven't been instantiated yet?  e.g. Bus error.
>>>    A related useful question is: Can we assume these registers are
>>>    accessible on offlined CPUs?  If they can be unsafe to access from
>>>    CPUs that are temporary powered down / offline then I think we are fine because
>>>    the CPPC code must guarantee not to access them. (I'm relying on this!)
>>>
>>> For the particular case Lifeng has run into, I think the code that matters
>>> (beyond instantiation of the infrastructure) is the creation of the
>>> domain info in acpi_get_psd(). I think _PSD can only be static data so
>>> shouldn't cause any register accesses to the powered down CPUs.
>>>
>>> So 'probably' fine + we'll not really know unless we get CPU HP and
>>> CPC.
>>>
>>> Alternative much more complex change would be to separate the grabbing of
>>> static data (done here) from setting up anything dynamic which would remain
>>> in the hotplug handler.  If those registers haven't been discovered we definitely
>>> can't access them from the cpu freq driver.
>>
>> I'm thinking that maybe cppc_cpufreq should be updated instead.
>>
>> I'm not really sure why it needs to collect information on offline
>> CPUs.  Surely, they don't matter until they are brought online.
> 
> This information is collected in order to generate related_cpus. Without
> doing so, a new policy will be created when the second CPU in the same
> domain comes online, instead of reusing the existing policy. And this will
> make a mess.
> 
> I can't find a good way to solve this problem in cppc_cpufreq or cpufreq.

Hi Rafael,

If we have to make sure not to use the information on offline CPUs, there
will be many things that need to be modified:

1. acpi_get_psd_map() in cppc_acpi.c: only use the information on online
CPUs, and update shared_cpu_map when a new CPU in the same domain goes
online.

2. cpufreq_online() in cpufreq.c: create a new policy and decide whether it
should be kept because it may turns out that the CPU should share an
already exist policy with other CPUs.

3. The init() callbacks in cppc_cpufreq, acpi-cpufreq and maybe other
drivers: verify whether the newly generated policy is necessary and return
the result.

...

Is it necessary to make such a big change for solving this problem? It
seems to me that it is reasonable and fewer changes to parse the _CPC table
for all CPUs in advance because it never change, isn't it?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-02-06  9:57         ` zhenglifeng (A)
@ 2026-04-02  8:36           ` zhenglifeng (A)
  0 siblings, 0 replies; 10+ messages in thread
From: zhenglifeng (A) @ 2026-04-02  8:36 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jonathan Cameron, catalin.marinas, linux-acpi, linux-kernel,
	linuxarm, gshan, miguel.luis, guohanjun, zhanjie9, lihuisong,
	yubowen8, zhangpengjie2, wangzhi12, linhongye, salil.mehta,
	Viresh Kumar, Linux PM

On 2/6/2026 5:57 PM, zhenglifeng (A) wrote:
> On 2026/1/29 20:45, zhenglifeng (A) wrote:
>>
>>
>> On 2026/1/28 2:00, Rafael J. Wysocki wrote:
>>> +linux-pm and Viresh
>>>
>>> On Tue, Jan 27, 2026 at 5:58 PM Jonathan Cameron
>>> <jonathan.cameron@huawei.com> wrote:
>>>>
>>>> On Tue, 27 Jan 2026 15:42:16 +0100
>>>> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>>>>
>>>>> On Tue, Jan 20, 2026 at 12:33 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote:
>>>>>>
>>>>>> Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
>>>>>> will fail to register. Because it requires the domain information of all
>>>>>> possible CPUs to construct shared_cpu_map, which shows the CPUs that share
>>>>>> the same domain.
>>>>>>
>>>>>> Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
>>>>>> same path for cold and hotplug") removes probe() of acpi_processor_driver
>>>>>> and makes acpi_cppc_processor_probe() only being called the first time CPU
>>>>>> goes online. This means that CPUs that haven't yet gone online will not
>>>>>> have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
>>>>>>
>>>>>> Add acpi_processor_start() back as the probe() callback of
>>>>>> acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
>>>>>> sure all _CPC tables will be parsed when acpi_processor_driver registered.
>>>>>>
>>>>>> Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
>>>>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>>>>>> ---
>>>>>>  drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
>>>>>>  1 file changed, 26 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>>>>>> index 65e779be64ff..c8b4daf580b0 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,
>>>>>>  };
>>>>>>
>>>>>> @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
>>>>>>         if (!pr)
>>>>>>                 return -ENODEV;
>>>>>>
>>>>>> -       result = acpi_cppc_processor_probe(pr);
>>>>>> -       if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
>>>>>> -               dev_dbg(&device->dev, "CPPC data invalid or not present\n");
>>>>>> -
>>>>>>         if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>>>>>                 acpi_processor_power_init(pr);
>>>>>>
>>>>>> @@ -192,6 +190,30 @@ 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);
>>>>>> +       struct acpi_processor *pr;
>>>>>> +       int result;
>>>>>> +
>>>>>> +       if (!device)
>>>>>> +               return -ENODEV;
>>>>>> +
>>>>>> +       pr = acpi_driver_data(device);
>>>>>> +       if (!pr)
>>>>>> +               return -ENODEV;
>>>>>> +
>>>>>> +       /* Protect against concurrent CPU hotplug operations */
>>>>>> +       cpu_hotplug_disable();
>>>>>> +       result = acpi_cppc_processor_probe(pr);
>>>>>> +       cpu_hotplug_enable();
>>>>>
>>>>> This means that CPPC will be initialized for vCPUs that are not
>>>>> enabled on ARM if I'm not mistaken.
>>>>
>>>> If we are just talking powered down at boot it used to do that
>>>> so I assume it was fine. The corner case is ones we are explicitly
>>>> saying are not onlineable yet but marked online capable and will
>>>> turn up later.
>>>>
>>>>>
>>>>> I'm not sure if it is valid to do so.
>>>>
>>>> The conclusion of the following is I think this is fine but I'm not
>>>> entirely confident about it.
>>>>
>>>> I'm struggling to figure out the right answer to this and
>>>> it's not easy to test. I vaguely recall having some nasty emulation
>>>> hacks to poke some x86 related _CPC stuff a while back.
>>>> I might be able to hack something up for this as well and try to
>>>> create pathological corner cases.
>>>>
>>>> The short answer is CPPC + hotplug isn't a thing today in KVM + QEMU,
>>>> but that's not to say it never will be if someone virtualizes CPC for
>>>> a guest.  Let's consider that hypothetical virtualization / emulation.
>>>>
>>>> So the questions:
>>>> 1) Does simply making this acpi_cppc_processor_probe() result in any
>>>>    register accesses to the registers that might be found in _CPC or
>>>>    used via other ACPI methods?
>>>> 2) Can we rely on a a VMM not do something nasty if those are accessed
>>>>    on CPUs that haven't been instantiated yet?  e.g. Bus error.
>>>>    A related useful question is: Can we assume these registers are
>>>>    accessible on offlined CPUs?  If they can be unsafe to access from
>>>>    CPUs that are temporary powered down / offline then I think we are fine because
>>>>    the CPPC code must guarantee not to access them. (I'm relying on this!)
>>>>
>>>> For the particular case Lifeng has run into, I think the code that matters
>>>> (beyond instantiation of the infrastructure) is the creation of the
>>>> domain info in acpi_get_psd(). I think _PSD can only be static data so
>>>> shouldn't cause any register accesses to the powered down CPUs.
>>>>
>>>> So 'probably' fine + we'll not really know unless we get CPU HP and
>>>> CPC.
>>>>
>>>> Alternative much more complex change would be to separate the grabbing of
>>>> static data (done here) from setting up anything dynamic which would remain
>>>> in the hotplug handler.  If those registers haven't been discovered we definitely
>>>> can't access them from the cpu freq driver.
>>>
>>> I'm thinking that maybe cppc_cpufreq should be updated instead.
>>>
>>> I'm not really sure why it needs to collect information on offline
>>> CPUs.  Surely, they don't matter until they are brought online.
>>
>> This information is collected in order to generate related_cpus. Without
>> doing so, a new policy will be created when the second CPU in the same
>> domain comes online, instead of reusing the existing policy. And this will
>> make a mess.
>>
>> I can't find a good way to solve this problem in cppc_cpufreq or cpufreq.
> 
> Hi Rafael,
> 
> If we have to make sure not to use the information on offline CPUs, there
> will be many things that need to be modified:
> 
> 1. acpi_get_psd_map() in cppc_acpi.c: only use the information on online
> CPUs, and update shared_cpu_map when a new CPU in the same domain goes
> online.
> 
> 2. cpufreq_online() in cpufreq.c: create a new policy and decide whether it
> should be kept because it may turns out that the CPU should share an
> already exist policy with other CPUs.
> 
> 3. The init() callbacks in cppc_cpufreq, acpi-cpufreq and maybe other
> drivers: verify whether the newly generated policy is necessary and return
> the result.
> 
> ...
> 
> Is it necessary to make such a big change for solving this problem? It
> seems to me that it is reasonable and fewer changes to parse the _CPC table
> for all CPUs in advance because it never change, isn't it?
> 

Hi Rafael,

Although some time has passed, I still look forward to your advice.

Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-01-20 11:32 [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online Lifeng Zheng
  2026-01-27 14:42 ` Rafael J. Wysocki
@ 2026-08-26  7:59 ` Pengjie Zhang
  2026-08-26 10:31   ` Rafael J. Wysocki (Intel)
  2026-08-27  1:39   ` zhenglifeng (A)
  1 sibling, 2 replies; 10+ messages in thread
From: Pengjie Zhang @ 2026-08-26  7:59 UTC (permalink / raw)
  To: Lifeng Zheng, rafael, catalin.marinas, Jonathan.Cameron, lenb
  Cc: linux-acpi, linux-kernel, linuxarm, gshan, miguel.luis,
	guohanjun, zhanjie9, lihuisong, yubowen8, wangzhi12, linhongye

Hi Lifeng,

On 1/20/2026 7:32 PM, Lifeng Zheng wrote:
> Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
> will fail to register. Because it requires the domain information of all
> possible CPUs to construct shared_cpu_map, which shows the CPUs that share
> the same domain.
>
> Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
> same path for cold and hotplug") removes probe() of acpi_processor_driver
> and makes acpi_cppc_processor_probe() only being called the first time CPU
> goes online. This means that CPUs that haven't yet gone online will not
> have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
>
> Add acpi_processor_start() back as the probe() callback of
> acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
> sure all _CPC tables will be parsed when acpi_processor_driver registered.
>
> Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> ---
>   drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
>   1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 65e779be64ff..c8b4daf580b0 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,
>   };
>   
> @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
>   	if (!pr)
>   		return -ENODEV;
>   
> -	result = acpi_cppc_processor_probe(pr);
> -	if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> -		dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> -
>   	if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>   		acpi_processor_power_init(pr);
>   
> @@ -192,6 +190,30 @@ 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);
> +	struct acpi_processor *pr;
> +	int result;
> +
> +	if (!device)
> +		return -ENODEV;
> +
> +	pr = acpi_driver_data(device);
> +	if (!pr)
> +		return -ENODEV;
> +
> +	/* Protect against concurrent CPU hotplug operations */
> +	cpu_hotplug_disable();
> +	result = acpi_cppc_processor_probe(pr);
> +	cpu_hotplug_enable();
> +
> +	if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> +		dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> +
> +	return 0;
> +}
> +
>   static int acpi_processor_stop(struct device *dev)
>   {
>   	struct acpi_device *device = ACPI_COMPANION(dev);
I reproduced the issue described by this patch on the latest kernel at
commit 45c13f3f9e3bb15f.

On my system, CPU0 and CPU1 belong to the same software-coordinated
frequency domain. After booting with maxcpus=1, CPU1 is present but
offline, and its CPC descriptor is not parsed.

Consequently, acpi_get_psd_map() skips CPU1 and constructs an
incomplete shared_cpu_map containing only CPU0.

When CPU1 is subsequently brought online:

echo 1 > /sys/devices/system/cpu/cpu1/online

the cpufreq core creates an overlapping policy and attempts to create
the existing cpu0/cpufreq symbolic link again, resulting in the
following warnings:
...
sysfs_warn_dup
sysfs_do_create_link_sd
sysfs_create_link
add_cpu_dev_symlink
cpufreq_policy_online
cpufreq_online
cpuhp_cpufreq_online
...
processor cpu0: cpufreq symlink creation failed

freq_qos_add_request() called for active request
WARNING: kernel/power/qos.c:658 at freq_qos_add_request

The affected CPU masks are also inconsistent:

$ cat /sys/devices/system/cpu/cpu0/cpufreq/affected_cpus
0

$ cat /sys/devices/system/cpu/cpu1/cpufreq/affected_cpus
0 1

After applying this patch on top of commit 45c13f3f9e3bb15f, the CPC
descriptors are parsed before the CPUs are brought online. The
shared_cpu_map is constructed correctly, and CPU1 can be brought
online without triggering the duplicate sysfs link or active QoS
request warnings.

This patch fixes the issue in my testing. so,

Tested-by: Pengjie Zhang <zhangpengjie2@huawei.com>
Reviewed-by: Pengjie Zhang <zhangpengjie2@huawei.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-08-26  7:59 ` Pengjie Zhang
@ 2026-08-26 10:31   ` Rafael J. Wysocki (Intel)
  2026-08-27  1:39   ` zhenglifeng (A)
  1 sibling, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-08-26 10:31 UTC (permalink / raw)
  To: Pengjie Zhang
  Cc: Lifeng Zheng, rafael, catalin.marinas, Jonathan.Cameron, lenb,
	linux-acpi, linux-kernel, linuxarm, gshan, miguel.luis,
	guohanjun, zhanjie9, lihuisong, yubowen8, wangzhi12, linhongye

On Wed, Aug 26, 2026 at 10:00 AM Pengjie Zhang <zhangpengjie2@huawei.com> wrote:
>
> Hi Lifeng,
>
> On 1/20/2026 7:32 PM, Lifeng Zheng wrote:
> > Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
> > will fail to register. Because it requires the domain information of all
> > possible CPUs to construct shared_cpu_map, which shows the CPUs that share
> > the same domain.
> >
> > Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
> > same path for cold and hotplug") removes probe() of acpi_processor_driver
> > and makes acpi_cppc_processor_probe() only being called the first time CPU
> > goes online. This means that CPUs that haven't yet gone online will not
> > have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
> >
> > Add acpi_processor_start() back as the probe() callback of
> > acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
> > sure all _CPC tables will be parsed when acpi_processor_driver registered.
> >
> > Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
> > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
> > ---
> >   drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
> >   1 file changed, 26 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> > index 65e779be64ff..c8b4daf580b0 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,
> >   };
> >
> > @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
> >       if (!pr)
> >               return -ENODEV;
> >
> > -     result = acpi_cppc_processor_probe(pr);
> > -     if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> > -             dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> > -
> >       if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
> >               acpi_processor_power_init(pr);
> >
> > @@ -192,6 +190,30 @@ 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);
> > +     struct acpi_processor *pr;
> > +     int result;
> > +
> > +     if (!device)
> > +             return -ENODEV;
> > +
> > +     pr = acpi_driver_data(device);
> > +     if (!pr)
> > +             return -ENODEV;
> > +
> > +     /* Protect against concurrent CPU hotplug operations */
> > +     cpu_hotplug_disable();
> > +     result = acpi_cppc_processor_probe(pr);
> > +     cpu_hotplug_enable();
> > +
> > +     if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
> > +             dev_dbg(&device->dev, "CPPC data invalid or not present\n");
> > +
> > +     return 0;
> > +}
> > +
> >   static int acpi_processor_stop(struct device *dev)
> >   {
> >       struct acpi_device *device = ACPI_COMPANION(dev);
> I reproduced the issue described by this patch on the latest kernel at
> commit 45c13f3f9e3bb15f.
>
> On my system, CPU0 and CPU1 belong to the same software-coordinated
> frequency domain. After booting with maxcpus=1, CPU1 is present but
> offline, and its CPC descriptor is not parsed.
>
> Consequently, acpi_get_psd_map() skips CPU1 and constructs an
> incomplete shared_cpu_map containing only CPU0.
>
> When CPU1 is subsequently brought online:
>
> echo 1 > /sys/devices/system/cpu/cpu1/online
>
> the cpufreq core creates an overlapping policy and attempts to create
> the existing cpu0/cpufreq symbolic link again, resulting in the
> following warnings:
> ...
> sysfs_warn_dup
> sysfs_do_create_link_sd
> sysfs_create_link
> add_cpu_dev_symlink
> cpufreq_policy_online
> cpufreq_online
> cpuhp_cpufreq_online
> ...
> processor cpu0: cpufreq symlink creation failed
>
> freq_qos_add_request() called for active request
> WARNING: kernel/power/qos.c:658 at freq_qos_add_request
>
> The affected CPU masks are also inconsistent:
>
> $ cat /sys/devices/system/cpu/cpu0/cpufreq/affected_cpus
> 0
>
> $ cat /sys/devices/system/cpu/cpu1/cpufreq/affected_cpus
> 0 1
>
> After applying this patch on top of commit 45c13f3f9e3bb15f, the CPC
> descriptors are parsed before the CPUs are brought online. The
> shared_cpu_map is constructed correctly, and CPU1 can be brought
> online without triggering the duplicate sysfs link or active QoS
> request warnings.
>
> This patch fixes the issue in my testing. so,
>
> Tested-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> Reviewed-by: Pengjie Zhang <zhangpengjie2@huawei.com>

Thanks for this information!

I'll consider queuing up the patch as a fix for 7.3-rc next week.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online
  2026-08-26  7:59 ` Pengjie Zhang
  2026-08-26 10:31   ` Rafael J. Wysocki (Intel)
@ 2026-08-27  1:39   ` zhenglifeng (A)
  1 sibling, 0 replies; 10+ messages in thread
From: zhenglifeng (A) @ 2026-08-27  1:39 UTC (permalink / raw)
  To: Pengjie Zhang, rafael, catalin.marinas, lenb, jonathan.cameron
  Cc: linux-acpi, linux-kernel, linuxarm, gshan, miguel.luis,
	guohanjun, zhanjie9, lihuisong, yubowen8, wangzhi12, linhongye

Hi Pengjie,

On 8/26/2026 3:59 PM, Pengjie Zhang wrote:
> Hi Lifeng,
> 
> On 1/20/2026 7:32 PM, Lifeng Zheng wrote:
>> Currently, if boot with maxcpus less than NR_CPUS, the cppc_cpufreq driver
>> will fail to register. Because it requires the domain information of all
>> possible CPUs to construct shared_cpu_map, which shows the CPUs that share
>> the same domain.
>>
>> Commit c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use
>> same path for cold and hotplug") removes probe() of acpi_processor_driver
>> and makes acpi_cppc_processor_probe() only being called the first time CPU
>> goes online. This means that CPUs that haven't yet gone online will not
>> have pre-parsed _CPC objects and causes cppc_cpufreq driver register fail.
>>
>> Add acpi_processor_start() back as the probe() callback of
>> acpi_processor_driver and call acpi_cppc_processor_probe() in it to make
>> sure all _CPC tables will be parsed when acpi_processor_driver registered.
>>
>> Fixes: c1385c1f0ba3 ("ACPI: processor: Simplify initial onlining to use same path for cold and hotplug")
>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
>> ---
>>   drivers/acpi/processor_driver.c | 30 ++++++++++++++++++++++++++----
>>   1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 65e779be64ff..c8b4daf580b0 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,
>>   };
>>   @@ -162,10 +164,6 @@ static int __acpi_processor_start(struct acpi_device *device)
>>       if (!pr)
>>           return -ENODEV;
>>   -    result = acpi_cppc_processor_probe(pr);
>> -    if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
>> -        dev_dbg(&device->dev, "CPPC data invalid or not present\n");
>> -
>>       if (!cpuidle_get_driver() || cpuidle_get_driver() == &acpi_idle_driver)
>>           acpi_processor_power_init(pr);
>>   @@ -192,6 +190,30 @@ 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);
>> +    struct acpi_processor *pr;
>> +    int result;
>> +
>> +    if (!device)
>> +        return -ENODEV;
>> +
>> +    pr = acpi_driver_data(device);
>> +    if (!pr)
>> +        return -ENODEV;
>> +
>> +    /* Protect against concurrent CPU hotplug operations */
>> +    cpu_hotplug_disable();
>> +    result = acpi_cppc_processor_probe(pr);
>> +    cpu_hotplug_enable();
>> +
>> +    if (result && !IS_ENABLED(CONFIG_ACPI_CPU_FREQ_PSS))
>> +        dev_dbg(&device->dev, "CPPC data invalid or not present\n");
>> +
>> +    return 0;
>> +}
>> +
>>   static int acpi_processor_stop(struct device *dev)
>>   {
>>       struct acpi_device *device = ACPI_COMPANION(dev);
> I reproduced the issue described by this patch on the latest kernel at
> commit 45c13f3f9e3bb15f.
> 
> On my system, CPU0 and CPU1 belong to the same software-coordinated
> frequency domain. After booting with maxcpus=1, CPU1 is present but
> offline, and its CPC descriptor is not parsed.
> 
> Consequently, acpi_get_psd_map() skips CPU1 and constructs an
> incomplete shared_cpu_map containing only CPU0.
> 
> When CPU1 is subsequently brought online:
> 
> echo 1 > /sys/devices/system/cpu/cpu1/online
> 
> the cpufreq core creates an overlapping policy and attempts to create
> the existing cpu0/cpufreq symbolic link again, resulting in the
> following warnings:
> ...
> sysfs_warn_dup
> sysfs_do_create_link_sd
> sysfs_create_link
> add_cpu_dev_symlink
> cpufreq_policy_online
> cpufreq_online
> cpuhp_cpufreq_online
> ...
> processor cpu0: cpufreq symlink creation failed
> 
> freq_qos_add_request() called for active request
> WARNING: kernel/power/qos.c:658 at freq_qos_add_request
> 
> The affected CPU masks are also inconsistent:
> 
> $ cat /sys/devices/system/cpu/cpu0/cpufreq/affected_cpus
> 0
> 
> $ cat /sys/devices/system/cpu/cpu1/cpufreq/affected_cpus
> 0 1
> 
> After applying this patch on top of commit 45c13f3f9e3bb15f, the CPC
> descriptors are parsed before the CPUs are brought online. The
> shared_cpu_map is constructed correctly, and CPU1 can be brought
> online without triggering the duplicate sysfs link or active QoS
> request warnings.
> 
> This patch fixes the issue in my testing. so,
> 
> Tested-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> Reviewed-by: Pengjie Zhang <zhangpengjie2@huawei.com>
> 

Thanks for the testing! 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-27  1:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-20 11:32 [PATCH] ACPI: processor: Add acpi_processor_start() back to parse _CPC tables before CPU online Lifeng Zheng
2026-01-27 14:42 ` Rafael J. Wysocki
2026-01-27 16:58   ` Jonathan Cameron
2026-01-27 18:00     ` Rafael J. Wysocki
2026-01-29 12:45       ` zhenglifeng (A)
2026-02-06  9:57         ` zhenglifeng (A)
2026-04-02  8:36           ` zhenglifeng (A)
2026-08-26  7:59 ` Pengjie Zhang
2026-08-26 10:31   ` Rafael J. Wysocki (Intel)
2026-08-27  1:39   ` zhenglifeng (A)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®