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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c0580922-608a-410c-aec6-08249087943e@intel.com>
Date: Fri, 12 Sep 2025 17:51:49 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Borislav Petkov <bp@...en8.de>
CC: <linux-kernel@...r.kernel.org>, <x86@...nel.org>, <tglx@...utronix.de>,
	<mingo@...hat.com>, <dave.hansen@...ux.intel.com>, <chao.gao@...el.com>,
	<abusse@...zon.de>
Subject: Re: [PATCH v5 6/7] x86/microcode/intel: Support mailbox transfer

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);