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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.11.2024 11:57, Andrew Cooper wrote:
> On 12/11/2024 10:49 am, Jan Beulich wrote:
>> On 12.11.2024 11:36, Andrew Cooper wrote:
>>> 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.
>> What's the intended overall sequence of patches then? With two series that
>> (aiui) now have grown some sort of dependency, and with this series have
>> gained a 4/3 patch, having a clear picture would certainly help. Might it
>> be best if you merge both series and re-submit as a single one?
> 
> The order turns out to be as emailed out and threaded.

Yet above you said "after the latter series, this patch happens to be accurate."
Which suggest to me that at least part of the latter series needs to be in place
for the change here to be correct. IOW - I'm confused now.

Jan