diff mbox series

[v3,1/8] x86/vmx: handle writes to MISC_ENABLE MSR

Message ID 20200901105445.22277-2-roger.pau@citrix.com
State Superseded
Headers show
Series x86: switch default MSR behavior | expand

Commit Message

Roger Pau Monne Sept. 1, 2020, 10:54 a.m. UTC
Such handling consist in checking that no bits have been changed from
the read value, if that's the case silently drop the write, otherwise
inject a fault.

At least Windows guests will expect to write to the MISC_ENABLE MSR
with the same value that's been read from it.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Jan Beulich Sept. 4, 2020, 8:34 a.m. UTC | #1
On 01.09.2020 12:54, Roger Pau Monne wrote:
> Such handling consist in checking that no bits have been changed from
> the read value, if that's the case silently drop the write, otherwise
> inject a fault.
> 
> At least Windows guests will expect to write to the MISC_ENABLE MSR
> with the same value that's been read from it.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Btw - early next week I think I'll time out waiting for a VMX side
ack for this change.

Jan
Tian, Kevin Sept. 7, 2020, 3:25 a.m. UTC | #2
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: Tuesday, September 1, 2020 6:55 PM
> 
> Such handling consist in checking that no bits have been changed from
> the read value, if that's the case silently drop the write, otherwise
> inject a fault.
> 
> At least Windows guests will expect to write to the MISC_ENABLE MSR
> with the same value that's been read from it.

for better readability could you also add this line to the code comment
below?

with that:

	Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/x86/hvm/vmx/vmx.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index a0d58ffbe2..4717e50d4a 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3163,7 +3163,7 @@ static int vmx_msr_write_intercept(unsigned int
> msr, uint64_t msr_content)
> 
>      switch ( msr )
>      {
> -        uint64_t rsvd;
> +        uint64_t rsvd, tmp;
> 
>      case MSR_IA32_SYSENTER_CS:
>          __vmwrite(GUEST_SYSENTER_CS, msr_content);
> @@ -3301,6 +3301,13 @@ static int vmx_msr_write_intercept(unsigned int
> msr, uint64_t msr_content)
>          /* None of these MSRs are writeable. */
>          goto gp_fault;
> 
> +    case MSR_IA32_MISC_ENABLE:
> +        /* Silently drop writes that don't change the reported value. */
> +        if ( vmx_msr_read_intercept(msr, &tmp) != X86EMUL_OKAY ||
> +             tmp != msr_content )
> +            goto gp_fault;
> +        break;
> +
>      case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
>      case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(7):
>      case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
> --
> 2.28.0
Jan Beulich Sept. 7, 2020, 7:22 a.m. UTC | #3
On 07.09.2020 05:25, Tian, Kevin wrote:
>> From: Roger Pau Monne <roger.pau@citrix.com>
>> Sent: Tuesday, September 1, 2020 6:55 PM
>>
>> Such handling consist in checking that no bits have been changed from
>> the read value, if that's the case silently drop the write, otherwise
>> inject a fault.
>>
>> At least Windows guests will expect to write to the MISC_ENABLE MSR
>> with the same value that's been read from it.
> 
> for better readability could you also add this line to the code comment
> below?
> 
> with that:
> 
> 	Reviewed-by: Kevin Tian <kevin.tian@intel.com>

I'll fold this in while committing.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index a0d58ffbe2..4717e50d4a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3163,7 +3163,7 @@  static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
 
     switch ( msr )
     {
-        uint64_t rsvd;
+        uint64_t rsvd, tmp;
 
     case MSR_IA32_SYSENTER_CS:
         __vmwrite(GUEST_SYSENTER_CS, msr_content);
@@ -3301,6 +3301,13 @@  static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
         /* None of these MSRs are writeable. */
         goto gp_fault;
 
+    case MSR_IA32_MISC_ENABLE:
+        /* Silently drop writes that don't change the reported value. */
+        if ( vmx_msr_read_intercept(msr, &tmp) != X86EMUL_OKAY ||
+             tmp != msr_content )
+            goto gp_fault;
+        break;
+
     case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
     case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(7):
     case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2: