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

[Jan Beulich <jbeulich@xxxxxxxx>]

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. In the
course of determining that I'm afraid I also found the first sentence of this
paragraph rather misleading than helpful: 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).

>  For
> good measure, the apply_microcode() hooks also keep the revision correct as
> load attempts are made.
> 
> This finally gets us down to a single call per CPU on boot / S3 resume, and no
> calls during late-load hypercalls.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Preferably with the problematic sentence dropped or clarified:
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan