Skip to contentSkip to conversation

KVM: x86: Save&restore interrupt shadow mask

PATCH In review Jan Kiszka 2 participants 4 messages Feb 14, 2010 – Feb 15, 2010

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

Jan Kiszka

author
#0 lore raw

PATCH 1/1 KVM: x86: Save&restore interrupt shadow mask

From: Jan Kiszka <[email protected]>

The interrupt shadow created by STI or MOV-SS-like operations is part of the VCPU state and must be preserved across migration. Transfer it in the spare padding field of kvm_vcpu_events.interrupt.

As a side effect we now have to make vmx_set_interrupt_shadow robust against both shadow types being set. Give MOV SS a higher priority and skip STI in that case to avoid that VMX throws a fault on next entry.

Signed-off-by Jan Kiszka <[email protected]>

---

 arch/x86/include/asm/kvm.h         |    5 ++++-
 arch/x86/include/asm/kvm_emulate.h |    3 ---
 arch/x86/kvm/vmx.c                 |    2 +-
 arch/x86/kvm/x86.c                 |    4 ++++
 4 files changed, 9 insertions(+), 5 deletions(-)
arch/x86/include/asm/kvm.h +4 -1
index f46b79f..4dd01e2 100644
@@ -258,6 +258,9 @@ struct kvm_reinject_control {
 #define KVM_VCPUEVENT_VALID_NMI_PENDING	0x00000001
 #define KVM_VCPUEVENT_VALID_SIPI_VECTOR	0x00000002
 
+#define X86_SHADOW_INT_MOV_SS  1
+#define X86_SHADOW_INT_STI     2
+
 /* for KVM_GET/SET_VCPU_EVENTS */
 struct kvm_vcpu_events {
 	struct {

@@ -271,7 +274,7 @@ struct kvm_vcpu_events {
 		__u8 injected;
 		__u8 nr;
 		__u8 soft;
-		__u8 pad;
+		__u8 shadow;
 	} interrupt;
 	struct {
 		__u8 injected;
arch/x86/include/asm/kvm_emulate.h +0 -3
index 7a6f54f..2666d7a 100644
@@ -153,9 +153,6 @@ struct decode_cache {
 	struct fetch_cache fetch;
 };
 
-#define X86_SHADOW_INT_MOV_SS  1
-#define X86_SHADOW_INT_STI     2
-
 struct x86_emulate_ctxt {
 	/* Register state before/after emulation. */
 	struct kvm_vcpu *vcpu;
arch/x86/kvm/vmx.c +1 -1
index e9f64e8..ecec95a 100644
@@ -854,7 +854,7 @@ static void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask)
 
 	if (mask & X86_SHADOW_INT_MOV_SS)
 		interruptibility |= GUEST_INTR_STATE_MOV_SS;
-	if (mask & X86_SHADOW_INT_STI)
+	else if (mask & X86_SHADOW_INT_STI)
 		interruptibility |= GUEST_INTR_STATE_STI;
 
 	if ((interruptibility != interruptibility_old))
arch/x86/kvm/x86.c +4 -0
index 50d1d2a..4795d78 100644
@@ -2132,6 +2132,9 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
 		vcpu->arch.interrupt.pending && !vcpu->arch.interrupt.soft;
 	events->interrupt.nr = vcpu->arch.interrupt.nr;
 	events->interrupt.soft = 0;
+	events->interrupt.shadow =
+		kvm_x86_ops->get_interrupt_shadow(vcpu,
+				X86_SHADOW_INT_MOV_SS | X86_SHADOW_INT_STI);
 
 	events->nmi.injected = vcpu->arch.nmi_injected;
 	events->nmi.pending = vcpu->arch.nmi_pending;

@@ -2164,6 +2167,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 	vcpu->arch.interrupt.soft = events->interrupt.soft;
 	if (vcpu->arch.interrupt.pending && irqchip_in_kernel(vcpu->kvm))
 		kvm_pic_clear_isr_ack(vcpu->kvm);
+	kvm_x86_ops->set_interrupt_shadow(vcpu, events->interrupt.shadow);
 
 	vcpu->arch.nmi_injected = events->nmi.injected;
 	if (events->flags & KVM_VCPUEVENT_VALID_NMI_PENDING)
#1 lore raw
Quoted 16 lines · Jan Kiszka

From: Jan Kiszka<[email protected]>

The interrupt shadow created by STI or MOV-SS-like operations is part of the VCPU state and must be preserved across migration. Transfer it in the spare padding field of kvm_vcpu_events.interrupt.

As a side effect we now have to make vmx_set_interrupt_shadow robust against both shadow types being set. Give MOV SS a higher priority and skip STI in that case to avoid that VMX throws a fault on next entry.

arch/x86/include/asm/kvm.h +0 -0
index f46b79f..4dd01e2 100644

arch/x86/include/asm/kvm.h struct kvm_reinject_control { · line 264

Quoted 7 lines
arch/x86/include/asm/kvm.h +3 -0
@@ -258,6 +258,9 @@ struct kvm_reinject_control {
  #define KVM_VCPUEVENT_VALID_NMI_PENDING	0x00000001
  #define KVM_VCPUEVENT_VALID_SIPI_VECTOR	0x00000002
 
+#define X86_SHADOW_INT_MOV_SS  1
+#define X86_SHADOW_INT_STI     2
+

svm doesn't make a difference. Is it worth exposing this to userspace (and thus providing wrong information on svm)?

(in any case, needs a KVM_ prefix).

Needs a KVM_CAP_ and flags bit.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#2 lore raw
Quoted 20 lines · Avi Kivity
Quoted 19 lines · Jan Kiszka

From: Jan Kiszka<[email protected]>

The interrupt shadow created by STI or MOV-SS-like operations is part of the VCPU state and must be preserved across migration. Transfer it in the spare padding field of kvm_vcpu_events.interrupt.

As a side effect we now have to make vmx_set_interrupt_shadow robust against both shadow types being set. Give MOV SS a higher priority and skip STI in that case to avoid that VMX throws a fault on next entry.

arch/x86/include/asm/kvm.h +0 -0
index f46b79f..4dd01e2 100644
@@ -258,6 +258,9 @@ struct kvm_reinject_control {
  #define KVM_VCPUEVENT_VALID_NMI_PENDING    0x00000001
  #define KVM_VCPUEVENT_VALID_SIPI_VECTOR    0x00000002
Quoted 7 lines

arch/x86/include/asm/kvm.h struct kvm_reinject_control {

Quoted 4 lines
arch/x86/include/asm/kvm.h +3 -0
+#define X86_SHADOW_INT_MOV_SS  1
+#define X86_SHADOW_INT_STI     2
+

svm doesn't make a difference. Is it worth exposing this to userspace (and thus providing wrong information on svm)?

SVM already folds both reasons into one. Don't know if we should do it at the generic layer already - probably.

Quoted 2 lines

(in any case, needs a KVM_ prefix).

...and then this is no longer required. OK, will export this with boolean meaning.

Quoted 2 lines

Needs a KVM_CAP_ and flags bit.

Why if merged before 2.6.33? We haven't released KVM_CAP_VCPU_EVENTS yet.

Jan

#3 lore raw
Quoted 4 lines · Jan Kiszka
Quoted 2 lines

Needs a KVM_CAP_ and flags bit.

Why if merged before 2.6.33? We haven't released KVM_CAP_VCPU_EVENTS yet.

Userspace already knows the interrupt shadow mask free variant. Maybe it all works out, but KVM_CAP_ bits are cheaper than checking (and possibly getting it wrong).

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.