[PATCH 0/8] x86/microcode: Address GNR errata and follow-up

[PATCH 0/8] x86/microcode: Address GNR errata and follow-up

[Chang S. Bae]

Dear x86 maintainers,

We recently found a couple of microcode loading issues [1]. This series
addresses them from both short-term and long-term perspectives.

It would be much appreciated if the short-term, one-off fix (patch1)
could be sorted out first to allow backporting.

== Issue Summary ==

As background, the minimum revision check was introduced by commit
cf5ab01c8703 ("x86/microcode/intel: Add a minimum required revision for
late loading"). It was originally intended to address changes visible to
software at runtime, and has therefore been limited to the late loading
path.

The two errata are essentially about dependencies between microcode
revisions:

 1 GNR98 involves a dependency on an older revision 0x1000405, which
   contains internal changes required by subsequent revisions. Since this
   is not an OS-visible change, the dependency applies to both early and
   late loading. The minimum revision check only applies to late loading.

 2 GNR101 describes an incorrect minimum revision value in revision
   0x1000423, in spite of the same dependency on 0x1000405. The
   late-loading minimum revision check does not prevent 0x1000423 from
   being loaded directly on an older revision.

== Short-term Remedy ==

There are therefore gaps in both boot-time and runtime loading. The
one-off short-term fix is to reject the problematic loading cases on both
paths.

With this change, systems running an older revision need to update
firmware. See `FIT Microcode Update` in [2]. There may be other ways to
deploy the required update more efficiently, but those are outside the
scope of this kernel patch series.

== Long-term Direction ==

The issues above also raise a couple of questions for the longer term.

 1 How can we address microcode dependencies during early loading?

   The minimum revision information can serve as a more general revision
   dependency check, rather than being limited to runtime changes. The
   series applies the minimum revision checker for early loading
   (patch5-6).

 2 Can we do better for late loading?

   Looking at Intel microcode releases [3], the repository traditionally
   provides the latest blob. When updating from an older revision, admins
   may need to retrieve prerequisite older revisions from the git
   history. This can be inefficient and error-prone.

   Intel will now concatenate prerequisite older blobs with the latest
   blob together into a multi-blob image. The kernel, however, currently
   selects the highest loadable revision, or at best one prerequisite
   when the minimum revision check is enforced. Admins may need to
   trigger multiple loads manually.

   Instead the kernel loader can facilitate iterative loading to apply
   the available blobs incrementally. There are some possible ways to
   implement this.

   One is to rely on the minimum revision check and select the highest
   revision whose prerequisite has been loaded already. Another is to
   load the blobs sequentially according to the order in the image.

   Both approaches have drawbacks: the former depends on the minimum
   revision information being correct, while the latter is sensitive to
   blob ordering.

   The approach here selects the lowest loadable revision on each
   iteration, so that the blobs to be applied incrementally (patch7-8).

== Patch Set ==

The series started as a single patch to block the problematic loading
cases. It was then expanded to address the underlying gaps in early and
late loading, with some fixes along the way.

  Part1, patch1: One-off fix to reject the errata loading cases
  Part2, patch2-4: Preparatory fixes
  Part3, patch5-6: Early loading: apply minimum revision check
  Part4, patch7-8: Late loading: support iterative loading

== Tests ==

Andrew helped providing useful scripts to concatenate blobs from the
repository [3] into a test image. Thanks! I could grab one production
system and test changed paths with the concatenated test image.

The patch set is available in this repository:
 git://github.com/intel-staging/microcode.git gnr-errata-followup-v1

Thanks,
Chang

[1] https://edc.intel.com/content/www/jp/ja/design/products-and-solutions/processors-and-chipsets/birch-stream/xeon-6900-6700-6500-series-processors-with-p-cores-specification-update/016US/errata-summary-table/

   GNR98. MCE When Early OS Microcode Update to MCU 0x1000405 Release or Later
   Problem: The 0x1000405 microcode release introduced changes to the
   inter-microcode communication interface that are incompatible with
   prior versions.

   GNR101. Incorrect Minimum Runtime Microcode Update Revision in 0x1000423 Release
   Problem: The 0x1000423 microcode release does not correctly set the
   Minimum Runtime Microcode Update Revision, document number 783834, ID
   in the microcode header to reference the 0x1000405 release.

[2] Microcode Update Guidance
    https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/microcode-update-guidance.html

[3] https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files.git

Chang S. Bae (8):
  x86/microcode/intel: Reject problematic loading on GNR systems
  x86/microcode: Solidify base_rev= option parsing
  x86/microcode: Accept a boolean for force_minrev parameter
  x86/microcode: Mark early_data __initdata
  x86/microcode: Decouple minimum revision check from late loading
  x86/microcode/intel: Apply minimum revision check to early loading
  x86/microcode: Introduce iterative late loading
  x86/microcode/intel: Select the lowest loadable revision for iterative
    loading

 .../admin-guide/kernel-parameters.txt         |  11 +-
 arch/x86/Kconfig                              |  31 +++--
 arch/x86/kernel/cpu/microcode/amd.c           |  11 +-
 arch/x86/kernel/cpu/microcode/core.c          |  86 ++++++++++++--
 arch/x86/kernel/cpu/microcode/intel.c         | 110 ++++++++++++++----
 arch/x86/kernel/cpu/microcode/internal.h      |   2 +
 6 files changed, 202 insertions(+), 49 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0

[PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

[Chang S. Bae]

Revision 0x1000405 contains internal microcode changes that are required
by subsequent revisions to avoid #MC during loading. This dependency
logically fits the minimum revision requirement.

The minimum revision check, however, currently applies only to the late
loading path, since the dependency was primarily intended for OS-visible
changes. The early loading path is therefore still vulnerable to this
issue.

Furthermore, one of the subsequent revisions does not correctly specify
the minimum revision, so unfortunately the late loading cannot rely on
that check either in this case.

Prevent loading 0x1000405 or later when the system has not yet been
updated to 0x1000405 or later. Apply this blocking to both early- and
late-loading paths.

Rename is_blacklisted() to is_late_loading_denied() so the new function
that covers both loading paths is not confused with the late-load only
one.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: <stable@vger.kernel.org>
---
The GNR errata page:
https://edc.intel.com/content/www/jp/ja/design/products-and-solutions/processors-and-chipsets/birch-stream/xeon-6900-6700-6500-series-processors-with-p-cores-specification-update/016US/errata-summary-table/

Thanks to Sohil, I noticed the naming guideline in
Documentation/process/coding-style.rst:
    For symbol names and documentation, avoid introducing new usage of
    'master / slave' (or 'slave' independent of 'master') and 'blacklist /
    whitelist'.
---
 arch/x86/kernel/cpu/microcode/intel.c | 42 +++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 1142183c950c..c502138fd8e9 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -309,6 +309,38 @@ static void save_microcode_patch(struct microcode_intel *patch)
 		pr_err("Unable to allocate microcode memory size: %u\n", size);
 }
 
+static bool is_loading_denied(struct cpu_signature *sig, u32 rev)
+{
+	u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig));
+
+	/*
+	 * Revision 0x1000405 contains prerequisite changes for subsequent
+	 * microcode updates on Granite Rapids systems. Updates directly from
+	 * an older revision to this or a newer one can result in #MC (GNR98).
+	 *
+	 * This dependency can be indicated from the minimum revision field.
+	 * However, revision 0x1000423 has an incorrect minimum revision in its
+	 * header (GNR101).
+	 *
+	 * Prevent loading 0x1000405 or later unless the CPU has already been
+	 * updated to 0x1000405 or later.
+	 */
+	if (vfm == INTEL_GRANITERAPIDS_X &&
+	    x86_stepping(sig->sig) == 1 &&
+	    sig->pf & 0x95 &&
+	    sig->rev < 0x1000405 &&
+	    rev >= 0x1000405) {
+		if (rev == 0x1000405)
+			pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n");
+		else
+			pr_err_once("Erratum GNR98: 0x1000405 is required before 0x%x.\n", rev);
+		pr_err_once("Please update the system BIOS or firmware.\n");
+		return true;
+	}
+
+	return false;
+}
+
 /* Scan blob for microcode matching the boot CPUs family, model, stepping */
 static __init struct microcode_intel *scan_microcode(void *data, size_t size,
 						     struct ucode_cpu_info *uci,
@@ -330,6 +362,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size,
 		if (!intel_find_matching_signature(data, &uci->cpu_sig))
 			continue;
 
+		if (is_loading_denied(&uci->cpu_sig, mc_header->rev))
+			continue;
+
 		/*
 		 * For saving the early microcode, find the matching revision which
 		 * was loaded on the BSP.
@@ -878,6 +913,9 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
 		if (!intel_find_matching_signature(mc, &uci->cpu_sig))
 			continue;
 
+		if (is_loading_denied(&uci->cpu_sig, mc_header.rev))
+			continue;
+
 		is_safe = ucode_validate_minrev(&mc_header);
 		if (force_minrev && !is_safe)
 			continue;
@@ -905,7 +943,7 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
 	return UCODE_ERROR;
 }
 
-static bool is_blacklisted(unsigned int cpu)
+static bool is_late_loading_denied(unsigned int cpu)
 {
 	struct cpuinfo_x86 *c = &cpu_data(cpu);
 
@@ -936,7 +974,7 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
 	struct kvec kvec;
 	char name[30];
 
-	if (is_blacklisted(cpu))
+	if (is_late_loading_denied(cpu))
 		return UCODE_NFOUND;
 
 	sprintf(name, "intel-ucode/%02x-%02x-%02x",
-- 
2.53.0

[PATCH 2/8] x86/microcode: Solidify base_rev= option parsing

[Chang S. Bae]

After base_rev= has been parsed, the remainder of the token is still
compared against the other option names. Along with this, the substring
check strstr() may also produce a spurious match.

Move on to the next token once the option has been parsed, and use
str_has_prefix() instead.

Fixes: 43181a47263d ("x86/microcode: Add microcode loader debugging functionality")
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: <stable@vger.kernel.org>
---
 arch/x86/kernel/cpu/microcode/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 0dd0c7241c57..303b0d0b4573 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -146,10 +146,11 @@ static void __init early_parse_cmdline(void)
 	if (cmdline_find_option(boot_command_line, "microcode", cmd_buf, sizeof(cmd_buf)) > 0) {
 		while ((s = strsep(&p, ","))) {
 			if (IS_ENABLED(CONFIG_MICROCODE_DBG)) {
-				if (strstr(s, "base_rev=")) {
+				if (str_has_prefix(s, "base_rev=")) {
 					/* advance to the option arg */
 					strsep(&s, "=");
 					if (kstrtouint(s, 16, &base_rev)) { ; }
+					continue;
 				}
 			}
 
-- 
2.53.0

[PATCH 3/8] x86/microcode: Accept a boolean for force_minrev parameter

[Chang S. Bae]

The microcode= command-line option allows controlling all microcode
loading options. However, force_minrev can only enable minimum revision
enforcement, not disable it.

The previous microcode.force_minrev= option accepted both boolean values,
and the current behavior differs from the documentation in
kernel-parameters.txt.

Add `force_minrev=` as a sub-option to explicitly enable or disable the
minimum revision enforcement. Update the documentation accordingly.

While doing so, wrap strsep() in a descriptively named helper.

Fixes: 632ff6170647 ("x86/microcode: Add microcode= cmdline parsing")
Reported-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Cc: <stable@vger.kernel.org>
---
 Documentation/admin-guide/kernel-parameters.txt |  7 ++++---
 arch/x86/kernel/cpu/microcode/core.c            | 15 ++++++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 68647ff4bdd2..cde092017cd8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4071,9 +4071,10 @@ Kernel parameters
 
 			dis_ucode_ldr: disable the microcode loader
 
-			force_minrev:
-			Enable or disable the microcode minimal revision
-			enforcement for the runtime microcode loader.
+			force_minrev[=<bool>]:
+			Enable or disable microcode minimal revision enforcement
+			for the runtime microcode loader according to <bool>. If
+			<bool> is not given, enable the enforcement.
 
 	mini2440=	[ARM,HW,KNL]
 			Format:[0..2][b][c][t]
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 303b0d0b4573..af7196d94540 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -138,6 +138,11 @@ bool __init microcode_loader_disabled(void)
 	return dis_ucode_ldr;
 }
 
+static inline void advance_option_argument(char **s)
+{
+	strsep(s, "=");
+}
+
 static void __init early_parse_cmdline(void)
 {
 	char cmd_buf[64] = {};
@@ -147,15 +152,19 @@ static void __init early_parse_cmdline(void)
 		while ((s = strsep(&p, ","))) {
 			if (IS_ENABLED(CONFIG_MICROCODE_DBG)) {
 				if (str_has_prefix(s, "base_rev=")) {
-					/* advance to the option arg */
-					strsep(&s, "=");
+					advance_option_argument(&s);
 					if (kstrtouint(s, 16, &base_rev)) { ; }
 					continue;
 				}
 			}
 
-			if (!strcmp("force_minrev", s))
+			if (!strcmp("force_minrev", s)) {
 				force_minrev = true;
+			} else if (str_has_prefix(s, "force_minrev=")) {
+				advance_option_argument(&s);
+				if (kstrtobool(s, &force_minrev)) { ; }
+				continue;
+			}
 
 			if (!strcmp(s, "dis_ucode_ldr"))
 				dis_ucode_ldr = true;
-- 
2.53.0

[PATCH 4/8] x86/microcode: Mark early_data __initdata

[Chang S. Bae]

early_data records the revisions seen during early loading, and
microcode_init() is its last reader.

Mark the storage reclaimable afterwards.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/kernel/cpu/microcode/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index af7196d94540..ed1eb6b753d6 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -83,7 +83,7 @@ static u32 final_levels[] = {
 	0, /* T-101 terminator */
 };
 
-struct early_load_data early_data;
+struct early_load_data early_data __initdata;
 
 /*
  * Check the current patch level on this CPU.
-- 
2.53.0

[PATCH RFC 5/8] x86/microcode: Decouple minimum revision check from late loading

[Chang S. Bae]

The microcode minimum revision check was originally introduced to express
dependencies involving OS-visible changes, thus it has been limited to
the late-load path.

In fact this can serve as a general guard against unsafe microcode
loading. Make the check independent of the loading path in preparation
for applying it to the early path as well.

For now, reject early loading if enforced. Taint the kernel on early
loading because the early path does not perform the check yet.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
Note for the review:
The AMD loader does not have the minrev checker anywhere (yet). So,
force_minrev is effectively dis_ucode_ldr which may appear excessive.
Without enforcement though, it is going to taint the kernel on every
early loading then. This side-effect should be reviewed.
---
 .../admin-guide/kernel-parameters.txt         |  4 ++--
 arch/x86/Kconfig                              | 19 +++++++++----------
 arch/x86/kernel/cpu/microcode/amd.c           |  8 +++++++-
 arch/x86/kernel/cpu/microcode/core.c          | 13 ++++++++++---
 arch/x86/kernel/cpu/microcode/intel.c         | 17 ++++++++++++-----
 arch/x86/kernel/cpu/microcode/internal.h      |  1 +
 6 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cde092017cd8..b5f7a8142f58 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4073,8 +4073,8 @@ Kernel parameters
 
 			force_minrev[=<bool>]:
 			Enable or disable microcode minimal revision enforcement
-			for the runtime microcode loader according to <bool>. If
-			<bool> is not given, enable the enforcement.
+			for the microcode loader according to <bool>. If <bool>
+			is not given, enable the enforcement.
 
 	mini2440=	[ARM,HW,KNL]
 			Format:[0..2][b][c][t]
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 15fd9ec5ecac..6c503004775e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1338,18 +1338,17 @@ config MICROCODE_LATE_LOADING
 	  minimal revision check. This minimal revision check can be enforced on
 	  the kernel command line with "microcode=force_minrev".
 
-config MICROCODE_LATE_FORCE_MINREV
-	bool "Enforce late microcode loading minimal revision check"
+config MICROCODE_FORCE_MINREV
+	bool "Enforce microcode loading minimal revision check"
 	default n
-	depends on MICROCODE_LATE_LOADING
+	depends on MICROCODE
 	help
-	  To prevent that users load microcode late which modifies already
-	  in use features, newer microcode patches have a minimum revision field
-	  in the microcode header, which tells the kernel which minimum
-	  revision must be active in the CPU to safely load that new microcode
-	  late into the running system. If disabled the check will not
-	  be enforced but the kernel will be tainted when the minimal
-	  revision check fails.
+	  To prevent that users load microcode which modifies already in use
+	  features, newer microcode patches have a minimum revision field in the
+	  microcode header, which tells the kernel which minimum revision must
+	  be active in the CPU to safely load that new microcode into the system.
+	  If disabled the check will not be enforced but the kernel will be
+	  tainted when the minimal revision check fails.
 
 	  This minimal revision check can also be controlled via the
 	  "microcode=force_minrev" parameter on the kernel command line.
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 6cdc410e7547..0625d5e8eb7e 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -800,6 +800,11 @@ void __init load_ucode_amd_bsp(struct early_load_data *ed, unsigned int cpuid_1_
 	/* Needed in load_microcode_amd() */
 	ucode_cpu_info[0].cpu_sig.sig = cpuid_1_eax;
 
+	if (force_minrev) {
+		pr_warn_once("No early load: minimum revision check is not implemented.\n");
+		return;
+	}
+
 	if (!find_blobs_in_containers(&cp))
 		return;
 
@@ -1202,7 +1207,8 @@ static int __init save_microcode_in_initrd(void)
 	enum ucode_state ret;
 	struct cpio_data cp;
 
-	if (microcode_loader_disabled() || c->x86_vendor != X86_VENDOR_AMD || c->x86 < 0x10)
+	if (microcode_loader_disabled() || c->x86_vendor != X86_VENDOR_AMD || c->x86 < 0x10 ||
+	    force_minrev)
 		return 0;
 
 	cpuid_1_eax = native_cpuid_eax(1);
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index ed1eb6b753d6..7d6caf191795 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -46,7 +46,7 @@
 static struct microcode_ops *microcode_ops;
 static bool dis_ucode_ldr;
 
-bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_LATE_FORCE_MINREV);
+bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_FORCE_MINREV);
 
 /*
  * Those below should be behind CONFIG_MICROCODE_DBG ifdeffery but in
@@ -603,7 +603,7 @@ static int load_late_stop_cpus(bool is_safe)
 
 	if (!is_safe) {
 		pr_err("Late microcode loading without minimal revision check.\n");
-		pr_err("You should switch to early loading, if possible.\n");
+		pr_err("You should update microcode incrementally.\n");
 	}
 
 	/*
@@ -912,9 +912,16 @@ static int __init microcode_init(void)
 
 	pr_info_once("Current revision: 0x%08x\n", (early_data.new_rev ?: early_data.old_rev));
 
-	if (early_data.new_rev)
+	if (early_data.new_rev) {
 		pr_info_once("Updated early from: 0x%08x\n", early_data.old_rev);
 
+		if (!early_data.is_safe) {
+			pr_err("Early microcode loading without minimal revision check.\n");
+			pr_err("You should update microcode incrementally.\n");
+			add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
+		}
+	}
+
 	microcode_fdev = faux_device_create("microcode", NULL, NULL);
 	if (!microcode_fdev)
 		return -ENODEV;
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index c502138fd8e9..3247c619eaa1 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -786,7 +786,15 @@ void __init load_ucode_intel_bsp(struct early_load_data *ed)
 	uci.mc = get_microcode_blob(&uci, false);
 	ed->old_rev = uci.cpu_sig.rev;
 
-	if (uci.mc && apply_microcode_early(&uci) == UCODE_UPDATED) {
+	if (!uci.mc)
+		return;
+
+	if (force_minrev) {
+		pr_warn_once("No early load: minimum revision check is not implemented.\n");
+		return;
+	}
+
+	if (apply_microcode_early(&uci) == UCODE_UPDATED) {
 		ucode_patch_va = UCODE_BSP_LOADED;
 		ed->new_rev = uci.cpu_sig.rev;
 	}
@@ -842,9 +850,8 @@ static bool ucode_validate_minrev(struct microcode_header_intel *mc_header)
 	int cur_rev = boot_cpu_data.microcode;
 
 	/*
-	 * When late-loading, ensure the header declares a minimum revision
-	 * required to perform a late-load. The previously reserved field
-	 * is 0 in older microcode blobs.
+	 * Ensure the header declares a minimum revision required to perform a
+	 * load. The previously reserved field is 0 in older microcode blobs.
 	 */
 	if (!mc_header->min_req_ver) {
 		pr_info("Unsafe microcode update: Microcode header does not specify a required min version\n");
@@ -857,7 +864,7 @@ static bool ucode_validate_minrev(struct microcode_header_intel *mc_header)
 	 */
 	if (cur_rev < mc_header->min_req_ver) {
 		pr_info("Unsafe microcode update: Current revision 0x%x too old\n", cur_rev);
-		pr_info("Current should be at 0x%x or higher. Use early loading instead\n", mc_header->min_req_ver);
+		pr_info("Current should be at 0x%x or higher. Update incrementally.\n", mc_header->min_req_ver);
 		return false;
 	}
 	return true;
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index a10b547eda1e..1b35f9099580 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -42,6 +42,7 @@ struct microcode_ops {
 struct early_load_data {
 	u32 old_rev;
 	u32 new_rev;
+	bool is_safe;
 };
 
 extern struct early_load_data early_data;
-- 
2.53.0

[PATCH RFC 6/8] x86/microcode/intel: Apply minimum revision check to early loading

[Chang S. Bae]

The minimum revision check is now generally applicable. Establish the
minimum revision check on early loading. Follow the late loading behavior
when the check fails:

 * Load but taint the kernel if not enforced, or
 * Reject the loading if enforced.

Adjust the check function so that the early path can pass the current
revision.

With this establishment, do not reject early loading anymore when
enforced.

Suggested-by: Arjan van de Ven <arjan.van.de.ven@intel.com>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/kernel/cpu/microcode/intel.c | 64 ++++++++++++---------------
 1 file changed, 29 insertions(+), 35 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 3247c619eaa1..ddd5b72b27a1 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -341,6 +341,29 @@ static bool is_loading_denied(struct cpu_signature *sig, u32 rev)
 	return false;
 }
 
+static bool ucode_validate_minrev(u32 cur_rev, struct microcode_header_intel *mc_header)
+{
+	/*
+	 * Ensure the header declares a minimum revision required to perform a
+	 * load. The previously reserved field is 0 in older microcode blobs.
+	 */
+	if (!mc_header->min_req_ver) {
+		pr_info("Unsafe microcode update: Microcode header does not specify a required min version\n");
+		return false;
+	}
+
+	/*
+	 * Check whether the current revision is either greater or equal to
+	 * the minimum revision specified in the header.
+	 */
+	if (cur_rev < mc_header->min_req_ver) {
+		pr_info("Unsafe microcode update: Current revision 0x%x too old.\n", cur_rev);
+		pr_info("Current should be at 0x%x or higher. Update incrementally.\n", mc_header->min_req_ver);
+		return false;
+	}
+	return true;
+}
+
 /* Scan blob for microcode matching the boot CPUs family, model, stepping */
 static __init struct microcode_intel *scan_microcode(void *data, size_t size,
 						     struct ucode_cpu_info *uci,
@@ -365,6 +388,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size,
 		if (is_loading_denied(&uci->cpu_sig, mc_header->rev))
 			continue;
 
+		if (force_minrev && !ucode_validate_minrev(uci->cpu_sig.rev, mc_header))
+			continue;
+
 		/*
 		 * For saving the early microcode, find the matching revision which
 		 * was loaded on the BSP.
@@ -786,17 +812,10 @@ void __init load_ucode_intel_bsp(struct early_load_data *ed)
 	uci.mc = get_microcode_blob(&uci, false);
 	ed->old_rev = uci.cpu_sig.rev;
 
-	if (!uci.mc)
-		return;
-
-	if (force_minrev) {
-		pr_warn_once("No early load: minimum revision check is not implemented.\n");
-		return;
-	}
-
-	if (apply_microcode_early(&uci) == UCODE_UPDATED) {
+	if (uci.mc && apply_microcode_early(&uci) == UCODE_UPDATED) {
 		ucode_patch_va = UCODE_BSP_LOADED;
 		ed->new_rev = uci.cpu_sig.rev;
+		ed->is_safe = ucode_validate_minrev(ed->old_rev, uci.mc);
 	}
 }
 
@@ -845,31 +864,6 @@ static enum ucode_state apply_microcode_late(int cpu)
 	return ret;
 }
 
-static bool ucode_validate_minrev(struct microcode_header_intel *mc_header)
-{
-	int cur_rev = boot_cpu_data.microcode;
-
-	/*
-	 * Ensure the header declares a minimum revision required to perform a
-	 * load. The previously reserved field is 0 in older microcode blobs.
-	 */
-	if (!mc_header->min_req_ver) {
-		pr_info("Unsafe microcode update: Microcode header does not specify a required min version\n");
-		return false;
-	}
-
-	/*
-	 * Check whether the current revision is either greater or equal to
-	 * to the minimum revision specified in the header.
-	 */
-	if (cur_rev < mc_header->min_req_ver) {
-		pr_info("Unsafe microcode update: Current revision 0x%x too old\n", cur_rev);
-		pr_info("Current should be at 0x%x or higher. Update incrementally.\n", mc_header->min_req_ver);
-		return false;
-	}
-	return true;
-}
-
 static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
@@ -923,7 +917,7 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
 		if (is_loading_denied(&uci->cpu_sig, mc_header.rev))
 			continue;
 
-		is_safe = ucode_validate_minrev(&mc_header);
+		is_safe = ucode_validate_minrev(uci->cpu_sig.rev, &mc_header);
 		if (force_minrev && !is_safe)
 			continue;
 
-- 
2.53.0

[PATCH RFC 7/8] x86/microcode: Introduce iterative late loading

[Chang S. Bae]

The minimum revision requirement was introduced to handle dependencies
between microcode revisions. On the other hand, Intel has traditionally
distributed only the latest microcode blob in the repository [1].

Instead, to better accommodate those dependent blobs at hand, it is
useful to concatenate blobs together into a single image. This allows
loading the prerequisite revisions in sequence.

The loader currently handles a multi-blob image by selecting the highest
revision without minimum revision enforcement. This loading approach may
not achieve the safety requirement.

If enforced, it skips revisions whose prerequisites have not been loaded,
possibly leaving the loading process at an intermediate revision. Then,
admins have to manually trigger another load to apply the remaining
higher revisions.

To avoid this manual step, introduce iterative late loading. After each
successful update, let the vendor-specific parser select the next blob.
If the parser selects the lowest loadable revision on each pass, the
loader can apply the blobs incrementally until no more blobs are
available.

For now, vendor code rejects this loading because the blob parser is not
ready yet to support incremental blob selection.

To retain the existing behavior by default, add a Kconfig option and a
kernel command-line option to select iterative loading.

[1] https://github.com/intel/Intel-Linux-Processor-Microcode-Data-Files.git

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 .../admin-guide/kernel-parameters.txt         |  4 ++
 arch/x86/Kconfig                              | 12 +++++
 arch/x86/kernel/cpu/microcode/amd.c           |  3 +-
 arch/x86/kernel/cpu/microcode/core.c          | 53 +++++++++++++++++--
 arch/x86/kernel/cpu/microcode/intel.c         |  3 ++
 arch/x86/kernel/cpu/microcode/internal.h      |  1 +
 6 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b5f7a8142f58..67f5b233315f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4076,6 +4076,10 @@ Kernel parameters
 			for the microcode loader according to <bool>. If <bool>
 			is not given, enable the enforcement.
 
+			iterative_loading=<bool>:
+			Enable or disable iterative microcode patch application
+			for the runtime microcode loader according to <bool>.
+
 	mini2440=	[ARM,HW,KNL]
 			Format:[0..2][b][c][t]
 			Default: "0tb"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 6c503004775e..021c77a6ad4b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1355,6 +1355,18 @@ config MICROCODE_FORCE_MINREV
 
 	  If unsure say Y.
 
+config MICROCODE_LATE_ITERATIVE_LOADING
+	bool "Iterative microcode late loading"
+	default n
+	depends on MICROCODE_LATE_LOADING
+	help
+	  Apply the microcode blobs in a concatenated image one at a time, in
+	  ascending revision order, so that revisions with prerequisites can be
+	  applied in a single process.
+
+	  This loading option can also be controlled via the
+	  "microcode=iterative_loading=" parameter on the kernel command line.
+
 config MICROCODE_DBG
 	bool "Enable microcode loader debugging"
 	default n
diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 0625d5e8eb7e..97b30da9e5e6 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -1251,7 +1251,8 @@ static enum ucode_state request_microcode_amd(int cpu, struct device *device)
 	enum ucode_state ret = UCODE_NFOUND;
 	const struct firmware *fw;
 
-	if (force_minrev)
+	/* The blob parser does not support these features yet. */
+	if (force_minrev || iterative_loading)
 		return UCODE_NFOUND;
 
 	if (c->x86 >= 0x15)
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 7d6caf191795..e7e159479975 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -47,6 +47,7 @@ static struct microcode_ops *microcode_ops;
 static bool dis_ucode_ldr;
 
 bool force_minrev = IS_ENABLED(CONFIG_MICROCODE_FORCE_MINREV);
+bool iterative_loading = IS_ENABLED(CONFIG_MICROCODE_LATE_ITERATIVE_LOADING);
 
 /*
  * Those below should be behind CONFIG_MICROCODE_DBG ifdeffery but in
@@ -166,6 +167,12 @@ static void __init early_parse_cmdline(void)
 				continue;
 			}
 
+			if (str_has_prefix(s, "iterative_loading=")) {
+				advance_option_argument(&s);
+				if (kstrtobool(s, &iterative_loading)) { ; }
+				continue;
+			}
+
 			if (!strcmp(s, "dis_ucode_ldr"))
 				dis_ucode_ldr = true;
 		}
@@ -750,16 +757,34 @@ static bool setup_cpus(void)
 	return true;
 }
 
+static void reset_ucode_ctrl(void)
+{
+	struct microcode_ctrl ctrl = { .ctrl = SCTRL_WAIT, .result = -1, };
+	unsigned int cpu;
+
+	for_each_cpu_and(cpu, cpu_present_mask, &cpus_booted_once_mask) {
+		ctrl.ctrl_cpu = per_cpu(ucode_ctrl.ctrl_cpu, cpu);
+		per_cpu(ucode_ctrl, cpu) = ctrl;
+	}
+}
+
 static int load_late_locked(void)
 {
+	enum ucode_state state;
+	int err;
+
 	if (!setup_cpus())
 		return -EBUSY;
 
-	switch (microcode_ops->request_microcode_fw(0, &microcode_fdev->dev)) {
+	state = microcode_ops->request_microcode_fw(0, &microcode_fdev->dev);
+next:
+	switch (state) {
 	case UCODE_NEW:
-		return load_late_stop_cpus(false);
+		err = load_late_stop_cpus(false);
+		break;
 	case UCODE_NEW_SAFE:
-		return load_late_stop_cpus(true);
+		err = load_late_stop_cpus(true);
+		break;
 	case UCODE_NFOUND:
 		return -ENOENT;
 	case UCODE_OK:
@@ -767,6 +792,28 @@ static int load_late_locked(void)
 	default:
 		return -EBADFD;
 	}
+
+	if (err)
+		return err;
+
+	/*
+	 * A multi-blob image is traditionally handled by selecting the highest
+	 * revision to load it in one shot. With iterative loading, the
+	 * vendor-specific parser instead selects the lowest loadable revision.
+	 *
+	 * After each successful update, find the next loadable blob to continue
+	 * the iteration. Stop if no more blobs are found.
+	 */
+	if (iterative_loading) {
+		state = microcode_ops->request_microcode_fw(0, &microcode_fdev->dev);
+		if (state == UCODE_NFOUND)
+			return 0;
+
+		reset_ucode_ctrl();
+		goto next;
+	}
+
+	return 0;
 }
 
 static ssize_t reload_store(struct device *dev,
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index ddd5b72b27a1..48e7d023ea41 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -978,6 +978,9 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
 	if (is_late_loading_denied(cpu))
 		return UCODE_NFOUND;
 
+	if (iterative_loading)
+		return UCODE_NFOUND;
+
 	sprintf(name, "intel-ucode/%02x-%02x-%02x",
 		c->x86, c->x86_model, c->x86_stepping);
 
diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index 1b35f9099580..311e885819a2 100644
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -101,6 +101,7 @@ static inline unsigned int x86_cpuid_family(void)
 }
 
 extern bool force_minrev;
+extern bool iterative_loading;
 
 #ifdef CONFIG_CPU_SUP_AMD
 void load_ucode_amd_bsp(struct early_load_data *ed, unsigned int family);
-- 
2.53.0

[PATCH RFC 8/8] x86/microcode/intel: Select the lowest loadable revision for iterative loading

[Chang S. Bae]

The iterative loading logic in the core can repeatedly request a
microcode patch. The vendor parser currently selects the highest loadable
revision, which means no further patch can be selected after the first
update.

To load a concatenated multi-blob image incrementally, adjust the blob
parser to select the lowest loadable revision. This allows patches to be
applied in ascending revision order.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
---
 arch/x86/kernel/cpu/microcode/intel.c | 34 ++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 48e7d023ea41..3a699e361fe0 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -864,14 +864,39 @@ static enum ucode_state apply_microcode_late(int cpu)
 	return ret;
 }
 
+static bool is_revision_candidate(unsigned int cur_rev, unsigned int rev)
+{
+	/*
+	 * A revision must be newer than the active microcode revision to be
+	 * loadable.
+	 */
+	if (rev <= boot_cpu_data.microcode)
+		return false;
+
+	/*
+	 * Iterative loading selects the lowest loadable revision so that
+	 * revisions can be applied incrementally.
+	 */
+	if (iterative_loading)
+		return rev < cur_rev;
+
+	/* Legacy late loading selects the highest loadable revision. */
+	return rev > cur_rev;
+}
+
 static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
 {
 	struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
+	unsigned int cur_rev, curr_mc_size = 0;
 	bool is_safe, new_is_safe = false;
-	int cur_rev = uci->cpu_sig.rev;
-	unsigned int curr_mc_size = 0;
 	u8 *new_mc = NULL, *mc = NULL;
 
+	/*
+	 * Start from the boundary value for the revision search. With iterative
+	 * loading search walks downward but the legacy search walks upward.
+	 */
+	cur_rev = iterative_loading ? UINT_MAX : 0;
+
 	while (iov_iter_count(iter)) {
 		struct microcode_header_intel mc_header;
 		unsigned int mc_size, data_size;
@@ -908,7 +933,7 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
 		    intel_microcode_sanity_check(mc, true, MC_HEADER_TYPE_MICROCODE) < 0)
 			goto fail;
 
-		if (cur_rev >= mc_header.rev)
+		if (!is_revision_candidate(cur_rev, mc_header.rev))
 			continue;
 
 		if (!intel_find_matching_signature(mc, &uci->cpu_sig))
@@ -978,9 +1003,6 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
 	if (is_late_loading_denied(cpu))
 		return UCODE_NFOUND;
 
-	if (iterative_loading)
-		return UCODE_NFOUND;
-
 	sprintf(name, "intel-ucode/%02x-%02x-%02x",
 		c->x86, c->x86_model, c->x86_stepping);
 
-- 
2.53.0

Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

[Sohil Mehta]

On 9/1/2026 4:16 PM, Chang S. Bae wrote:
> Revision 0x1000405 contains internal microcode changes that are required
> by subsequent revisions to avoid #MC during loading. 

Before going to the solution space, can we add some more
context/background here?

For example:

GNR revision 0x1000405 introduces a breaking change that causes a #MC
when the OS updates the microcode from any version older than 0x1000405
to a newer version. This is applicable to late load as well as early
loading during boot.

A BIOS or firmware update is required to update the revision to
0x1000405 or newer before any OS updates can be safely loaded.
This dependency ...

> This dependency logically fits the minimum revision requirement.
> 
> The minimum revision check, however, currently applies only to the late
> loading path, since the dependency was primarily intended for OS-visible
> changes. The early loading path is therefore still vulnerable to this
> issue.
> 
> Furthermore, one of the subsequent revisions does not correctly specify
> the minimum revision, so unfortunately the late loading cannot rely on
> that check either in this case.
> 
> Prevent loading 0x1000405 or later when the system has not yet been
> updated to 0x1000405 or later. 

This is a bit confusing. Should it say jumping from any anything older
than 0x1000405 to anything newer?

> Apply this blocking to both early- and late-loading paths.
> 

> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 1142183c950c..c502138fd8e9 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -309,6 +309,38 @@ static void save_microcode_patch(struct microcode_intel *patch)
>  		pr_err("Unable to allocate microcode memory size: %u\n", size);
>  }
>  
> +static bool is_loading_denied(struct cpu_signature *sig, u32 rev)
> +{
> +	u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig));
> +
> +	/*
> +	 * Revision 0x1000405 contains prerequisite changes for subsequent
> +	 * microcode updates on Granite Rapids systems. Updates directly from
> +	 * an older revision to this or a newer one can result in #MC (GNR98).
> +	 *
> +	 * This dependency can be indicated from the minimum revision field.
> +	 * However, revision 0x1000423 has an incorrect minimum revision in its
> +	 * header (GNR101).
> +	 *
> +	 * Prevent loading 0x1000405 or later unless the CPU has already been
> +	 * updated to 0x1000405 or later.
> +	 */
> +	if (vfm == INTEL_GRANITERAPIDS_X &&
> +	    x86_stepping(sig->sig) == 1 &&
> +	    sig->pf & 0x95 &&
> +	    sig->rev < 0x1000405 &&
> +	    rev >= 0x1000405) {

I don't think we have a helper that can be used here directly. Also,
this is tagged for stable so adding a new one probably doesn't make sense.

But, 0x1000405 is repeated way too many times in this function :)
At a minimum, can we add something like this?

#define GNR98_UCODE_MIN_REV 	0x1000405

Maybe add defines for the PF as well?


> +		if (rev == 0x1000405)
> +			pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n");

						   ^ revision

With early load, his is probably one of the first messages that folks
will see but it will only be displayed on platforms that have the issue
until they update their microcode. Should we be more verbose here?


> +		else
> +			pr_err_once("Erratum GNR98: 0x1000405 is required before 0x%x.\n", rev);

"Erratum GNR98: revision 0x1000405 is required before revision 0x%x can
be loaded. \n"

Should this be 0x1000405 *or later* ? We don't want common users to
specifically try to find the revision 0x1000405 and load it, right?

I am wondering what is the use of the if-else? Is that intended to guide
the late loading users? I think for other users the second message would
be confusing.


> +		pr_err_once("Please update the system BIOS or firmware.\n");
> +		return true;
> +	}
> +
> +	return false;
> +}
> +

RE: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

[Van De Ven, Arjan]

> On 9/1/2026 4:16 PM, Chang S. Bae wrote:
> > Revision 0x1000405 contains internal microcode changes that are required
> > by subsequent revisions to avoid #MC during loading.
> 
> Before going to the solution space, can we add some more
> context/background here?

I think, taking as step back, the world is sort of simple and more examples maybe helping or maybe not.

At the request of "linux folks" (many on the CC), Intel added a "minrev" field for the microcode files, to express a "don't load this unless you are at least at version XYZ", for various compatibility reasons.

At the time, we added this as a check to "runtime" loading, but not during early loading. In hindsight this was a mistake, compatibility issues are not limited to late runtime loading, but can also happen during earlier-in-the-OS loading.

The example mentioned is a case where this became no longer theoretical.... 

Now, these kind of compat breaks are somewhat rare (we can debate what rare means I suppose), but clearly they happen (the bug mentioned is clear existence proof).

The more detailed nicer solution is in later patches -- this patch just makes the kernel honor the minrev field more consistently, which gets the kernel into a "safe" space again. The reason this patch is first is one of -stable backports. One can imagine that -stable folks like to take the most simple solution back first before even considering a more complex solution.... 

Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

[Dave Hansen]

On 9/1/26 16:16, Chang S. Bae wrote:
> Revision 0x1000405 contains internal microcode changes that are required
> by subsequent revisions to avoid #MC during loading. This dependency
> logically fits the minimum revision requirement.
> 
> The minimum revision check, however, currently applies only to the late
> loading path, since the dependency was primarily intended for OS-visible
> changes. The early loading path is therefore still vulnerable to this
> issue.

Honestly, I think minrevs are a distraction here. Let's keep this short
and sweet:

Microcode updates can usually jump revisions. However, there is an
erratum on Granite Rapids systems. If they "jump over" revision
0x1000405, they machine check <or whatever, I forget how they die>.

> Furthermore, one of the subsequent revisions does not correctly specify
> the minimum revision, so unfortunately the late loading cannot rely on
> that check either in this case.
> 
> Prevent loading 0x1000405 or later when the system has not yet been
> updated to 0x1000405 or later. Apply this blocking to both early- and
> late-loading paths.
> 
> Rename is_blacklisted() to is_late_loading_denied() so the new function
> that covers both loading paths is not confused with the late-load only
> one.

Whee, and get rid of a chunk of old terminology while we're at it.

Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

[Borislav Petkov]

On Tue, Sep 01, 2026 at 11:16:26PM +0000, Chang S. Bae wrote:
> +static bool is_loading_denied(struct cpu_signature *sig, u32 rev)

This naming is not better, sorry.

Is loading denied means the loading in general is denied because <raisin> or
are you trying to check whether this particular revision should not be loaded?

I think it is latter.

So you wanna say

	revision_blacklisted()

or so.

> +{
> +	u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig));
> +
> +	/*
> +	 * Revision 0x1000405 contains prerequisite changes for subsequent
> +	 * microcode updates on Granite Rapids systems. Updates directly from
> +	 * an older revision to this or a newer one can result in #MC (GNR98).
> +	 *
> +	 * This dependency can be indicated from the minimum revision field.
> +	 * However, revision 0x1000423 has an incorrect minimum revision in its

So you lost me here: 0x1000423 is not tested anywhere - just mentioned here.

So what I understand is: usually, dependencies like that can be expressed with
minrev but *in addition* to the current issue, patch 0x1000423 has minrev
wrong so that dependency cannot be upheld there either.

But then why even mention it if you're not going to test it?

Why do we care about GNR101 at all?

> +	 * header (GNR101).
> +	 *
> +	 * Prevent loading 0x1000405 or later unless the CPU has already been
> +	 * updated to 0x1000405 or later.
> +	 */
> +	if (vfm == INTEL_GRANITERAPIDS_X &&
> +	    x86_stepping(sig->sig) == 1 &&
> +	    sig->pf & 0x95 &&
> +	    sig->rev < 0x1000405 &&
> +	    rev >= 0x1000405) {

You don't really need to test rev here - it is enough that sig->rev is
< 0x1000405 - that already makes you susceptible and then you can check rev
inside the { }.

> +		if (rev == 0x1000405)
> +			pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n");
> +		else
> +			pr_err_once("Erratum GNR98: 0x1000405 is required before 0x%x.\n", rev);
> +		pr_err_once("Please update the system BIOS or firmware.\n");

This is useless most of the time because client won't usually get BIOS
updates. You can tell people they should update their microcode packages
instead. That's where we can really help.

> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  /* Scan blob for microcode matching the boot CPUs family, model, stepping */
>  static __init struct microcode_intel *scan_microcode(void *data, size_t size,
>  						     struct ucode_cpu_info *uci,
> @@ -330,6 +362,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size,
>  		if (!intel_find_matching_signature(data, &uci->cpu_sig))
>  			continue;
>  
> +		if (is_loading_denied(&uci->cpu_sig, mc_header->rev))
> +			continue;
> +
>  		/*
>  		 * For saving the early microcode, find the matching revision which
>  		 * was loaded on the BSP.
> @@ -878,6 +913,9 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
>  		if (!intel_find_matching_signature(mc, &uci->cpu_sig))
>  			continue;
>  
> +		if (is_loading_denied(&uci->cpu_sig, mc_header.rev))
> +			continue;
> +
>  		is_safe = ucode_validate_minrev(&mc_header);
>  		if (force_minrev && !is_safe)
>  			continue;
> @@ -905,7 +943,7 @@ static enum ucode_state parse_microcode_blobs(int cpu, struct iov_iter *iter)
>  	return UCODE_ERROR;
>  }
>  
> -static bool is_blacklisted(unsigned int cpu)
> +static bool is_late_loading_denied(unsigned int cpu)
>  {
>  	struct cpuinfo_x86 *c = &cpu_data(cpu);
>  
> @@ -936,7 +974,7 @@ static enum ucode_state request_microcode_fw(int cpu, struct device *device)
>  	struct kvec kvec;
>  	char name[30];
>  
> -	if (is_blacklisted(cpu))
> +	if (is_late_loading_denied(cpu))

Aaaah, you wanna be politically correct and can't use "blacklisted" anymore.

Well, you're not introducing new usage so you don't have to touch old usage.
And "is denied" does not express the situation properly. Try a better one.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems

[Chang S. Bae]

On 9/2/2026 6:51 PM, Borislav Petkov wrote:
> On Tue, Sep 01, 2026 at 11:16:26PM +0000, Chang S. Bae wrote:
>> +static bool is_loading_denied(struct cpu_signature *sig, u32 rev)
> 
> This naming is not better, sorry.
> 
> Is loading denied means the loading in general is denied because <raisin> or
> are you trying to check whether this particular revision should not be loaded?
> 
> I think it is latter.
> 
> So you wanna say
> 
> 	revision_blacklisted()
> 
> or so.

Maybe revision_banned()?

> 
>> +{
>> +	u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig));
>> +
>> +	/*
>> +	 * Revision 0x1000405 contains prerequisite changes for subsequent
>> +	 * microcode updates on Granite Rapids systems. Updates directly from
>> +	 * an older revision to this or a newer one can result in #MC (GNR98).
>> +	 *
>> +	 * This dependency can be indicated from the minimum revision field.
>> +	 * However, revision 0x1000423 has an incorrect minimum revision in its
> 
> So you lost me here: 0x1000423 is not tested anywhere - just mentioned here.
> 
> So what I understand is: usually, dependencies like that can be expressed with
> minrev but *in addition* to the current issue, patch 0x1000423 has minrev
> wrong so that dependency cannot be upheld there either.
> 
> But then why even mention it if you're not going to test it?
> 
> Why do we care about GNR101 at all?
> 

My original intention was to explain why rejection is need for the late 
loading path. There is a minrev check, but broken with that case.

But to preserve the legacy behavior, the minrev check isn't enforced by 
default. So what's the point of mentioning? Yes, if we need to ban 
particular revisions, we need to do it everywhere.

So mentioning minrev is just distracting readers as long as rejection is 
based on revision number in the first place. Let me remove minrev 
wording from this patch.

>> +	 * header (GNR101).
>> +	 *
>> +	 * Prevent loading 0x1000405 or later unless the CPU has already been
>> +	 * updated to 0x1000405 or later.
>> +	 */
>> +	if (vfm == INTEL_GRANITERAPIDS_X &&
>> +	    x86_stepping(sig->sig) == 1 &&
>> +	    sig->pf & 0x95 &&
>> +	    sig->rev < 0x1000405 &&
>> +	    rev >= 0x1000405) {
> 
> You don't really need to test rev here - it is enough that sig->rev is
> < 0x1000405 - that already makes you susceptible and then you can check rev
> inside the { }.

Yeah, I'd like to simplify like that, though. Intel repository looks to 
have three published revisions before 0x1000405. So there can be a valid 
update between those revisions.

So still need to allow:
   old_rev < 0x1000405 -> new_rev < 0x1000405

while rejecting:
   old_rev < 0x1000405 -> new_rev >= 0x1000405

> 
>> +		if (rev == 0x1000405)
>> +			pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n");
>> +		else
>> +			pr_err_once("Erratum GNR98: 0x1000405 is required before 0x%x.\n", rev);
>> +		pr_err_once("Please update the system BIOS or firmware.\n");
> 
> This is useless most of the time because client won't usually get BIOS
> updates. You can tell people they should update their microcode packages
> instead. That's where we can really help.
> 

I see.

I think anyone seeing this message has probably already tried updating 
microcode. With `GNR98` in the message, they would still need to decode 
it by looking up the erratum and its workaround if want to follow up on it:

   Workaround: None identified. Software should avoid updating MCU
   versions earlier than 0x1000405 to 0x1000405 or later until the system
   is Firmware Interface Table (FIT) loaded with UEFI FW/BIOS to
   0x1000405 or later.

Then there isn't much point of the additional message, I suppose. I'll 
remove it.

>>   
>> -	if (is_blacklisted(cpu))
>> +	if (is_late_loading_denied(cpu))
> 
> Aaaah, you wanna be politically correct and can't use "blacklisted" anymore.
> 
> Well, you're not introducing new usage so you don't have to touch old usage.
> And "is denied" does not express the situation properly. Try a better one.
Okay. I'll leave the existing "is_blacklisted(cpu)" as it-is since it 
already exists and probably descriptive enough. Also assuming it will 
remain distinctive from the new one, e.g. revision_banned(...).

Thanks,
Chang