linux-kernel - RE: [PATCH] x86/cpu/intel: Fix MTRR verification for TME enabled platforms

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SA1PR11MB67344EB62BF798C9FA991552A8FEA@SA1PR11MB6734.namprd11.prod.outlook.com>
Date:   Sat, 23 Sep 2023 07:37:12 +0000
From:   "Li, Xin3" <xin3.li@...el.com>
To:     "Compostella, Jeremy" <jeremy.compostella@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: RE: [PATCH] x86/cpu/intel: Fix MTRR verification for TME enabled
 platforms

Hi Jeremy,

> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index be4045628fd3..34c54432bf00 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -184,6 +184,90 @@ static bool bad_spectre_microcode(struct cpuinfo_x86 *c)
>  	return false;
>  }
> 
> +#define MSR_IA32_TME_ACTIVATE		0x982

I know you're just moving the definitions, however we usually define MSRs
and their bits in arch/x86/include/asm/msr-index.h.

> +
> +/* Helpers to access TME_ACTIVATE MSR */
> +#define TME_ACTIVATE_LOCKED(x)		(x & 0x1)
> +#define TME_ACTIVATE_ENABLED(x)		(x & 0x2)

What about:

#define TME_ACTIVATE_LOCKED(x)		(x & BIT(0))
#define TME_ACTIVATE_ENABLED(x)		(x & BIT(1))

> +
> +#define TME_ACTIVATE_POLICY(x)		((x >> 4) & 0xf)	/* Bits 7:4 */

And:

/* Bits 7:4 are TME activate policy bits */
#define TME_ACTIVATE_POLICY_OFFSET		4
#define TME_ACTIVATE_POLICY_MASK		0xf
#define TME_ACTIVATE_POLICY(x)						\
	((x >> TME_ACTIVATE_POLICY_OFFSET) & TME_ACTIVATE_POLICY_MASK)

> +#define TME_ACTIVATE_KEYID_BITS(x)	((x >> 32) & 0xf)	/* Bits 35:32 */
> +
> +#define TME_ACTIVATE_CRYPTO_ALGS(x)	((x >> 48) & 0xffff)	/* Bits
> 63:48 */

ditto

> @@ -335,6 +419,9 @@ static void early_init_intel(struct cpuinfo_x86 *c)
>  	 */
>  	if (detect_extended_topology_early(c) < 0)
>  		detect_ht_early(c);
> +

Please add a comment here explaining why detect_tme() needs to be called
in early_init_intel().

> +	if (cpu_has(c, X86_FEATURE_TME))
> +		detect_tme(c);
>  }
> 
>  static void bsp_init_intel(struct cpuinfo_x86 *c)

Thanks!
    Xin