KVM: VCPU state extensions

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]>

---
 Documentation/kvm/api.txt  |   11 ++++++++++-
 arch/x86/include/asm/kvm.h |    3 ++-
 arch/x86/kvm/vmx.c         |    2 +-
 arch/x86/kvm/x86.c         |   12 ++++++++++--
 include/linux/kvm.h        |    1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

Documentation/kvm/api.txt +10 -1

index c6416a3..8770b67 100644
@@ -656,6 +656,7 @@ struct kvm_clock_data {
 4.29 KVM_GET_VCPU_EVENTS
 
 Capability: KVM_CAP_VCPU_EVENTS
+Extended by: KVM_CAP_INTR_SHADOW
 Architectures: x86
 Type: vm ioctl
 Parameters: struct kvm_vcpu_event (out)

@@ -676,7 +677,7 @@ struct kvm_vcpu_events {
 		__u8 injected;
 		__u8 nr;
 		__u8 soft;
-		__u8 pad;
+		__u8 shadow;
 	} interrupt;
 	struct {
 		__u8 injected;

@@ -688,9 +689,13 @@ struct kvm_vcpu_events {
 	__u32 flags;
 };
 
+KVM_VCPUEVENT_VALID_SHADOW may be set in the flags field to signal that
+interrupt.shadow contains a valid state. Otherwise, this field is undefined.
+
 4.30 KVM_SET_VCPU_EVENTS
 
 Capability: KVM_CAP_VCPU_EVENTS
+Extended by: KVM_CAP_INTR_SHADOW
 Architectures: x86
 Type: vm ioctl
 Parameters: struct kvm_vcpu_event (in)

@@ -709,6 +714,10 @@ current in-kernel state. The bits are:
 KVM_VCPUEVENT_VALID_NMI_PENDING - transfer nmi.pending to the kernel
 KVM_VCPUEVENT_VALID_SIPI_VECTOR - transfer sipi_vector
 
+If KVM_CAP_INTR_SHADOW is available, KVM_VCPUEVENT_VALID_SHADOW can be set in
+the flags field to signal that interrupt.shadow contains a valid state and
+shall be written into the VCPU.
+
 
 5. The kvm_run structure
 

arch/x86/include/asm/kvm.h +2 -1

index f46b79f..dc6cd24 100644
@@ -257,6 +257,7 @@ struct kvm_reinject_control {
 /* When set in flags, include corresponding fields on KVM_SET_VCPU_EVENTS */
 #define KVM_VCPUEVENT_VALID_NMI_PENDING	0x00000001
 #define KVM_VCPUEVENT_VALID_SIPI_VECTOR	0x00000002
+#define KVM_VCPUEVENT_VALID_SHADOW	0x00000004
 
 /* for KVM_GET/SET_VCPU_EVENTS */
 struct kvm_vcpu_events {

@@ -271,7 +272,7 @@ struct kvm_vcpu_events {
 		__u8 injected;
 		__u8 nr;
 		__u8 soft;
-		__u8 pad;
+		__u8 shadow;
 	} interrupt;
 	struct {
 		__u8 injected;

arch/x86/kvm/vmx.c +1 -1

index f82b072..0fa74d0 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 +7 -2

index 50d1d2a..60e6341 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;

@@ -2140,7 +2143,8 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
 	events->sipi_vector = vcpu->arch.sipi_vector;
 
 	events->flags = (KVM_VCPUEVENT_VALID_NMI_PENDING
-			 | KVM_VCPUEVENT_VALID_SIPI_VECTOR);
+			 | KVM_VCPUEVENT_VALID_SIPI_VECTOR
+			 | KVM_VCPUEVENT_VALID_SHADOW);
 
 	vcpu_put(vcpu);
 }

@@ -2149,7 +2153,8 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 					      struct kvm_vcpu_events *events)
 {
 	if (events->flags & ~(KVM_VCPUEVENT_VALID_NMI_PENDING
-			      | KVM_VCPUEVENT_VALID_SIPI_VECTOR))
+			      | KVM_VCPUEVENT_VALID_SIPI_VECTOR
+			      | KVM_VCPUEVENT_VALID_SHADOW))
 		return -EINVAL;
 
 	vcpu_load(vcpu);

Quoted 3 lines · Jan Kiszka

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.

Quoted 4 lines · Marcelo Tosatti

Quoted 3 lines · Jan Kiszka

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.

STI and MOV-SS interrupt shadow are both treated differently by hardware. Any attempt to unify them into a single field is wrong, especially so in a hardware virtualization context, where they are actually represented by different fields in the undocumented but nevertheless extant format that can be inferred from the hardware virtualization context used by specific vendors.

Quoted 5 lines · Marcelo Tosatti

Quoted 3 lines · Jan Kiszka

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.

STI and MOV-SS interrupt shadow are both treated differently by hardware. Any attempt to unify them into a single field is wrong, especially so in a hardware virtualization context, where they are actually represented by different fields in the undocumented but nevertheless extant format that can be inferred from the hardware virtualization context used by specific vendors.

Quoted 132 lines · Jan Kiszka

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]>

---
 Documentation/kvm/api.txt  |   11 ++++++++++-
 arch/x86/include/asm/kvm.h |    3 ++-
 arch/x86/kvm/vmx.c         |    2 +-
 arch/x86/kvm/x86.c         |   12 ++++++++++--
 include/linux/kvm.h        |    1 +
 5 files changed, 24 insertions(+), 5 deletions(-)

Documentation/kvm/api.txt +10 -1

index c6416a3..8770b67 100644
@@ -656,6 +656,7 @@ struct kvm_clock_data {
 4.29 KVM_GET_VCPU_EVENTS
 
 Capability: KVM_CAP_VCPU_EVENTS
+Extended by: KVM_CAP_INTR_SHADOW
 Architectures: x86
 Type: vm ioctl
 Parameters: struct kvm_vcpu_event (out)

@@ -676,7 +677,7 @@ struct kvm_vcpu_events {
 		__u8 injected;
 		__u8 nr;
 		__u8 soft;
-		__u8 pad;
+		__u8 shadow;
 	} interrupt;
 	struct {
 		__u8 injected;

@@ -688,9 +689,13 @@ struct kvm_vcpu_events {
 	__u32 flags;
 };
 
+KVM_VCPUEVENT_VALID_SHADOW may be set in the flags field to signal that
+interrupt.shadow contains a valid state. Otherwise, this field is undefined.
+
 4.30 KVM_SET_VCPU_EVENTS
 
 Capability: KVM_CAP_VCPU_EVENTS
+Extended by: KVM_CAP_INTR_SHADOW
 Architectures: x86
 Type: vm ioctl
 Parameters: struct kvm_vcpu_event (in)

@@ -709,6 +714,10 @@ current in-kernel state. The bits are:
 KVM_VCPUEVENT_VALID_NMI_PENDING - transfer nmi.pending to the kernel
 KVM_VCPUEVENT_VALID_SIPI_VECTOR - transfer sipi_vector
 
+If KVM_CAP_INTR_SHADOW is available, KVM_VCPUEVENT_VALID_SHADOW can be set in
+the flags field to signal that interrupt.shadow contains a valid state and
+shall be written into the VCPU.
+
 
 5. The kvm_run structure
 

arch/x86/include/asm/kvm.h +2 -1

index f46b79f..dc6cd24 100644
@@ -257,6 +257,7 @@ struct kvm_reinject_control {
 /* When set in flags, include corresponding fields on KVM_SET_VCPU_EVENTS */
 #define KVM_VCPUEVENT_VALID_NMI_PENDING	0x00000001
 #define KVM_VCPUEVENT_VALID_SIPI_VECTOR	0x00000002
+#define KVM_VCPUEVENT_VALID_SHADOW	0x00000004
 
 /* for KVM_GET/SET_VCPU_EVENTS */
 struct kvm_vcpu_events {

@@ -271,7 +272,7 @@ struct kvm_vcpu_events {
 		__u8 injected;
 		__u8 nr;
 		__u8 soft;
-		__u8 pad;
+		__u8 shadow;
 	} interrupt;
 	struct {
 		__u8 injected;

arch/x86/kvm/vmx.c +1 -1

index f82b072..0fa74d0 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 +7 -2

index 50d1d2a..60e6341 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;

@@ -2140,7 +2143,8 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
 	events->sipi_vector = vcpu->arch.sipi_vector;
 
 	events->flags = (KVM_VCPUEVENT_VALID_NMI_PENDING
-			 | KVM_VCPUEVENT_VALID_SIPI_VECTOR);
+			 | KVM_VCPUEVENT_VALID_SIPI_VECTOR
+			 | KVM_VCPUEVENT_VALID_SHADOW);
 
 	vcpu_put(vcpu);
 }

@@ -2149,7 +2153,8 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 					      struct kvm_vcpu_events *events)
 {
 	if (events->flags & ~(KVM_VCPUEVENT_VALID_NMI_PENDING
-			      | KVM_VCPUEVENT_VALID_SIPI_VECTOR))
+			      | KVM_VCPUEVENT_VALID_SIPI_VECTOR
+			      | KVM_VCPUEVENT_VALID_SHADOW))
 		return -EINVAL;
 
 	vcpu_load(vcpu);

arch/x86/kvm/x86.c static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu, · line 2174

Quoted 6 lines

arch/x86/kvm/x86.c +3 -0

@@ -2164,6 +2169,9 @@ 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);
+	if (events->flags & KVM_VCPUEVENT_VALID_SHADOW)
+		kvm_x86_ops->set_interrupt_shadow(vcpu,
+			events->interrupt.shadow ? X86_SHADOW_INT_MOV_SS : 0);

Its hackish to transform blocking-by-sti into blocking-by-mov-ss (sti does not block debug exceptions for the next instruction).

Any special reason you are doing this?

Also, as Avi mentioned it would be better to avoid this. Is it not possible to disallow migration while interrupt shadow is present?

Quoted 3 lines

Also, as Avi mentioned it would be better to avoid this. Is it not possible to disallow migration while interrupt shadow is present?

Which means disallowing user space exists while the shadow it set? Or should we introduce some flag for user space that tells it "do not migration now, resume the guest till next exit"?

Quoted 4 lines · Jan Kiszka

Quoted 3 lines

Also, as Avi mentioned it would be better to avoid this. Is it not possible to disallow migration while interrupt shadow is present?

Quoted 4 lines

Which means disallowing user space exists while the shadow it set? Or should we introduce some flag for user space that tells it "do not migration now, resume the guest till next exit"?

I think disabling migration is a slippery slope. Guest may abuse it. May be it will be hard to do with interrupt shadow, but the mechanism will be used for other cases too. I remember there was an argument that we should not migrate while vcpu is in a nested guest mode.

These patches do not technically depend on each other but overlap, so I'm pushing them now in a series.

Patch 1 is a repost. Patch 2 is reworked and comes with the following changes:

- expose only a boolean to user space, mapping it on X86_SHADOW_INT_MOV_SS during write
- do not move X86_SHADOW_INT_* flags around
- Signal capability via KVM_CAP_INTR_SHADOW and manage the new kvm_vcpu_events field via KVM_VCPUEVENT_VALID_SHADOW
- Update docs

Finally, patch 3 is new, plugging the debug register migration (and reset) hole.

You can also pull from

git://git.kiszka.org/linux-kvm vcpu-state

Jan Kiszka (3):
KVM: x86: Do not return soft events in vcpu_events
KVM: x86: Save&restore interrupt shadow mask
KVM: x86: Add support for saving&restoring debug registers

 Documentation/kvm/api.txt  |   42 ++++++++++++++++++++++++-
 arch/x86/include/asm/kvm.h |   13 +++++++-
 arch/x86/kvm/vmx.c         |    2 +-
 arch/x86/kvm/x86.c         |   75 +++++++++++++++++++++++++++++++++++++++++---
 include/linux/kvm.h        |    7 ++++
 5 files changed, 131 insertions(+), 8 deletions(-)

[Trying to sort my patch queues]

Ping on this series with updated patch 2. Any open issues remaining?

Quoted 4 lines · Jan Kiszka

[Trying to sort my patch queues]

Ping on this series with updated patch 2. Any open issues remaining?

My default algorithm on patches with a lot of noise is to wait until the dust settles. I'll look at it now.