linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Ashok Raj <ashok.raj@intel.com>, Borislav Petkov <bp@alien8.de>
Cc: Ashok Raj <ashok.raj@intel.com>,
	LKML <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Tony Luck <tony.luck@intel.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Stefan Talpalaru <stefantalpalaru@yahoo.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Jonathan Corbet <corbet@lwn.net>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Peter Zilstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>
Subject: Re: [PATCH v1 Part2 2/5] x86/microcode/intel: Add minimum required revision to microcode header
Date: Thu, 19 Jan 2023 23:03:14 +0100	[thread overview]
Message-ID: <873586i3ml.ffs@tglx> (raw)
In-Reply-To: <20230113172920.113612-3-ashok.raj@intel.com>

On Fri, Jan 13 2023 at 09:29, Ashok Raj wrote:
> In general users don't have the necessary information to determine
> whether a late loading of a new microcode version has removed any feature
> (MSR, CPUID etc) between what is currently loaded and this new microcode.

s/this new microcode/a newer microcode revision/

> To address this issue, Intel has added a "minimum required version" field
> to a previously reserved field in the file header. Microcode updates

s/file header/microcode header/ perhaps?

> should only be applied if the current microcode version is equal
> to, or greater than this minimum required version.
>
> Thomas made some suggestions[1] on how meta-data in the microcode file
> could provide Linux with information to decide if the new microcode is
> suitable candidate for late loading. But even the "simpler" option#1
> requires a lot of metadata and corresponding kernel code to parse it.
>
> The proposal here is an even simpler option.

IIRC this was also suggested by this Thomas dude, right?

> Simply "OS visible features" such as CPUID and MSRs are the only two
> examples. The microcode must not change these OS visible features
> because they cause problems after late loading. When microcode changes
> features, microcode will change the min_rev to prevent such microcodes
> from being late loaded.
>
> Pseudo code for late loading is as follows:
>
> if header.min_required_id == 0
>         This is old format microcode, block late loading
> else if current_ucode_version < header.min_required_id
>         Current version is too old, block late loading of this microcode.
> else
>         OK to proceed with late loading.
>
> Any microcode that modifies the interface to an OS-visible feature
> will set the min_version to itself. This will enforce this microcode is
> not suitable for late loading unless the currently loaded revision is
> greater or equal to the new microcode affecting the change.

Up to this paragraph the changelog made sense.

If the currently loaded revision is the same as the to be loaded
revision, then there is nothing to do.

If the currently loaded revision is greater than the to be loaded
revision then it is not loaded as the kernel does not support
downgrading in the first place.

Even if it would support downgrading then this would be outright wrong
for this case:

Rev:        10
Min-Rev:    10

Rev:        20
Min-Rev:    20

If Rev 20 is loaded, then you absolutely cannot load Rev 10 because that
would have the reverse side effects due to which Rev 20 prevents late
loading.

See?

Thanks,

        tglx

  reply	other threads:[~2023-01-19 22:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13 17:29 [PATCH v1 Part2 0/5] Declare safe late loadable microcode Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 1/5] x86/microcode: Move late load warning to the same function that taints kernel Ashok Raj
2023-01-19 21:48   ` Thomas Gleixner
2023-01-19 22:05     ` Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 2/5] x86/microcode/intel: Add minimum required revision to microcode header Ashok Raj
2023-01-19 22:03   ` Thomas Gleixner [this message]
2023-01-19 22:34     ` Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 3/5] x86/microcode: Add a generic mechanism to declare support for minrev Ashok Raj
2023-01-20  0:15   ` Thomas Gleixner
2023-01-20  2:19     ` Ashok Raj
2023-01-21 21:35     ` [Part 2 v2[cleanup] 0/4] Some additional cleanups in microcode Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 1/4] x86/microcode: Taint kernel only if microcode loading was successful Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 2/4] x86/microcode: Report invalid writes to reload sysfs file Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 3/4] x86/microcode/intel: Fix collect_cpu_info() to reflect current microcode Ashok Raj
2023-01-21 21:35       ` [Part 2 v2[cleanup] 4/4] x86/microcode: Do not call apply_microde() on sibling threads Ashok Raj
2023-01-22  3:36         ` Ashok Raj
2023-01-26 16:52       ` [Part 2 v2[cleanup] 0/4] Some additional cleanups in microcode Ashok Raj
2023-01-27 13:49         ` Borislav Petkov
2023-01-13 17:29 ` [PATCH v1 Part2 4/5] x86/microcode/intel: Drop wbinvd() from microcode loading Ashok Raj
2023-01-13 17:29 ` [PATCH v1 Part2 5/5] x86/microcode: Provide an option to override minrev enforcement Ashok Raj

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=873586i3ml.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=Andrew.Cooper3@citrix.com \
    --cc=alison.schofield@intel.com \
    --cc=ashok.raj@intel.com \
    --cc=benh@kernel.crashing.org \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=dave.hansen@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=reinette.chatre@intel.com \
    --cc=stefantalpalaru@yahoo.com \
    --cc=thomas.lendacky@amd.com \
    --cc=tony.luck@intel.com \
    --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).