[PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case

Ingo Molnar posted 1 patch 9 months, 1 week ago
There is a newer version of this series
arch/x86/include/asm/microcode.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case
Posted by Ingo Molnar 9 months, 1 week ago

Fix this build bug:

  ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]

by adding the 'static' storage class to the !CONFIG_MICROCODE 
prototype.

Also, while at it, add all the other storage classes as well for this 
block of prototypes, 'extern' and 'static', respectively.

( Omitting 'extern' just because it's technically not needed
  is a bad habit for header prototypes and leads to bugs like
  this one. )

Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/microcode.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 6aa12aecbc95..c23849246d0a 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -16,15 +16,15 @@ struct ucode_cpu_info {
 };
 
 #ifdef CONFIG_MICROCODE
-void load_ucode_bsp(void);
-void load_ucode_ap(void);
-void microcode_bsp_resume(void);
-bool __init microcode_loader_disabled(void);
+extern void load_ucode_bsp(void);
+extern void load_ucode_ap(void);
+extern void microcode_bsp_resume(void);
+extern bool __init microcode_loader_disabled(void);
 #else
 static inline void load_ucode_bsp(void)	{ }
 static inline void load_ucode_ap(void) { }
 static inline void microcode_bsp_resume(void) { }
-bool __init microcode_loader_disabled(void) { return false; }
+static inline bool __init microcode_loader_disabled(void) { return false; }
 #endif
 
 extern unsigned long initrd_start_early;
Re: [PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case
Posted by Borislav Petkov 9 months, 1 week ago
On Mon, May 05, 2025 at 07:15:00AM +0200, Ingo Molnar wrote:
> 
> Fix this build bug:
> 
>   ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]
> 
> by adding the 'static' storage class to the !CONFIG_MICROCODE 
> prototype.

Thanks, I'll merge it with my patch.

> Also, while at it, add all the other storage classes as well for this 
> block of prototypes, 'extern' and 'static', respectively.

This I won't add as it is unnecessary.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case
Posted by Ingo Molnar 9 months, 1 week ago
* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, May 05, 2025 at 07:15:00AM +0200, Ingo Molnar wrote:
> > 
> > Fix this build bug:
> > 
> >   ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]
> > 
> > by adding the 'static' storage class to the !CONFIG_MICROCODE 
> > prototype.
> 
> Thanks, I'll merge it with my patch.

Please don't forcibly rebase trees like that, because you've just 
reintroduced a bug this way in tip:x86/urgent:

  # 5214a9f6c0f5 x86/microcode: Consolidate the loader enablement checking

  +static inline bool __init microcode_loader_disabled(void) { return false; }

static inlines don't need an __init tag ...

This was done correctly in the commit you removed:

  59e820c6de60 ("x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case")

  +static inline bool microcode_loader_disabled(void) { return false; }

> > Also, while at it, add all the other storage classes as well for this 
> > block of prototypes, 'extern' and 'static', respectively.
> 
> This I won't add as it is unnecessary.

Technically the 'int' in 'unsigned int' is 'unnecessary' and could be 
skipped as well with 'unsigned', right? Yet clean kernel code uses it 
because it's easier to read and more consistent. Likewise, the 'extern' 
storage class is a well-known and widespread way in the kernel to 
document public APIs, a counterpart to 'static'. Most of our major 
headers use that style.

Your change is a doubly poor choice in this particular case of 
<asm/microcode.h> as well, because the header continues with an 
'extern':

  extern unsigned long initrd_start_early;

So by dropping that cleanup you reintroduced an inconsistency as 
well...

Thanks,

	Ingo
Re: [PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case
Posted by Borislav Petkov 9 months, 1 week ago
On Tue, May 06, 2025 at 12:57:08PM +0200, Ingo Molnar wrote:
> Please don't forcibly rebase trees like that, because you've just 
> reintroduced a bug this way in tip:x86/urgent:
> 
>   # 5214a9f6c0f5 x86/microcode: Consolidate the loader enablement checking
> 
>   +static inline bool __init microcode_loader_disabled(void) { return false; }
> 
> static inlines don't need an __init tag ...

Oh please do tell what kind of bug that is. Especially if in
a CONFIG_MICROCODE=n build *all* the callsites of that function are gone!

Geez.

> Technically the 'int' in 'unsigned int' is 'unnecessary' and could be 
> skipped as well with 'unsigned', right?

And this is the same how exactly?

> Yet clean kernel code uses it because it's easier to read and more
> consistent. Likewise, the 'extern' storage class is a well-known and
> widespread way in the kernel to document public APIs, a counterpart to
> 'static'. Most of our major headers use that style.

Does this header need it?

No. Not really.

If it fixes something, yes, sure, by all means.

Gratuitous cleanups without any point: no. 

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
[tip: x86/microcode] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case
Posted by tip-bot2 for Ingo Molnar 9 months, 1 week ago
The following commit has been merged into the x86/microcode branch of tip:

Commit-ID:     59e820c6de60e81757211fbb846129485e337ee0
Gitweb:        https://git.kernel.org/tip/59e820c6de60e81757211fbb846129485e337ee0
Author:        Ingo Molnar <mingo@kernel.org>
AuthorDate:    Mon, 05 May 2025 07:15:04 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Mon, 05 May 2025 09:17:02 +02:00

x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case

Fix this build bug:

  ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]

by adding the 'static' storage class to the !CONFIG_MICROCODE
prototype.

Also, while at it, add all the other storage classes as well for this
block of prototypes, 'extern' and 'static', respectively.

( Omitting 'extern' just because it's technically not needed
  is a bad habit for header prototypes and leads to bugs like
  this one. )

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: "Borislav Petkov (AMD)" <bp@alien8.de>
Link: https://lore.kernel.org/r/aBhJVJDTlw2Y8owu@gmail.com
---
 arch/x86/include/asm/microcode.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index d53148f..b68fc9d 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -14,15 +14,15 @@ struct ucode_cpu_info {
 };
 
 #ifdef CONFIG_MICROCODE
-void load_ucode_bsp(void);
-void load_ucode_ap(void);
-void microcode_bsp_resume(void);
-bool __init microcode_loader_disabled(void);
+extern void load_ucode_bsp(void);
+extern void load_ucode_ap(void);
+extern void microcode_bsp_resume(void);
+extern bool __init microcode_loader_disabled(void);
 #else
 static inline void load_ucode_bsp(void)	{ }
 static inline void load_ucode_ap(void) { }
 static inline void microcode_bsp_resume(void) { }
-bool __init microcode_loader_disabled(void) { return false; }
+static inline bool microcode_loader_disabled(void) { return false; }
 #endif
 
 extern unsigned long initrd_start_early;
[tip: x86/microcode] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case
Posted by tip-bot2 for Ingo Molnar 9 months, 1 week ago
The following commit has been merged into the x86/microcode branch of tip:

Commit-ID:     e26d770a995f6f1ddb7c0c6d24f1aa81fb41eaa4
Gitweb:        https://git.kernel.org/tip/e26d770a995f6f1ddb7c0c6d24f1aa81fb41eaa4
Author:        Ingo Molnar <mingo@kernel.org>
AuthorDate:    Mon, 05 May 2025 07:15:04 +02:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Mon, 05 May 2025 07:15:27 +02:00

x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case

Fix this build bug:

  ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]

by adding the 'static' storage class to the !CONFIG_MICROCODE
prototype.

Also, while at it, add all the other storage classes as well for this
block of prototypes, 'extern' and 'static', respectively.

( Omitting 'extern' just because it's technically not needed
  is a bad habit for header prototypes and leads to bugs like
  this one. )

Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: "Borislav Petkov (AMD)" <bp@alien8.de>
Link: https://lore.kernel.org/r/aBhJVJDTlw2Y8owu@gmail.com
---
 arch/x86/include/asm/microcode.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index d53148f..af33bbf 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -14,15 +14,15 @@ struct ucode_cpu_info {
 };
 
 #ifdef CONFIG_MICROCODE
-void load_ucode_bsp(void);
-void load_ucode_ap(void);
-void microcode_bsp_resume(void);
-bool __init microcode_loader_disabled(void);
+extern void load_ucode_bsp(void);
+extern void load_ucode_ap(void);
+extern void microcode_bsp_resume(void);
+extern bool __init microcode_loader_disabled(void);
 #else
 static inline void load_ucode_bsp(void)	{ }
 static inline void load_ucode_ap(void) { }
 static inline void microcode_bsp_resume(void) { }
-bool __init microcode_loader_disabled(void) { return false; }
+static inline bool __init microcode_loader_disabled(void) { return false; }
 #endif
 
 extern unsigned long initrd_start_early;