Re: [PATCH v2 4/4] Add R3MWAIT to CPU features

[Borislav Petkov <bp@suse.de>]

On Wed, Oct 12, 2016 at 02:16:25PM +0200, Grzegorz Andrejczuk wrote:
> Add cpu feature for ring 3 monitor/mwait.
> 
> Change-Id: Iba4d20639efd8d3637d37db9294cbc43a98f009a

Please take your time when incorporating review comments - these
internal commit IDs have no meaning when submitting upstream so please
remove them. I already pointed that out previously...

Also, please do not set v3 immediately but give other people a couple of
days to take a look at those patches and give you review comments.

> Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 2 ++
>  arch/x86/kernel/cpu/common.c       | 3 +++
>  arch/x86/kernel/cpu/scattered.c    | 5 +++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 92a8308..9caf9c4 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -71,6 +71,8 @@
>  #define X86_FEATURE_RECOVERY	( 2*32+ 0) /* CPU in recovery mode */
>  #define X86_FEATURE_LONGRUN	( 2*32+ 1) /* Longrun power control */
>  #define X86_FEATURE_LRTI	( 2*32+ 3) /* LongRun table interface */
> +/* non architectural Intel-defined CPU features not present in CPUID */

No need for that comment - we have other synthetic CPUID bits already.

> +#define X86_FEATURE_PHIR3MWAIT	(2*32+ 4)
>  
>  /* Other features, Linux-defined mapping, word 3 */
>  /* This range is used for feature bits which conflict or are synthesized */
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 93ffaa5..15fe27f 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1108,6 +1108,9 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  #endif
>  	/* The boot/hotplug time assigment got cleared, restore it */
>  	c->logical_proc_id = topology_phys_to_logical_pkg(c->phys_proc_id);
> +
> +	if (cpu_has(c, X86_FEATURE_PHIR3MWAIT))
> +		elf_hwcap2 |= HWCAP2_PHIR3MWAIT;
>  }
>  
>  /*
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 8cb57df..e4ff3d0 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -29,6 +29,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>  	u32 max_level;
>  	u32 regs[4];
>  	const struct cpuid_bit *cb;
> +	u64 misc_thd_enable;
>  
>  	static const struct cpuid_bit cpuid_bits[] = {
>  		{ X86_FEATURE_INTEL_PT,		CR_EBX,25, 0x00000007, 0 },
> @@ -54,4 +55,8 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>  		if (regs[cb->reg] & (1 << cb->bit))
>  			set_cpu_cap(c, cb->feature);
>  	}
> +
> +	rdmsrl(MSR_PHI_MISC_THD_FEATURE, misc_thd_enable);
> +	if ((misc_thd_enable & MSR_PHI_MISC_THD_FEATURE_R3MWAIT) != 0)
> +		set_cpu_cap(c, X86_FEATURE_PHIR3MWAIT);

I didn't realize this bit is not a CPUID bit, sorry.

In that case, you can move that code into init_intel() in
arch/x86/kernel/cpu/intel.c, i.e., vendor-specific code.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--