Skip to contentSkip to conversation

KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

PATCH Applied Sheng Yang 3 participants 7 messages Dec 15, 2009

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Sheng Yang

author
#0 lore raw

PATCH 1/1 KVM: VMX: Trap and invalid MWAIT/MONITOR instruction

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 <[email protected]>
---
 arch/x86/include/asm/vmx.h |    1 +
 arch/x86/kvm/vmx.c         |   10 ++++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)
arch/x86/include/asm/vmx.h +1 -0
index 2b49454..8f6b011 100644
@@ -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
arch/x86/kvm/vmx.c +10 -0
index 9a0a2cf..da7e76b 100644
@@ -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 =
Signature · 1 line
1.5.4.5
#1 lore raw
Quoted 3 lines · 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.

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

#2 lore raw
Quoted 11 lines · Alexander Graf
Quoted 4 lines · 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.

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#3 lore raw
Quoted 14 lines · Avi Kivity
Quoted 13 lines · Alexander Graf
Quoted 6 lines · 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.

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.

Quoted 6 lines
Quoted 4 lines

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

#4 lore raw
Quoted 7 lines · Alexander Graf
Quoted 3 lines

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
Applied
#5 lore raw
In reply to Sheng Yang
Quoted 4 lines · 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.

Applied, and queued for stable. Thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function
#6 lore raw
In reply to Sheng Yang

arch/x86/kvm/vmx.c static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) · line 1231

Quoted 13 lines · 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.

CPU_BASED_MOV_DR_EXITING |
CPU_BASED_USE_TSC_OFFSETING |

arch/x86/kvm/vmx.c +2 -0
+	      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).

Signature · 1 line
error compiling committee.c: too many arguments to function