From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Nadav Har'El" Subject: Re: [PATCH 13/24] Implement VMREAD and VMWRITE Date: Wed, 4 Aug 2010 14:46:50 +0300 Message-ID: <20100804114650.GA15156@fermat.math.technion.ac.il> References: <1276431753-nyh@il.ibm.com> <201006131229.o5DCTDhk013030@rice.haifa.ibm.com> <20100616150314.GE523@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: avi@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mailgw13.technion.ac.il ([132.68.225.13]:18463 "EHLO mailgw13.technion.ac.il" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757407Ab0HDLqx (ORCPT ); Wed, 4 Aug 2010 07:46:53 -0400 Content-Disposition: inline In-Reply-To: <20100616150314.GE523@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jun 16, 2010, Gleb Natapov wrote about "Re: [PATCH 13/24] Implement VMREAD and VMWRITE": > > + set_rflags_to_vmx_fail_valid(vcpu); > > + vmcs_write32(VM_INSTRUCTION_ERROR, 12); > VM_INSTRUCTION_ERROR is read only and when do you transfer it to vmcs12 anyway?. > I think set_rflags_to_vmx_fail_valid() should get vm_instruction_error > as a parameter and put it into vmcs12, that way you'll never forget to > provide error code on fail_valid case, compiler will remind you. Good catch, and I now do exactly what you suggested. Both you and Eddie Dong noticed that the functions that set the success and failure flags weren't quite doing the right thing, and certainly the vm_instruction_error needs to be set on vmcs12, not vmcs02 - and this needs to be done on every failValid - not only some of the places as the code now had. I'm fixing all these cases. I attach a new patch with just the 3 success/failure functions, and the list of error codes (from the spec, vol 2B table 5-1). > What about checking that vmcs field is read only? Good idea - I'll do that. ---- Subject: [PATCH 09/26] nVMX: Success/failure of VMX instructions. VMX instructions specify success or failure by setting certain RFLAGS bits. This patch contains common functions to do this, and they will be used in the following patches which emulate the various VMX instructions. Signed-off-by: Nadav Har'El --- arch/x86/include/asm/vmx.h | 31 +++++++++++++++++++++++++++++++ arch/x86/kvm/vmx.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) --- .before/arch/x86/kvm/vmx.c 2010-08-04 14:40:56.000000000 +0300 +++ .after/arch/x86/kvm/vmx.c 2010-08-04 14:40:56.000000000 +0300 @@ -3817,6 +3817,36 @@ static int read_guest_vmcs_gpa(struct kv return 0; } +/* + * The following 3 functions, nested_vmx_succeed()/failValid()/failInvalid(), + * set the success or error code of an emulated VMX instruction, as specified + * by Vol 2B, VMX Instruction Reference, "Conventions". + */ +static void nested_vmx_succeed(struct kvm_vcpu *vcpu) +{ + vmx_set_rflags(vcpu, vmx_get_rflags(vcpu) + & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | + X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)); +} + +static void nested_vmx_failInvalid(struct kvm_vcpu *vcpu) +{ + vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu) + & ~(X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_ZF | + X86_EFLAGS_SF | X86_EFLAGS_OF)) + | X86_EFLAGS_CF); +} + +static void nested_vmx_failValid(struct kvm_vcpu *vcpu, + u32 vm_instruction_error) +{ + vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu) + & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | + X86_EFLAGS_SF | X86_EFLAGS_OF)) + | X86_EFLAGS_ZF); + get_vmcs12_fields(vcpu)->vm_instruction_error = vm_instruction_error; +} + static int handle_invlpg(struct kvm_vcpu *vcpu) { unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION); --- .before/arch/x86/include/asm/vmx.h 2010-08-04 14:40:56.000000000 +0300 +++ .after/arch/x86/include/asm/vmx.h 2010-08-04 14:40:56.000000000 +0300 @@ -409,4 +409,35 @@ struct vmx_msr_entry { u64 value; } __aligned(16); +/* + * VM-instruction error numbers + */ +enum vm_instruction_error_number { + VMXERR_VMCALL_IN_VMX_ROOT_OPERATION = 1, + VMXERR_VMCLEAR_INVALID_ADDRESS = 2, + VMXERR_VMCLEAR_VMXON_POINTER = 3, + VMXERR_VMLAUNCH_NONCLEAR_VMCS = 4, + VMXERR_VMRESUME_NONLAUNCHED_VMCS = 5, + VMXERR_VMRESUME_CORRUPTED_VMCS = 6, + VMXERR_ENTRY_INVALID_CONTROL_FIELD = 7, + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD = 8, + VMXERR_VMPTRLD_INVALID_ADDRESS = 9, + VMXERR_VMPTRLD_VMXON_POINTER = 10, + VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID = 11, + VMXERR_UNSUPPORTED_VMCS_COMPONENT = 12, + VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT = 13, + VMXERR_VMXON_IN_VMX_ROOT_OPERATION = 15, + VMXERR_ENTRY_INVALID_EXECUTIVE_VMCS_POINTER = 16, + VMXERR_ENTRY_NONLAUNCHED_EXECUTIVE_VMCS = 17, + VMXERR_ENTRY_EXECUTIVE_VMCS_POINTER_NOT_VMXON_POINTER = 18, + VMXERR_VMCALL_NONCLEAR_VMCS = 19, + VMXERR_VMCALL_INVALID_VM_EXIT_CONTROL_FIELDS = 20, + VMXERR_VMCALL_INCORRECT_MSEG_REVISION_ID = 22, + VMXERR_VMXOFF_UNDER_DUAL_MONITOR_TREATMENT_OF_SMIS_AND_SMM = 23, + VMXERR_VMCALL_INVALID_SMM_MONITOR_FEATURES = 24, + VMXERR_ENTRY_INVALID_VM_EXECUTION_CONTROL_FIELDS_IN_EXECUTIVE_VMCS = 25, + VMXERR_ENTRY_EVENTS_BLOCKED_BY_MOV_SS = 26, + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID = 28, +}; + #endif -- Nadav Har'El | Wednesday, Aug 4 2010, 24 Av 5770 nyh@math.technion.ac.il |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |"A mathematician is a device for turning http://nadav.harel.org.il |coffee into theorems" -- P. Erdos