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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/11/2024 9:58 pm, Andrew Cooper wrote:
> On 07/11/2024 12:21 pm, 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().  
>> 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>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>
>> Slightly RFC.
>>
>> Just before posting, I've realised that cpu_request_microcode() does actually
>> use the current CPU revision, and it's buggy, and it's the cause of 
>> `xen-ucode
>> --force` not working as expected.
>>
>> I'm tempted to do another series cleaning that up in isolation, such that 
>> this
>> patch becomes true in this form.
> Actually no.  Having tried a bit, I think it's easier to do with patch 2
> already in place.
>
> So instead I'm tempted to edit the middle paragraph to note that it
> currently uses the revision but that's going to be fixed shortly.  The
> rest of the paragraph explains why it's still safe anyway.

So, after the latter series, this patch happens to be accurate.

cpu_request_microcode() does read the revision, but discards the result
of the calculation which used it.

~Andrew