From: "Huang, Kai" <kai.huang@intel.com>
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Dave Hansen <dave.hansen@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>,
<linux-kernel@vger.kernel.org>, <pbonzini@redhat.com>,
<tglx@linutronix.de>, <x86@kernel.org>, <bp@alien8.de>
Subject: Re: [RFC][PATCH 11/34] x86/cpu/intel: Prepare MKTME for "address configuration" infrastructure
Date: Wed, 28 Feb 2024 10:48:19 +1300 [thread overview]
Message-ID: <b32cdda4-5cca-4608-b403-30ab6ce668de@intel.com> (raw)
In-Reply-To: <wtdmrkjfvlf4b5mkpqw537u6xuxkdajix2knbo5ivanjzzpvvg@qqlw7gaetujj>
On 27/02/2024 1:14 am, Kirill A. Shutemov wrote:
> On Fri, Feb 23, 2024 at 08:22:16AM -0800, Dave Hansen wrote:
>> On 2/23/24 03:33, Kirill A. Shutemov wrote:
>>> On Thu, Feb 22, 2024 at 10:39:41AM -0800, Dave Hansen wrote:
>>>> From: Dave Hansen <dave.hansen@linux.intel.com>
>>>>
>>>> Intel also does memory encryption and also fiddles with the physical
>>>> address bits. This is currently called for *each* CPU, but practically
>>>> only done on the boot CPU because of 'mktme_status'.
>>>>
>>>> Move it from the "each CPU" ->c_init() function to ->c_bsp_init() where
>>>> the whole thing only gets called once ever. This also necessitates moving
>>>> detect_tme() and its entourage around in the file.
>>> The state machine around mktme_state doesn't make sense if we only call it
>>> on boot CPU, so detect_tme() can be drastically simplified. I can do it on
>>> top of the patchset.
>>
>> That would be great. Looking at it again, the (tme_activate !=
>> tme_activate_cpu0) block is total cruft now. It probably just needs to
>> get moved to secondary CPU startup.
>
> I have never saw the check to be useful. I think it can be just dropped.
>
> The patch below makes detect_tme() only enumerate TME and MKTME. And
> report number of keyid bits. Kernel doesn't care about anything else.
>
> Any comments?
>
> From 1080535093d21f025d46fd610de5ad788591f4b5 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Date: Mon, 26 Feb 2024 14:01:01 +0200
> Subject: [PATCH] x86/cpu/intel: Simplify detect_tme()
>
> The detect_tme() function is now only called by the boot CPU. The logic
> for cross-checking TME configuration between CPUs is no longer used. It
> has never identified a real problem and can be safely removed.
>
> The kernel does not use MKTME and is not concerned with MKTME policy or
> encryption algorithms. There is no need to check them.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
> arch/x86/kernel/cpu/intel.c | 44 ++-----------------------------------
> 1 file changed, 2 insertions(+), 42 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 4192aa4576f4..60918b49344c 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -329,55 +329,20 @@ static void early_init_intel(struct cpuinfo_x86 *c)
> #define TME_ACTIVATE_CRYPTO_ALGS(x) ((x >> 48) & 0xffff) /* Bits 63:48 */
> #define TME_ACTIVATE_CRYPTO_AES_XTS_128 1
>
> -/* Values for mktme_status (SW only construct) */
> -#define MKTME_ENABLED 0
> -#define MKTME_DISABLED 1
> -#define MKTME_UNINITIALIZED 2
> -static int mktme_status = MKTME_UNINITIALIZED;
> -
> static int detect_tme(struct cpuinfo_x86 *c)
> {
> - u64 tme_activate, tme_policy, tme_crypto_algs;
> - int keyid_bits = 0, nr_keyids = 0;
> - static u64 tme_activate_cpu0 = 0;
> + int keyid_bits, nr_keyids;
> + u64 tme_activate;
>
> rdmsrl(MSR_IA32_TME_ACTIVATE, tme_activate);
>
> - if (mktme_status != MKTME_UNINITIALIZED) {
> - if (tme_activate != tme_activate_cpu0) {
> - /* Broken BIOS? */
> - pr_err_once("x86/tme: configuration is inconsistent between CPUs\n");
> - pr_err_once("x86/tme: MKTME is not usable\n");
> - mktme_status = MKTME_DISABLED;
> -
> - /* Proceed. We may need to exclude bits from x86_phys_bits. */
> - }
> - } else {
> - tme_activate_cpu0 = tme_activate;
> - }
> -
> if (!TME_ACTIVATE_LOCKED(tme_activate) || !TME_ACTIVATE_ENABLED(tme_activate)) {
> pr_info_once("x86/tme: not enabled by BIOS\n");
Since now detect_tme() is only called on BSP, seems we can change to
pr_info(), which is used ...
> - mktme_status = MKTME_DISABLED;
> return 0;
> }
>
> - if (mktme_status != MKTME_UNINITIALIZED)
> - goto detect_keyid_bits;
> -
> pr_info("x86/tme: enabled by BIOS\n");
... right here anyway.
>
> - tme_policy = TME_ACTIVATE_POLICY(tme_activate);
> - if (tme_policy != TME_ACTIVATE_POLICY_AES_XTS_128)
> - pr_warn("x86/tme: Unknown policy is active: %#llx\n", tme_policy);
> -
> - tme_crypto_algs = TME_ACTIVATE_CRYPTO_ALGS(tme_activate);
> - if (!(tme_crypto_algs & TME_ACTIVATE_CRYPTO_AES_XTS_128)) {
> - pr_err("x86/mktme: No known encryption algorithm is supported: %#llx\n",
> - tme_crypto_algs);
> - mktme_status = MKTME_DISABLED;
> - }
> -detect_keyid_bits:
> keyid_bits = TME_ACTIVATE_KEYID_BITS(tme_activate);
> nr_keyids = (1UL << keyid_bits) - 1;
> if (nr_keyids) {
> @@ -387,11 +352,6 @@ static int detect_tme(struct cpuinfo_x86 *c)
> pr_info_once("x86/mktme: disabled by BIOS\n");
> }
>
> - if (mktme_status == MKTME_UNINITIALIZED) {
> - /* MKTME is usable */
> - mktme_status = MKTME_ENABLED;
> - }
> -
> return keyid_bits;
Other than that the code change LGTM.
Reviewed-by: Kai Huang <kai.huang@intel.com>
next prev parent reply other threads:[~2024-02-27 21:48 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-22 18:39 [RFC][PATCH 00/34] [RFC] x86: Rework system-wide configuration masquerading as per-cpu data Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 01/34] x86/xen: Explain why calling get_cpu_cap() so early is a hack Dave Hansen
2024-03-07 16:27 ` Borislav Petkov
2024-02-22 18:39 ` [RFC][PATCH 02/34] x86/xen: Remove early "debug" physical address lookups Dave Hansen
2024-03-07 17:01 ` Borislav Petkov
2024-03-11 13:16 ` Jürgen Groß
2024-02-22 18:39 ` [RFC][PATCH 03/34] x86/pci: Assume that clflush size is always provided Dave Hansen
2024-03-08 8:57 ` Borislav Petkov
2024-02-22 18:39 ` [RFC][PATCH 04/34] x86/mm: Introduce physical address limit helper Dave Hansen
2024-02-27 11:05 ` Huang, Kai
2024-02-22 18:39 ` [RFC][PATCH 05/34] x86/cpu: Move /proc per-cpu ->x86_phys_bits reference to global one Dave Hansen
2024-02-27 11:05 ` Huang, Kai
2024-02-22 18:39 ` [RFC][PATCH 06/34] x86/boot: Use consistent value for iomem_resource.end Dave Hansen
2024-02-27 10:59 ` Huang, Kai
2024-02-28 14:22 ` Zhang, Rui
2024-02-22 18:39 ` [RFC][PATCH 07/34] x86/mm: Introduce virtual address space limit helper Dave Hansen
2024-02-27 11:09 ` Huang, Kai
2024-02-22 18:39 ` [RFC][PATCH 08/34] x86/cpu: Add CLFLUSH size helper Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 09/34] x86/cpu: Introduce address configuration structure Dave Hansen
2024-02-27 23:47 ` Huang, Kai
2024-02-28 17:29 ` Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 10/34] x86/cpu/amd: Use new "address configuration" infrastructure Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 11/34] x86/cpu/intel: Prepare MKTME for " Dave Hansen
2024-02-23 11:33 ` Kirill A. Shutemov
2024-02-23 16:22 ` Dave Hansen
2024-02-26 12:14 ` Kirill A. Shutemov
2024-02-27 21:48 ` Huang, Kai [this message]
2024-02-28 15:20 ` Kirill A. Shutemov
2024-02-28 16:57 ` Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 12/34] x86/cpu/intel: Actually use "address configuration" infrastructure for MKTME Dave Hansen
2024-02-23 11:41 ` Kirill A. Shutemov
2024-02-23 16:16 ` Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 13/34] x86/boot: Use address reduction config to handle erratum Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 14/34] x86/cpu: Remove default physical address space settings Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 15/34] x86/cpu: Remove default x86_phys_bits assignment Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 16/34] x86/cpu: Move physical address limit out of cpuinfo_x86 Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 17/34] x86/cpu: Move virtual " Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 18/34] x86/cpu/centaur: Move cache alignment override to BSP init Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 19/34] x86/cpu: Introduce cache alignment multiplier Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 20/34] x86/cpu: Remove superfluous cache alignment assignments Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 21/34] x86/cpu: Consolidate CLFLUSH size setting Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 22/34] x86/cpu: Move CLFLUSH size into global config Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 23/34] x86/cpu: Move cache alignment configuration to global struct Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 24/34] x86/cpu: Establish 'min_cache_bits' configuration Dave Hansen
2024-02-22 18:39 ` [RFC][PATCH 25/34] x86/cpu: Move cache bits to global config Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 26/34] x86/cpu: Zap superfluous get_cpu_address_sizes() Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 27/34] x86/cpu: Enforce read-only x86_config state (lightly) Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 28/34] x86/cpu: Return sane defaults for early x86_config reads Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 29/34] x86/xen: Remove extra get_cpu_address_sizes() call Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 30/34] x86/cpu/centaur: Mark BSP init function as __init Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 31/34] x86/cpu/intel: " Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 32/34] x86/cpu/amd: Move memory encryption detection Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 33/34] x86/cpu: Make get_cpu_address_sizes() static and __init Dave Hansen
2024-02-22 18:40 ` [RFC][PATCH 34/34] x86/cpu: Mark new boot CPU and config structures appropriately Dave Hansen
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=b32cdda4-5cca-4608-b403-30ab6ce668de@intel.com \
--to=kai.huang@intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--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).