From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 5/5] Nested VMX patch 5 implements vmlaunch and vmresume Date: Tue, 20 Oct 2009 13:56:39 +0900 Message-ID: <4ADD4307.1030302@redhat.com> References: <1255617706-13564-1-git-send-email-oritw@il.ibm.com> <1255617706-13564-2-git-send-email-oritw@il.ibm.com> <1255617706-13564-3-git-send-email-oritw@il.ibm.com> <1255617706-13564-4-git-send-email-oritw@il.ibm.com> <1255617706-13564-5-git-send-email-oritw@il.ibm.com> <1255617706-13564-6-git-send-email-oritw@il.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, benami@il.ibm.com, abelg@il.ibm.com, muli@il.ibm.com, aliguori@us.ibm.com, mdday@us.ibm.com To: oritw@il.ibm.com Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26447 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752899AbZJTE4n (ORCPT ); Tue, 20 Oct 2009 00:56:43 -0400 In-Reply-To: <1255617706-13564-6-git-send-email-oritw@il.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/15/2009 11:41 PM, oritw@il.ibm.com wrote: > From: Orit Wasserman > > --- > arch/x86/kvm/vmx.c | 1173 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 files changed, 1148 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 6a4c252..e814029 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -209,6 +209,7 @@ struct __attribute__ ((__packed__)) level_state { > struct vmcs *vmcs; > int cpu; > int launched; > + bool first_launch; > }; > > struct nested_vmx { > @@ -216,6 +217,12 @@ struct nested_vmx { > bool vmxon; > /* What is the location of the vmcs l1 keeps for l2? (in level1 gpa) */ > u64 vmptr; > + /* Are we running nested guest */ > + bool nested_mode; > + /* L1 requested VMLAUNCH or VMRESUME but we didn't run L2 yet */ > + bool nested_run_pending; > + /* flag indicating if there was a valid IDT after exiting from l2 */ > + bool nested_valid_idt; > Did you mean valid_idt_vectoring_info? No need to prefix everything with nested_ inside nested_vmx. > +void prepare_vmcs_12(struct kvm_vcpu *vcpu) > +{ > + struct shadow_vmcs *l2_shadow_vmcs = > + get_shadow_vmcs(vcpu); > + > + l2_shadow_vmcs->guest_es_selector = vmcs_read16(GUEST_ES_SELECTOR); > + l2_shadow_vmcs->guest_cs_selector = vmcs_read16(GUEST_CS_SELECTOR); > + l2_shadow_vmcs->guest_ss_selector = vmcs_read16(GUEST_SS_SELECTOR); > + l2_shadow_vmcs->guest_ds_selector = vmcs_read16(GUEST_DS_SELECTOR); > + l2_shadow_vmcs->guest_fs_selector = vmcs_read16(GUEST_FS_SELECTOR); > + l2_shadow_vmcs->guest_gs_selector = vmcs_read16(GUEST_GS_SELECTOR); > + l2_shadow_vmcs->guest_ldtr_selector = vmcs_read16(GUEST_LDTR_SELECTOR); > + l2_shadow_vmcs->guest_tr_selector = vmcs_read16(GUEST_TR_SELECTOR); > + > + l2_shadow_vmcs->tsc_offset = vmcs_read64(TSC_OFFSET); > + l2_shadow_vmcs->guest_physical_address = > + vmcs_read64(GUEST_PHYSICAL_ADDRESS); > + l2_shadow_vmcs->vmcs_link_pointer = vmcs_read64(VMCS_LINK_POINTER); > Physical addresses need translation, no? > + l2_shadow_vmcs->guest_cr0 = vmcs_readl(GUEST_CR0); > + > + l2_shadow_vmcs->guest_cr4 = vmcs_readl(GUEST_CR4); > We don't allow the guest to modify these, so no need to read them. If you do, you need to remove the bits that we modify. > + > +int load_vmcs_common(struct shadow_vmcs *src) > +{ > + > + vmcs_write64(VMCS_LINK_POINTER, src->vmcs_link_pointer); > Why load this? > + vmcs_write64(GUEST_IA32_DEBUGCTL, src->guest_ia32_debugctl); > I think some features there are dangerous. > + vmcs_write32(VM_ENTRY_MSR_LOAD_COUNT, src->vm_entry_msr_load_count); > Need to verify? Also need to validate the loaded MSRs and run them through kvm_set_msr() instead of letting the cpu do it. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.