All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.