linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State
@ 2022-01-06  7:43 Huang Rui
  2022-01-06  7:43 ` [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc Huang Rui
  2022-01-06 17:34 ` [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State Rafael J. Wysocki
  0 siblings, 2 replies; 13+ messages in thread
From: Huang Rui @ 2022-01-06  7:43 UTC (permalink / raw)
  To: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, linux-pm,
	linux-kernel
  Cc: Perry Yuan, Jinzhou Su, Xiaojian Du, Huang Rui

The AMD P-State driver is based on ACPI CPPC function, so ACPI should be
dependence of this driver in the kernel config.

In file included from ../drivers/cpufreq/amd-pstate.c:40:0:
../include/acpi/processor.h:226:2: error: unknown type name ‘phys_cpuid_t’
  phys_cpuid_t phys_id; /* CPU hardware ID such as APIC ID for x86 */
  ^~~~~~~~~~~~
../include/acpi/processor.h:355:1: error: unknown type name ‘phys_cpuid_t’; did you mean ‘phys_addr_t’?
 phys_cpuid_t acpi_get_phys_id(acpi_handle, int type, u32 acpi_id);
 ^~~~~~~~~~~~
 phys_addr_t
  CC      drivers/rtc/rtc-rv3029c2.o
../include/acpi/processor.h:356:1: error: unknown type name ‘phys_cpuid_t’; did you mean ‘phys_addr_t’?
 phys_cpuid_t acpi_map_madt_entry(u32 acpi_id);
 ^~~~~~~~~~~~
 phys_addr_t
../include/acpi/processor.h:357:20: error: unknown type name ‘phys_cpuid_t’; did you mean ‘phys_addr_t’?
 int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id);
                    ^~~~~~~~~~~~
                    phys_addr_t

See https://lore.kernel.org/lkml/20e286d4-25d7-fb6e-31a1-4349c805aae3@infradead.org/.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 drivers/cpufreq/Kconfig.x86 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index a951768c3ebb..55516043b656 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -36,9 +36,9 @@ config X86_PCC_CPUFREQ
 
 config X86_AMD_PSTATE
 	tristate "AMD Processor P-State driver"
-	depends on X86
-	select ACPI_PROCESSOR if ACPI
-	select ACPI_CPPC_LIB if X86_64 && ACPI
+	depends on X86 && ACPI
+	select ACPI_PROCESSOR
+	select ACPI_CPPC_LIB if X86_64
 	select CPU_FREQ_GOV_SCHEDUTIL if SMP
 	help
 	  This driver adds a CPUFreq driver which utilizes a fine grain
-- 
2.25.1


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

* [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06  7:43 [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State Huang Rui
@ 2022-01-06  7:43 ` Huang Rui
  2022-01-06 12:16   ` Borislav Petkov
  2022-01-06 17:12   ` Rafael J. Wysocki
  2022-01-06 17:34 ` [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State Rafael J. Wysocki
  1 sibling, 2 replies; 13+ messages in thread
From: Huang Rui @ 2022-01-06  7:43 UTC (permalink / raw)
  To: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, linux-pm,
	linux-kernel
  Cc: Perry Yuan, Jinzhou Su, Xiaojian Du, Huang Rui,
	kernel test robot, Borislav Petkov, Ingo Molnar, Peter Zijlstra,
	x86, stable

The init_freq_invariance_cppc function is implemented in smpboot and depends on
CONFIG_SMP.

  MODPOST vmlinux.symvers
  MODINFO modules.builtin.modinfo
  GEN     modules.builtin
  LD      .tmp_vmlinux.kallsyms1
ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
/home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
make: *** [Makefile:1161: vmlinux] Error 1

See https://lore.kernel.org/lkml/484af487-7511-647e-5c5b-33d4429acdec@infradead.org/.

Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: x86@kernel.org
Cc: stable@vger.kernel.org
---
 arch/x86/include/asm/topology.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index cc164777e661..2f0b6be8eaab 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
 }
 #endif
 
-#ifdef CONFIG_ACPI_CPPC_LIB
+#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
 void init_freq_invariance_cppc(void);
 #define init_freq_invariance_cppc init_freq_invariance_cppc
 #endif
-- 
2.25.1


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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06  7:43 ` [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc Huang Rui
@ 2022-01-06 12:16   ` Borislav Petkov
  2022-01-06 14:37     ` Huang Rui
  2022-01-06 17:12   ` Rafael J. Wysocki
  1 sibling, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2022-01-06 12:16 UTC (permalink / raw)
  To: Huang Rui
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, linux-pm,
	linux-kernel, Perry Yuan, Jinzhou Su, Xiaojian Du,
	kernel test robot, Ingo Molnar, Peter Zijlstra, x86, stable

On Thu, Jan 06, 2022 at 03:43:06PM +0800, Huang Rui wrote:
> The init_freq_invariance_cppc function is implemented in smpboot and depends on
> CONFIG_SMP.
> 
>   MODPOST vmlinux.symvers
>   MODINFO modules.builtin.modinfo
>   GEN     modules.builtin
>   LD      .tmp_vmlinux.kallsyms1
> ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
> /home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
> make: *** [Makefile:1161: vmlinux] Error 1
> 
> See https://lore.kernel.org/lkml/484af487-7511-647e-5c5b-33d4429acdec@infradead.org/.
> 
> Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: x86@kernel.org
> Cc: stable@vger.kernel.org
> ---
>  arch/x86/include/asm/topology.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index cc164777e661..2f0b6be8eaab 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
>  }
>  #endif
>  
> -#ifdef CONFIG_ACPI_CPPC_LIB
> +#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
>  void init_freq_invariance_cppc(void);
>  #define init_freq_invariance_cppc init_freq_invariance_cppc
>  #endif
> -- 

Well, since that function is in smpboot.c then the logic should be that
CPPC depends on functionality in smpboot.c for proper operation.

IOW, ACPI_CPPC_LIB should have "depends on CONFIG_SMP" in Kconfig, no?

Instead of adding more ifdeffery around...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 12:16   ` Borislav Petkov
@ 2022-01-06 14:37     ` Huang Rui
  2022-01-06 14:55       ` Huang Rui
  0 siblings, 1 reply; 13+ messages in thread
From: Huang Rui @ 2022-01-06 14:37 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, linux-pm,
	linux-kernel, Yuan, Perry, Su, Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Ingo Molnar, Peter Zijlstra,
	x86, stable

On Thu, Jan 06, 2022 at 08:16:43PM +0800, Borislav Petkov wrote:
> On Thu, Jan 06, 2022 at 03:43:06PM +0800, Huang Rui wrote:
> > The init_freq_invariance_cppc function is implemented in smpboot and depends on
> > CONFIG_SMP.
> > 
> >   MODPOST vmlinux.symvers
> >   MODINFO modules.builtin.modinfo
> >   GEN     modules.builtin
> >   LD      .tmp_vmlinux.kallsyms1
> > ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
> > /home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
> > make: *** [Makefile:1161: vmlinux] Error 1
> > 
> > See https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F484af487-7511-647e-5c5b-33d4429acdec%40infradead.org%2F&amp;data=04%7C01%7Cray.huang%40amd.com%7C4c696d3f23ac4479dda108d9d10e6a53%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637770682133383506%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=9lC2az1xGeYn7fNputkUMsy7PIhmkqW8jdpDUsaWthI%3D&amp;reserved=0.
> > 
> > Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: x86@kernel.org
> > Cc: stable@vger.kernel.org
> > ---
> >  arch/x86/include/asm/topology.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> > index cc164777e661..2f0b6be8eaab 100644
> > --- a/arch/x86/include/asm/topology.h
> > +++ b/arch/x86/include/asm/topology.h
> > @@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
> >  }
> >  #endif
> >  
> > -#ifdef CONFIG_ACPI_CPPC_LIB
> > +#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
> >  void init_freq_invariance_cppc(void);
> >  #define init_freq_invariance_cppc init_freq_invariance_cppc
> >  #endif
> > -- 
> 
> Well, since that function is in smpboot.c then the logic should be that
> CPPC depends on functionality in smpboot.c for proper operation.
> 
> IOW, ACPI_CPPC_LIB should have "depends on CONFIG_SMP" in Kconfig, no?
> 
> Instead of adding more ifdeffery around...
> 

Hmm, yes, that's fine. I will modify it in V2.

Thanks,
Ray

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 14:37     ` Huang Rui
@ 2022-01-06 14:55       ` Huang Rui
  2022-01-06 15:54         ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Huang Rui @ 2022-01-06 14:55 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, linux-pm,
	linux-kernel, Yuan, Perry, Su, Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Ingo Molnar, Peter Zijlstra,
	x86, stable

On Thu, Jan 06, 2022 at 10:37:59PM +0800, Huang Rui wrote:
> On Thu, Jan 06, 2022 at 08:16:43PM +0800, Borislav Petkov wrote:
> > On Thu, Jan 06, 2022 at 03:43:06PM +0800, Huang Rui wrote:
> > > The init_freq_invariance_cppc function is implemented in smpboot and depends on
> > > CONFIG_SMP.
> > > 
> > >   MODPOST vmlinux.symvers
> > >   MODINFO modules.builtin.modinfo
> > >   GEN     modules.builtin
> > >   LD      .tmp_vmlinux.kallsyms1
> > > ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
> > > /home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
> > > make: *** [Makefile:1161: vmlinux] Error 1
> > > 
> > > See https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F484af487-7511-647e-5c5b-33d4429acdec%40infradead.org%2F&amp;data=04%7C01%7Cray.huang%40amd.com%7C4c696d3f23ac4479dda108d9d10e6a53%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637770682133383506%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=9lC2az1xGeYn7fNputkUMsy7PIhmkqW8jdpDUsaWthI%3D&amp;reserved=0.
> > > 
> > > Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Cc: Borislav Petkov <bp@alien8.de>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: x86@kernel.org
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  arch/x86/include/asm/topology.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> > > index cc164777e661..2f0b6be8eaab 100644
> > > --- a/arch/x86/include/asm/topology.h
> > > +++ b/arch/x86/include/asm/topology.h
> > > @@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
> > >  }
> > >  #endif
> > >  
> > > -#ifdef CONFIG_ACPI_CPPC_LIB
> > > +#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
> > >  void init_freq_invariance_cppc(void);
> > >  #define init_freq_invariance_cppc init_freq_invariance_cppc
> > >  #endif
> > > -- 
> > 
> > Well, since that function is in smpboot.c then the logic should be that
> > CPPC depends on functionality in smpboot.c for proper operation.
> > 
> > IOW, ACPI_CPPC_LIB should have "depends on CONFIG_SMP" in Kconfig, no?
> > 
> > Instead of adding more ifdeffery around...
> > 
> 
> Hmm, yes, that's fine. I will modify it in V2.
> 

I just thought the CPPC function should be able to work on single core even
SMP is disabled.

Thanks,
Ray

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 14:55       ` Huang Rui
@ 2022-01-06 15:54         ` Borislav Petkov
  2022-01-06 16:12           ` Rafael J. Wysocki
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2022-01-06 15:54 UTC (permalink / raw)
  To: Huang Rui
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, linux-pm,
	linux-kernel, Yuan, Perry, Su, Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Ingo Molnar, Peter Zijlstra,
	x86, stable

On Thu, Jan 06, 2022 at 10:55:20PM +0800, Huang Rui wrote:
> I just thought the CPPC function should be able to work on single core even
> SMP is disabled.

Why, because SMP=n is a real use case?!

FWIW, we were even speculating on removing SMP=n so how practical is
using CPPC on SMP=n at all?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 15:54         ` Borislav Petkov
@ 2022-01-06 16:12           ` Rafael J. Wysocki
  2022-01-06 16:23             ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2022-01-06 16:12 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Huang Rui, Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell,
	linux-pm, linux-kernel, Yuan, Perry, Su, Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Ingo Molnar, Peter Zijlstra,
	x86, stable

On Thu, Jan 6, 2022 at 4:55 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Jan 06, 2022 at 10:55:20PM +0800, Huang Rui wrote:
> > I just thought the CPPC function should be able to work on single core even
> > SMP is disabled.
>
> Why, because SMP=n is a real use case?!

And why can't it be a real use case?

> FWIW, we were even speculating on removing SMP=n so how practical is
> using CPPC on SMP=n at all?

The honest answer is that we don't know.

Moreover, AFAICS the requisite #ifdeffery is there already and  the
problem is that the init_freq_invariance_cppc() defined in smpboot.c
is not exported to modules and the CPPC code is modular in this build.

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 16:12           ` Rafael J. Wysocki
@ 2022-01-06 16:23             ` Borislav Petkov
  2022-01-06 16:49               ` Rafael J. Wysocki
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2022-01-06 16:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Huang Rui, Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell,
	linux-pm, linux-kernel, Yuan, Perry, Su, Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Ingo Molnar, Peter Zijlstra,
	x86, stable

On Thu, Jan 06, 2022 at 05:12:51PM +0100, Rafael J. Wysocki wrote:
> And why can't it be a real use case?

You mean there's someone out there running SMP=n kernels on current
hardware which has CPPC too? Yeah, right.

> The honest answer is that we don't know.
>
> Moreover, AFAICS the requisite #ifdeffery is there already and  the
> problem is that the init_freq_invariance_cppc() defined in smpboot.c
> is not exported to modules and the CPPC code is modular in this build.

Yah, I saw that. And that's why I'm saying CPPC should depend on SMP -
because it needs that functionality which is defined there.

But if you really wanna support SMP=n, I don't care that much to debate
this more - I just think it is silly.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 16:23             ` Borislav Petkov
@ 2022-01-06 16:49               ` Rafael J. Wysocki
  0 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2022-01-06 16:49 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Rafael J. Wysocki, Huang Rui, Rafael J . Wysocki, Randy Dunlap,
	Stephen Rothwell, linux-pm, linux-kernel, Yuan, Perry, Su,
	Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Ingo Molnar, Peter Zijlstra,
	x86, stable

On Thu, Jan 6, 2022 at 5:23 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Jan 06, 2022 at 05:12:51PM +0100, Rafael J. Wysocki wrote:
> > And why can't it be a real use case?
>
> You mean there's someone out there running SMP=n kernels on current
> hardware which has CPPC too? Yeah, right.
>
> > The honest answer is that we don't know.
> >
> > Moreover, AFAICS the requisite #ifdeffery is there already and  the
> > problem is that the init_freq_invariance_cppc() defined in smpboot.c
> > is not exported to modules and the CPPC code is modular in this build.
>
> Yah, I saw that. And that's why I'm saying CPPC should depend on SMP -
> because it needs that functionality which is defined there.

In fact, the CPPC code itself doesn't need that functionality.

The init_freq_invariance_cppc() call is in there, because
amd_set_max_freq_ratio() depends on CPPC and it is pointless to run it
when CPPC is not supported, not the other way around.

> But if you really wanna support SMP=n, I don't care that much to debate
> this more - I just think it is silly.

Well, I just don't want to stop supporting SMP=n just because we can't
possibly get our build dependencies right.

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06  7:43 ` [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc Huang Rui
  2022-01-06 12:16   ` Borislav Petkov
@ 2022-01-06 17:12   ` Rafael J. Wysocki
  2022-01-06 17:46     ` Rafael J. Wysocki
  1 sibling, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2022-01-06 17:12 UTC (permalink / raw)
  To: Huang Rui
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, Linux PM,
	Linux Kernel Mailing List, Perry Yuan, Jinzhou Su, Xiaojian Du,
	kernel test robot, Borislav Petkov, Ingo Molnar, Peter Zijlstra,
	the arch/x86 maintainers, Stable

On Thu, Jan 6, 2022 at 8:43 AM Huang Rui <ray.huang@amd.com> wrote:
>
> The init_freq_invariance_cppc function is implemented in smpboot and depends on
> CONFIG_SMP.
>
>   MODPOST vmlinux.symvers
>   MODINFO modules.builtin.modinfo
>   GEN     modules.builtin
>   LD      .tmp_vmlinux.kallsyms1
> ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
> /home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
> make: *** [Makefile:1161: vmlinux] Error 1
>
> See https://lore.kernel.org/lkml/484af487-7511-647e-5c5b-33d4429acdec@infradead.org/.
>
> Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: x86@kernel.org
> Cc: stable@vger.kernel.org
> ---
>  arch/x86/include/asm/topology.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> index cc164777e661..2f0b6be8eaab 100644
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
>  }
>  #endif
>
> -#ifdef CONFIG_ACPI_CPPC_LIB
> +#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
>  void init_freq_invariance_cppc(void);
>  #define init_freq_invariance_cppc init_freq_invariance_cppc

Why don't you check CONFIG_SMP instead of this symbol in cppc_acpi.c?
That file depends on CONFIG_ACPI_CPPC_LIB anyway.

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

* Re: [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State
  2022-01-06  7:43 [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State Huang Rui
  2022-01-06  7:43 ` [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc Huang Rui
@ 2022-01-06 17:34 ` Rafael J. Wysocki
  1 sibling, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2022-01-06 17:34 UTC (permalink / raw)
  To: Huang Rui
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, Linux PM,
	Linux Kernel Mailing List, Perry Yuan, Jinzhou Su, Xiaojian Du

On Thu, Jan 6, 2022 at 8:43 AM Huang Rui <ray.huang@amd.com> wrote:
>
> The AMD P-State driver is based on ACPI CPPC function, so ACPI should be
> dependence of this driver in the kernel config.
>
> In file included from ../drivers/cpufreq/amd-pstate.c:40:0:
> ../include/acpi/processor.h:226:2: error: unknown type name ‘phys_cpuid_t’
>   phys_cpuid_t phys_id; /* CPU hardware ID such as APIC ID for x86 */
>   ^~~~~~~~~~~~
> ../include/acpi/processor.h:355:1: error: unknown type name ‘phys_cpuid_t’; did you mean ‘phys_addr_t’?
>  phys_cpuid_t acpi_get_phys_id(acpi_handle, int type, u32 acpi_id);
>  ^~~~~~~~~~~~
>  phys_addr_t
>   CC      drivers/rtc/rtc-rv3029c2.o
> ../include/acpi/processor.h:356:1: error: unknown type name ‘phys_cpuid_t’; did you mean ‘phys_addr_t’?
>  phys_cpuid_t acpi_map_madt_entry(u32 acpi_id);
>  ^~~~~~~~~~~~
>  phys_addr_t
> ../include/acpi/processor.h:357:20: error: unknown type name ‘phys_cpuid_t’; did you mean ‘phys_addr_t’?
>  int acpi_map_cpuid(phys_cpuid_t phys_id, u32 acpi_id);
>                     ^~~~~~~~~~~~
>                     phys_addr_t
>
> See https://lore.kernel.org/lkml/20e286d4-25d7-fb6e-31a1-4349c805aae3@infradead.org/.
>
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  drivers/cpufreq/Kconfig.x86 | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index a951768c3ebb..55516043b656 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -36,9 +36,9 @@ config X86_PCC_CPUFREQ
>
>  config X86_AMD_PSTATE
>         tristate "AMD Processor P-State driver"
> -       depends on X86
> -       select ACPI_PROCESSOR if ACPI
> -       select ACPI_CPPC_LIB if X86_64 && ACPI
> +       depends on X86 && ACPI
> +       select ACPI_PROCESSOR
> +       select ACPI_CPPC_LIB if X86_64
>         select CPU_FREQ_GOV_SCHEDUTIL if SMP
>         help
>           This driver adds a CPUFreq driver which utilizes a fine grain
> --

Applied (under a modified subject), thanks!

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 17:12   ` Rafael J. Wysocki
@ 2022-01-06 17:46     ` Rafael J. Wysocki
  2022-01-07  1:23       ` Huang Rui
  0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2022-01-06 17:46 UTC (permalink / raw)
  To: Huang Rui
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, Linux PM,
	Linux Kernel Mailing List, Perry Yuan, Jinzhou Su, Xiaojian Du,
	kernel test robot, Borislav Petkov, Ingo Molnar, Peter Zijlstra,
	the arch/x86 maintainers, Stable

On Thu, Jan 6, 2022 at 6:12 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, Jan 6, 2022 at 8:43 AM Huang Rui <ray.huang@amd.com> wrote:
> >
> > The init_freq_invariance_cppc function is implemented in smpboot and depends on
> > CONFIG_SMP.
> >
> >   MODPOST vmlinux.symvers
> >   MODINFO modules.builtin.modinfo
> >   GEN     modules.builtin
> >   LD      .tmp_vmlinux.kallsyms1
> > ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
> > /home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
> > make: *** [Makefile:1161: vmlinux] Error 1
> >
> > See https://lore.kernel.org/lkml/484af487-7511-647e-5c5b-33d4429acdec@infradead.org/.
> >
> > Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: x86@kernel.org
> > Cc: stable@vger.kernel.org
> > ---
> >  arch/x86/include/asm/topology.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> > index cc164777e661..2f0b6be8eaab 100644
> > --- a/arch/x86/include/asm/topology.h
> > +++ b/arch/x86/include/asm/topology.h
> > @@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
> >  }
> >  #endif
> >
> > -#ifdef CONFIG_ACPI_CPPC_LIB
> > +#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
> >  void init_freq_invariance_cppc(void);
> >  #define init_freq_invariance_cppc init_freq_invariance_cppc
>
> Why don't you check CONFIG_SMP instead of this symbol in cppc_acpi.c?
> That file depends on CONFIG_ACPI_CPPC_LIB anyway.

Scratch that, it needs to compile on non-x86 too.

The $subject patch is cleaner than all of the alternatives I have
considered, so I'm going to apply it.

However, I'm not really happy with the dependencies between CPPC and
smpboot.c going both ways.

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

* Re: [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc
  2022-01-06 17:46     ` Rafael J. Wysocki
@ 2022-01-07  1:23       ` Huang Rui
  0 siblings, 0 replies; 13+ messages in thread
From: Huang Rui @ 2022-01-07  1:23 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J . Wysocki, Randy Dunlap, Stephen Rothwell, Linux PM,
	Linux Kernel Mailing List, Yuan, Perry, Su, Jinzhou (Joe),
	Du, Xiaojian, kernel test robot, Borislav Petkov, Ingo Molnar,
	Peter Zijlstra, the arch/x86 maintainers, Stable

On Fri, Jan 07, 2022 at 01:46:04AM +0800, Rafael J. Wysocki wrote:
> On Thu, Jan 6, 2022 at 6:12 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Thu, Jan 6, 2022 at 8:43 AM Huang Rui <ray.huang@amd.com> wrote:
> > >
> > > The init_freq_invariance_cppc function is implemented in smpboot and depends on
> > > CONFIG_SMP.
> > >
> > >   MODPOST vmlinux.symvers
> > >   MODINFO modules.builtin.modinfo
> > >   GEN     modules.builtin
> > >   LD      .tmp_vmlinux.kallsyms1
> > > ld: drivers/acpi/cppc_acpi.o: in function `acpi_cppc_processor_probe':
> > > /home/ray/brahma3/linux/drivers/acpi/cppc_acpi.c:819: undefined reference to `init_freq_invariance_cppc'
> > > make: *** [Makefile:1161: vmlinux] Error 1
> > >
> > > See https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F484af487-7511-647e-5c5b-33d4429acdec%40infradead.org%2F&amp;data=04%7C01%7Cray.huang%40amd.com%7Cde64292b38394cf2cac508d9d13c701c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637770879795785268%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=BtO45nsVaYM%2BBytS%2B5GOPQuIadaiUEnzzaSmydsl0Jo%3D&amp;reserved=0.
> > >
> > > Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Cc: Borislav Petkov <bp@alien8.de>
> > > Cc: Ingo Molnar <mingo@kernel.org>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: x86@kernel.org
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  arch/x86/include/asm/topology.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
> > > index cc164777e661..2f0b6be8eaab 100644
> > > --- a/arch/x86/include/asm/topology.h
> > > +++ b/arch/x86/include/asm/topology.h
> > > @@ -221,7 +221,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled)
> > >  }
> > >  #endif
> > >
> > > -#ifdef CONFIG_ACPI_CPPC_LIB
> > > +#if defined(CONFIG_ACPI_CPPC_LIB) && defined(CONFIG_SMP)
> > >  void init_freq_invariance_cppc(void);
> > >  #define init_freq_invariance_cppc init_freq_invariance_cppc
> >
> > Why don't you check CONFIG_SMP instead of this symbol in cppc_acpi.c?
> > That file depends on CONFIG_ACPI_CPPC_LIB anyway.
> 
> Scratch that, it needs to compile on non-x86 too.
> 
> The $subject patch is cleaner than all of the alternatives I have
> considered, so I'm going to apply it.
> 
> However, I'm not really happy with the dependencies between CPPC and
> smpboot.c going both ways.

Thanks, for urgent fix for linux-next, I think we can use this patch at
this moment.
Let me think out a better way to handle the dependencies next step. :-)

Thanks,
Ray

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

end of thread, other threads:[~2022-01-07  1:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-06  7:43 [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State Huang Rui
2022-01-06  7:43 ` [PATCH 2/2] x86, sched: Fix the undefined reference building error of init_freq_invariance_cppc Huang Rui
2022-01-06 12:16   ` Borislav Petkov
2022-01-06 14:37     ` Huang Rui
2022-01-06 14:55       ` Huang Rui
2022-01-06 15:54         ` Borislav Petkov
2022-01-06 16:12           ` Rafael J. Wysocki
2022-01-06 16:23             ` Borislav Petkov
2022-01-06 16:49               ` Rafael J. Wysocki
2022-01-06 17:12   ` Rafael J. Wysocki
2022-01-06 17:46     ` Rafael J. Wysocki
2022-01-07  1:23       ` Huang Rui
2022-01-06 17:34 ` [PATCH 1/2] cpufreq: amd-pstate: Fix the dependence issue of AMD P-State Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).