diff mbox series

[v1,Part2,1/5] x86/microcode: Move late load warning to the same function that taints kernel

Message ID 20230113172920.113612-2-ashok.raj@intel.com
State New
Headers
Series Declare safe late loadable microcode |

Commit Message

Ashok Raj Jan. 13, 2023, 5:29 p.m. UTC 
  Currently the warning about late loading and tainting are issued from two
different functions.

Later patches will re-enable microcode late-loading.

Having both messages in the same function helps issuing warnings only
when required.

Move the warning from microcode_reload_late() -> reload_store() where the
kernel tainting also happens.

No functional changes.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
Cc: LKML <linux-kernel@vger.kernel.org>
Cc: x86 <x86@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
Cc: Thomas Gleixner (Intel) <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Stefan Talpalaru <stefantalpalaru@yahoo.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Rafael J. Wysocki <rafael@kernel.org>
Cc: Peter Zilstra (Intel) <peterz@infradead.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>
---
 arch/x86/kernel/cpu/microcode/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Thomas Gleixner Jan. 19, 2023, 9:48 p.m. UTC | #1 
On Fri, Jan 13 2023 at 09:29, Ashok Raj wrote:
> Currently the warning about late loading and tainting are issued from two
> different functions.
>
> Later patches will re-enable microcode late-loading.
>
> Having both messages in the same function helps issuing warnings only
> when required.
>
> Move the warning from microcode_reload_late() -> reload_store() where the
> kernel tainting also happens.
>
> No functional change.

I had to read this more than once to make sense of it. Let me try a
translation:

  Late microcode loading issues a warning and taints the
  kernel. Tainting the kernel and emitting the warning happens in two
  different functions.

  The upcoming support for safe late loading under certain conditions
  needs to prevent both the warning and the tainting when the safe
  conditions are met. That would require to hand the result of the safe
  condition check into the function which emits the warning.

  To avoid this awkward construct, move the warning into reload_store()
  next to the taint() invocation as that is also the function which will
  later contain the safe condition check.

  No functional change.

Did my decoder get that right?

Thanks,

        tglx
Ashok Raj Jan. 19, 2023, 10:05 p.m. UTC | #2 
On Thu, Jan 19, 2023 at 10:48:30PM +0100, Thomas Gleixner wrote:
> On Fri, Jan 13 2023 at 09:29, Ashok Raj wrote:
> > Currently the warning about late loading and tainting are issued from two
> > different functions.
> >
> > Later patches will re-enable microcode late-loading.
> >
> > Having both messages in the same function helps issuing warnings only
> > when required.
> >
> > Move the warning from microcode_reload_late() -> reload_store() where the
> > kernel tainting also happens.
> >
> > No functional change.
> 
> I had to read this more than once to make sense of it. Let me try a
> translation:
> 
>   Late microcode loading issues a warning and taints the
>   kernel. Tainting the kernel and emitting the warning happens in two
>   different functions.
> 
>   The upcoming support for safe late loading under certain conditions
>   needs to prevent both the warning and the tainting when the safe
>   conditions are met. That would require to hand the result of the safe
>   condition check into the function which emits the warning.
> 
>   To avoid this awkward construct, move the warning into reload_store()
>   next to the taint() invocation as that is also the function which will
>   later contain the safe condition check.
> 
>   No functional change.
> 
> Did my decoder get that right?
> 

Yes, that is accurate.. inspite of my awkward phrasing :-(

I should copy this commit verbatim before i resend :-)

Cheers,
Ashok
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index d7cbc83df9b6..c361882baf63 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -441,9 +441,6 @@  static int microcode_reload_late(void)
 	int old = boot_cpu_data.microcode, ret;
 	struct cpuinfo_x86 prev_info;
 
-	pr_err("Attempting late microcode loading - it is dangerous and taints the kernel.\n");
-	pr_err("You should switch to early loading, if possible.\n");
-
 	atomic_set(&late_cpus_in,  0);
 	atomic_set(&late_cpus_out, 0);
 
@@ -494,6 +491,9 @@  static ssize_t reload_store(struct device *dev,
 	if (tmp_ret != UCODE_NEW)
 		goto put;
 
+	pr_err("Attempting late microcode loading - it is dangerous and taints the kernel.\n");
+	pr_err("You should switch to early loading, if possible.\n");
+
 	mutex_lock(&microcode_mutex);
 	ret = microcode_reload_late();
 	mutex_unlock(&microcode_mutex);