[PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Sheng Yang]

We don't support these instructions, but guest can execute them even if the
feature('monitor') haven't been exposed in CPUID. So we would trap and inject
a #UD if guest try this way.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/vmx.h |    1 +
 arch/x86/kvm/vmx.c         |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 2b49454..8f6b011 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -251,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_MONITOR_INSTRUCTION 39
 #define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9a0a2cf..da7e76b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1224,6 +1224,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	      CPU_BASED_USE_IO_BITMAPS |
 	      CPU_BASED_MOV_DR_EXITING |
 	      CPU_BASED_USE_TSC_OFFSETING |
+	      CPU_BASED_MWAIT_EXITING |
+	      CPU_BASED_MONITOR_EXITING |
 	      CPU_BASED_INVLPG_EXITING;
 	opt = CPU_BASED_TPR_SHADOW |
 	      CPU_BASED_USE_MSR_BITMAPS |
@@ -3415,6 +3417,12 @@ static int handle_pause(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+static int handle_invalid_op(struct kvm_vcpu *vcpu)
+{
+	kvm_queue_exception(vcpu, UD_VECTOR);
+	return 1;
+}
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
@@ -3452,6 +3460,8 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
 	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
 	[EXIT_REASON_PAUSE_INSTRUCTION]       = handle_pause,
+	[EXIT_REASON_MWAIT_INSTRUCTION]	      = handle_invalid_op,
+	[EXIT_REASON_MONITOR_INSTRUCTION]     = handle_invalid_op,
 };
 
 static const int kvm_vmx_max_exit_handlers =
-- 
1.5.4.5

Re: [PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Alexander Graf]

On 15.12.2009, at 06:29, Sheng Yang wrote:

> We don't support these instructions, but guest can execute them even if the
> feature('monitor') haven't been exposed in CPUID. So we would trap and inject
> a #UD if guest try this way.

Wouldn't it be a lot better to emulate them? I actually have emulation of MONITOR/MWAIT on my todo list for some time, since OSX requires it.

Basically what they do is they guarantee that a MONITOR'ed region is on a page boundary, so all we need to do is not map the monitor'ed page to wake up from the mwait.

Alex

Re: [PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Avi Kivity]

On 12/15/2009 09:36 AM, Alexander Graf wrote:
> On 15.12.2009, at 06:29, Sheng Yang wrote:
>
>    
>> We don't support these instructions, but guest can execute them even if the
>> feature('monitor') haven't been exposed in CPUID. So we would trap and inject
>> a #UD if guest try this way.
>>      
> Wouldn't it be a lot better to emulate them? I actually have emulation of MONITOR/MWAIT on my todo list for some time, since OSX requires it.
>
> Basically what they do is they guarantee that a MONITOR'ed region is on a page boundary, so all we need to do is not map the monitor'ed page to wake up from the mwait.
>    

You'll need to break up large page mappings for this.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Alexander Graf]

Avi Kivity wrote:
> On 12/15/2009 09:36 AM, Alexander Graf wrote:
>> On 15.12.2009, at 06:29, Sheng Yang wrote:
>>
>>   
>>> We don't support these instructions, but guest can execute them even
>>> if the
>>> feature('monitor') haven't been exposed in CPUID. So we would trap
>>> and inject
>>> a #UD if guest try this way.
>>>      
>> Wouldn't it be a lot better to emulate them? I actually have
>> emulation of MONITOR/MWAIT on my todo list for some time, since OSX
>> requires it.
>>
>> Basically what they do is they guarantee that a MONITOR'ed region is
>> on a page boundary, so all we need to do is not map the monitor'ed
>> page to wake up from the mwait.
>>    
>
> You'll need to break up large page mappings for this.


Right. Same for PCI device assignment. Breaking up large page mappings
should be a fairly normal operation.

Alex

Re: [PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Avi Kivity]

On 12/15/2009 11:36 AM, Alexander Graf wrote:
>
>> You'll need to break up large page mappings for this.
>>      
>
> Right. Same for PCI device assignment. Breaking up large page mappings
> should be a fairly normal operation.
>    

Sure, just pointing out that this isn't free.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Avi Kivity]

On 12/15/2009 07:29 AM, Sheng Yang wrote:
> We don't support these instructions, but guest can execute them even if the
> feature('monitor') haven't been exposed in CPUID. So we would trap and inject
> a #UD if guest try this way.
>    

Applied, and queued for stable.  Thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Avi Kivity]

On 12/15/2009 07:29 AM, Sheng Yang wrote:
> We don't support these instructions, but guest can execute them even if the
> feature('monitor') haven't been exposed in CPUID. So we would trap and inject
> a #UD if guest try this way.
>    

>   	CPU_BASED_MOV_DR_EXITING |
>   	      CPU_BASED_USE_TSC_OFFSETING |
> +	      CPU_BASED_MWAIT_EXITING |
> +	      CPU_BASED_MONITOR_EXITING |
>   	      CPU_BASED_INVLPG_EXITING;
>   	opt = CPU_BASED_TPR_SHADOW |
>   	      CPU_BASED_USE_MSR_BITMAPS |
>    

Shouldn't these be in the 'opt' section?  Not all processors support 
MONITOR/MWAIT exiting (presumably those don't support the instructions).

-- 
error compiling committee.c: too many arguments to function

[PATCH] KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

[Sheng Yang]

We don't support these instructions, but guest can execute them even if the
feature('monitor') haven't been exposed in CPUID. So we would trap and inject
a #UD if guest try this way.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/vmx.h |    1 +
 arch/x86/kvm/vmx.c         |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 2b49454..8f6b011 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -251,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_MONITOR_INSTRUCTION 39
 #define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9a0a2cf..da7e76b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1224,6 +1224,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	      CPU_BASED_USE_IO_BITMAPS |
 	      CPU_BASED_MOV_DR_EXITING |
 	      CPU_BASED_USE_TSC_OFFSETING |
+	      CPU_BASED_MWAIT_EXITING |
+	      CPU_BASED_MONITOR_EXITING |
 	      CPU_BASED_INVLPG_EXITING;
 	opt = CPU_BASED_TPR_SHADOW |
 	      CPU_BASED_USE_MSR_BITMAPS |
@@ -3415,6 +3417,12 @@ static int handle_pause(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+static int handle_invalid_op(struct kvm_vcpu *vcpu)
+{
+	kvm_queue_exception(vcpu, UD_VECTOR);
+	return 1;
+}
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
@@ -3452,6 +3460,8 @@ static int (*kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
 	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
 	[EXIT_REASON_PAUSE_INSTRUCTION]       = handle_pause,
+	[EXIT_REASON_MWAIT_INSTRUCTION]	      = handle_invalid_op,
+	[EXIT_REASON_MONITOR_INSTRUCTION]     = handle_invalid_op,
 };
 
 static const int kvm_vmx_max_exit_handlers =
-- 
1.5.4.5