KVM: inject #UD if instruction emulation fails while vcpu is in cpl==3

RFC PATCH In review Gleb Natapov 2 participants 8 messages Apr 30, 2010 – May 6, 2010

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Gleb Natapov

author

2010-04-30 17:18 UTC #0 lore raw

RFC PATCH 1/1 KVM: inject #UD if instruction emulation fails while vcpu is in cpl==3

Do not kill VM If instruction emulation fails while vcpu is in userspace. Inject #UD instead in a hope that guest OS will kill offending process. Emulation failure is still traced by ftrace point to help analyze the problem.

Signed-off-by Gleb Natapov <[email protected]>

---

This patch goes on top of previous patchset.

arch/x86/include/asm/kvm_host.h +0 -1

index ed48904..5aa0944 100644
@@ -575,7 +575,6 @@ enum emulation_result {
 #define EMULTYPE_SKIP		    (1 << 2)
 int emulate_instruction(struct kvm_vcpu *vcpu,
 			unsigned long cr2, u16 error_code, int emulation_type);
-void kvm_report_emulation_failure(struct kvm_vcpu *cvpu, const char *context);
 void realmode_lgdt(struct kvm_vcpu *vcpu, u16 size, unsigned long address);
 void realmode_lidt(struct kvm_vcpu *vcpu, u16 size, unsigned long address);

arch/x86/kvm/mmu.c +1 -4

index ddfa865..4ddcb1b 100644
@@ -2776,11 +2776,8 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code)
 		return 1;
 	case EMULATE_DO_MMIO:
 		++vcpu->stat.mmio_exits;
-		return 0;
+		/* fall through */
 	case EMULATE_FAIL:
-		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
-		vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
-		vcpu->run->internal.ndata = 0;
 		return 0;
 	default:
 		BUG();

arch/x86/kvm/svm.c +3 -7

index 889f660..b2eed27 100644
@@ -1449,7 +1449,7 @@ static int io_interception(struct vcpu_svm *svm)
 	string = (io_info & SVM_IOIO_STR_MASK) != 0;
 	in = (io_info & SVM_IOIO_TYPE_MASK) != 0;
 	if (string || in)
-		return !(emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO);
+		return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
 
 	port = io_info >> 16;
 	size = (io_info & SVM_IOIO_SIZE_MASK) >> SVM_IOIO_SIZE_SHIFT;

@@ -2297,16 +2297,12 @@ static int iret_interception(struct vcpu_svm *svm)
 
 static int invlpg_interception(struct vcpu_svm *svm)
 {
-	if (emulate_instruction(&svm->vcpu, 0, 0, 0) != EMULATE_DONE)
-		pr_unimpl(&svm->vcpu, "%s: failed\n", __func__);
-	return 1;
+	return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE;
 }
 
 static int emulate_on_interception(struct vcpu_svm *svm)
 {
-	if (emulate_instruction(&svm->vcpu, 0, 0, 0) != EMULATE_DONE)
-		pr_unimpl(&svm->vcpu, "%s: failed\n", __func__);
-	return 1;
+	return emulate_instruction(&svm->vcpu, 0, 0, 0) == EMULATE_DONE;
 }
 
 static int cr8_write_interception(struct vcpu_svm *svm)

arch/x86/kvm/vmx.c +4 ~~-24~~

index 875b785..d05bc46 100644
@@ -3003,7 +3003,7 @@ static int handle_io(struct kvm_vcpu *vcpu)
 	++vcpu->stat.io_exits;
 
 	if (string || in)
-		return !(emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DO_MMIO);
+		return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
 
 	port = exit_qualification >> 16;
 	size = (exit_qualification & 7) + 1;

@@ -3260,22 +3260,7 @@ static int handle_wbinvd(struct kvm_vcpu *vcpu)
 
 static int handle_apic_access(struct kvm_vcpu *vcpu)
 {
-	unsigned long exit_qualification;
-	enum emulation_result er;
-	unsigned long offset;
-
-	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
-	offset = exit_qualification & 0xffful;
-
-	er = emulate_instruction(vcpu, 0, 0, 0);
-
-	if (er !=  EMULATE_DONE) {
-		printk(KERN_ERR
-		       "Fail to handle apic access vmexit! Offset is 0x%lx\n",
-		       offset);
-		return -ENOEXEC;
-	}
-	return 1;
+	return emulate_instruction(vcpu, 0, 0, 0) == EMULATE_DONE;
 }
 
 static int handle_task_switch(struct kvm_vcpu *vcpu)

@@ -3487,13 +3472,8 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
 			goto out;
 		}
 
-		if (err != EMULATE_DONE) {
-			vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
-			vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
-			vcpu->run->internal.ndata = 0;
-			ret = 0;
-			goto out;
-		}
+		if (err != EMULATE_DONE)
+			return 0;
 
 		if (signal_pending(current))
 			goto out;

arch/x86/kvm/x86.c +26 ~~-25~~

83 lines not on this page · raw message

--
Gleb.

Avi Kivity

2010-05-06 09:16 UTC #1 lore raw

Quoted 5 lines · Gleb Natapov

Do not kill VM If instruction emulation fails while vcpu is in userspace. Inject #UD instead in a hope that guest OS will kill offending process. Emulation failure is still traced by ftrace point to help analyze the problem.

Still there's the risk here that a critical failure goes unnoticed. ftrace isn't on at all times.

We can probably inject a #UD unconditionally and exit to userspace. Userspace would then report the problem to the user and reenter the guest, which would then recover or not.

Signature · 1 line

error compiling committee.c: too many arguments to function

Gleb Natapov

author

2010-05-06 10:06 UTC #2 lore raw

Quoted 9 lines · Avi Kivity

Quoted 4 lines · Gleb Natapov
Do not kill VM If instruction emulation fails while vcpu is in userspace. Inject #UD instead in a hope that guest OS will kill offending process. Emulation failure is still traced by ftrace point to help analyze the problem.
Still there's the risk here that a critical failure goes unnoticed. ftrace isn't on at all times.

Kvm_stat will still show that there was emulation failure, so if strange application behaviour is reported kvm_stat output will have hints where to look. Next step in analyzing the problem will be enabling emulator tracing.

Quoted 4 lines

We can probably inject a #UD unconditionally and exit to userspace. Userspace would then report the problem to the user and reenter the guest, which would then recover or not.

By "unconditionally" you mean even if guest is in kernel mode? There is no point in trying to continue after that happens. Instead of getting paused VM at exact place where problem happened and easily analyzable we will get misbehaved VM with undefined state.

--
Gleb.

Avi Kivity

2010-05-06 10:13 UTC #3 lore raw

Quoted 9 lines · Gleb Natapov

Quoted 7 lines · Avi Kivity
Quoted 5 lines · Gleb Natapov
Do not kill VM If instruction emulation fails while vcpu is in userspace. Inject #UD instead in a hope that guest OS will kill offending process. Emulation failure is still traced by ftrace point to help analyze the problem.

Quoted 9 lines

Quoted 4 lines
Still there's the risk here that a critical failure goes unnoticed. ftrace isn't on at all times.
Kvm_stat will still show that there was emulation failure, so if strange application behaviour is reported kvm_stat output will have hints where to look. Next step in analyzing the problem will be enabling emulator tracing.

We can expect that from a developer or a user subscribed to kvm@. But what about some random user running virt-manager?

dmesg and kvm_stat will not go up the management stack.

Quoted 6 lines

Quoted 5 lines
We can probably inject a #UD unconditionally and exit to userspace. Userspace would then report the problem to the user and reenter the guest, which would then recover or not.
By "unconditionally" you mean even if guest is in kernel mode?

Yes.

Quoted 5 lines

There is
no point in trying to continue after that happens. Instead of getting paused VM at exact place where problem happened and easily analyzable we will get misbehaved VM with undefined state.

True. But the same problem exists with cpl>0 #UD. It may be a critical driver in userspace (say, video driver).

Also need to think consider nested kernels (which are userspace for this purpose).

How about default to unconditional #UD and report, and pause if requested (in userspace)? Usually emulation failures will be 100% reproducible, so the user can rerun their workload.

Signature · 1 line

error compiling committee.c: too many arguments to function

Gleb Natapov

author

2010-05-06 12:25 UTC #4 lore raw

Quoted 10 lines · Avi Kivity

Quoted 9 lines · Gleb Natapov
Quoted 8 lines · Avi Kivity
Quoted 4 lines · Gleb Natapov
Do not kill VM If instruction emulation fails while vcpu is in userspace. Inject #UD instead in a hope that guest OS will kill offending process. Emulation failure is still traced by ftrace point to help analyze the problem.
Still there's the risk here that a critical failure goes unnoticed. ftrace isn't on at all times.

Quoted 8 lines

Quoted 4 lines
Kvm_stat will still show that there was emulation failure, so if strange application behaviour is reported kvm_stat output will have hints where to look. Next step in analyzing the problem will be enabling emulator tracing.
We can expect that from a developer or a user subscribed to kvm@. But what about some random user running virt-manager?

What virt-manager would do about such error?

Quoted 15 lines

dmesg and kvm_stat will not go up the management stack.
Quoted 5 lines
Quoted 4 lines
We can probably inject a #UD unconditionally and exit to userspace. Userspace would then report the problem to the user and reenter the guest, which would then recover or not.
By "unconditionally" you mean even if guest is in kernel mode?
Yes.
Quoted 4 lines
There is
no point in trying to continue after that happens. Instead of getting paused VM at exact place where problem happened and easily analyzable we will get misbehaved VM with undefined state.

Quoted 5 lines

True. But the same problem exists with cpl>0 #UD. It may be a critical driver in userspace (say, video driver).
Also need to think consider nested kernels (which are userspace for this purpose).

Ugh, we can check if vcpu is in nested mode.

Quoted 5 lines

How about default to unconditional #UD and report, and pause if requested (in userspace)? Usually emulation failures will be 100% reproducible, so the user can rerun their workload.

Didn't what to involve userspace in this, but it can be done of course.

--
Gleb.

Avi Kivity

2010-05-06 12:33 UTC #5 lore raw

Quoted 7 lines · Gleb Natapov

Quoted 5 lines
We can expect that from a developer or a user subscribed to kvm@. But what about some random user running virt-manager?
What virt-manager would do about such error?

Call up abrt.

Quoted 8 lines

Quoted 6 lines
True. But the same problem exists with cpl>0 #UD. It may be a critical driver in userspace (say, video driver).
Also need to think consider nested kernels (which are userspace for this purpose).
Ugh, we can check if vcpu is in nested mode.

And do what? Inject #UD to the guest? Or force some vmexit?

Quoted 7 lines

Quoted 5 lines
How about default to unconditional #UD and report, and pause if requested (in userspace)? Usually emulation failures will be 100% reproducible, so the user can rerun their workload.
Didn't what to involve userspace in this, but it can be done of course.

Whenever we have to make a decision, we involve userspace.

Signature · 1 line

error compiling committee.c: too many arguments to function

Gleb Natapov

author

2010-05-06 12:41 UTC #6 lore raw

Quoted 9 lines · Avi Kivity

Quoted 5 lines · Gleb Natapov
Quoted 4 lines
We can expect that from a developer or a user subscribed to kvm@. But what about some random user running virt-manager?
What virt-manager would do about such error?
Call up abrt.

The idea is not to let userspace process running in a VM kill the VM.

Quoted 9 lines

Quoted 6 lines
Quoted 5 lines
True. But the same problem exists with cpl>0 #UD. It may be a critical driver in userspace (say, video driver).
Also need to think consider nested kernels (which are userspace for this purpose).
Ugh, we can check if vcpu is in nested mode.
And do what? Inject #UD to the guest? Or force some vmexit?

Does host emulator will ever run on behalf of nested guest? We have emulator inside nested guest for this.

Quoted 10 lines

Quoted 5 lines
Quoted 4 lines
How about default to unconditional #UD and report, and pause if requested (in userspace)? Usually emulation failures will be 100% reproducible, so the user can rerun their workload.
Didn't what to involve userspace in this, but it can be done of course.
Whenever we have to make a decision, we involve userspace.
Signature · 1 line
error compiling committee.c: too many arguments to function

--
Gleb.

Avi Kivity

2010-05-06 12:48 UTC #7 lore raw

Quoted 9 lines · Gleb Natapov

Quoted 7 lines · Avi Kivity
Quoted 5 lines · Gleb Natapov
Quoted 5 lines
We can expect that from a developer or a user subscribed to kvm@. But what about some random user running virt-manager?

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
What virt-manager would do about such error?
Call up abrt.
The idea is not to let userspace process running in a VM kill the VM.

Well, log the problem (including registers and instruction code), and continue with the #UD.

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
True. But the same problem exists with cpl>0 #UD. It may be a critical driver in userspace (say, video driver).
Also need to think consider nested kernels (which are userspace for this purpose).

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Ugh, we can check if vcpu is in nested mode.
And do what? Inject #UD to the guest? Or force some vmexit?
Does host emulator will ever run on behalf of nested guest? We have emulator inside nested guest for this.

If the guest doesn't map the page, it will emulate. If it does map the page, and the host doesn't, the host emulates.

Signature · 1 line

error compiling committee.c: too many arguments to function