diff mbox series

x86: Fix amd_check_microcode() declaration

Message ID 20230725082638.956675-1-arnd@kernel.org
State New
Headers
Series x86: Fix amd_check_microcode() declaration |

Commit Message

Arnd Bergmann July 25, 2023, 8:26 a.m. UTC 
  From: Arnd Bergmann <arnd@arndb.de>

The newly added amd_check_microcode() function has two conflicting definitions
if CONFIG_CPU_SUP_AMD is enabled and CONFIG_MICROCODE_AMD is disabled. Since
the header with the stub definition is not included in cpu/amd.c, this only
causes a -Wmissing-prototype warning with W=1

arch/x86/kernel/cpu/amd.c:1289:6: error: no previous prototype for 'amd_check_microcode' [-Werror=missing-prototypes]

Adding the missing #include shows the other problem:

arch/x86/kernel/cpu/amd.c:1290:6: error: redefinition of 'amd_check_microcode'
arch/x86/include/asm/microcode_amd.h:58:20: note: previous definition of 'amd_check_microcode' with type 'void(void)'

Change the stub function to use the matching #ifdef check for the amd cpu
support instead of the microcode and include the header to avoid both and
make it behave consistently.

Fixes: 522b1d69219d8 ("x86/cpu/amd: Add a Zenbleed fix")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/include/asm/microcode_amd.h | 7 ++++++-
 arch/x86/kernel/cpu/amd.c            | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Borislav Petkov July 25, 2023, 9:20 a.m. UTC | #1 
On Tue, Jul 25, 2023 at 10:26:13AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The newly added amd_check_microcode() function has two conflicting definitions
> if CONFIG_CPU_SUP_AMD is enabled and CONFIG_MICROCODE_AMD is disabled. Since
> the header with the stub definition is not included in cpu/amd.c, this only
> causes a -Wmissing-prototype warning with W=1

Can we please promote -Wmissing-prototype to default or is it too noisy
yet?

> diff --git a/arch/x86/include/asm/microcode_amd.h b/arch/x86/include/asm/microcode_amd.h
> index 9675c621c1ca4..6a860d40b0411 100644
> --- a/arch/x86/include/asm/microcode_amd.h
> +++ b/arch/x86/include/asm/microcode_amd.h
> @@ -48,13 +48,18 @@ extern void __init load_ucode_amd_bsp(unsigned int family);
>  extern void load_ucode_amd_ap(unsigned int family);
>  extern int __init save_microcode_in_initrd_amd(unsigned int family);
>  void reload_ucode_amd(unsigned int cpu);
> -extern void amd_check_microcode(void);
>  #else
>  static inline void __init load_ucode_amd_bsp(unsigned int family) {}
>  static inline void load_ucode_amd_ap(unsigned int family) {}
>  static inline int __init
>  save_microcode_in_initrd_amd(unsigned int family) { return -EINVAL; }
>  static inline void reload_ucode_amd(unsigned int cpu) {}
> +#endif
> +
> +#ifdef CONFIG_CPU_SUP_AMD
> +extern void amd_check_microcode(void);
> +#else
>  static inline void amd_check_microcode(void) {}
>  #endif
> +
>  #endif /* _ASM_X86_MICROCODE_AMD_H */

Considering how cpu/amd.c provides the function implementation, that
header gunk should not be in microcode_amd.h but in ...asm/processor.h,
I'd say.

Thx.
Arnd Bergmann July 25, 2023, 12:15 p.m. UTC | #2 
On Tue, Jul 25, 2023, at 11:20, Borislav Petkov wrote:
> On Tue, Jul 25, 2023 at 10:26:13AM +0200, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The newly added amd_check_microcode() function has two conflicting definitions
>> if CONFIG_CPU_SUP_AMD is enabled and CONFIG_MICROCODE_AMD is disabled. Since
>> the header with the stub definition is not included in cpu/amd.c, this only
>> causes a -Wmissing-prototype warning with W=1
>
> Can we please promote -Wmissing-prototype to default or is it too noisy
> yet?

I'm working on it, currently missing just 15 patches (down from 70 in 6.3)
before all randconfigs on x86 and arm are clean.

>> diff --git a/arch/x86/include/asm/microcode_amd.h b/arch/x86/include/asm/microcode_amd.h
>> index 9675c621c1ca4..6a860d40b0411 100644
>> --- a/arch/x86/include/asm/microcode_amd.h
>> +++ b/arch/x86/include/asm/microcode_amd.h

>> +#ifdef CONFIG_CPU_SUP_AMD
>> +extern void amd_check_microcode(void);
>> +#else
>>  static inline void amd_check_microcode(void) {}
>>  #endif
>> +
>>  #endif /* _ASM_X86_MICROCODE_AMD_H */
>
> Considering how cpu/amd.c provides the function implementation, that
> header gunk should not be in microcode_amd.h but in ...asm/processor.h,
> I'd say.

Ok, I'll send a v2 with that changed.

     Arnd
diff mbox series

Patch

diff --git a/arch/x86/include/asm/microcode_amd.h b/arch/x86/include/asm/microcode_amd.h
index 9675c621c1ca4..6a860d40b0411 100644
--- a/arch/x86/include/asm/microcode_amd.h
+++ b/arch/x86/include/asm/microcode_amd.h
@@ -48,13 +48,18 @@  extern void __init load_ucode_amd_bsp(unsigned int family);
 extern void load_ucode_amd_ap(unsigned int family);
 extern int __init save_microcode_in_initrd_amd(unsigned int family);
 void reload_ucode_amd(unsigned int cpu);
-extern void amd_check_microcode(void);
 #else
 static inline void __init load_ucode_amd_bsp(unsigned int family) {}
 static inline void load_ucode_amd_ap(unsigned int family) {}
 static inline int __init
 save_microcode_in_initrd_amd(unsigned int family) { return -EINVAL; }
 static inline void reload_ucode_amd(unsigned int cpu) {}
+#endif
+
+#ifdef CONFIG_CPU_SUP_AMD
+extern void amd_check_microcode(void);
+#else
 static inline void amd_check_microcode(void) {}
 #endif
+
 #endif /* _ASM_X86_MICROCODE_AMD_H */
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 26ad7ca423e7c..24596e3c106b7 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -15,6 +15,7 @@ 
 #include <asm/cpu.h>
 #include <asm/spec-ctrl.h>
 #include <asm/smp.h>
+#include <asm/microcode_amd.h>
 #include <asm/numa.h>
 #include <asm/pci-direct.h>
 #include <asm/delay.h>