Re: [PATCH v5 6/7] x86/microcode/intel: Support mailbox transfer

["Chang S. Bae" <chang.seok.bae@intel.com>]

On 9/12/2025 9:34 AM, Borislav Petkov wrote:
> On Sat, Aug 23, 2025 at 08:52:09AM -0700, Chang S. Bae wrote:
> 
> I don't understand what the point of this explanation is if all you end up
> doing is writing to "io_addr". Why introduce the unnecessary confusion?

Yikes, it looks like describing the format and interface went into too 
much detail when that's already covered in the spec.

>> Note: The kernel has support for similar mailboxes. But none of them are
>> compatible with this one. Trying to share code resulted in a bloated
>> mess, so this code is standalone.
> 
> Now that belongs in a commit message. Stuff which is non-obvious etc.

Okay, maybe the changelog could be something like this concisely:

The functions for sending microcode data and retrieving the next offset
were previously placeholders, as they need to handle a specific mailbox
format.

While the kernel supports similar mailboxes, none of them are compatible
with this one. Attempts to share code led to unnecessary complexity, so
add a dedicated implementation instead.


>> +	/*
>> +	 * The MMIO space is mapped as Uncached (UC). Each write arrives
>> +	 * at the device as an individual transaction in program order.
>> +	 * The device can then resemble the sequence accordingly.
> 
> reassemble?

Yep, fixed.

>> +	/*
>> +	 * Hardware is neither responded to the action nor signaled any
> 
> s/is/has/

Also fixed -- and I'll do a more careful pass to catch typos like that.

> You might want to dump some of the actual values in those pr_* statements so
> that it dumps more breadcrumbs and those prints are more useful when debugging
> issues.

Yeah, good suggestion -- maybe it could dump invalid header and offset 
values like these:

   pr_err_once("staging: invalid response header (0x%llx)\n", header);

   pr_err_once("staging: invalid offset (%u) after the image end (%u)",
               offset, ss->ucode_len);