Re: [PATCH v15 6/9] x86/arch_prctl: Add ARCH_[GET|SET]_CPUID

From: Thomas Gleixner
Date: Tue Mar 14 2017 - 16:38:15 EST


On Sat, 11 Mar 2017, Kyle Huey wrote:
> static void init_intel_misc_features(struct cpuinfo_x86 *c)
> {
> u64 msr;
>
> + if (rdmsrl_safe(MSR_MISC_FEATURES_ENABLES, &msr))
> + return;
> +
> + msr = 0;
> + wrmsrl(MSR_MISC_FEATURES_ENABLES, msr);
> + this_cpu_write(msr_misc_features_shadow, msr);
> +
> if (!rdmsrl_safe(MSR_PLATFORM_INFO, &msr)) {
> if (msr & MSR_PLATFORM_INFO_CPUID_FAULT)
> set_cpu_cap(c, X86_FEATURE_CPUID_FAULT);
> }
>
> probe_xeon_phi_r3mwait(c);

The way you are doing it breaks the ring3 mwait feature because you
overwrite the R3MWAIT bit the first time you update the MSR on context
switch.

> }

What you really want is to fixup the r3mwait part:

static void probe_xeon_phi_r3mwait(c)
{
....
if (ring3mwait_disabled)
return;

set_cpu_cap(c, X86_FEATURE_RING3MWAIT);
this_cpu_or(msr_misc_features_shadow,
1UL << MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);

if (c == &boot_cpu_data)
ELF_HWCAP2 |= HWCAP2_RING3MWAIT;
}

and have a function for cpuid fault:

static void init_cpuid_fault(c)
{
if (rdmsrl_safe(MSR_PLATFORM_INFO, &msr))
return;
if ((msr & MSR_PLATFORM_INFO_CPUID_FAULT))
set_cpu_cap(c, X86_FEATURE_CPUID_FAULT);
}

and then do:

static void init_intel_misc_features(struct cpuinfo_x86 *c)
{
u64 msr;

if (rdmsrl_safe(MSR_MISC_FEATURES_ENABLES, &msr))
return;

/* Clear all MISC features */
this_cpu_write(msr_misc_features_shadow, 0);

/* Check the features and update the shadow control bits */
init_cpuid_fault(c);
probe_xeon_phi_r3mwait(c);

msr_write(MSR_MISC_FEATURE_ENABLES,
this_cpu_read(msr_misc_features_shadow);
}

Hmm?

Thanks,

tglx