Re: [PATCH v2 1/3] x86/ucode: Remove the collect_cpu_info() call from parse_blob()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.11.2024 16:59, Andrew Cooper wrote:
> On 14/11/2024 11:11 am, Jan Beulich wrote:
>> On 12.11.2024 22:19, Andrew Cooper wrote:
>>> With the tangle of logic starting to come under control, it is now plain to
>>> see that parse_blob()'s side effect of re-gathering the signature/revision 
>>> is
>>> pointless.
>>>
>>> The cpu_request_microcode() hooks need the signature only.  The BSP gathers
>>> this in early_microcode_init(), the APs and S3 in microcode_update_cpu().
>> That's microcode_update_one() after 502478bc1d9d if I'm not mistaken.
> 
> I wouldn't necessarily say "after".

My point was merely that there's no microcode_update_cpu() anymore, as of
that commit.

> microcode_update_cpu() has been the way the APs and S3 get this
> information for ages, whether its in the function directly, or in an
> immediate callee.
> 
>>  In the
>> course of determining that I'm afraid I also found the first sentence of this
>> paragraph rather misleading than helpful:
> 
> Do you mean "The cpu_request_microcode() hooks need the signature only" ?

Yes.

>> While it is true what is being said,
>> in both cases it is collect_cpu_info() that is being invoked, retrieving both
>> signature and revision. IOW logic needing the signature only doesn't really
>> matter here (and the sentence made me hunt for cases where we would read just
>> the signature, aiming at verifying that leaving the revision field unset
>> would indeed not be a problem).
> 
> It probably doesn't come as a surprise that I'm intending to rework
> collect_cpu_info() entirely.  It's a mess.
> 
> The signature and platform flags are invariants for a CPU.  (In fact,
> Platform Flags had better be the same for an entire system).  The
> revision does change with type, but apply_microcode() keeps it up to date.
> 
> Yet we had logic which was throwing the details away and re-gathering
> (which is quite expensive) for basically every microcode operation.
> 
> 
> What I'm trying to express is "this information is collected once at the
> start of day, and kept up to date, so collect_cpu_info() should not be
> called under any other circumstance".
> 
> Perhaps I should just say that directly?

That may be a good thing, yes. The main point still being though that the
way that 1st sentence in the paragraph was written, it ended up confusing.

Jan