print_ucode_rev() takes a struct ucode_cpu_info argument. The sole purpose
of it is to print the microcode revision.
The only available ucode_cpu_info always describes the currently loaded
microcode revision. After a microcode update is successful, this is the new
revision, or on failure it is the original revision.
Subsequent changes need to print both the original and new revision, but
the original version will be cached in a plain integer, which makes the
code inconsistent.
Replace the struct ucode_cpu_info argument with a plain integer which
contains the revision number and adjust the call sites accordingly.
No functional change.
Signed-off-by: Ashok Raj <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Cc: LKML <[email protected]>
Cc: x86 <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Alison Schofield <[email protected]>
Cc: Reinette Chatre <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
Changes since V1:
Thomas:
- Updated commit log as suggested
- Remove the line break after static void before print_ucode_info
---
arch/x86/kernel/cpu/microcode/intel.c | 31 ++++++++-------------------
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 6bebc46ad8b1..1d709b72cfd0 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -310,13 +310,10 @@ static bool load_builtin_intel_microcode(struct cpio_data *cp)
/*
* Print ucode update info.
*/
-static void
-print_ucode_info(struct ucode_cpu_info *uci, unsigned int date)
+static void print_ucode_info(unsigned int new_rev, unsigned int date)
{
pr_info_once("microcode updated early to revision 0x%x, date = %04x-%02x-%02x\n",
- uci->cpu_sig.rev,
- date & 0xffff,
- date >> 24,
+ new_rev, date & 0xffff, date >> 24,
(date >> 16) & 0xff);
}
@@ -334,7 +331,7 @@ void show_ucode_info_early(void)
if (delay_ucode_info) {
intel_cpu_collect_info(&uci);
- print_ucode_info(&uci, current_mc_date);
+ print_ucode_info(uci.cpu_sig.rev. current_mc_date);
delay_ucode_info = 0;
}
}
@@ -343,33 +340,23 @@ void show_ucode_info_early(void)
* At this point, we can not call printk() yet. Delay printing microcode info in
* show_ucode_info_early() until printk() works.
*/
-static void print_ucode(struct ucode_cpu_info *uci)
+static void print_ucode(int new_rev, int date)
{
struct microcode_intel *mc;
int *delay_ucode_info_p;
int *current_mc_date_p;
- mc = uci->mc;
- if (!mc)
- return;
-
delay_ucode_info_p = (int *)__pa_nodebug(&delay_ucode_info);
current_mc_date_p = (int *)__pa_nodebug(¤t_mc_date);
*delay_ucode_info_p = 1;
- *current_mc_date_p = mc->hdr.date;
+ *current_mc_date_p = date;
}
#else
-static inline void print_ucode(struct ucode_cpu_info *uci)
+static inline void print_ucode(int new_rev, int date)
{
- struct microcode_intel *mc;
-
- mc = uci->mc;
- if (!mc)
- return;
-
- print_ucode_info(uci, mc->hdr.date);
+ print_ucode_info(new_rev, date);
}
#endif
@@ -409,9 +396,9 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early)
uci->cpu_sig.rev = rev;
if (early)
- print_ucode(uci);
+ print_ucode(uci->cpu_sig.rev, mc->hdr.date);
else
- print_ucode_info(uci, mc->hdr.date);
+ print_ucode_info(uci->cpu_sig.rev, mc->hdr.date);
return 0;
}
--
2.34.1
On Mon, Jan 09, 2023 at 07:35:53AM -0800, Ashok Raj wrote:
> @@ -343,33 +340,23 @@ void show_ucode_info_early(void)
> * At this point, we can not call printk() yet. Delay printing microcode info in
> * show_ucode_info_early() until printk() works.
> */
> -static void print_ucode(struct ucode_cpu_info *uci)
> +static void print_ucode(int new_rev, int date)
> {
> struct microcode_intel *mc;
> int *delay_ucode_info_p;
> int *current_mc_date_p;
>
> - mc = uci->mc;
> - if (!mc)
> - return;
> -
> delay_ucode_info_p = (int *)__pa_nodebug(&delay_ucode_info);
> current_mc_date_p = (int *)__pa_nodebug(¤t_mc_date);
>
> *delay_ucode_info_p = 1;
> - *current_mc_date_p = mc->hdr.date;
> + *current_mc_date_p = date;
Here's how I know you haven't tested this on 32-bit:
arch/x86/kernel/cpu/microcode/intel.c: In function ‘print_ucode’:
arch/x86/kernel/cpu/microcode/intel.c:344:33: error: unused variable ‘mc’ [-Werror=unused-variable]
344 | struct microcode_intel *mc;
| ^~
cc1: all warnings being treated as errors
make[5]: *** [scripts/Makefile.build:252: arch/x86/kernel/cpu/microcode/intel.o] Error 1
make[4]: *** [scripts/Makefile.build:504: arch/x86/kernel/cpu/microcode] Error 2
make[3]: *** [scripts/Makefile.build:504: arch/x86/kernel/cpu] Error 2
make[2]: *** [scripts/Makefile.build:504: arch/x86/kernel] Error 2
make[1]: *** [scripts/Makefile.build:504: arch/x86] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:2008: .] Error 2
Testing is overrated, right?
The maintainers can do that, ofc.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Mon, Jan 09, 2023 at 07:35:53AM -0800, Ashok Raj wrote:
> @@ -334,7 +331,7 @@ void show_ucode_info_early(void)
>
> if (delay_ucode_info) {
> intel_cpu_collect_info(&uci);
> - print_ucode_info(&uci, current_mc_date);
> + print_ucode_info(uci.cpu_sig.rev. current_mc_date);
You must be kidding:
arch/x86/kernel/cpu/microcode/intel.c: In function ‘show_ucode_info_early’:
arch/x86/kernel/cpu/microcode/intel.c:332:49: error: request for member ‘current_mc_date’ in something not a structure or union
332 | print_ucode_info(uci.cpu_sig.rev. current_mc_date);
| ^
arch/x86/kernel/cpu/microcode/intel.c:332:17: error: too few arguments to function ‘print_ucode_info’
332 | print_ucode_info(uci.cpu_sig.rev. current_mc_date);
| ^~~~~~~~~~~~~~~~
arch/x86/kernel/cpu/microcode/intel.c:311:13: note: declared here
311 | static void print_ucode_info(unsigned int new_rev, unsigned int date)
| ^~~~~~~~~~~~~~~~
arch/x86/kernel/cpu/microcode/intel.c: In function ‘print_ucode’:
arch/x86/kernel/cpu/microcode/intel.c:343:33: error: unused variable ‘mc’ [-Werror=unused-variable]
343 | struct microcode_intel *mc;
| ^~
cc1: all warnings being treated as errors
make[5]: *** [scripts/Makefile.build:252: arch/x86/kernel/cpu/microcode/intel.o] Error 1
make[4]: *** [scripts/Makefile.build:504: arch/x86/kernel/cpu/microcode] Error 2
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:504: arch/x86/kernel/cpu] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:504: arch/x86/kernel] Error 2
make[1]: *** [scripts/Makefile.build:504: arch/x86] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:2008: .] Error 2
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Sun, Jan 15, 2023 at 08:39:18PM +0100, Borislav Petkov wrote:
> On Mon, Jan 09, 2023 at 07:35:53AM -0800, Ashok Raj wrote:
> > @@ -334,7 +331,7 @@ void show_ucode_info_early(void)
> >
> > if (delay_ucode_info) {
> > intel_cpu_collect_info(&uci);
> > - print_ucode_info(&uci, current_mc_date);
> > + print_ucode_info(uci.cpu_sig.rev. current_mc_date);
>
> You must be kidding:
Oversight completely.. Apologize. I remember seeing them but unfortunately
I ended up fixing the issue in patch5 instead of patch4.
>
> arch/x86/kernel/cpu/microcode/intel.c: In function ‘show_ucode_info_early’:
> arch/x86/kernel/cpu/microcode/intel.c:332:49: error: request for member ‘current_mc_date’ in something not a structure or union
> 332 | print_ucode_info(uci.cpu_sig.rev. current_mc_date);
> | ^
> arch/x86/kernel/cpu/microcode/intel.c:332:17: error: too few arguments to function ‘print_ucode_info’
> 332 | print_ucode_info(uci.cpu_sig.rev. current_mc_date);
> | ^~~~~~~~~~~~~~~~
> arch/x86/kernel/cpu/microcode/intel.c:311:13: note: declared here
> 311 | static void print_ucode_info(unsigned int new_rev, unsigned int date)
> | ^~~~~~~~~~~~~~~~
> arch/x86/kernel/cpu/microcode/intel.c: In function ‘print_ucode’:
> arch/x86/kernel/cpu/microcode/intel.c:343:33: error: unused variable ‘mc’ [-Werror=unused-variable]
> 343 | struct microcode_intel *mc;
> | ^~
I fixed in patch6 instead of this patch, my bad.
I updated my build to have CONFIG_WERROR, i had that accidently turned off
in one of my configs.
I have fixed up the right version and can repost if there are no additional
comments.
Cheers,
Ashok
On Tue, Jan 17, 2023 at 08:05:38AM -0800, Ashok Raj wrote:
> I updated my build to have CONFIG_WERROR, i had that accidently turned off
> in one of my configs.
You need to build every patch for bisection purposes, obviously. In addition
CONFIG_ERROR.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette