linux-kernel - Re: [PATCH v5 5/7] x86/microcode/intel: Implement staging handler

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: <4ed4c815-12ca-48e5-a6fe-a2f2dd367730@intel.com>
Date: Wed, 10 Sep 2025 14:31:55 -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 5/7] x86/microcode/intel: Implement staging handler

On 9/10/2025 11:33 AM, Borislav Petkov wrote:
> On Sat, Aug 23, 2025 at 08:52:08AM -0700, Chang S. Bae wrote:
>> diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
>> index 3ca22457d839..a1b13202330d 100644
>> --- a/arch/x86/kernel/cpu/microcode/intel.c
>> +++ b/arch/x86/kernel/cpu/microcode/intel.c
>> @@ -20,6 +20,8 @@
>>   #include <linux/cpu.h>
>>   #include <linux/uio.h>
>>   #include <linux/mm.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
> 
> You do see those are sorted by header name length in a reverse order, right?

Okay, fixed -- and I adjusted patch6 as well:

  #include <linux/initrd.h>
  #include <linux/io.h>
  #include <linux/kernel.h>
+#include <linux/pci_ids.h>
  #include <linux/slab.h>
  #include <linux/cpu.h>
  #include <linux/uio.h>

>> +/* Defines for the microcode staging mailbox interface */
>> +
> 
> ^ Superfluous newline.

Dropped.

>> +/*
>> + * Return PAGE_SIZE, or remaining bytes if this is the final chunk
>> + */
>> +static inline unsigned int calc_next_chunk_size(unsigned int ucode_len, unsigned int offset)
>> +{
>> +	return min(PAGE_SIZE, ucode_len - offset);
>> +}
> 
> That oneliner looks useless - sticking a comment over tne min() and putting it
> at the single callsite below is good enough.

Agreed -- removed the helper and moved them.

>> +/*
>> + * 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)
>> +{
>> +	ss->chunk_size = calc_next_chunk_size(ss->ucode_len, ss->offset);
>> +	/*
>> +	 * Each microcode image is divided into chunks, each at most
>> +	 * one page size. A 10-chunk  image would typically require 10
> 				   ^^^^

Fixed.

Just to make sure, include the diff here.

Thanks for the careful review and for sticking with this set.
View attachment "tmp.diff" of type "text/plain" (2015 bytes)