[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] x86, amd_ucode: Verify max allowed patch size before apply
On 4/28/2014 3:49 AM, Jan Beulich wrote: Okay, So let me just modify the patch to make sure I don't change original behavior.On 25.04.14 at 22:30, <andrew.cooper3@xxxxxxxxxx> wrote:On 25/04/14 20:48, Aravind Gopalakrishnan wrote:Could someone please help me understand why there are two initcalls - 'microcode_presmp_init' && 'microcode_init' that perform the same tasks? It results in these functions - 'collect_cpu_info', 'cpu_request_microcode' to run a second time through 'microcode_init' when we have already accomplished the job of updating cpu with microcode patches through 'microcode_presmp_init' and 'microcode_resume_cpu' If there is particular reason for 'microcode_init''s existence, then I'll modify the patch so that the error handling around 'microcode_fits' is not altered. But if not, then I simply have to just remove the 'break' statement.Good grief this looks mad. Ignoring for a moment what the code actually does, lets consider what it should do. The administrator has an option of providing a microcode blob in a multiboot module, and instructing Xen where to look. In this case if a valid blob is found, Xen will want to load the microcode ASAP and cache it for the other processors. This can be done as early as is suitable on the BSP. The administrator has an option of providing a microcode blob by hypercall (either automatically as part of their kernel microcode patching code, or perhaps from a userspace tool in dom0). For a valid blob, Xen will want to cache it and apply to all active processors. This can be done by whichever cpu processes the hypercall. At any point, a new cpu coming online (AP boot, S3 etc) should look at the cached microcode and try to load it. So it would occur to me to have something such as as presmp initcall which sets up a cpunotifier for any cpu coming online. It checks to see whether there are any blobs provided at boot, tries to load them and caches the best result. The hypercall tries to apply the blob from userspace and if successful, caches the new microcode and instructs all other cpus to load this new microcode. I dont see a need for two initcalls in the slightest, nor do I see a need for using tasklets on boot. If the earlier initcall hasn't found any microcode in a multiboot module then there will be nothing for the latter one to do. If the earlier did find microcode, then it should be applied automatically whenever a new cpu comes up, rather than when explicitly prodded by the later initcall.The code indeed looks more complicated than one would think it needs to be, but simplifying it would likely require adding another, later CPU notification that is being issued _on_ the CPU being brought up, or an IPI issued from e.g. the CPU_ONLINE callback (but I'm rather uncertain about it being a good idea to do the ucode update in an interrupt handler). The whole purpose of the later initcall and the use of a tasklet there is to have a vehicle to get code executed _on_ the new CPU, with CPU_STARTING being too early to hook onto. and worry about simplifying this code at a later time.. Will send out a V2.. Thanks Jan, Andrew -Aravind _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |