linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ashok Raj <ashok.raj@intel.com>
To: Borislav Petkov <bp@alien8.de>, Thomas Gleixner <tglx@linutronix.de>
Cc: Ashok Raj <ashok.raj@intel.com>,
	LKML <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Tony Luck <tony.luck@intel.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Stefan Talpalaru <stefantalpalaru@yahoo.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Jonathan Corbet <corbet@lwn.net>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Peter Zilstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>
Subject: [PATCH v1 Part2 2/5] x86/microcode/intel: Add minimum required revision to microcode header
Date: Fri, 13 Jan 2023 09:29:17 -0800	[thread overview]
Message-ID: <20230113172920.113612-3-ashok.raj@intel.com> (raw)
In-Reply-To: <20230113172920.113612-1-ashok.raj@intel.com>

In general users don't have the necessary information to determine
whether a late loading of a new microcode version has removed any feature
(MSR, CPUID etc) between what is currently loaded and this new microcode.
To address this issue, Intel has added a "minimum required version" field
to a previously reserved field in the file header. Microcode updates
should only be applied if the current microcode version is equal
to, or greater than this minimum required version.

Thomas made some suggestions[1] on how meta-data in the microcode file
could provide Linux with information to decide if the new microcode is
suitable candidate for late loading. But even the "simpler" option#1
requires a lot of metadata and corresponding kernel code to parse it.

The proposal here is an even simpler option. Simply "OS visible features"
such as CPUID and MSRs are the only two examples. The microcode must not
change these OS visible features because they cause problems after late
loading. When microcode changes features, microcode will change the min_rev
to prevent such microcodes from being late loaded.

Pseudo code for late loading is as follows:

if header.min_required_id == 0
        This is old format microcode, block late loading
else if current_ucode_version < header.min_required_id
        Current version is too old, block late loading of this microcode.
else
        OK to proceed with late loading.

Any microcode that modifies the interface to an OS-visible feature
will set the min_version to itself. This will enforce this microcode is
not suitable for late loading unless the currently loaded revision is
greater or equal to the new microcode affecting the change.

The enforcement is not in hardware and limited to kernel loader enforcing
the requirement. It is not required for early loading of microcode to
enforce this requirement, since the new features are only
evaluated after early loading in the boot process.

Check if the new microcode specifies the minimum version for safe late
loading. Otherwise reject late load.

Test cases covered:

1. With new kernel, attempting to load an older format microcode with the
   min_rev=0 should be blocked by kernel.

   [  210.541802] Late loading denied: Microcode header does not specify a
   required min version.

2. New microcode with a non-zero min_rev in the header, but the specified
   min_rev is greater than what is currently loaded in the CPU should be
   blocked by kernel.

   245.139828] microcode: Late loading denied: Current revision 0x8f685300 is too old to update, must be at 0xaa000050 version or higher. Use early loading instead.

3. New microcode with a min_rev < currently loaded should allow loading the
   microcode

4. Build initrd with microcode that has min_rev=0, or min_rev > currently
   loaded should permit early loading microcode from initrd.

[1] https://lore.kernel.org/linux-kernel/alpine.DEB.2.21.1909062237580.1902@nanos.tec.linutronix.de/

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ashok Raj <ashok.raj@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/include/asm/microcode_intel.h |  3 ++-
 arch/x86/kernel/cpu/microcode/intel.c  | 34 +++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index f1fa979e05bf..e83afe919b10 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -15,7 +15,8 @@ struct microcode_header_intel {
 	unsigned int            datasize;
 	unsigned int            totalsize;
 	unsigned int            metasize;
-	unsigned int            reserved[2];
+	unsigned int		min_req_ver;
+	unsigned int		reserved3;
 };
 
 struct microcode_intel {
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 0cdff9ed2a4e..6046f90a47b2 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -137,6 +137,33 @@ static void save_microcode_patch(struct ucode_cpu_info *uci, void *data, unsigne
 		intel_ucode_patch = p->data;
 }
 
+static int is_lateload_safe(struct microcode_header_intel *mc_header)
+{
+	struct ucode_cpu_info uci;
+
+	/*
+	 * When late-loading, ensure the header declares a minimum revision
+	 * required to perform a late-load.
+	 */
+	if (!mc_header->min_req_ver) {
+		pr_warn("Late loading denied: Microcode header does not specify a required min version\n");
+		return -EINVAL;
+	}
+
+	intel_cpu_collect_info(&uci);
+
+	/*
+	 * Enforce the minimum revision specified in the header is either
+	 * greater or equal to the current revision.
+	 */
+	if (uci.cpu_sig.rev < mc_header->min_req_ver) {
+		pr_warn("Late loading denied: Current revision 0x%x too old to update, must be at 0x%x or higher. Use early loading instead\n",
+			uci.cpu_sig.rev, mc_header->min_req_ver);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Get microcode matching with BSP's model. Only CPUs with the same model as
  * BSP can stay in the platform.
@@ -678,7 +705,9 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
 		memcpy(mc, &mc_header, sizeof(mc_header));
 		data = mc + sizeof(mc_header);
 		if (!copy_from_iter_full(data, data_size, iter) ||
-		    intel_microcode_sanity_check(mc, true, MC_HEADER_TYPE_MICROCODE) < 0) {
+		    intel_microcode_sanity_check(mc, true, MC_HEADER_TYPE_MICROCODE) < 0 ||
+		    is_lateload_safe(&mc_header)) {
+			ret = UCODE_ERROR;
 			break;
 		}
 
@@ -701,6 +730,9 @@ static enum ucode_state generic_load_microcode(int cpu, struct iov_iter *iter)
 		return UCODE_ERROR;
 	}
 
+	if (ret == UCODE_ERROR)
+		return ret;
+
 	if (!new_mc)
 		return UCODE_NFOUND;
 
-- 
2.34.1


  parent reply	other threads:[~2023-01-13 17:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13 17:29 [PATCH v1 Part2 0/5] Declare safe late loadable microcode Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 1/5] x86/microcode: Move late load warning to the same function that taints kernel Ashok Raj
2023-01-19 21:48   ` Thomas Gleixner
2023-01-19 22:05     ` Ashok Raj
2023-01-13 17:29 ` Ashok Raj [this message]
2023-01-19 22:03   ` [PATCH v1 Part2 2/5] x86/microcode/intel: Add minimum required revision to microcode header Thomas Gleixner
2023-01-19 22:34     ` Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 3/5] x86/microcode: Add a generic mechanism to declare support for minrev Ashok Raj
2023-01-20  0:15   ` Thomas Gleixner
2023-01-20  2:19     ` Ashok Raj
2023-01-21 21:35     ` [Part 2 v2[cleanup] 0/4] Some additional cleanups in microcode Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 1/4] x86/microcode: Taint kernel only if microcode loading was successful Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 2/4] x86/microcode: Report invalid writes to reload sysfs file Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 3/4] x86/microcode/intel: Fix collect_cpu_info() to reflect current microcode Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 4/4] x86/microcode: Do not call apply_microde() on sibling threads Ashok Raj
2023-01-22  3:36         ` Ashok Raj
2023-01-26 16:52       ` [Part 2 v2[cleanup] 0/4] Some additional cleanups in microcode Ashok Raj
2023-01-27 13:49         ` Borislav Petkov
2023-01-13 17:29 ` [PATCH v1 Part2 4/5] x86/microcode/intel: Drop wbinvd() from microcode loading Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 5/5] x86/microcode: Provide an option to override minrev enforcement Ashok Raj

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230113172920.113612-3-ashok.raj@intel.com \
    --to=ashok.raj@intel.com \
    --cc=Andrew.Cooper3@citrix.com \
    --cc=alison.schofield@intel.com \
    --cc=benh@kernel.crashing.org \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=reinette.chatre@intel.com \
    --cc=stefantalpalaru@yahoo.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).