| Message ID | 20241107122117.4073266-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/ucode: Simplify/fix loading paths further | expand |
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@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > > 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. ~Andrew
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> 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. --- xen/arch/x86/cpu/microcode/core.c | 2 -- 1 file changed, 2 deletions(-)