Re: [Xen-devel] [PATCH] x86/Intel: hide CPUID faulting capability from guests

[Tian, Kevin]

> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Friday, September 16, 2016 2:32 PM
> 
> We don't currently emulate it, so guests should not be misguided to
> believe they can (try to) use it.
> 
> For now, simply return zero to guests for platform MSR reads, and only
> accept (by discarding) writes of zero. If ever there will be bits we
> can safely expose to guests, let's handle them by white listing.
> 
> (As a side note - according to SDM version 059 bit 31 is reserved on
> all known families.)
> 
> Reported-by: Kyle Huey 
> Signed-off-by: Jan Beulich 
> 

Acked-by: Kevin Tian 

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

Re: [Xen-devel] [PATCH] x86/Intel: hide CPUID faulting capability from guests

[Kyle Huey]

On Thu, Sep 15, 2016 at 11:32 PM, Jan Beulich  wrote:
> We don't currently emulate it, so guests should not be misguided to
> believe they can (try to) use it.
>
> For now, simply return zero to guests for platform MSR reads, and only
> accept (by discarding) writes of zero. If ever there will be bits we
> can safely expose to guests, let's handle them by white listing.
>
> (As a side note - according to SDM version 059 bit 31 is reserved on
> all known families.)
>
> Reported-by: Kyle Huey 
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2699,6 +2699,13 @@ static int vmx_msr_read_intercept(unsign
>  if ( vpmu_do_rdmsr(msr, msr_content) )
>  goto gp_fault;
>  break;
> +
> +case MSR_INTEL_PLATFORM_INFO:
> +if ( rdmsr_safe(MSR_INTEL_PLATFORM_INFO, *msr_content) )
> +goto gp_fault;
> +*msr_content = 0;
> +break;
> +
>  default:
>  if ( passive_domain_do_rdmsr(msr, msr_content) )
>  goto done;
> @@ -2918,6 +2925,13 @@ static int vmx_msr_write_intercept(unsig
>   if ( vpmu_do_wrmsr(msr, msr_content, 0) )
>  goto gp_fault;
>  break;
> +
> +case MSR_INTEL_PLATFORM_INFO:
> +if ( msr_content ||
> + rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
> +goto gp_fault;
> +break;
> +
>  default:
>  if ( passive_domain_do_wrmsr(msr, msr_content) )
>  return X86EMUL_OKAY;
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -2938,6 +2938,14 @@ static int emulate_privileged_op(struct
>  if ( v->arch.debugreg[7] & DR7_ACTIVE_MASK )
>  wrmsrl(regs->_ecx, msr_content);
>  break;
> +
> +case MSR_INTEL_PLATFORM_INFO:
> +if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> + msr_content ||
> + rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
> +goto fail;
> +break;
> +
>  case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
>  case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(3):
>  case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
> @@ -3066,6 +3074,14 @@ static int emulate_privileged_op(struct
>  /* No extra capabilities are supported */
>  regs->eax = regs->edx = 0;
>  break;
> +
> +case MSR_INTEL_PLATFORM_INFO:
> +if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> + rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val) )
> +goto fail;
> +regs->eax = regs->edx = 0;
> +break;
> +
>  case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
>  case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(3):
>  case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
>
>
>

Excellent.  Thank you for writing this.

- Kyle

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

Re: [Xen-devel] [PATCH] x86/Intel: hide CPUID faulting capability from guests

[Jan Beulich]

>>> On 16.09.16 at 11:46,  wrote:
> On 16/09/16 07:32, Jan Beulich wrote:
>> We don't currently emulate it, so guests should not be misguided to
>> believe they can (try to) use it.
>>
>> For now, simply return zero to guests for platform MSR reads, and only
>> accept (by discarding) writes of zero. If ever there will be bits we
>> can safely expose to guests, let's handle them by white listing.
>>
>> (As a side note - according to SDM version 059 bit 31 is reserved on
>> all known families.)
>>
>> Reported-by: Kyle Huey 
>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Andrew Cooper , but I would also 
> suggest putting in a d->arch.x86_vendor check against Intel as well.

In general I agree, but then let's please do this for all instances
which currently use the host CPU vendor field at once: There's no
single example of this, and I have to admit that it's also not
immediately clear to me what the best behavior would be in some
of the cases if host and guest vendor disagree.

Jan


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

Re: [Xen-devel] [PATCH] x86/Intel: hide CPUID faulting capability from guests

[Andrew Cooper]

On 16/09/16 07:32, Jan Beulich wrote:

We don't currently emulate it, so guests should not be misguided to
believe they can (try to) use it.

For now, simply return zero to guests for platform MSR reads, and only
accept (by discarding) writes of zero. If ever there will be bits we
can safely expose to guests, let's handle them by white listing.

(As a side note - according to SDM version 059 bit 31 is reserved on
all known families.)

Reported-by: Kyle Huey 
Signed-off-by: Jan Beulich 


Acked-by: Andrew Cooper , but I would also 
suggest putting in a d->arch.x86_vendor check against Intel as well.


~Andrew

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

[Xen-devel] [PATCH] x86/Intel: hide CPUID faulting capability from guests

[Jan Beulich]

We don't currently emulate it, so guests should not be misguided to
believe they can (try to) use it.

For now, simply return zero to guests for platform MSR reads, and only
accept (by discarding) writes of zero. If ever there will be bits we
can safely expose to guests, let's handle them by white listing.

(As a side note - according to SDM version 059 bit 31 is reserved on
all known families.)

Reported-by: Kyle Huey 
Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2699,6 +2699,13 @@ static int vmx_msr_read_intercept(unsign
 if ( vpmu_do_rdmsr(msr, msr_content) )
 goto gp_fault;
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( rdmsr_safe(MSR_INTEL_PLATFORM_INFO, *msr_content) )
+goto gp_fault;
+*msr_content = 0;
+break;
+
 default:
 if ( passive_domain_do_rdmsr(msr, msr_content) )
 goto done;
@@ -2918,6 +2925,13 @@ static int vmx_msr_write_intercept(unsig
  if ( vpmu_do_wrmsr(msr, msr_content, 0) )
 goto gp_fault;
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( msr_content ||
+ rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
+goto gp_fault;
+break;
+
 default:
 if ( passive_domain_do_wrmsr(msr, msr_content) )
 return X86EMUL_OKAY;
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2938,6 +2938,14 @@ static int emulate_privileged_op(struct
 if ( v->arch.debugreg[7] & DR7_ACTIVE_MASK )
 wrmsrl(regs->_ecx, msr_content);
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ msr_content ||
+ rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
+goto fail;
+break;
+
 case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
 case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(3):
 case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
@@ -3066,6 +3074,14 @@ static int emulate_privileged_op(struct
 /* No extra capabilities are supported */
 regs->eax = regs->edx = 0;
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val) )
+goto fail;
+regs->eax = regs->edx = 0;
+break;
+
 case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
 case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(3):
 case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:



x86/Intel: hide CPUID faulting capability from guests

We don't currently emulate it, so guests should not be misguided to
believe they can (try to) use it.

For now, simply return zero to guests for platform MSR reads, and only
accept (by discarding) writes of zero. If ever there will be bits we
can safely expose to guests, let's handle them by white listing.

(As a side note - according to SDM version 059 bit 31 is reserved on
all known families.)

Reported-by: Kyle Huey 
Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2699,6 +2699,13 @@ static int vmx_msr_read_intercept(unsign
 if ( vpmu_do_rdmsr(msr, msr_content) )
 goto gp_fault;
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( rdmsr_safe(MSR_INTEL_PLATFORM_INFO, *msr_content) )
+goto gp_fault;
+*msr_content = 0;
+break;
+
 default:
 if ( passive_domain_do_rdmsr(msr, msr_content) )
 goto done;
@@ -2918,6 +2925,13 @@ static int vmx_msr_write_intercept(unsig
  if ( vpmu_do_wrmsr(msr, msr_content, 0) )
 goto gp_fault;
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( msr_content ||
+ rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
+goto gp_fault;
+break;
+
 default:
 if ( passive_domain_do_wrmsr(msr, msr_content) )
 return X86EMUL_OKAY;
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2938,6 +2938,14 @@ static int emulate_privileged_op(struct
 if ( v->arch.debugreg[7] & DR7_ACTIVE_MASK )
 wrmsrl(regs->_ecx, msr_content);
 break;
+
+case MSR_INTEL_PLATFORM_INFO:
+if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
+ msr_content ||
+ rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
+goto fail;
+break;
+
 case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
 case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(3):
 case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
@@ -3066,6 +3074,14 @@ static int emulate_privileged_op(struct
 /* No extra capabilities are supported */