* [PATCH] CPU hotplug, x86 microcode: Revalidate microcode before microcode update @ 2011-10-19 17:16 Srivatsa S. Bhat 2011-10-20 12:31 ` Borislav Petkov 0 siblings, 1 reply; 3+ messages in thread From: Srivatsa S. Bhat @ 2011-10-19 17:16 UTC (permalink / raw) To: Borislav Petkov Cc: Tejun Heo, Rafael J. Wysocki, tglx@linutronix.de, tigran@aivazian.fsnet.co.uk, mingo@elte.hu, hpa@zytor.com, x86@kernel.org, Linux PM mailing list, linux-kernel Hi, Boris, you had mentioned that hpa's new ucode loading solution would take care of hot-swap cases too. I am referring to: http://thread.gmane.org/gmane.linux.documentation/3414/focus=3454 However, since I am not aware of what hpa's new ucode loading solution is, nor how it will handle this case, I thought of posting this patch (now tested and fine-tuned), so that it can be applied in case this patch is deemed to be necessary/desirable to avoid breaking hot-swap cases. Thanks, Srivatsa S. Bhat --- This patch is intended to go with the patch posted at http://thread.gmane.org/gmane.linux.kernel/1200882/focus=1200991 Since that patch optimizes microcode update by keeping the microcode image in kernel memory and not freeing it up even when the CPU goes offline, it means we would be applying the same microcode image when the CPU comes back online. However, if physically hotplugging of CPUs is supported, then we could physically hot-unplug CPUs and then hot-plug new (and possibly a different type of) CPUs. In such a case, it would be wrong to blindly apply the same old microcode (that the kernel maintained for the old CPU) to this new CPU. Hence, this patch adds a validation to check whether the microcode revision we have in kernel memory and the revision that needs to be applied to the CPU that was just brought online, are the same or not. If they happen to be different, then it invalidates and frees the kernel's copy of the microcode, triggering a fresh request to userspace to get the appropriate microcode image for this CPU. Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> --- arch/x86/kernel/microcode_core.c | 35 +++++++++++++++++++++++++++++++++++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/arch/x86/kernel/microcode_core.c b/arch/x86/kernel/microcode_core.c index f924280..1b353a3 100644 --- a/arch/x86/kernel/microcode_core.c +++ b/arch/x86/kernel/microcode_core.c @@ -155,6 +155,30 @@ static int collect_cpu_info(int cpu) return ret; } +/* + * Compare the microcode revision that the kernel has in memory + * for this cpu and the microcode revision that we need to apply + * on this cpu. If they match, return 0, else return -1. + */ +static int compare_cpu_with_microcode(int cpu) +{ + struct ucode_cpu_info *uci_mem = ucode_cpu_info + cpu; + struct ucode_cpu_info uci_cpu; + int ret; + + memset(&uci_cpu, 0, sizeof(uci_cpu)); + + ret = collect_cpu_info_on_target(cpu, &uci_cpu.cpu_sig); + if (!ret) { + if (!(uci_mem->cpu_sig.sig == uci_cpu.cpu_sig.sig && + uci_mem->cpu_sig.pf == uci_cpu.cpu_sig.pf && + uci_mem->cpu_sig.rev == uci_cpu.cpu_sig.rev)) + ret = -1; + } + + return ret; +} + struct apply_microcode_ctx { int err; }; @@ -397,6 +421,17 @@ static enum ucode_state microcode_update_cpu(int cpu) struct ucode_cpu_info *uci = ucode_cpu_info + cpu; enum ucode_state ustate; + /* + * If the CPU on which we are going to update the + * microcode and the microcode which we currently + * have in kernel memory are incompatible, then + * invalidate the microcode that we have (and also + * free its memory), so that we can get the required + * microcode afresh. + */ + if (compare_cpu_with_microcode(cpu)) + microcode_fini_cpu(cpu); + if (uci->valid) ustate = microcode_resume_cpu(cpu); else ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] CPU hotplug, x86 microcode: Revalidate microcode before microcode update 2011-10-19 17:16 [PATCH] CPU hotplug, x86 microcode: Revalidate microcode before microcode update Srivatsa S. Bhat @ 2011-10-20 12:31 ` Borislav Petkov 2011-10-20 15:20 ` Srivatsa S. Bhat 0 siblings, 1 reply; 3+ messages in thread From: Borislav Petkov @ 2011-10-20 12:31 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Tejun Heo, Rafael J. Wysocki, tglx@linutronix.de, tigran@aivazian.fsnet.co.uk, mingo@elte.hu, hpa@zytor.com, x86@kernel.org, Linux PM mailing list, linux-kernel Hi Srivatsa, On Wed, Oct 19, 2011 at 01:16:41PM -0400, Srivatsa S. Bhat wrote: > However, since I am not aware of what hpa's new ucode loading solution > is, nor how it will handle this case, I thought of posting this patch > (now tested and fine-tuned), Hehe, please don't tell me you tested it by really hotswapping an x86 CPU :-). > so that it can be applied in case this > patch is deemed to be necessary/desirable to avoid breaking hot-swap cases. > > Thanks, > Srivatsa S. Bhat > > --- > > This patch is intended to go with the patch posted at > http://thread.gmane.org/gmane.linux.kernel/1200882/focus=1200991 > > Since that patch optimizes microcode update by keeping the microcode image > in kernel memory and not freeing it up even when the CPU goes offline, it > means we would be applying the same microcode image when the CPU comes back > online. > > However, if physically hotplugging of CPUs is supported, then we could > physically hot-unplug CPUs and then hot-plug new (and possibly a different > type of) CPUs. In such a case, it would be wrong to blindly apply the same > old microcode (that the kernel maintained for the old CPU) to this new CPU. > > Hence, this patch adds a validation to check whether the microcode revision > we have in kernel memory and the revision that needs to be applied to the > CPU that was just brought online, are the same or not. If they happen to be > different, then it invalidates and frees the kernel's copy of the microcode, > triggering a fresh request to userspace to get the appropriate microcode > image for this CPU. > > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Ok, seriously, I think your patch looks very good and it should be doing what it's supposed to and the commit message is very good in explaining what happens, but, unless there's a real problem it fixes, it is not needed. I know, I know, what about the hotswapping case, you'd say. Well, this is not real, I'd guess hotswapping on x86 is still no more than scribbles on some whiteboard (unless I'm proven wrong, of course) so fixing hypothetical cases is not something we should do - the kernel bloat level is not very low as it is right now and there are _much_ more _real_ issues which could make much better use of your expertise and patches. :-) So, this should probably be x86 maintainers' call, but I, for one, don't think this patch is necessary, IMHO. Thanks. -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] CPU hotplug, x86 microcode: Revalidate microcode before microcode update 2011-10-20 12:31 ` Borislav Petkov @ 2011-10-20 15:20 ` Srivatsa S. Bhat 0 siblings, 0 replies; 3+ messages in thread From: Srivatsa S. Bhat @ 2011-10-20 15:20 UTC (permalink / raw) To: Borislav Petkov Cc: Tejun Heo, Rafael J. Wysocki, tglx@linutronix.de, tigran@aivazian.fsnet.co.uk, mingo@elte.hu, hpa@zytor.com, x86@kernel.org, Linux PM mailing list, linux-kernel On 10/20/2011 06:01 PM, Borislav Petkov wrote: > Hi Srivatsa, > > On Wed, Oct 19, 2011 at 01:16:41PM -0400, Srivatsa S. Bhat wrote: >> However, since I am not aware of what hpa's new ucode loading solution >> is, nor how it will handle this case, I thought of posting this patch >> (now tested and fine-tuned), > > Hehe, please don't tell me you tested it by really hotswapping an x86 > CPU :-). > Of course not :-) I just saw to it that the kernel boots fine, and my usual test case (suspend/resume with other things) works well (as expected...) >> so that it can be applied in case this >> patch is deemed to be necessary/desirable to avoid breaking hot-swap cases. >> >> Thanks, >> Srivatsa S. Bhat >> >> --- >> >> This patch is intended to go with the patch posted at >> http://thread.gmane.org/gmane.linux.kernel/1200882/focus=1200991 >> >> Since that patch optimizes microcode update by keeping the microcode image >> in kernel memory and not freeing it up even when the CPU goes offline, it >> means we would be applying the same microcode image when the CPU comes back >> online. >> >> However, if physically hotplugging of CPUs is supported, then we could >> physically hot-unplug CPUs and then hot-plug new (and possibly a different >> type of) CPUs. In such a case, it would be wrong to blindly apply the same >> old microcode (that the kernel maintained for the old CPU) to this new CPU. >> >> Hence, this patch adds a validation to check whether the microcode revision >> we have in kernel memory and the revision that needs to be applied to the >> CPU that was just brought online, are the same or not. If they happen to be >> different, then it invalidates and frees the kernel's copy of the microcode, >> triggering a fresh request to userspace to get the appropriate microcode >> image for this CPU. >> >> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > > Ok, seriously, I think your patch looks very good and it should be doing > what it's supposed to and the commit message is very good in explaining > what happens, but, unless there's a real problem it fixes, it is not > needed. > > I know, I know, what about the hotswapping case, you'd say. Well, > this is not real, I'd guess hotswapping on x86 is still no more than > scribbles on some whiteboard (unless I'm proven wrong, of course) so > fixing hypothetical cases is not something we should do - the kernel > bloat level is not very low as it is right now and there are _much_ more > _real_ issues which could make much better use of your expertise and > patches. > > :-) > Thank you very much for appreciating my patch :-) I feel honoured! By the way, I am perfectly fine with not getting this patch into the kernel. My only intention in posting this patch was that, in case this becomes necessary for some reason, it will be handy to push into the kernel (since I had already written the patch by the time you indicated this won't really be necessary). And moreover, even I am not much aware of where hotswapping stands on x86, so I thought I'd let experts like you decide whether this patch is needed or not. > So, this should probably be x86 maintainers' call, but I, for one, don't > think this patch is necessary, IMHO. > Sure, thanks for your invaluable feedback! -- Regards, Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Linux Technology Center, IBM India Systems and Technology Lab ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-10-20 15:20 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-19 17:16 [PATCH] CPU hotplug, x86 microcode: Revalidate microcode before microcode update Srivatsa S. Bhat 2011-10-20 12:31 ` Borislav Petkov 2011-10-20 15:20 ` Srivatsa S. Bhat
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).