On Tue, Jun 15, 2010 at 04:50:35PM +0300, Avi Kivity wrote:
> On 06/15/2010 04:47 PM, Gleb Natapov wrote:
> >On Sun, Jun 13, 2010 at 03:27:10PM +0300, Nadav Har'El wrote:
> >>This patch implements the VMCLEAR instruction.
> >>
> >>Signed-off-by: Nadav Har'El<nyh@xxxxxxxxxx>
> >>---
> >>--- .before/arch/x86/kvm/vmx.c 2010-06-13 15:01:29.000000000 +0300
> >>+++ .after/arch/x86/kvm/vmx.c 2010-06-13 15:01:29.000000000 +0300
> >>@@ -138,6 +138,8 @@ struct __attribute__ ((__packed__)) vmcs
> >> */
> >> u32 revision_id;
> >> u32 abort;
> >>+
> >>+ bool launch_state; /* set to 0 by VMCLEAR, to 1 by VMLAUNCH */
> >> };
> >>
> >> struct vmcs_list {
> >>@@ -3827,6 +3829,46 @@ static int read_guest_vmcs_gpa(struct kv
> >> return 0;
> >> }
> >>
> >>+static void clear_rflags_cf_zf(struct kvm_vcpu *vcpu)
> >>+{
> >>+ unsigned long rflags;
> >>+ rflags = vmx_get_rflags(vcpu);
> >>+ rflags&= ~(X86_EFLAGS_CF | X86_EFLAGS_ZF);
> >>+ vmx_set_rflags(vcpu, rflags);
> >>+}
> >>+
> >>+/* Emulate the VMCLEAR instruction */
> >>+static int handle_vmclear(struct kvm_vcpu *vcpu)
> >>+{
> >>+ struct vcpu_vmx *vmx = to_vmx(vcpu);
> >>+ gpa_t guest_vmcs_addr, save_current_vmptr;
> >>+
> >>+ if (!nested_vmx_check_permission(vcpu))
> >>+ return 1;
> >>+
> >>+ if (read_guest_vmcs_gpa(vcpu,&guest_vmcs_addr))
> >>+ return 1;
> >>+
> >>+ save_current_vmptr = vmx->nested.current_vmptr;
> >>+
> >>+ vmx->nested.current_vmptr = guest_vmcs_addr;
> >>+ if (!nested_map_current(vcpu))
> >>+ return 1;
> >>+ vmx->nested.current_l2_page->launch_state = 0;
> >>+ nested_unmap_current(vcpu);
> >>+
> >>+ nested_free_current_vmcs(vcpu);
> >>+
> >>+ if (save_current_vmptr == guest_vmcs_addr)
> >>+ vmx->nested.current_vmptr = -1ull;
> >>+ else
> >>+ vmx->nested.current_vmptr = save_current_vmptr;
> >>+
> >>+ skip_emulated_instruction(vcpu);
> >>+ clear_rflags_cf_zf(vcpu);
> >>+ return 1;
> >>+}
> >>+
> >Shouldn't error cases update flags too?
>
> Architectural errors (bad alignment) should update flags. Internal
> errors (ENOMEM, vpmtr pointing outside of RAM) should not.
>
vpmtr pointing outside of RAM is architectural error (or not?). SDM
says "The operand of this instruction is always 64 bits and is always in
memory", but may be they mean "not in register". Anyway internal errors
should generate error exit to userspace which this patch is also
missing.
--
Gleb.
--
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
