On Mon, Jun 14, 2010, Avi Kivity wrote about "Re: [PATCH 10/24] Implement VMPTRLD": > >+ if (vmcs_page == NULL) > >+ return 0; > > > > Doesn't seem right. >... > >+ > >+ if (read_guest_vmcs_gpa(vcpu,&guest_vmcs_addr)) { > >+ set_rflags_to_vmx_fail_invalid(vcpu); > > > > Need to skip_emulated_instruction() in this case. Thanks. I've "cleaned up my act" regarding error checking, and am now much more careful to throw the right exception or set the right error code, and to call skip_emulated_instruction when necessary. Here is the new version, in case you want to look at it (of course, when I'm done I'll send the whole patch set again). ---- Subject: [PATCH 11/26] nVMX: Implement VMPTRLD This patch implements the VMPTRLD instruction. Signed-off-by: Nadav Har'El <nyh@xxxxxxxxxx> --- arch/x86/kvm/vmx.c | 64 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) --- .before/arch/x86/kvm/vmx.c 2010-08-05 14:12:24.000000000 +0300 +++ .after/arch/x86/kvm/vmx.c 2010-08-05 14:12:24.000000000 +0300 @@ -3882,6 +3882,68 @@ static int handle_vmclear(struct kvm_vcp return 1; } +/* Emulate the VMPTRLD instruction */ +static int handle_vmptrld(struct kvm_vcpu *vcpu) +{ + struct vcpu_vmx *vmx = to_vmx(vcpu); + gva_t gva; + gpa_t vmcs12_addr; + + if (!nested_vmx_check_permission(vcpu)) + return 1; + + if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION), + vmcs_read32(VMX_INSTRUCTION_INFO), &gva)) + return 1; + + if (kvm_read_guest_virt(gva, &vmcs12_addr, sizeof(vmcs12_addr), + vcpu, NULL)) { + kvm_queue_exception(vcpu, PF_VECTOR); + return 1; + } + + if (!IS_ALIGNED(vmcs12_addr, PAGE_SIZE)) { + nested_vmx_failValid(vcpu, VMXERR_VMPTRLD_INVALID_ADDRESS); + skip_emulated_instruction(vcpu); + return 1; + } + + if (vmx->nested.current_vmptr != vmcs12_addr) { + struct vmcs12 *new_vmcs12; + struct page *page; + page = nested_get_page(vcpu, vmcs12_addr); + if (page == NULL){ + nested_vmx_failInvalid(vcpu); + skip_emulated_instruction(vcpu); + return 1; + } + new_vmcs12 = kmap(page); + if (new_vmcs12->revision_id != VMCS12_REVISION){ + kunmap(page); + nested_release_page_clean(page); + nested_vmx_failValid(vcpu, + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID); + skip_emulated_instruction(vcpu); + return 1; + } + if (vmx->nested.current_vmptr != -1ull){ + kunmap(vmx->nested.current_vmcs12_page); + nested_release_page(vmx->nested.current_vmcs12_page); + } + + vmx->nested.current_vmptr = vmcs12_addr; + vmx->nested.current_vmcs12 = new_vmcs12; + vmx->nested.current_vmcs12_page = page; + + if (nested_create_current_vmcs(vcpu)) + return -ENOMEM; + } + + nested_vmx_succeed(vcpu); + skip_emulated_instruction(vcpu); + return 1; +} + static int handle_invlpg(struct kvm_vcpu *vcpu) { unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION); @@ -4166,7 +4228,7 @@ static int (*kvm_vmx_exit_handlers[])(st [EXIT_REASON_VMCALL] = handle_vmcall, [EXIT_REASON_VMCLEAR] = handle_vmclear, [EXIT_REASON_VMLAUNCH] = handle_vmx_insn, - [EXIT_REASON_VMPTRLD] = handle_vmx_insn, + [EXIT_REASON_VMPTRLD] = handle_vmptrld, [EXIT_REASON_VMPTRST] = handle_vmx_insn, [EXIT_REASON_VMREAD] = handle_vmx_insn, [EXIT_REASON_VMRESUME] = handle_vmx_insn, -- Nadav Har'El | Thursday, Aug 5 2010, 25 Av 5770 nyh@xxxxxxxxxxxxxxxxxxx |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |We could wipe out world hunger if we knew http://nadav.harel.org.il |how to make AOL's Free CD's edible! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html