Re: [PATCHv2] KVM: inject #UD if instruction emulation fails and exit to userspace
On Mon, May 10, 2010 at 11:16:56AM +0300, Gleb Natapov wrote:
> Do not kill VM when instruction emulation fails. Inject #UD and report
> failure to userspace instead. Userspace may choose to reenter guest if
> vcpu is in userspace (cpl == 3) in which case guest OS will kill
> offending process and continue running.
>
Please use this one instead. Compilation warning fixed.
Signed-off-by: Gleb Natapov
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index ed48904..5aa0944 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -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);
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 95bee9d..41d2de8 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2788,11 +2788,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();
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index cea916f..58c91f5 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -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;
@@ -2300,16 +2300,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)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 777e00d..e35c479 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3074,7 +3074,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;
@@ -3331,22 +3331,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)
@@ -3558,13 +3543,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;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cd8a606..eb845cf
Re: [PATCHv2] KVM: inject #UD if instruction emulation fails and exit to userspace
On Mon, May 10, 2010 at 1:25 PM, Gleb Natapov wrote:
> On Mon, May 10, 2010 at 11:16:56AM +0300, Gleb Natapov wrote:
>> Do not kill VM when instruction emulation fails. Inject #UD and report
>> failure to userspace instead. Userspace may choose to reenter guest if
>> vcpu is in userspace (cpl == 3) in which case guest OS will kill
>> offending process and continue running.
>>
I am curious to know what'd happen in case the vcpu is in kernel space
(cpl == 0). Is that case handled?
> Please use this one instead. Compilation warning fixed.
>
> Signed-off-by: Gleb Natapov
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index ed48904..5aa0944 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -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);
>
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 95bee9d..41d2de8 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -2788,11 +2788,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();
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index cea916f..58c91f5 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -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;
> @@ -2300,16 +2300,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)
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 777e00d..e35c479 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3074,7 +3074,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;
> @@ -3331,22 +3331,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)
> @@ -3558,13 +3543,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;
> -
Re: [PATCHv2] KVM: inject #UD if instruction emulation fails and exit to userspace
On Mon, May 10, 2010 at 07:06:05PM +0300, Mohammed Gamal wrote: > On Mon, May 10, 2010 at 1:25 PM, Gleb Natapov wrote: > > On Mon, May 10, 2010 at 11:16:56AM +0300, Gleb Natapov wrote: > >> Do not kill VM when instruction emulation fails. Inject #UD and report > >> failure to userspace instead. Userspace may choose to reenter guest if > >> vcpu is in userspace (cpl == 3) in which case guest OS will kill > >> offending process and continue running. > >> > > I am curious to know what'd happen in case the vcpu is in kernel space > (cpl == 0). Is that case handled? > Currently no matter where emulation fails VM is stopped and cpu state is printed on stderr. After that patch userspace may choose to continue VM execution after emulation error (#UD will be injected into VM though). The policy is in userspace, but I don't see the point to continue execution after emulation failed in kernel. How kernel can recover from the #UD? -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] KVM: inject #UD if instruction emulation fails and exit to userspace
(2010/05/11 2:33), Gleb Natapov wrote: On Mon, May 10, 2010 at 07:06:05PM +0300, Mohammed Gamal wrote: On Mon, May 10, 2010 at 1:25 PM, Gleb Natapov wrote: On Mon, May 10, 2010 at 11:16:56AM +0300, Gleb Natapov wrote: Do not kill VM when instruction emulation fails. Inject #UD and report failure to userspace instead. Userspace may choose to reenter guest if vcpu is in userspace (cpl == 3) in which case guest OS will kill offending process and continue running. I am curious to know what'd happen in case the vcpu is in kernel space (cpl == 0). Is that case handled? Currently no matter where emulation fails VM is stopped and cpu state is printed on stderr. After that patch userspace may choose to continue VM execution after emulation error (#UD will be injected into VM though). The policy is in userspace, but I don't see the point to continue execution after emulation failed in kernel. How kernel can recover from the #UD? I don't see what is the recommended(possible) way of trouble shooting yet. If the user is managing both the guest and the host, it's simple: no worth reentering the guest. The user will just see the stderr. But what about if the user can only see the guest? In the case of non-virt, usually the user sees oops log or something and calls to a support staff. Compared to that, if VM is silently stopped, what information can the user see? In such a case, catching up the trouble and calling to a support staff is host side management staff's job? If you give us preferred way of trouble shooting of KVM, from the developer's point of view, it will really help us to prepare for the future use of KVM. What we can do will depend on your development! And this is one of the reasons why I'm interested in the x86's emulation development. :-) Thanks, Takuya -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] KVM: inject #UD if instruction emulation fails and exit to userspace
On Mon, May 10, 2010 at 11:16:56AM +0300, Gleb Natapov wrote: > Do not kill VM when instruction emulation fails. Inject #UD and report > failure to userspace instead. Userspace may choose to reenter guest if > vcpu is in userspace (cpl == 3) in which case guest OS will kill > offending process and continue running. > > Signed-off-by: Gleb Natapov > --- > > Changelog: > v1->v2: > - always inject #UD and report failure to userspace. Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
