[PATCH] VMX: PLATFORM_INFO MSR is r/o

[PATCH] VMX: PLATFORM_INFO MSR is r/o

[Jan Beulich]

Therefore all write attempts should produce #GP, just like on real
hardware.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3138,10 +3138,7 @@ static int vmx_msr_write_intercept(unsig
         break;
 
     case MSR_INTEL_PLATFORM_INFO:
-        if ( msr_content ||
-             rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
-            goto gp_fault;
-        break;
+        goto gp_fault;
 
     case MSR_INTEL_MISC_FEATURES_ENABLES:
     {




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

Re: [PATCH] VMX: PLATFORM_INFO MSR is r/o

[Roger Pau Monné]

On Fri, Sep 22, 2017 at 03:07:44AM -0600, Jan Beulich wrote:
> Therefore all write attempts should produce #GP, just like on real
> hardware.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3138,10 +3138,7 @@ static int vmx_msr_write_intercept(unsig
>          break;
>  
>      case MSR_INTEL_PLATFORM_INFO:
> -        if ( msr_content ||
> -             rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
> -            goto gp_fault;
> -        break;
> +        goto gp_fault;

Could you place the label together with the MSR_IA32_FEATURE_CONTROL
one above? So that we don't add another case with just a gp_fault.

With that:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

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

Re: [PATCH] VMX: PLATFORM_INFO MSR is r/o

[Andrew Cooper]

On 22/09/17 10:07, Jan Beulich wrote:
> Therefore all write attempts should produce #GP, just like on real
> hardware.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Sergey has already posted a patch series to fix this.

What is the benefit of this version?  As far as I can tell, it isn't as 
complete as his work, because it doesn't fix the problems with this MSR 
on AMD systems.

~Andrew

>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3138,10 +3138,7 @@ static int vmx_msr_write_intercept(unsig
>           break;
>   
>       case MSR_INTEL_PLATFORM_INFO:
> -        if ( msr_content ||
> -             rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
> -            goto gp_fault;
> -        break;
> +        goto gp_fault;
>   
>       case MSR_INTEL_MISC_FEATURES_ENABLES:
>       {
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel


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

Re: [PATCH] VMX: PLATFORM_INFO MSR is r/o

[Jan Beulich]

>>> On 22.09.17 at 19:05, <roger.pau@citrix.com> wrote:
> On Fri, Sep 22, 2017 at 03:07:44AM -0600, Jan Beulich wrote:
>> Therefore all write attempts should produce #GP, just like on real
>> hardware.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -3138,10 +3138,7 @@ static int vmx_msr_write_intercept(unsig
>>          break;
>>  
>>      case MSR_INTEL_PLATFORM_INFO:
>> -        if ( msr_content ||
>> -             rdmsr_safe(MSR_INTEL_PLATFORM_INFO, msr_content) )
>> -            goto gp_fault;
>> -        break;
>> +        goto gp_fault;
> 
> Could you place the label together with the MSR_IA32_FEATURE_CONTROL
> one above? So that we don't add another case with just a gp_fault.

Oh, of course. I did think I had looked for something to group
it with, but clearly I hadn't (maybe that was the PV side change
then).

> With that:
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Jan

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

Re: [PATCH] VMX: PLATFORM_INFO MSR is r/o

[Jan Beulich]

>>> On 22.09.17 at 19:38, <andrew.cooper3@citrix.com> wrote:
> On 22/09/17 10:07, Jan Beulich wrote:
>> Therefore all write attempts should produce #GP, just like on real
>> hardware.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Sergey has already posted a patch series to fix this.
> 
> What is the benefit of this version?  As far as I can tell, it isn't as 
> complete as his work, because it doesn't fix the problems with this MSR 
> on AMD systems.

For one, I didn't get around to look at his series yet, so I simply
can't know that this series addresses the same issue. I'm fine
dropping the patch here, but as said in reply to a similar comment
by him the patch here (as well as the PV counterpart) has the
advantage of being a backporting candidate; the other series
pretty clearly isn't, afaict.

Jan


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