* [PATCH] x86: avoid #GP for PV guest MSR accesses
@ 2017-09-22 9:06 Jan Beulich
2017-09-22 10:32 ` Sergey Dyasli
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2017-09-22 9:06 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper
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.
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@suse.com>
--- 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);
+
expected_levelling_cap |= LCAP_faulting;
levelling_caps |= LCAP_faulting;
setup_force_cpu_cap(X86_FEATURE_CPUID_FAULTING);
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -941,8 +941,7 @@ static int read_msr(unsigned int reg, ui
return X86EMUL_OKAY;
case MSR_INTEL_PLATFORM_INFO:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
- rdmsr_safe(MSR_INTEL_PLATFORM_INFO, *val) )
+ if ( !boot_cpu_has(X86_FEATURE_MSR_PLATFORM_INFO) )
break;
*val = 0;
if ( this_cpu(cpuid_faulting_enabled) )
@@ -950,8 +949,7 @@ static int read_msr(unsigned int reg, ui
return X86EMUL_OKAY;
case MSR_INTEL_MISC_FEATURES_ENABLES:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
- rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES, *val) )
+ if ( !boot_cpu_has(X86_FEATURE_MSR_MISC_FEATURES) )
break;
*val = 0;
if ( curr->arch.cpuid_faulting )
@@ -1147,15 +1145,13 @@ static int write_msr(unsigned int reg, u
return X86EMUL_OKAY;
case MSR_INTEL_PLATFORM_INFO:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
- val || rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val) )
+ if ( !boot_cpu_has(X86_FEATURE_MSR_PLATFORM_INFO) || val )
break;
return X86EMUL_OKAY;
case MSR_INTEL_MISC_FEATURES_ENABLES:
- if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
- (val & ~MSR_MISC_FEATURES_CPUID_FAULTING) ||
- rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES, temp) )
+ if ( !boot_cpu_has(X86_FEATURE_MSR_MISC_FEATURES) ||
+ (val & ~MSR_MISC_FEATURES_CPUID_FAULTING) )
break;
if ( (val & MSR_MISC_FEATURES_CPUID_FAULTING) &&
!this_cpu(cpuid_faulting_enabled) )
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -22,3 +22,5 @@ XEN_CPUFEATURE(APERFMPERF, (FSCAPIN
XEN_CPUFEATURE(MFENCE_RDTSC, (FSCAPINTS+0)*32+ 9) /* MFENCE synchronizes RDTSC */
XEN_CPUFEATURE(XEN_SMEP, (FSCAPINTS+0)*32+10) /* SMEP gets used by Xen itself */
XEN_CPUFEATURE(XEN_SMAP, (FSCAPINTS+0)*32+11) /* SMAP gets used by Xen itself */
+XEN_CPUFEATURE(MSR_PLATFORM_INFO, (FSCAPINTS+0)*32+12) /* PLATFORM_INFO MSR present */
+XEN_CPUFEATURE(MSR_MISC_FEATURES, (FSCAPINTS+0)*32+13) /* MISC_FEATURES_ENABLES MSR present */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: avoid #GP for PV guest MSR accesses
2017-09-22 9:06 [PATCH] x86: avoid #GP for PV guest MSR accesses Jan Beulich
@ 2017-09-22 10:32 ` Sergey Dyasli
2017-09-22 10:44 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Sergey Dyasli @ 2017-09-22 10:32 UTC (permalink / raw)
To: JBeulich@suse.com
Cc: Sergey Dyasli, xen-devel@lists.xenproject.org, Andrew Cooper
On Fri, 2017-09-22 at 03:06 -0600, 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.
>
> 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@suse.com>
The intent of this patch (and the related "VMX: PLATFORM_INFO MSR is
r/o") is somewhat intersects with my series "Generic MSR policy:
infrastructure + cpuid_faulting". IMHO it's better to fix MSR-related
issues in the scope of the MSR policy work.
Also, I have one question below.
>
> --- 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);
> +
> expected_levelling_cap |= LCAP_faulting;
> levelling_caps |= LCAP_faulting;
> setup_force_cpu_cap(X86_FEATURE_CPUID_FAULTING);
> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -941,8 +941,7 @@ static int read_msr(unsigned int reg, ui
> return X86EMUL_OKAY;
>
> case MSR_INTEL_PLATFORM_INFO:
> - if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> - rdmsr_safe(MSR_INTEL_PLATFORM_INFO, *val) )
> + if ( !boot_cpu_has(X86_FEATURE_MSR_PLATFORM_INFO) )
> break;
> *val = 0;
> if ( this_cpu(cpuid_faulting_enabled) )
> @@ -950,8 +949,7 @@ static int read_msr(unsigned int reg, ui
> return X86EMUL_OKAY;
>
> case MSR_INTEL_MISC_FEATURES_ENABLES:
> - if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> - rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES, *val) )
> + if ( !boot_cpu_has(X86_FEATURE_MSR_MISC_FEATURES) )
> break;
> *val = 0;
> if ( curr->arch.cpuid_faulting )
> @@ -1147,15 +1145,13 @@ static int write_msr(unsigned int reg, u
> return X86EMUL_OKAY;
>
> case MSR_INTEL_PLATFORM_INFO:
> - if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> - val || rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val) )
> + if ( !boot_cpu_has(X86_FEATURE_MSR_PLATFORM_INFO) || val )
> break;
> return X86EMUL_OKAY;
Why writes to MSR_INTEL_PLATFORM_INFO shouldn't produce #GP faults for
PV guests?
>
> case MSR_INTEL_MISC_FEATURES_ENABLES:
> - if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> - (val & ~MSR_MISC_FEATURES_CPUID_FAULTING) ||
> - rdmsr_safe(MSR_INTEL_MISC_FEATURES_ENABLES, temp) )
> + if ( !boot_cpu_has(X86_FEATURE_MSR_MISC_FEATURES) ||
> + (val & ~MSR_MISC_FEATURES_CPUID_FAULTING) )
> break;
> if ( (val & MSR_MISC_FEATURES_CPUID_FAULTING) &&
> !this_cpu(cpuid_faulting_enabled) )
> --- a/xen/include/asm-x86/cpufeatures.h
> +++ b/xen/include/asm-x86/cpufeatures.h
> @@ -22,3 +22,5 @@ XEN_CPUFEATURE(APERFMPERF, (FSCAPIN
> XEN_CPUFEATURE(MFENCE_RDTSC, (FSCAPINTS+0)*32+ 9) /* MFENCE synchronizes RDTSC */
> XEN_CPUFEATURE(XEN_SMEP, (FSCAPINTS+0)*32+10) /* SMEP gets used by Xen itself */
> XEN_CPUFEATURE(XEN_SMAP, (FSCAPINTS+0)*32+11) /* SMAP gets used by Xen itself */
> +XEN_CPUFEATURE(MSR_PLATFORM_INFO, (FSCAPINTS+0)*32+12) /* PLATFORM_INFO MSR present */
> +XEN_CPUFEATURE(MSR_MISC_FEATURES, (FSCAPINTS+0)*32+13) /* MISC_FEATURES_ENABLES MSR present */
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
--
Thanks,
Sergey
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86: avoid #GP for PV guest MSR accesses
2017-09-22 10:32 ` Sergey Dyasli
@ 2017-09-22 10:44 ` Jan Beulich
0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2017-09-22 10:44 UTC (permalink / raw)
To: Sergey Dyasli; +Cc: Andrew Cooper, xen-devel@lists.xenproject.org
>>> On 22.09.17 at 12:32, <sergey.dyasli@citrix.com> wrote:
> On Fri, 2017-09-22 at 03:06 -0600, 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.
>>
>> 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@suse.com>
>
> The intent of this patch (and the related "VMX: PLATFORM_INFO MSR is
> r/o") is somewhat intersects with my series "Generic MSR policy:
> infrastructure + cpuid_faulting". IMHO it's better to fix MSR-related
> issues in the scope of the MSR policy work.
I'm of the opposite opinion, as doing what you suggest would
make the backport more difficult.
>> @@ -1147,15 +1145,13 @@ static int write_msr(unsigned int reg, u
>> return X86EMUL_OKAY;
>>
>> case MSR_INTEL_PLATFORM_INFO:
>> - if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
>> - val || rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val) )
>> + if ( !boot_cpu_has(X86_FEATURE_MSR_PLATFORM_INFO) || val )
>> break;
>> return X86EMUL_OKAY;
>
> Why writes to MSR_INTEL_PLATFORM_INFO shouldn't produce #GP faults for
> PV guests?
My mistake not carrying backwards what I had figured when
doing the VMX side change.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-09-22 10:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-22 9:06 [PATCH] x86: avoid #GP for PV guest MSR accesses Jan Beulich
2017-09-22 10:32 ` Sergey Dyasli
2017-09-22 10:44 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.