| Message ID | 20241112211915.1473121-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/ucode: Simplify/fix loading paths further | expand |
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@citrix.com> Preferably with the problematic sentence dropped or clarified: Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index fd4b08b45388..5897ec54032a 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -189,8 +189,6 @@ static struct patch_with_flags nmi_patch = */ static struct microcode_patch *parse_blob(const char *buf, size_t len) { - alternative_vcall(ucode_ops.collect_cpu_info); - return alternative_call(ucode_ops.cpu_request_microcode, buf, len, true); }
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@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/cpu/microcode/core.c | 2 -- 1 file changed, 2 deletions(-)