When microcode staging is initiated, operations are carried out through
an MMIO interface. Each package has a unique interface specified by the
IA32_MCU_STAGING_MBOX_ADDR MSR, which maps to a set of 32-bit registers.
Prepare staging with the following steps:
1. Ensure the microcode image is 32-bit aligned to match the MMIO
register size.
2. Identify each MMIO interface based on its per-package scope.
3. Invoke the staging function for each identified interface, which
will be implemented separately.
Also, define cpu_primary_thread_mask for the CONFIG_SMP=n case, allowing
consistent use when narrowing down primary threads to locate the
per-package interface.
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Tested-by: Anselm Busse <abusse@amazon.de>
Link: https://lore.kernel.org/all/871pznq229.ffs@tglx
---
V4 -> V5:
* Rebase on the primary thread cpumask fix (Dave)
* Clean up the rev print code (Dave)
* rdmsrl_on_cpu() -> rdmsrq_on_cpu (Chao)
V2 -> V3:
* Remove a global variable and adjust stage_microcode() (Dave).
* Simplify for_each_cpu() loop control code
* Handle rdmsrl_on_cpu() return code explicitly (Chao)
V1 -> V2:
* Adjust to reference the staging_state struct.
* Add lockdep_assert_cpus_held() (Boris)
---
arch/x86/include/asm/msr-index.h | 2 ++
arch/x86/kernel/cpu/microcode/intel.c | 49 +++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index b65c3ba5fa14..0356155f9264 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -913,6 +913,8 @@
#define MSR_IA32_UCODE_WRITE 0x00000079
#define MSR_IA32_UCODE_REV 0x0000008b
+#define MSR_IA32_MCU_STAGING_MBOX_ADDR 0x000007a5
+
/* Intel SGX Launch Enclave Public Key Hash MSRs */
#define MSR_IA32_SGXLEPUBKEYHASH0 0x0000008C
#define MSR_IA32_SGXLEPUBKEYHASH1 0x0000008D
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 371ca6eac00e..d309fb1f058f 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -299,6 +299,54 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size,
return size ? NULL : patch;
}
+/*
+ * Handle the staging process using the mailbox MMIO interface.
+ * Return the result state.
+ */
+static enum ucode_state do_stage(u64 mmio_pa)
+{
+ pr_debug_once("Staging implementation is pending.\n");
+ return UCODE_ERROR;
+}
+
+static void stage_microcode(void)
+{
+ unsigned int pkg_id = UINT_MAX;
+ enum ucode_state ret;
+ int cpu, err;
+ u64 mmio_pa;
+
+ if (!IS_ALIGNED(get_totalsize(&ucode_patch_late->hdr), sizeof(u32)))
+ return;
+
+ lockdep_assert_cpus_held();
+
+ /*
+ * The MMIO address is unique per package, and all the SMT
+ * primary threads are online here. Find each MMIO space by
+ * their package ids to avoid duplicate staging.
+ */
+ for_each_cpu(cpu, cpu_primary_thread_mask) {
+ if (topology_logical_package_id(cpu) == pkg_id)
+ continue;
+ pkg_id = topology_logical_package_id(cpu);
+
+ err = rdmsrq_on_cpu(cpu, MSR_IA32_MCU_STAGING_MBOX_ADDR, &mmio_pa);
+ if (WARN_ON_ONCE(err))
+ return;
+
+ ret = do_stage(mmio_pa);
+ if (ret != UCODE_OK) {
+ pr_err("Error: staging failed with %s for CPU%d at package %u.\n",
+ ret == UCODE_TIMEOUT ? "timeout" : "error state",
+ cpu, pkg_id);
+ return;
+ }
+ }
+
+ pr_info("Staging of patch revision 0x%x succeeded.\n", ucode_patch_late->hdr.rev);
+}
+
static enum ucode_state __apply_microcode(struct ucode_cpu_info *uci,
struct microcode_intel *mc,
u32 *cur_rev)
@@ -627,6 +675,7 @@ static struct microcode_ops microcode_intel_ops = {
.collect_cpu_info = collect_cpu_info,
.apply_microcode = apply_microcode_late,
.finalize_late_load = finalize_late_load,
+ .stage_microcode = stage_microcode,
.use_nmi = IS_ENABLED(CONFIG_X86_64),
};
--
2.48.1On Sat, Aug 23, 2025 at 08:52:06AM -0700, Chang S. Bae wrote:
> When microcode staging is initiated, operations are carried out through
> an MMIO interface. Each package has a unique interface specified by the
> IA32_MCU_STAGING_MBOX_ADDR MSR, which maps to a set of 32-bit registers.
>
> Prepare staging with the following steps:
>
> 1. Ensure the microcode image is 32-bit aligned to match the MMIO
> register size.
>
> 2. Identify each MMIO interface based on its per-package scope.
>
> 3. Invoke the staging function for each identified interface, which
> will be implemented separately.
>
> Also, define cpu_primary_thread_mask for the CONFIG_SMP=n case, allowing
> consistent use when narrowing down primary threads to locate the
> per-package interface.
This paragraph is stale now and can go.
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index b65c3ba5fa14..0356155f9264 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -913,6 +913,8 @@
> #define MSR_IA32_UCODE_WRITE 0x00000079
> #define MSR_IA32_UCODE_REV 0x0000008b
>
> +#define MSR_IA32_MCU_STAGING_MBOX_ADDR 0x000007a5
Doesn't look sorted to me.
> +
> /* Intel SGX Launch Enclave Public Key Hash MSRs */
> #define MSR_IA32_SGXLEPUBKEYHASH0 0x0000008C
> #define MSR_IA32_SGXLEPUBKEYHASH1 0x0000008D
> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
> index 371ca6eac00e..d309fb1f058f 100644
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -299,6 +299,54 @@ static __init struct microcode_intel *scan_microcode(void *data, size_t size,
> return size ? NULL : patch;
> }
>
> +/*
> + * Handle the staging process using the mailbox MMIO interface.
> + * Return the result state.
> + */
> +static enum ucode_state do_stage(u64 mmio_pa)
> +{
> + pr_debug_once("Staging implementation is pending.\n");
> + return UCODE_ERROR;
> +}
> +
> +static void stage_microcode(void)
> +{
> + unsigned int pkg_id = UINT_MAX;
> + enum ucode_state ret;
> + int cpu, err;
> + u64 mmio_pa;
> +
> + if (!IS_ALIGNED(get_totalsize(&ucode_patch_late->hdr), sizeof(u32)))
> + return;
> +
> + lockdep_assert_cpus_held();
> +
> + /*
> + * The MMIO address is unique per package, and all the SMT
> + * primary threads are online here. Find each MMIO space by
> + * their package ids to avoid duplicate staging.
> + */
> + for_each_cpu(cpu, cpu_primary_thread_mask) {
> + if (topology_logical_package_id(cpu) == pkg_id)
> + continue;
<---- newline here.
> + pkg_id = topology_logical_package_id(cpu);
> +
> + err = rdmsrq_on_cpu(cpu, MSR_IA32_MCU_STAGING_MBOX_ADDR, &mmio_pa);
> + if (WARN_ON_ONCE(err))
> + return;
> +
> + ret = do_stage(mmio_pa);
> + if (ret != UCODE_OK) {
> + pr_err("Error: staging failed with %s for CPU%d at package %u.\n",
> + ret == UCODE_TIMEOUT ? "timeout" : "error state",
What does "error state" mean?
Are we going to dump additional error state so that it is clear why it failed?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 9/4/2025 5:13 AM, Borislav Petkov wrote:
> On Sat, Aug 23, 2025 at 08:52:06AM -0700, Chang S. Bae wrote:
>>
>> Also, define cpu_primary_thread_mask for the CONFIG_SMP=n case, allowing
>> consistent use when narrowing down primary threads to locate the
>> per-package interface.
>
> This paragraph is stale now and can go.
Ah, right. Sorry, I had to chop it out.
>> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
>> index b65c3ba5fa14..0356155f9264 100644
>> --- a/arch/x86/include/asm/msr-index.h
>> +++ b/arch/x86/include/asm/msr-index.h
>> @@ -913,6 +913,8 @@
>> #define MSR_IA32_UCODE_WRITE 0x00000079
>> #define MSR_IA32_UCODE_REV 0x0000008b
>>
>> +#define MSR_IA32_MCU_STAGING_MBOX_ADDR 0x000007a5
>
> Doesn't look sorted to me.
Okay, I think this 'MCU' looks a bit fuzzy. Intel folks already use this
acronym elsewhere (e.g. MSR_IA32_MCU_OPT_CTRL). It might be clearer to
consolidate them under one section, like:
/* Intel microcode update MSRs */
#define MSR_IA32_MCU_OPT_CTRL 0x00000123
#define MSR_IA32_MCU_ENUMERATION 0x0000007b <- patch7 adds this
#define MSR_IA32_MCU_STAGING_MBOX_ADDR 0x000007a5
...
>> + /*
>> + * The MMIO address is unique per package, and all the SMT
>> + * primary threads are online here. Find each MMIO space by
>> + * their package ids to avoid duplicate staging.
>> + */
>> + for_each_cpu(cpu, cpu_primary_thread_mask) {
>> + if (topology_logical_package_id(cpu) == pkg_id)
>> + continue;
>
> <---- newline here.
Fixed. Thanks.
>> + ret = do_stage(mmio_pa);
>> + if (ret != UCODE_OK) {
>> + pr_err("Error: staging failed with %s for CPU%d at package %u.\n",
>> + ret == UCODE_TIMEOUT ? "timeout" : "error state",
>
> What does "error state" mean?
>
> Are we going to dump additional error state so that it is clear why it failed?
Yeah, right. The wording "error state" is vague.
The next two patches in this series introduce helpers that return an
error code and *also* update this ucode_state. The latter could go away.
Instead, the error code could be just down through here and decoded like:
static const char *staging_errstr(int code)
{
switch (code) {
case -ETIMEDOUT: return "timeout";
...
default: return "unknown error";
}
}
static void stage_microcode(void)
{
...
err = do_stage(mmio_pa);
if (err) {
pr_err("Error: staging failed with %s for CPU%d at package %u.\n",
staging_errstr(err), cpu, pkg_id);
return;
}
}
I've attached a diff on top of V5 to picture what it will look like.
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index 455b50514d87..ae4b08e71e20 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -98,7 +98,6 @@ struct extended_sigtable {
* @chunk_size: Size of each data piece
* @bytes_sent: Total bytes transmitted so far
* @offset: Current offset in the microcode image
- * @state: Current state of the staging process
*/
struct staging_state {
void __iomem *mmio_base;
@@ -106,7 +105,6 @@ struct staging_state {
unsigned int chunk_size;
unsigned int bytes_sent;
unsigned int offset;
- enum ucode_state state;
};
#define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
@@ -425,7 +423,7 @@ static inline unsigned int calc_next_chunk_size(unsigned int ucode_len, unsigned
* Update the chunk size and decide whether another chunk can be sent.
* This accounts for remaining data and retry limits.
*/
-static bool can_send_next_chunk(struct staging_state *ss)
+static bool can_send_next_chunk(struct staging_state *ss, int *err)
{
ss->chunk_size = calc_next_chunk_size(ss->ucode_len, ss->offset);
/*
@@ -445,10 +443,11 @@ static bool can_send_next_chunk(struct staging_state *ss)
* likely stuck and mark the state as timeout.
*/
if (ss->bytes_sent + ss->chunk_size > ss->ucode_len * 2) {
- ss->state = UCODE_TIMEOUT;
+ *err = -ETIMEDOUT;
return false;
}
+ *err = 0;
return true;
}
@@ -465,9 +464,9 @@ static inline bool is_end_offset(u32 offset)
* the end offset, or no more transactions are permitted (retry limit
* reached).
*/
-static inline bool staging_is_complete(struct staging_state *ss)
+static inline bool staging_is_complete(struct staging_state *ss, int *err)
{
- return is_end_offset(ss->offset) || !can_send_next_chunk(ss);
+ return is_end_offset(ss->offset) || !can_send_next_chunk(ss, err);
}
/*
@@ -489,21 +488,16 @@ static int wait_for_transaction(struct staging_state *ss)
}
/* Check for explicit error response */
- if (status & MASK_MBOX_STATUS_ERROR) {
- ss->state = UCODE_ERROR;
- return -EPROTO;
- }
+ if (status & MASK_MBOX_STATUS_ERROR)
+ return -EIO;
/*
* Hardware is neither responded to the action nor signaled any
* error. Treat this as timeout.
*/
- if (!(status & MASK_MBOX_STATUS_READY)) {
- ss->state = UCODE_TIMEOUT;
+ if (!(status & MASK_MBOX_STATUS_READY))
return -ETIMEDOUT;
- }
- ss->state = UCODE_OK;
return 0;
}
@@ -545,7 +539,6 @@ static int fetch_next_offset(struct staging_state *ss)
const u64 expected_header = MBOX_HEADER(MBOX_HEADER_SIZE + MBOX_RESPONSE_SIZE);
u32 offset, status;
u64 header;
- int err;
/*
* The 'response' mailbox returns three fields, in order:
@@ -558,55 +551,42 @@ static int fetch_next_offset(struct staging_state *ss)
status = read_mbox_dword(ss->mmio_base);
/* All valid responses must start with the expected header. */
- if (header != expected_header) {
- pr_err_once("staging: invalid response header\n");
- err = -EINVAL;
- goto err_out;
- }
+ if (header != expected_header)
+ return -EBADR;
/*
* Verify the offset: If not at the end marker, it must not
* exceed the microcode image length
*/
- if (!is_end_offset(offset) && offset > ss->ucode_len) {
- pr_err_once("staging: invalid response offset\n");
- err = -EINVAL;
- goto err_out;
- }
+ if (!is_end_offset(offset) && offset > ss->ucode_len)
+ return -EINVAL;
/* Hardware may report errors explicitly in the status field */
- if (status & MASK_MBOX_RESP_ERROR) {
- err = -EPROTO;
- goto err_out;
- }
+ if (status & MASK_MBOX_RESP_ERROR)
+ return -EPROTO;
ss->offset = offset;
- ss->state = UCODE_OK;
return 0;
-
-err_out:
- ss->state = UCODE_ERROR;
- return err;
}
/*
* Handle the staging process using the mailbox MMIO interface. The
* microcode image is transferred in chunks until completion. Return the
- * result state.
+ * error code.
*/
-static enum ucode_state do_stage(u64 mmio_pa)
+static int do_stage(u64 mmio_pa)
{
struct staging_state ss = {};
int err;
ss.mmio_base = ioremap(mmio_pa, MBOX_REG_NUM * MBOX_REG_SIZE);
if (WARN_ON_ONCE(!ss.mmio_base))
- return UCODE_ERROR;
+ return -EADDRNOTAVAIL;
init_stage(&ss);
/* Perform the staging process while within the retry limit */
- while (!staging_is_complete(&ss)) {
+ while (!staging_is_complete(&ss, &err)) {
/* Send a chunk of microcode each time: */
err = send_data_chunk(&ss, ucode_patch_late);
if (err)
@@ -622,17 +602,25 @@ static enum ucode_state do_stage(u64 mmio_pa)
iounmap(ss.mmio_base);
- /*
- * The helpers update ss.state on error. The final state is
- * returned to the caller.
- */
- return ss.state;
+ return err;
+}
+
+static const char *staging_errstr(int code)
+{
+ switch (code) {
+ case -EBADR: return "invalid response header";
+ case -EINVAL: return "invalid next offset";
+ case -EIO: return "transaction error";
+ case -ETIMEDOUT: return "timeout";
+ case -EPROTO: return "response error";
+ case -EADDRNOTAVAIL: return "ioremap() failure";
+ default: return "unknown error";
+ };
}
static void stage_microcode(void)
{
unsigned int pkg_id = UINT_MAX;
- enum ucode_state ret;
int cpu, err;
u64 mmio_pa;
@@ -656,11 +644,10 @@ static void stage_microcode(void)
if (WARN_ON_ONCE(err))
return;
- ret = do_stage(mmio_pa);
- if (ret != UCODE_OK) {
+ err = do_stage(mmio_pa);
+ if (err) {
pr_err("Error: staging failed with %s for CPU%d at package %u.\n",
- ret == UCODE_TIMEOUT ? "timeout" : "error state",
- cpu, pkg_id);
+ staging_errstr(err), cpu, pkg_id);
return;
}
}
On Thu, Sep 04, 2025 at 05:04:24PM -0700, Chang S. Bae wrote:
> Okay, I think this 'MCU' looks a bit fuzzy.
I mean we should try to sort them by MSR *number* so that they're easier to
find. I know, I know, the whole file needs sorting but we don't need the churn
right now. So let's put new ones where they should be, i.e.:
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 6eb0c62c7fbd..87be1f815211 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -922,8 +922,6 @@
#define MSR_IA32_UCODE_WRITE 0x00000079
#define MSR_IA32_UCODE_REV 0x0000008b
-#define MSR_IA32_MCU_STAGING_MBOX_ADDR 0x000007a5
-
/* Intel SGX Launch Enclave Public Key Hash MSRs */
#define MSR_IA32_SGXLEPUBKEYHASH0 0x0000008C
#define MSR_IA32_SGXLEPUBKEYHASH1 0x0000008D
@@ -1219,6 +1217,8 @@
#define MSR_IA32_VMX_VMFUNC 0x00000491
#define MSR_IA32_VMX_PROCBASED_CTLS3 0x00000492
+#define MSR_IA32_MCU_STAGING_MBOX_ADDR 0x000007a5
+
/* Resctrl MSRs: */
/* - Intel: */
#define MSR_IA32_L3_QOS_CFG 0xc81
> The next two patches in this series introduce helpers that return an error
> code and *also* update this ucode_state. The latter could go away. Instead,
> the error code could be just down through here and decoded like:
You don't need the strings decoding... yet. This stuff is going to be looked
at only by us so there's no problem with simply dumping the errval and then
going to consult the sources where that value comes from.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 9/5/2025 4:13 AM, Borislav Petkov wrote:
>
> So let's put new ones where they should be
I see.
> You don't need the strings decoding... yet. This stuff is going to be looked
> at only by us so there's no problem with simply dumping the errval and then
> going to consult the sources where that value comes from.
Makes sense. Then, it could be just like this:
pr_err("Error: staging failed (code = %d) for CPU%d at package %u.\n",
err, cpu, pkg_id);
Thanks,
Chang
© 2016 - 2025 Red Hat, Inc.