Re: [Xen-devel] [PATCH for-4.9 v2] x86: avoid #GP for PV guest MSR accesses

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/10/17 08:48, Jan Beulich wrote:
> Halfway recent Linux kernels probe MISC_FEATURES_ENABLES on all CPUs,
> leading to ugly recovered #GP fault messages with debug builds on older
> systems. We can do better, so introduce synthetic feature flags for
> both this and PLATFORM_INFO to avoid the rdmsr_safe() altogether.
>
> Note that the r/o nature of PLATFORM_INFO is now also being enforced.
>
> The rdmsr_safe() uses for MISC_ENABLE are left in place as benign - it
> exists for all 64-bit capable Intel CPUs (see e.g. early_init_intel()).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Force PLATFORM_INFO writes to fail.
>
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -21,10 +21,19 @@ static bool __init probe_intel_cpuid_fau
>  {
>       uint64_t x;
>  
> -     if (rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) ||
> -         !(x & MSR_PLATFORM_INFO_CPUID_FAULTING))
> +     if (rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x))
>               return 0;
>  
> +     setup_force_cpu_cap(X86_FEATURE_MSR_PLATFORM_INFO);
> +
> +     if (!(x & MSR_PLATFORM_INFO_CPUID_FAULTING)) {
> +             if (!rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES, x))
> +                     setup_force_cpu_cap(X86_FEATURE_MSR_MISC_FEATURES);
> +             return 0;
> +     }
> +
> +     setup_force_cpu_cap(X86_FEATURE_MSR_MISC_FEATURES);

Why this odd arrangement with the double setup_force_cpu_cap()?  As
neither of these MSRs are architectural, would it not be better to probe
both of them, rather than assuming MISC_FEATURES_ENABLES is available if
faulting is reported?

Otherwise, everything else looks ok.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel