Messages in this thread | | | Date | Thu, 7 Sep 2023 00:17:18 +0200 | From | Ingo Molnar <> | Subject | Re: [patch 1/2] x86/microcode/32: Move early loading after paging enable |
| |
* Thomas Gleixner <tglx@linutronix.de> wrote:
> 32-bit loads microcode before paging is enabled. The commit which > introduced that has zero justification in the changelog. The cover letter > has slightly more content, but it does not give any technical justification > either: > > "The problem in current microcode loading method is that we load a > microcode way, way too late; ideally we should load it before turning > paging on. This may only be practical on 32 bits since we can't get to > 64-bit mode without paging on, but we should still do it as early as at > all possible." > > Handwaving word salad with zero technical content. > > Someone claimed in an offlist conversation that this is required for curing > the ATOM erratum AAE44/AAF40/AAG38/AAH41. That erratum requires an > microcode update in order to make the usage of PSE safe. But during early > boot PSE is completely irrelevant and it is evaluated way later. > > Neither is it relevant for the AP on single core HT enabled CPUs as the > microcode loading on the AP is not doing anything. > > On dual core CPUs there is a theoretical problem if a split of an > executable large page between enabling paging including PSE and loading the > microcode happens. But that's only theoretical, it's practically irrelevant > because the affected dual core CPUs are 64bit enabled and therefore have > paging and PSE enabled before loading the microcode on the second core. So > why would it work on 64-bit but not on 32-bit? > > The erratum: > > "AAG38 Code Fetch May Occur to Incorrect Address After a Large Page is > Split Into 4-Kbyte Pages > > Problem: If software clears the PS (page size) bit in a present PDE > (page directory entry), that will cause linear addresses mapped through > this PDE to use 4-KByte pages instead of using a large page after old > TLB entries are invalidated. Due to this erratum, if a code fetch uses > this PDE before the TLB entry for the large page is invalidated then it > may fetch from a different physical address than specified by either the > old large page translation or the new 4-KByte page translation. This > erratum may also cause speculative code fetches from incorrect addresses." > > The practical relevance for this is exactly zero because there is no > splitting of large text pages during early boot-time, i.e. between paging > enable and microcode loading, and neither during CPU hotplug. > > IOW, this load microcode before paging enable is yet another voodoo > programming solution in search of a problem. What's worse is that it causes > at least two serious problems: > > 1) When stackprotector is enabled then the microcode loader code has the > stackprotector mechanics enabled. The read from the per CPU variable > __stack_chk_guard is always accessing the virtual address either > directly on UP or via FS on SMP. In physical address mode this results > in an access to memory above 3GB. So this works by chance as the > hardware returns the same value when there is no RAM at this physical > address. When there is RAM populated above 3G then the read is by > chance the same as nothing changes that memory during the very early > boot stage. That's not necessarily true during runtime CPU hotplug. > > 2) When function tracing is enabled, then the relevant microcode loader > functions and the functions invoked from there will call into the > tracing code and evaluate global and per CPU variables in physical > address mode. What could potentially go wrong? > > Cure this and move the microcode loading after the early paging enable and > remove the gunk in the microcode loader which is required to handle > physical address mode. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: Fenghua Yu <fenghua.yu@intel.com> > Cc: Peter Anvin <hpa@zytor.com> > Link: https://lore.kernel.org/lkml/1356075872-3054-1-git-send-email-fenghua.yu@intel.com > --- > arch/x86/kernel/cpu/microcode/amd.c | 31 +------- > arch/x86/kernel/cpu/microcode/core.c | 40 ++--------- > arch/x86/kernel/cpu/microcode/intel.c | 108 +++---------------------------- > arch/x86/kernel/cpu/microcode/internal.h | 2 > arch/x86/kernel/head32.c | 3 > arch/x86/kernel/head_32.S | 10 -- > arch/x86/kernel/smpboot.c | 12 +-- > 7 files changed, 35 insertions(+), 171 deletions(-)
Reviewed-by: Ingo Molnar <mingo@kernel.org>
Frankly, the general principle is that we should run as little Linux kernel code before paging is enabled as possible.
If a system is so broken that it requires a microcode update before it can even enable paging is IMO so terminally broken that it should be firmware patched at the factory (to have a proper microcode), or recalled from the market. It's not something we should make the kernel more fragile for.
Thanks,
Ingo
| |