Re: [PATCH] KVM: x86: Add instruction length to VCPU event state

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3929 bytes --]

Gleb Natapov wrote:
> On Sat, Feb 13, 2010 at 10:51:40AM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> VMX requires a properly set instruction length VM entry field when
>> trying to inject soft exception and interrupts. We have to preserve this
>> state across VM save/restore to avoid breaking the re-injection of such
>> events on Intel. So add it to the new VCPU event state.
>>
> We shouldn't re-inject soft exceptions/interrupts after migration, but
> re-execute instruction instead. Instruction length field doesn't exist
> on SVM and migration shouldn't expose implementation details.
> 

Hmm, then I guess this totally untested patch should fly:

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f9a2f66..f87c3a5 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -315,8 +315,6 @@ struct kvm_vcpu_arch {
 	struct kvm_pio_request pio;
 	void *pio_data;
 
-	u8 event_exit_inst_len;
-
 	struct kvm_queued_exception {
 		bool pending;
 		bool has_error_code;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f82b072..a7111da 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -889,8 +889,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
 		vmx->rmode.irq.vector = nr;
 		vmx->rmode.irq.rip = kvm_rip_read(vcpu);
 		if (kvm_exception_is_soft(nr))
-			vmx->rmode.irq.rip +=
-				vmx->vcpu.arch.event_exit_inst_len;
+			vmx->rmode.irq.rip++;
 		intr_info |= INTR_TYPE_SOFT_INTR;
 		vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr_info);
 		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 1);
@@ -899,8 +898,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
 	}
 
 	if (kvm_exception_is_soft(nr)) {
-		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN,
-			     vmx->vcpu.arch.event_exit_inst_len);
+		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 1);
 		intr_info |= INTR_TYPE_SOFT_EXCEPTION;
 	} else
 		intr_info |= INTR_TYPE_HARD_EXCEPTION;
@@ -2639,8 +2637,7 @@ static void vmx_inject_irq(struct kvm_vcpu *vcpu)
 		vmx->rmode.irq.vector = irq;
 		vmx->rmode.irq.rip = kvm_rip_read(vcpu);
 		if (vcpu->arch.interrupt.soft)
-			vmx->rmode.irq.rip +=
-				vmx->vcpu.arch.event_exit_inst_len;
+			vmx->rmode.irq.rip += 2;
 		vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
 			     irq | INTR_TYPE_SOFT_INTR | INTR_INFO_VALID_MASK);
 		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 1);
@@ -2650,8 +2647,7 @@ static void vmx_inject_irq(struct kvm_vcpu *vcpu)
 	intr = irq | INTR_INFO_VALID_MASK;
 	if (vcpu->arch.interrupt.soft) {
 		intr |= INTR_TYPE_SOFT_INTR;
-		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN,
-			     vmx->vcpu.arch.event_exit_inst_len);
+		vmcs_write32(VM_ENTRY_INSTRUCTION_LEN, 2);
 	} else
 		intr |= INTR_TYPE_EXT_INTR;
 	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr);
@@ -3688,9 +3684,6 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx)
 				GUEST_INTR_STATE_NMI);
 		break;
 	case INTR_TYPE_SOFT_EXCEPTION:
-		vmx->vcpu.arch.event_exit_inst_len =
-			vmcs_read32(VM_EXIT_INSTRUCTION_LEN);
-		/* fall through */
 	case INTR_TYPE_HARD_EXCEPTION:
 		if (idt_vectoring_info & VECTORING_INFO_DELIVER_CODE_MASK) {
 			u32 err = vmcs_read32(IDT_VECTORING_ERROR_CODE);
@@ -3699,9 +3692,6 @@ static void vmx_complete_interrupts(struct vcpu_vmx *vmx)
 			kvm_queue_exception(&vmx->vcpu, vector);
 		break;
 	case INTR_TYPE_SOFT_INTR:
-		vmx->vcpu.arch.event_exit_inst_len =
-			vmcs_read32(VM_EXIT_INSTRUCTION_LEN);
-		/* fall through */
 	case INTR_TYPE_EXT_INTR:
 		kvm_queue_interrupt(&vmx->vcpu, vector,
 			type == INTR_TYPE_SOFT_INTR);


As we actually do not support privileged soft exceptions, we are left
with int3 and into as single-byte instructions triggering soft
exceptions and int N as a two-byte instruction a the reason for soft
interrupts. Am I missing something?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]