linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Borislav Petkov <bp@alien8.de>,
	x86@kernel.org, Ashok Raj <ashok.raj@intel.com>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>, Peter Anvin <hpa@zytor.com>
Subject: [patch 0/2] x86/microcode: Make 32-bit early loading robust and correct
Date: Tue, 22 Aug 2023 14:20:07 +0200 (CEST)	[thread overview]
Message-ID: <20230822121936.476984181@linutronix.de> (raw)

While working on the microcode loader I stumbled over the 32bit early
loading mechanism which runs before paging enable in physical address mode.

Aside of being a source of code horror and debug trouble, it also turned
out that this is fundamentally broken vs. stackprotector and tracing:

 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?

While it would be possible to mark all the involved functions to be non
instrumentable and excluded from stack protector, the obvious question
arose why this is necessary at all.

The changelog of the commit which introduced this is completely useless to
answer this question and the cover letter consists mostly of handwaving
word salad without technical content:

  "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."

So after asking around, it turned out that this was implemented to handle
the erratum of early ATOM CPUs which affects PSE.

So I sat down and did a proper technical analysis of that erratum and the
resulting requirements. It turns out that the early loading before paging
is enabled is just a pointless exercise due to lack of a thorough analysis.

More details about the reasons why this is a pointless exercise can be
found in patch 1/2.

Patch 2/2 is not microcode related, but the fallout of the analysis. It's
the only other C-function invoked on 32-bit before paging is enabled and it
is obviously affected by the stack protector issue as well and on top lacks
the removal of tracing.

The series applies on top of

    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/microcode

Thanks,

	tglx
---
 Makefile                 |    1 
 cpu/microcode/amd.c      |   31 ++-----------
 cpu/microcode/core.c     |   40 +++--------------
 cpu/microcode/intel.c    |  108 +++++------------------------------------------
 cpu/microcode/internal.h |    2 
 head32.c                 |    6 ++
 head_32.S                |   10 ----
 smpboot.c                |   12 +----
 8 files changed, 38 insertions(+), 172 deletions(-)


             reply	other threads:[~2023-08-22 12:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-22 12:20 Thomas Gleixner [this message]
2023-08-22 12:20 ` [patch 1/2] x86/microcode/32: Move early loading after paging enable Thomas Gleixner
2023-08-23 10:16   ` [patch V2 " Thomas Gleixner
2023-09-06 22:17   ` [patch " Ingo Molnar
2023-09-06 22:18     ` Ingo Molnar
2023-08-22 12:20 ` [patch 2/2] x86/boot/32: Disable stackprotector and tracing for mk_early_pgtbl_32() Thomas Gleixner
2023-09-06 22:17   ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230822121936.476984181@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=arjan@linux.intel.com \
    --cc=ashok.raj@intel.com \
    --cc=bp@alien8.de \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).