* [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems [not found] <20260901231634.714144-1-chang.seok.bae@intel.com> @ 2026-09-01 23:16 ` Chang S. Bae 2026-09-02 13:11 ` Sohil Mehta ` (3 more replies) 2026-09-01 23:16 ` [PATCH 2/8] x86/microcode: Solidify base_rev= option parsing Chang S. Bae 2026-09-01 23:16 ` [PATCH 3/8] x86/microcode: Accept a boolean for force_minrev parameter Chang S. Bae 2 siblings, 4 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-01 23:16 UTC (permalink / raw) To: linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, chang.seok.bae, stable 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems 2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae @ 2026-09-02 13:11 ` Sohil Mehta 2026-09-02 13:21 ` Van De Ven, Arjan 2026-09-08 23:03 ` Chang S. Bae 2026-09-02 14:23 ` Dave Hansen ` (2 subsequent siblings) 3 siblings, 2 replies; 26+ messages in thread From: Sohil Mehta @ 2026-09-02 13:11 UTC (permalink / raw) To: Chang S. Bae, linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable 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; > +} > + ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems 2026-09-02 13:11 ` Sohil Mehta @ 2026-09-02 13:21 ` Van De Ven, Arjan 2026-09-08 23:03 ` Chang S. Bae 1 sibling, 0 replies; 26+ messages in thread From: Van De Ven, Arjan @ 2026-09-02 13:21 UTC (permalink / raw) To: Mehta, Sohil, Bae, Chang Seok, linux-kernel@vger.kernel.org Cc: x86@kernel.org, tglx@kernel.org, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, andrew.cooper3@citrix.com, stable@vger.kernel.org > 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.... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems 2026-09-02 13:11 ` Sohil Mehta 2026-09-02 13:21 ` Van De Ven, Arjan @ 2026-09-08 23:03 ` Chang S. Bae 1 sibling, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-08 23:03 UTC (permalink / raw) To: Sohil Mehta, linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable On 9/2/2026 6:11 AM, Sohil Mehta wrote: > 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 ... Thanks. I ended up taking Dave's version for V2. >> 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? I tweaked it in V2 to: "Prevent loading 0x1000405 or later unless the running revision is already at least 0x1000405." >> + 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? I saw exactly the same suggestion from AI, but I was not sure much value beyond more cosmetic. The revision is a one-off specific to the blocking check, so putting the revision value directly instead of a new define. > > >> + if (rev == 0x1000405) >> + pr_err_once("Erratum GNR98: 0x1000405 is not loadable.\n"); > > ^ revision Let me add `revision` in the message. Thanks. > > 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? For the error messages, `GNR98` needs to be decoded anyway, since the full context of the erratum is needed to understand for the next step. The message then is intended to point the erratum with a brief explanation. So I don't think it is too terse. > > 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. When the incoming revision is exactly 0x1000405, saying "revision 0x1000405 is required" doesn't make any sense. For the later revisions, the second message can explain more directly too. So I split the two cases. Thanks, Chang ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems 2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae 2026-09-02 13:11 ` Sohil Mehta @ 2026-09-02 14:23 ` Dave Hansen 2026-09-03 1:51 ` Borislav Petkov 2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae 3 siblings, 0 replies; 26+ messages in thread From: Dave Hansen @ 2026-09-02 14:23 UTC (permalink / raw) To: Chang S. Bae, linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems 2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae 2026-09-02 13:11 ` Sohil Mehta 2026-09-02 14:23 ` Dave Hansen @ 2026-09-03 1:51 ` Borislav Petkov 2026-09-03 21:04 ` Chang S. Bae 2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae 3 siblings, 1 reply; 26+ messages in thread From: Borislav Petkov @ 2026-09-03 1:51 UTC (permalink / raw) To: Chang S. Bae Cc: linux-kernel, x86, tglx, mingo, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems 2026-09-03 1:51 ` Borislav Petkov @ 2026-09-03 21:04 ` Chang S. Bae 0 siblings, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-03 21:04 UTC (permalink / raw) To: Borislav Petkov Cc: linux-kernel, x86, tglx, mingo, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae ` (2 preceding siblings ...) 2026-09-03 1:51 ` Borislav Petkov @ 2026-09-08 22:32 ` Chang S. Bae 2026-09-08 23:16 ` Dave Hansen ` (3 more replies) 3 siblings, 4 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-08 22:32 UTC (permalink / raw) To: linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, chang.seok.bae, stable Microcode updates can usually jump revisions. However, there is an erratum on Granite Rapids systems. If they "jump over" to revision 0x1000405 or later, they result in #MC. Prevent loading 0x1000405 or later unless the running revision is already at least 0x1000405. Apply this blocking to both early- and late-loading paths. Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> Cc: <stable@vger.kernel.org> --- V1 -> V2: * Cut the code comments and print messages (Boris) * Rename the new function and keep the old function as it-is (Boris) * Rewrote the changelog (Dave) * Add `revision` in the error messages (Sohil) --- arch/x86/kernel/cpu/microcode/intel.c | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 1142183c950c..61ad280497e9 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -309,6 +309,32 @@ static void save_microcode_patch(struct microcode_intel *patch) pr_err("Unable to allocate microcode memory size: %u\n", size); } +static bool revision_banned(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. This is + * documented item GNR98, #835486 (Intel Xeon 6900/6700/6500-Series + * Processors with P-Cores). + */ + 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: revision 0x1000405 is not loadable.\n"); + else + pr_err_once("Erratum GNR98: revision 0x1000405 is required before 0x%x.\n", rev); + 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 +356,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size, if (!intel_find_matching_signature(data, &uci->cpu_sig)) continue; + if (revision_banned(&uci->cpu_sig, mc_header->rev)) + continue; + /* * For saving the early microcode, find the matching revision which * was loaded on the BSP. @@ -878,6 +907,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 (revision_banned(&uci->cpu_sig, mc_header.rev)) + continue; + is_safe = ucode_validate_minrev(&mc_header); if (force_minrev && !is_safe) continue; -- 2.53.0 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae @ 2026-09-08 23:16 ` Dave Hansen 2026-09-09 0:13 ` Borislav Petkov ` (2 subsequent siblings) 3 siblings, 0 replies; 26+ messages in thread From: Dave Hansen @ 2026-09-08 23:16 UTC (permalink / raw) To: Chang S. Bae, linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable On 9/8/26 15:32, Chang S. Bae wrote: > Microcode updates can usually jump revisions. However, there is an > erratum on Granite Rapids systems. If they "jump over" to revision > 0x1000405 or later, they result in #MC. > > Prevent loading 0x1000405 or later unless the running revision is already > at least 0x1000405. Apply this blocking to both early- and late-loading > paths. This is a kinda wonky-looking patch but I don't know a nicer way to do it, so: Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> > +static bool revision_banned(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. This is > + * documented item GNR98, #835486 (Intel Xeon 6900/6700/6500-Series > + * Processors with P-Cores). > + */ I honestly still prefer that this comment be more short and sweet. Does this miss anything? /* * Erratum GNR98 can cause #MC's if "jumping over" * revision 0x1000405. Avoid the jumps. */ Also, we don't need to put whole document titles or numbers in here. They're fine, but if you can Google "erratum GNR98" and get the doc for the first hit, what does it matter? Leave the precision and long chit chat for the changelog, I say. But we can edit this in place when it gets applied. > + 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: revision 0x1000405 is not loadable.\n"); > + else > + pr_err_once("Erratum GNR98: revision 0x1000405 is required before 0x%x.\n", rev); > + 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 +356,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size, > if (!intel_find_matching_signature(data, &uci->cpu_sig)) > continue; > > + if (revision_banned(&uci->cpu_sig, mc_header->rev)) > + continue; > + > /* > * For saving the early microcode, find the matching revision which > * was loaded on the BSP. > @@ -878,6 +907,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 (revision_banned(&uci->cpu_sig, mc_header.rev)) > + continue; > + > is_safe = ucode_validate_minrev(&mc_header); > if (force_minrev && !is_safe) > continue; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae 2026-09-08 23:16 ` Dave Hansen @ 2026-09-09 0:13 ` Borislav Petkov 2026-09-09 0:39 ` Chang S. Bae 2026-09-09 0:14 ` Andrew Cooper 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae 3 siblings, 1 reply; 26+ messages in thread From: Borislav Petkov @ 2026-09-09 0:13 UTC (permalink / raw) To: Chang S. Bae Cc: linux-kernel, x86, tglx, mingo, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable On Tue, Sep 08, 2026 at 10:32:09PM +0000, Chang S. Bae wrote: > Microcode updates can usually jump revisions. However, there is an > erratum on Granite Rapids systems. If they "jump over" to revision > 0x1000405 or later, they result in #MC. > > Prevent loading 0x1000405 or later unless the running revision is already > at least 0x1000405. Apply this blocking to both early- and late-loading > paths. People have got to stop explaining the patch in the commit message. That should be obvious from the diff. If you have to explain it then there is something very non-obvious here which I don't see it... > Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> > Cc: <stable@vger.kernel.org> > --- > V1 -> V2: > * Cut the code comments and print messages (Boris) > * Rename the new function and keep the old function as it-is (Boris) > * Rewrote the changelog (Dave) > * Add `revision` in the error messages (Sohil) > --- > arch/x86/kernel/cpu/microcode/intel.c | 32 +++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c > index 1142183c950c..61ad280497e9 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -309,6 +309,32 @@ static void save_microcode_patch(struct microcode_intel *patch) > pr_err("Unable to allocate microcode memory size: %u\n", size); > } > > +static bool revision_banned(struct cpu_signature *sig, u32 rev) I like Andy's naming: https://lore.kernel.org/xen-devel/20260908171525.3196765-1-andrew.cooper3@citrix.com/T/#u ...is_safe is much better than 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. This is > + * documented item GNR98, #835486 (Intel Xeon 6900/6700/6500-Series > + * Processors with P-Cores). > + */ What dhansen said - keep this short'n'sweet. > + if (vfm == INTEL_GRANITERAPIDS_X && > + x86_stepping(sig->sig) == 1 && > + sig->pf & 0x95 && > + sig->rev < 0x1000405 && > + rev >= 0x1000405) { > + if (rev == 0x1000405) I also like Andy's testing of the patch revs: + ((cpu_sig->rev < 0x01000380 && mc->rev >= 0x01000405) || + (cpu_sig->rev < 0x01000405 && mc->rev > 0x01000405)) ) > + pr_err_once("Erratum GNR98: revision 0x1000405 is not loadable.\n"); > + else > + pr_err_once("Erratum GNR98: revision 0x1000405 is required before 0x%x.\n", rev); And you don't need those semi-identical strings here. > + return true; > + } > + > + return false; > +} Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-09 0:13 ` Borislav Petkov @ 2026-09-09 0:39 ` Chang S. Bae 0 siblings, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-09 0:39 UTC (permalink / raw) To: Borislav Petkov Cc: linux-kernel, x86, tglx, mingo, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable On 9/8/2026 5:13 PM, Borislav Petkov wrote: > On Tue, Sep 08, 2026 at 10:32:09PM +0000, Chang S. Bae wrote: >> Microcode updates can usually jump revisions. However, there is an >> erratum on Granite Rapids systems. If they "jump over" to revision >> 0x1000405 or later, they result in #MC. >> >> Prevent loading 0x1000405 or later unless the running revision is already >> at least 0x1000405. Apply this blocking to both early- and late-loading >> paths. > > People have got to stop explaining the patch in the commit message. That > should be obvious from the diff. If you have to explain it then there is > something very non-obvious here which I don't see it... ... If they "jump over" to revision 0x1000405 or later, they result in #MC. Avoid it. >> >> +static bool revision_banned(struct cpu_signature *sig, u32 rev) > > I like Andy's naming: > > https://lore.kernel.org/xen-devel/20260908171525.3196765-1-andrew.cooper3@citrix.com/T/#u > > ...is_safe is much better than banned. s/revision_banned/revision_is_safe/ in V3. > >> +{ >> + 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. This is >> + * documented item GNR98, #835486 (Intel Xeon 6900/6700/6500-Series >> + * Processors with P-Cores). >> + */ > > What dhansen said - keep this short'n'sweet. Will replace the above with Dave's. > >> + if (vfm == INTEL_GRANITERAPIDS_X && >> + x86_stepping(sig->sig) == 1 && >> + sig->pf & 0x95 && >> + sig->rev < 0x1000405 && >> + rev >= 0x1000405) { >> + if (rev == 0x1000405) > > I also like Andy's testing of the patch revs: > > + ((cpu_sig->rev < 0x01000380 && mc->rev >= 0x01000405) || > + (cpu_sig->rev < 0x01000405 && mc->rev > 0x01000405)) ) Let me double-check this with ucode folks, first. Then will take. > >> + pr_err_once("Erratum GNR98: revision 0x1000405 is not loadable.\n"); >> + else >> + pr_err_once("Erratum GNR98: revision 0x1000405 is required before 0x%x.\n", rev); > > And you don't need those semi-identical strings here. pr_err_once("Erratum GNR98: skipping revision 0x%x.\n", rev); Yeah, overall I can hear "keep it simple & short" messages. Thanks, Chang ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae 2026-09-08 23:16 ` Dave Hansen 2026-09-09 0:13 ` Borislav Petkov @ 2026-09-09 0:14 ` Andrew Cooper 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae 3 siblings, 0 replies; 26+ messages in thread From: Andrew Cooper @ 2026-09-09 0:14 UTC (permalink / raw) To: Chang S. Bae, linux-kernel Cc: Andrew Cooper, x86, tglx, mingo, bp, dave.hansen, hpa, arjan.van.de.ven, sohil.mehta, stable On 08/09/2026 11:32 pm, Chang S. Bae wrote: > diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c > index 1142183c950c..61ad280497e9 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -309,6 +309,32 @@ static void save_microcode_patch(struct microcode_intel *patch) > pr_err("Unable to allocate microcode memory size: %u\n", size); > } > > +static bool revision_banned(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. This is > + * documented item GNR98, #835486 (Intel Xeon 6900/6700/6500-Series > + * Processors with P-Cores). > + */ > + 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: revision 0x1000405 is not loadable.\n"); > + else > + pr_err_once("Erratum GNR98: revision 0x1000405 is required before 0x%x.\n", rev); > + return true; > + } This logic is going to need editing when the muti-blob finally appears. This is what I'm doing for Xen: https://lore.kernel.org/xen-devel/20260908171525.3196765-1-andrew.cooper3@citrix.com/T/#u Notably, 0x1000405 is safe to load if 0x1000380 is in FIT, and this is a necessary hoop to jump through in due course. ~Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae ` (2 preceding siblings ...) 2026-09-09 0:14 ` Andrew Cooper @ 2026-09-16 22:59 ` Chang S. Bae 2026-09-17 9:14 ` David Laight ` (4 more replies) 3 siblings, 5 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-16 22:59 UTC (permalink / raw) To: linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, chang.seok.bae, stable Microcode updates can usually jump revisions. However, there is an erratum on Granite Rapids systems. If they "jump over" revision 0x1000405, they result in #MC. Avoid it. Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> Cc: <stable@vger.kernel.org> --- V2 -> V3: * Shorten the changelog and rename the function (Boris) * Reduce the code comment (Dave) * Allow 0x1000405 loading. Thanks to Andrew, this fix got attention. * Collect Dave review tag. Thanks, Dave! Note: * GNR98 currently describes loading 0x1000405 itself is unsafe, but it will be updated to say okay with that. I will watch out the GNR98 changes. * Jumping from < 0x1000380 to 0x1000405 was identified as an issue, but 0x1000380 is the first revision as GNR products. So loading 0x1000405 in production systems should be okay. --- arch/x86/kernel/cpu/microcode/intel.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 1142183c950c..30a22388d4b1 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -309,6 +309,26 @@ static void save_microcode_patch(struct microcode_intel *patch) pr_err("Unable to allocate microcode memory size: %u\n", size); } +static bool revision_is_safe(struct cpu_signature *sig, u32 rev) +{ + u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig)); + + /* + * Erratum GNR98 can cause #MC's if "jumping over" revision 0x1000405. + * Avoid the jumps. + */ + if (vfm == INTEL_GRANITERAPIDS_X && + x86_stepping(sig->sig) == 1 && + sig->pf & 0x95 && + sig->rev < 0x1000405 && + rev > 0x1000405) { + pr_err_once("Erratum GNR98: skipping revision 0x%x.\n", rev); + 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, @@ -330,6 +350,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size, if (!intel_find_matching_signature(data, &uci->cpu_sig)) continue; + if (!revision_is_safe(&uci->cpu_sig, mc_header->rev)) + continue; + /* * For saving the early microcode, find the matching revision which * was loaded on the BSP. @@ -878,6 +901,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 (!revision_is_safe(&uci->cpu_sig, mc_header.rev)) + continue; + is_safe = ucode_validate_minrev(&mc_header); if (force_minrev && !is_safe) continue; -- 2.53.0 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae @ 2026-09-17 9:14 ` David Laight 2026-09-17 10:50 ` Andrew Cooper 2026-09-17 13:35 ` Van De Ven, Arjan 2026-09-17 17:52 ` Sohil Mehta ` (3 subsequent siblings) 4 siblings, 2 replies; 26+ messages in thread From: David Laight @ 2026-09-17 9:14 UTC (permalink / raw) To: Chang S. Bae Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable On Wed, 16 Sep 2026 22:59:39 +0000 "Chang S. Bae" <chang.seok.bae@intel.com> wrote: > Microcode updates can usually jump revisions. However, there is an > erratum on Granite Rapids systems. If they "jump over" revision > 0x1000405, they result in #MC. Avoid it. A probably silly question. Is it valid to downgrade microcode? David > > Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> > Cc: <stable@vger.kernel.org> > --- > V2 -> V3: > * Shorten the changelog and rename the function (Boris) > * Reduce the code comment (Dave) > * Allow 0x1000405 loading. Thanks to Andrew, this fix got attention. > * Collect Dave review tag. Thanks, Dave! > > Note: > * GNR98 currently describes loading 0x1000405 itself is unsafe, but it > will be updated to say okay with that. I will watch out the GNR98 > changes. > * Jumping from < 0x1000380 to 0x1000405 was identified as an issue, but > 0x1000380 is the first revision as GNR products. So loading 0x1000405 > in production systems should be okay. > --- > arch/x86/kernel/cpu/microcode/intel.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c > index 1142183c950c..30a22388d4b1 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -309,6 +309,26 @@ static void save_microcode_patch(struct microcode_intel *patch) > pr_err("Unable to allocate microcode memory size: %u\n", size); > } > > +static bool revision_is_safe(struct cpu_signature *sig, u32 rev) > +{ > + u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig)); > + > + /* > + * Erratum GNR98 can cause #MC's if "jumping over" revision 0x1000405. > + * Avoid the jumps. > + */ > + if (vfm == INTEL_GRANITERAPIDS_X && > + x86_stepping(sig->sig) == 1 && > + sig->pf & 0x95 && > + sig->rev < 0x1000405 && > + rev > 0x1000405) { > + pr_err_once("Erratum GNR98: skipping revision 0x%x.\n", rev); > + 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, > @@ -330,6 +350,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size, > if (!intel_find_matching_signature(data, &uci->cpu_sig)) > continue; > > + if (!revision_is_safe(&uci->cpu_sig, mc_header->rev)) > + continue; > + > /* > * For saving the early microcode, find the matching revision which > * was loaded on the BSP. > @@ -878,6 +901,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 (!revision_is_safe(&uci->cpu_sig, mc_header.rev)) > + continue; > + > is_safe = ucode_validate_minrev(&mc_header); > if (force_minrev && !is_safe) > continue; ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-17 9:14 ` David Laight @ 2026-09-17 10:50 ` Andrew Cooper 2026-09-17 13:35 ` Van De Ven, Arjan 1 sibling, 0 replies; 26+ messages in thread From: Andrew Cooper @ 2026-09-17 10:50 UTC (permalink / raw) To: David Laight, Chang S. Bae Cc: Andrew Cooper, linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa, arjan.van.de.ven, sohil.mehta, stable On 17/09/2026 11:14 am, David Laight wrote: > On Wed, 16 Sep 2026 22:59:39 +0000 > "Chang S. Bae" <chang.seok.bae@intel.com> wrote: > >> Microcode updates can usually jump revisions. However, there is an >> erratum on Granite Rapids systems. If they "jump over" revision >> 0x1000405, they result in #MC. Avoid it. > A probably silly question. > Is it valid to downgrade microcode? Yes, and it does happen in practice. Intel microcode contains a field called the Security Version Number. The SVN bumps as infrequently as possible, but does bump when necessary. There is a hard block on the SVN going backwards, but as long as you're not violating this constraint, you can upgrade or downgrade microcode as desired. ~Andrew ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-17 9:14 ` David Laight 2026-09-17 10:50 ` Andrew Cooper @ 2026-09-17 13:35 ` Van De Ven, Arjan 1 sibling, 0 replies; 26+ messages in thread From: Van De Ven, Arjan @ 2026-09-17 13:35 UTC (permalink / raw) To: David Laight, Bae, Chang Seok Cc: linux-kernel@vger.kernel.org, x86@kernel.org, tglx@kernel.org, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, andrew.cooper3@citrix.com, Mehta, Sohil, stable@vger.kernel.org > > Microcode updates can usually jump revisions. However, there is an > > erratum on Granite Rapids systems. If they "jump over" revision > > 0x1000405, they result in #MC. Avoid it. > > A probably silly question. > Is it valid to downgrade microcode? Within SVN in theory it can be done In practice, unless you have very special circumstances (and check with Intel if the exact downgrade you have in mind has been tested for downgrading) I would very strongly recommend against doing so. It may appear to work, but note that A -> B -> A Is NOT identical to just A (staying at A) as part of the microcode load, some microcode runs as "setup" which may change settings in the CPU (versus runtime behavior changes) -- and that setup step is not undone as you go back to A.... which means you run A with a (partial or whole) setup of B. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae 2026-09-17 9:14 ` David Laight @ 2026-09-17 17:52 ` Sohil Mehta 2026-09-17 18:11 ` Chang S. Bae 2026-09-18 0:41 ` [tip: x86/urgent] " tip-bot2 for Chang S. Bae ` (2 subsequent siblings) 4 siblings, 1 reply; 26+ messages in thread From: Sohil Mehta @ 2026-09-17 17:52 UTC (permalink / raw) To: Chang S. Bae, linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable > /* 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 +350,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size, > if (!intel_find_matching_signature(data, &uci->cpu_sig)) > continue; > > + if (!revision_is_safe(&uci->cpu_sig, mc_header->rev)) > + continue; > + > /* > * For saving the early microcode, find the matching revision which > * was loaded on the BSP. > @@ -878,6 +901,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 (!revision_is_safe(&uci->cpu_sig, mc_header.rev)) > + continue; > + > is_safe = ucode_validate_minrev(&mc_header); > if (force_minrev && !is_safe) > continue; TL;dr: Should there be a revision_is_safe() check during __apply_microcode()? The patch only adds the revision check during blob selection. Have we evaluated the corner cases hinted by sashiko when this check might be bypassed? It talks about suspend/resume and the CPU hotplug cases. v1: https://sashiko.dev/#/patchset/20260901231634.714144-1-chang.seok.bae%40intel.com?part=1 v3: https://sashiko.dev/#/patchset/20260916225939.1144524-1-chang.seok.bae%40intel.com?part=1 The suspend/resume path probably doesn't matter for GNR servers and most of the CPU hotplug flows also seem to be covered. But, what about ucode update on cores that are not enabled at boot? If there are brought online later, would the ucode update skip the above check? For example, maxcpus=N prevents certain CPUs from showing up in cpus_booted_once_mask. So the checks in setup_cpus() during late-loading would not catch them. IIUC, the BIOS version can be < 0x1000405, early load can bump the booted cpus to 0x1000405, and then late-loading can load newer versions and only cache the latest ucode revision. All this while, the non-booted cores would be stuck at a revision less than 0x1000405. So, when they are brought online later, could they directly jump to a revision greater than 0x1000405? Would it be safer to add the revision_is_safe() check in __apply_microcode() so that all of such cases are covered? Maybe sashiko just made me paranoid. I only started looking at this because it complained! :( ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-17 17:52 ` Sohil Mehta @ 2026-09-17 18:11 ` Chang S. Bae 2026-09-17 18:43 ` Van De Ven, Arjan 0 siblings, 1 reply; 26+ messages in thread From: Chang S. Bae @ 2026-09-17 18:11 UTC (permalink / raw) To: Sohil Mehta, linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, stable On 9/17/2026 10:52 AM, Sohil Mehta wrote: > > TL;dr: Should there be a revision_is_safe() check during > __apply_microcode()? > > The patch only adds the revision check during blob selection. Have we > evaluated the corner cases hinted by sashiko when this check might be > bypassed? It talks about suspend/resume and the CPU hotplug cases. > > v1: > https://sashiko.dev/#/patchset/20260901231634.714144-1-chang.seok.bae%40intel.com?part=1 > > v3: > https://sashiko.dev/#/patchset/20260916225939.1144524-1-chang.seok.bae%40intel.com?part=1 > > The suspend/resume path probably doesn't matter for GNR servers and most > of the CPU hotplug flows also seem to be covered. But, what about ucode > update on cores that are not enabled at boot? If there are brought > online later, would the ucode update skip the above check? > > For example, maxcpus=N prevents certain CPUs from showing up in > cpus_booted_once_mask. So the checks in setup_cpus() during late-loading > would not catch them. IIUC, the BIOS version can be < 0x1000405, early > load can bump the booted cpus to 0x1000405, and then late-loading can > load newer versions and only cache the latest ucode revision. Currently, what setup_cpus() does as its comment says is first ensure all CPUs that are present and has been booted up have their primary threads online. It just allows its secondary thread offline with nosmt. I don't think the current logic is broken there. Then, those sibling threads assuming late-loading while soft-offlined will see an updated revision on its bringup because the loading scope is per-core by default, meaning the update performed by the primary thread also applies to its sibling. Also, the NMI stop-machine rendezvous includes those soft-offlined CPUs. They are brought into the rendezvous and wait there while the update is being performed. Now, I think it could be misleading if we put the revision check toward the end right before the application. For example, suppose a multi-blob image that is bundled with revision 0x1000405 and later ones and currently running < 0x1000405. With the check during blob selection, the parser can identify 0x1000405 as the loadable revision and reject later revisions. If we move the check to just before application, the parser may instead select a revision newer than 0x1000405. The application would then be rejected, and the same blob would be selected again on the next attempt, resulting in the loading process repeatedly aborting without ever making progress. So the revision check needs to remain part of blob selection, where it can affect which revision is considered loadable, rather than being only an application-time check. Thanks, Chang ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-17 18:11 ` Chang S. Bae @ 2026-09-17 18:43 ` Van De Ven, Arjan 2026-09-17 18:25 ` Chang S. Bae 0 siblings, 1 reply; 26+ messages in thread From: Van De Ven, Arjan @ 2026-09-17 18:43 UTC (permalink / raw) To: Bae, Chang Seok, Mehta, Sohil, linux-kernel@vger.kernel.org Cc: x86@kernel.org, tglx@kernel.org, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, andrew.cooper3@citrix.com, stable@vger.kernel.org > If we move the check to just before application, the parser may instead > select a revision newer than 0x1000405. The application would then be > rejected, and the same blob would be selected again on the next attempt, > resulting in the loading process repeatedly aborting without ever making > progress. > > So the revision check needs to remain part of blob selection, where it > can affect which revision is considered loadable, rather than being only > an application-time check. Agreed that we need the early check. But we could, on the actual application, at least check if the ucode already got upgraded to EXACTLY the target version, and skip the actual application if it has been upgraded. That is a simple check with basically no policy ("identity") but with huge savings ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-17 18:43 ` Van De Ven, Arjan @ 2026-09-17 18:25 ` Chang S. Bae 0 siblings, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-17 18:25 UTC (permalink / raw) To: Van De Ven, Arjan, Mehta, Sohil, linux-kernel@vger.kernel.org Cc: x86@kernel.org, tglx@kernel.org, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, andrew.cooper3@citrix.com, stable@vger.kernel.org On 9/17/2026 11:43 AM, Van De Ven, Arjan wrote: > > But we could, on the actual application, at least check if the ucode > already got upgraded to EXACTLY the target version, and skip the > actual application if it has been upgraded. That is a simple check > with basically no policy ("identity") but with huge savings Yes, I think that' what we do now: __apply_microcode(..., struct microcode_intel *mc, ...) { u32 rev; if (!mc) return UCODE_NFOUND; *cur_rev = intel_get_microcode_revision(); ^ read MSR0x8b (aka MSR_IA32_UCODE_REV) if (*cur_rev >= mc->hdr.rev) { uci->cpu_sig.rev = *cur_rev; return UCODE_OK; ^ we skip the loading if updated } /* write microcode via MSR 0x79 */ native_wrmsrq(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits); ... } https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/microcode/intel.c#n646 Thanks, Chang ^ permalink raw reply [flat|nested] 26+ messages in thread
* [tip: x86/urgent] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae 2026-09-17 9:14 ` David Laight 2026-09-17 17:52 ` Sohil Mehta @ 2026-09-18 0:41 ` tip-bot2 for Chang S. Bae 2026-09-18 6:54 ` [PATCH v3] " Ingo Molnar 2026-10-20 18:09 ` Chang S. Bae 4 siblings, 0 replies; 26+ messages in thread From: tip-bot2 for Chang S. Bae @ 2026-09-18 0:41 UTC (permalink / raw) To: linux-tip-commits Cc: Chang S. Bae, Borislav Petkov (AMD), Dave Hansen, stable, x86, linux-kernel The following commit has been merged into the x86/urgent branch of tip: Commit-ID: e7d3e2f46dd5a69046e6d95a0f189155a5516b93 Gitweb: https://git.kernel.org/tip/e7d3e2f46dd5a69046e6d95a0f189155a5516b93 Author: Chang S. Bae <chang.seok.bae@intel.com> AuthorDate: Wed, 16 Sep 2026 22:59:39 Committer: Borislav Petkov (AMD) <bp@alien8.de> CommitterDate: Thu, 17 Sep 2026 16:54:20 -07:00 x86/microcode/intel: Reject problematic loading on Granite Rapids systems Microcode updates can usually jump revisions. However, there is an erratum on Granite Rapids systems. If they "jump over" revision 0x1000405, they result in an #MC. Avoid it. Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20260916225939.1144524-1-chang.seok.bae@intel.com --- arch/x86/kernel/cpu/microcode/intel.c | 26 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 1142183..9f09d38 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -309,6 +309,26 @@ static void save_microcode_patch(struct microcode_intel *patch) pr_err("Unable to allocate microcode memory size: %u\n", size); } +static bool revision_is_safe(struct cpu_signature *sig, u32 rev) +{ + u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig)); + + /* + * Erratum GNR98 can cause #MCs if "jumping over" revision 0x1000405. + * Avoid the jumps. + */ + if (vfm == INTEL_GRANITERAPIDS_X && + x86_stepping(sig->sig) == 1 && + sig->pf & 0x95 && + sig->rev < 0x1000405 && + rev > 0x1000405) { + pr_err_once("Erratum GNR98: skipping revision 0x%x.\n", rev); + 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, @@ -330,6 +350,9 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size, if (!intel_find_matching_signature(data, &uci->cpu_sig)) continue; + if (!revision_is_safe(&uci->cpu_sig, mc_header->rev)) + continue; + /* * For saving the early microcode, find the matching revision which * was loaded on the BSP. @@ -878,6 +901,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 (!revision_is_safe(&uci->cpu_sig, mc_header.rev)) + continue; + is_safe = ucode_validate_minrev(&mc_header); if (force_minrev && !is_safe) continue; ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae ` (2 preceding siblings ...) 2026-09-18 0:41 ` [tip: x86/urgent] " tip-bot2 for Chang S. Bae @ 2026-09-18 6:54 ` Ingo Molnar 2026-09-18 15:37 ` Borislav Petkov 2026-10-20 18:09 ` Chang S. Bae 4 siblings, 1 reply; 26+ messages in thread From: Ingo Molnar @ 2026-09-18 6:54 UTC (permalink / raw) To: Chang S. Bae Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable * Chang S. Bae <chang.seok.bae@intel.com> wrote: > Microcode updates can usually jump revisions. However, there is an > erratum on Granite Rapids systems. If they "jump over" revision > 0x1000405, they result in #MC. Avoid it. > > Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com> > Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com> > Cc: <stable@vger.kernel.org> > --- > V2 -> V3: > * Shorten the changelog and rename the function (Boris) > * Reduce the code comment (Dave) > * Allow 0x1000405 loading. Thanks to Andrew, this fix got attention. > * Collect Dave review tag. Thanks, Dave! > > Note: > * GNR98 currently describes loading 0x1000405 itself is unsafe, but it > will be updated to say okay with that. I will watch out the GNR98 > changes. > * Jumping from < 0x1000380 to 0x1000405 was identified as an issue, but > 0x1000380 is the first revision as GNR products. So loading 0x1000405 > in production systems should be okay. > --- > arch/x86/kernel/cpu/microcode/intel.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c > index 1142183c950c..30a22388d4b1 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -309,6 +309,26 @@ static void save_microcode_patch(struct microcode_intel *patch) > pr_err("Unable to allocate microcode memory size: %u\n", size); > } > > +static bool revision_is_safe(struct cpu_signature *sig, u32 rev) > +{ > + u32 vfm = IFM(x86_family(sig->sig), x86_model(sig->sig)); > + > + /* > + * Erratum GNR98 can cause #MC's if "jumping over" revision 0x1000405. > + * Avoid the jumps. > + */ > + if (vfm == INTEL_GRANITERAPIDS_X && > + x86_stepping(sig->sig) == 1 && > + sig->pf & 0x95 && > + sig->rev < 0x1000405 && > + rev > 0x1000405) { > + pr_err_once("Erratum GNR98: skipping revision 0x%x.\n", rev); So this was explained in a really confusing way, I had to read the changelog and comments trice and then the code to figure out what's going on: - There's a microcode bug that makes it unsafe to apply current post-0x1000405 revisions on Granite Rapid CPUs if the current microcode version is below 0x1000405. The interim 0x1000405 version *must* be applied first for it to be safe to upgrade GNR CPUs. Will this be a problem perpetually? Will it be unsafe to have an older GNR CPU and simply apply fresh microcode to it, without first loading the interim 0x1000405 version? Will GNR microcode upgrades on pre-0x1000405 CPUs will always be a two-step process? - The new code doesn't declare it, but this patch creates a hidden, permanent microcode version upgrade barrier if user-space firmware/microcode tools do not provide the 0x1000405 microcode version reliably and implement the two-step upgrade workaround. - The message the kernel prints is rather passive-aggressive as well: Erratum GNR98: skipping revision 0x%x It does not explain *why* the fresh microcode upgrade is skipped, and if firmware tooling does not apply 0x1000405 then the kernel stays in this state indefinitely. It should at minimum say something like: Erratum GNR98: new revision %x is unsafe to apply until 0x1000405 is applied first, skipping it. Please upgrade firmware tools. Because, presumably, this scenario should not be possible with new user-space tooling, right? The comment and changelog should be updated accordingly as well. So this patch is still problematic IMO. Thanks, Ingo ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-18 6:54 ` [PATCH v3] " Ingo Molnar @ 2026-09-18 15:37 ` Borislav Petkov 0 siblings, 0 replies; 26+ messages in thread From: Borislav Petkov @ 2026-09-18 15:37 UTC (permalink / raw) To: Ingo Molnar Cc: Chang S. Bae, linux-kernel, x86, tglx, mingo, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable On Fri, Sep 18, 2026 at 08:54:01AM +0200, Ingo Molnar wrote: > It should at minimum say something like: > > Erratum GNR98: new revision %x is unsafe to apply until 0x1000405 is applied first, skipping it. Please upgrade firmware tools. Even if you put prose in a warning message, it still doesn't explain the issue fully. For stuff like that you need some properly written longer explanation in the documentation. Which I'm hoping we'll get eventually once the dust has settled down. That's what the rest of this thread is trying to do but I haven't reviewed yet. > So this patch is still problematic IMO. This patch is a minimal stable fix to stop it from #MCing. The real stuff comes later. It all has been discussed on this very thread. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3] x86/microcode/intel: Reject problematic loading on Granite Rapids systems 2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae ` (3 preceding siblings ...) 2026-09-18 6:54 ` [PATCH v3] " Ingo Molnar @ 2026-10-20 18:09 ` Chang S. Bae 4 siblings, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-10-20 18:09 UTC (permalink / raw) To: linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, sohil.mehta, stable On 9/16/2026 3:59 PM, Chang S. Bae wrote: > > * GNR98 currently describes loading 0x1000405 itself is unsafe, but it > will be updated to say okay with that. I will watch out the GNR98 > changes. The erratum documentation [*] appears to have been updated since last week. GNR98 now describes correctly: ... Due to this erratum, an Early OS Microcode Update from a microcode version earlier than 0x1000405 to a release _after_ 0x1000405 may result in a Machine Check Error (MCE). [*] https://www.intel.com/content/www/us/en/content-details/835486/intel-xeon-6900-6700-6500-series-processors-with-p-cores-specification-update.html Thanks, Chang ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/8] x86/microcode: Solidify base_rev= option parsing [not found] <20260901231634.714144-1-chang.seok.bae@intel.com> 2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae @ 2026-09-01 23:16 ` Chang S. Bae 2026-09-01 23:16 ` [PATCH 3/8] x86/microcode: Accept a boolean for force_minrev parameter Chang S. Bae 2 siblings, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-01 23:16 UTC (permalink / raw) To: linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, chang.seok.bae, stable 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 3/8] x86/microcode: Accept a boolean for force_minrev parameter [not found] <20260901231634.714144-1-chang.seok.bae@intel.com> 2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae 2026-09-01 23:16 ` [PATCH 2/8] x86/microcode: Solidify base_rev= option parsing Chang S. Bae @ 2026-09-01 23:16 ` Chang S. Bae 2 siblings, 0 replies; 26+ messages in thread From: Chang S. Bae @ 2026-09-01 23:16 UTC (permalink / raw) To: linux-kernel Cc: x86, tglx, mingo, bp, dave.hansen, hpa, andrew.cooper3, arjan.van.de.ven, chang.seok.bae, Sohil Mehta, stable 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 ^ permalink raw reply related [flat|nested] 26+ messages in thread
end of thread, other threads:[~2026-10-20 18:09 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260901231634.714144-1-chang.seok.bae@intel.com>
2026-09-01 23:16 ` [PATCH 1/8] x86/microcode/intel: Reject problematic loading on GNR systems Chang S. Bae
2026-09-02 13:11 ` Sohil Mehta
2026-09-02 13:21 ` Van De Ven, Arjan
2026-09-08 23:03 ` Chang S. Bae
2026-09-02 14:23 ` Dave Hansen
2026-09-03 1:51 ` Borislav Petkov
2026-09-03 21:04 ` Chang S. Bae
2026-09-08 22:32 ` [PATCH v2] x86/microcode/intel: Reject problematic loading on Granite Rapids systems Chang S. Bae
2026-09-08 23:16 ` Dave Hansen
2026-09-09 0:13 ` Borislav Petkov
2026-09-09 0:39 ` Chang S. Bae
2026-09-09 0:14 ` Andrew Cooper
2026-09-16 22:59 ` [PATCH v3] " Chang S. Bae
2026-09-17 9:14 ` David Laight
2026-09-17 10:50 ` Andrew Cooper
2026-09-17 13:35 ` Van De Ven, Arjan
2026-09-17 17:52 ` Sohil Mehta
2026-09-17 18:11 ` Chang S. Bae
2026-09-17 18:43 ` Van De Ven, Arjan
2026-09-17 18:25 ` Chang S. Bae
2026-09-18 0:41 ` [tip: x86/urgent] " tip-bot2 for Chang S. Bae
2026-09-18 6:54 ` [PATCH v3] " Ingo Molnar
2026-09-18 15:37 ` Borislav Petkov
2026-10-20 18:09 ` Chang S. Bae
2026-09-01 23:16 ` [PATCH 2/8] x86/microcode: Solidify base_rev= option parsing Chang S. Bae
2026-09-01 23:16 ` [PATCH 3/8] x86/microcode: Accept a boolean for force_minrev parameter Chang S. Bae
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox