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

[Li, Xin3]

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