| Message ID | 1353488677-29937-5-git-send-email-dongxiao.xu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 21, 2012 at 05:04:37PM +0800, Dongxiao Xu wrote: > The launch state is not a member in the VMCS area, use a separate > variable (list) to store it instead. > Why? Guest shouldn't be aware of the format of VMCS area. > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> > --- > arch/x86/kvm/vmx.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 files changed, 81 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 6687fb6..d03ab4e 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -177,8 +177,7 @@ struct __packed vmcs12 { > u32 revision_id; > u32 abort; > > - u32 launch_state; /* set to 0 by VMCLEAR, to 1 by VMLAUNCH */ > - u32 padding[7]; /* room for future expansion */ > + u32 padding[8]; /* room for future expansion */ > > u64 io_bitmap_a; > u64 io_bitmap_b; > @@ -339,6 +338,11 @@ struct vmcs02_list { > struct loaded_vmcs vmcs02; > }; > > +struct vmcs12_list { > + unsigned long vmcs12_pa; > + struct list_head node; > +}; > + > /* > * The nested_vmx structure is part of vcpu_vmx, and holds information we need > * for correct emulation of VMX (i.e., nested VMX) on this vcpu. > @@ -364,6 +368,8 @@ struct nested_vmx { > * we must keep them pinned while L2 runs. > */ > struct page *apic_access_page; > + /* vmcs12_pool contains the launched vmcs12. */ > + struct list_head vmcs12_pool; > }; > > struct vcpu_vmx { > @@ -614,6 +620,58 @@ static void nested_release_page_clean(struct page *page) > kvm_release_page_clean(page); > } > > +static int vmcs12_launched(struct list_head *vmcs12_pool, > + unsigned long vmcs12_pa) > +{ > + struct vmcs12_list *iter; > + struct list_head *pos; > + int launched = 0; > + > + list_for_each(pos, vmcs12_pool) { > + iter = list_entry(pos, struct vmcs12_list, node); > + if (vmcs12_pa == iter->vmcs12_pa) { > + launched = 1; > + break; > + } > + } > + > + return launched; > +} > + > +static int set_vmcs12_launched(struct list_head *vmcs12_pool, > + unsigned long vmcs12_pa) > +{ > + struct vmcs12_list *vmcs12; > + > + if (vmcs12_launched(vmcs12_pool, vmcs12_pa)) > + return 0; > + > + vmcs12 = kzalloc(sizeof(struct vmcs12_list), GFP_KERNEL); > + if (!vmcs12) > + return -ENOMEM; > + > + vmcs12->vmcs12_pa = vmcs12_pa; > + list_add(&vmcs12->node, vmcs12_pool); > + > + return 0; > +} > + > +static void clear_vmcs12_launched(struct list_head *vmcs12_pool, > + unsigned long vmcs12_pa) > +{ > + struct vmcs12_list *iter; > + struct list_head *pos; > + > + list_for_each(pos, vmcs12_pool) { > + iter = list_entry(pos, struct vmcs12_list, node); > + if (vmcs12_pa == iter->vmcs12_pa) { > + list_del(&iter->node); > + kfree(iter); > + break; > + } > + } > +} > + > static u64 construct_eptp(unsigned long root_hpa); > static void kvm_cpu_vmxon(u64 addr); > static void kvm_cpu_vmxoff(void); > @@ -5111,6 +5169,18 @@ static void nested_free_all_saved_vmcss(struct vcpu_vmx *vmx) > } > > /* > + * Free the vmcs12 list. > + */ > +static void nested_free_vmcs12_list(struct vcpu_vmx *vmx) > +{ > + struct vmcs12_list *item, *n; > + list_for_each_entry_safe(item, n, &vmx->nested.vmcs12_pool, node) { > + list_del(&item->node); > + kfree(item); > + } > +} > + > +/* > * Emulate the VMXON instruction. > * Currently, we just remember that VMX is active, and do not save or even > * inspect the argument to VMXON (the so-called "VMXON pointer") because we > @@ -5207,6 +5277,7 @@ static void free_nested(struct vcpu_vmx *vmx) > } > > nested_free_all_saved_vmcss(vmx); > + nested_free_vmcs12_list(vmx); > } > > /* Emulate the VMXOFF instruction */ > @@ -5359,7 +5430,7 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) > return 1; > } > vmcs12 = kmap(page); > - vmcs12->launch_state = 0; > + clear_vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)); > kunmap(page); > nested_release_page(page); > > @@ -6467,6 +6538,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) > > vmx->nested.current_vmptr = -1ull; > vmx->nested.current_vmcs12 = NULL; > + INIT_LIST_HEAD(&vmx->nested.vmcs12_pool); > > return &vmx->vcpu; > > @@ -6846,6 +6918,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > struct vcpu_vmx *vmx = to_vmx(vcpu); > int cpu; > struct loaded_vmcs *vmcs02; > + int is_launched; > > if (!nested_vmx_check_permission(vcpu) || > !nested_vmx_check_vmcs12(vcpu)) > @@ -6864,7 +6937,9 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > * for misconfigurations which will anyway be caught by the processor > * when using the merged vmcs02. > */ > - if (vmcs12->launch_state == launch) { > + is_launched = > + vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)); > + if (is_launched == launch) { > nested_vmx_failValid(vcpu, > launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS > : VMXERR_VMRESUME_NONLAUNCHED_VMCS); > @@ -6953,7 +7028,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > vcpu->cpu = cpu; > put_cpu(); > > - vmcs12->launch_state = 1; > + if (set_vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)) < 0) > + return -ENOMEM; > > prepare_vmcs02(vcpu); > > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Gleb Natapov [mailto:gleb@redhat.com] > Sent: Wednesday, November 21, 2012 10:15 PM > To: Xu, Dongxiao > Cc: kvm@vger.kernel.org; mtosatti@redhat.com > Subject: Re: [PATCH 4/4] nested vmx: use a list to store the launched vmcs12 > for L1 VMM > > On Wed, Nov 21, 2012 at 05:04:37PM +0800, Dongxiao Xu wrote: > > The launch state is not a member in the VMCS area, use a separate > > variable (list) to store it instead. > > > Why? Guest shouldn't be aware of the format of VMCS area. Yes, I agree. Guest VMM/L1 VMM shouldn't be aware of the VMCS format. For Root VMM/L0 VMM, it need to track the launch state of the vmcs12, in order to correctly emulate the VMLAUNCH and VMRESUME instructions. Originally we store the launch state in the VMCS area, however in fact, there is no "launch state" field in VMCS. This patch is to move it out and use a separate list to store it. Thanks, Dongxiao > > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> > > --- > > arch/x86/kvm/vmx.c | 86 > +++++++++++++++++++++++++++++++++++++++++++++++++--- > > 1 files changed, 81 insertions(+), 5 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index > > 6687fb6..d03ab4e 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -177,8 +177,7 @@ struct __packed vmcs12 { > > u32 revision_id; > > u32 abort; > > > > - u32 launch_state; /* set to 0 by VMCLEAR, to 1 by VMLAUNCH */ > > - u32 padding[7]; /* room for future expansion */ > > + u32 padding[8]; /* room for future expansion */ > > > > u64 io_bitmap_a; > > u64 io_bitmap_b; > > @@ -339,6 +338,11 @@ struct vmcs02_list { > > struct loaded_vmcs vmcs02; > > }; > > > > +struct vmcs12_list { > > + unsigned long vmcs12_pa; > > + struct list_head node; > > +}; > > + > > /* > > * The nested_vmx structure is part of vcpu_vmx, and holds information we > need > > * for correct emulation of VMX (i.e., nested VMX) on this vcpu. > > @@ -364,6 +368,8 @@ struct nested_vmx { > > * we must keep them pinned while L2 runs. > > */ > > struct page *apic_access_page; > > + /* vmcs12_pool contains the launched vmcs12. */ > > + struct list_head vmcs12_pool; > > }; > > > > struct vcpu_vmx { > > @@ -614,6 +620,58 @@ static void nested_release_page_clean(struct page > *page) > > kvm_release_page_clean(page); > > } > > > > +static int vmcs12_launched(struct list_head *vmcs12_pool, > > + unsigned long vmcs12_pa) > > +{ > > + struct vmcs12_list *iter; > > + struct list_head *pos; > > + int launched = 0; > > + > > + list_for_each(pos, vmcs12_pool) { > > + iter = list_entry(pos, struct vmcs12_list, node); > > + if (vmcs12_pa == iter->vmcs12_pa) { > > + launched = 1; > > + break; > > + } > > + } > > + > > + return launched; > > +} > > + > > +static int set_vmcs12_launched(struct list_head *vmcs12_pool, > > + unsigned long vmcs12_pa) > > +{ > > + struct vmcs12_list *vmcs12; > > + > > + if (vmcs12_launched(vmcs12_pool, vmcs12_pa)) > > + return 0; > > + > > + vmcs12 = kzalloc(sizeof(struct vmcs12_list), GFP_KERNEL); > > + if (!vmcs12) > > + return -ENOMEM; > > + > > + vmcs12->vmcs12_pa = vmcs12_pa; > > + list_add(&vmcs12->node, vmcs12_pool); > > + > > + return 0; > > +} > > + > > +static void clear_vmcs12_launched(struct list_head *vmcs12_pool, > > + unsigned long vmcs12_pa) > > +{ > > + struct vmcs12_list *iter; > > + struct list_head *pos; > > + > > + list_for_each(pos, vmcs12_pool) { > > + iter = list_entry(pos, struct vmcs12_list, node); > > + if (vmcs12_pa == iter->vmcs12_pa) { > > + list_del(&iter->node); > > + kfree(iter); > > + break; > > + } > > + } > > +} > > + > > static u64 construct_eptp(unsigned long root_hpa); static void > > kvm_cpu_vmxon(u64 addr); static void kvm_cpu_vmxoff(void); @@ -5111,6 > > +5169,18 @@ static void nested_free_all_saved_vmcss(struct vcpu_vmx > > *vmx) } > > > > /* > > + * Free the vmcs12 list. > > + */ > > +static void nested_free_vmcs12_list(struct vcpu_vmx *vmx) { > > + struct vmcs12_list *item, *n; > > + list_for_each_entry_safe(item, n, &vmx->nested.vmcs12_pool, node) { > > + list_del(&item->node); > > + kfree(item); > > + } > > +} > > + > > +/* > > * Emulate the VMXON instruction. > > * Currently, we just remember that VMX is active, and do not save or even > > * inspect the argument to VMXON (the so-called "VMXON pointer") > > because we @@ -5207,6 +5277,7 @@ static void free_nested(struct > vcpu_vmx *vmx) > > } > > > > nested_free_all_saved_vmcss(vmx); > > + nested_free_vmcs12_list(vmx); > > } > > > > /* Emulate the VMXOFF instruction */ > > @@ -5359,7 +5430,7 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) > > return 1; > > } > > vmcs12 = kmap(page); > > - vmcs12->launch_state = 0; > > + clear_vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)); > > kunmap(page); > > nested_release_page(page); > > > > @@ -6467,6 +6538,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct > > kvm *kvm, unsigned int id) > > > > vmx->nested.current_vmptr = -1ull; > > vmx->nested.current_vmcs12 = NULL; > > + INIT_LIST_HEAD(&vmx->nested.vmcs12_pool); > > > > return &vmx->vcpu; > > > > @@ -6846,6 +6918,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, > bool launch) > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > int cpu; > > struct loaded_vmcs *vmcs02; > > + int is_launched; > > > > if (!nested_vmx_check_permission(vcpu) || > > !nested_vmx_check_vmcs12(vcpu)) > > @@ -6864,7 +6937,9 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, > bool launch) > > * for misconfigurations which will anyway be caught by the processor > > * when using the merged vmcs02. > > */ > > - if (vmcs12->launch_state == launch) { > > + is_launched = > > + vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)); > > + if (is_launched == launch) { > > nested_vmx_failValid(vcpu, > > launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS > > : VMXERR_VMRESUME_NONLAUNCHED_VMCS); > > @@ -6953,7 +7028,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, > bool launch) > > vcpu->cpu = cpu; > > put_cpu(); > > > > - vmcs12->launch_state = 1; > > + if (set_vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)) < 0) > > + return -ENOMEM; > > > > prepare_vmcs02(vcpu); > > > > -- > > 1.7.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe kvm" in the > > body of a message to majordomo@vger.kernel.org More majordomo info at > > http://vger.kernel.org/majordomo-info.html > > -- > Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 6687fb6..d03ab4e 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -177,8 +177,7 @@ struct __packed vmcs12 { u32 revision_id; u32 abort; - u32 launch_state; /* set to 0 by VMCLEAR, to 1 by VMLAUNCH */ - u32 padding[7]; /* room for future expansion */ + u32 padding[8]; /* room for future expansion */ u64 io_bitmap_a; u64 io_bitmap_b; @@ -339,6 +338,11 @@ struct vmcs02_list { struct loaded_vmcs vmcs02; }; +struct vmcs12_list { + unsigned long vmcs12_pa; + struct list_head node; +}; + /* * The nested_vmx structure is part of vcpu_vmx, and holds information we need * for correct emulation of VMX (i.e., nested VMX) on this vcpu. @@ -364,6 +368,8 @@ struct nested_vmx { * we must keep them pinned while L2 runs. */ struct page *apic_access_page; + /* vmcs12_pool contains the launched vmcs12. */ + struct list_head vmcs12_pool; }; struct vcpu_vmx { @@ -614,6 +620,58 @@ static void nested_release_page_clean(struct page *page) kvm_release_page_clean(page); } +static int vmcs12_launched(struct list_head *vmcs12_pool, + unsigned long vmcs12_pa) +{ + struct vmcs12_list *iter; + struct list_head *pos; + int launched = 0; + + list_for_each(pos, vmcs12_pool) { + iter = list_entry(pos, struct vmcs12_list, node); + if (vmcs12_pa == iter->vmcs12_pa) { + launched = 1; + break; + } + } + + return launched; +} + +static int set_vmcs12_launched(struct list_head *vmcs12_pool, + unsigned long vmcs12_pa) +{ + struct vmcs12_list *vmcs12; + + if (vmcs12_launched(vmcs12_pool, vmcs12_pa)) + return 0; + + vmcs12 = kzalloc(sizeof(struct vmcs12_list), GFP_KERNEL); + if (!vmcs12) + return -ENOMEM; + + vmcs12->vmcs12_pa = vmcs12_pa; + list_add(&vmcs12->node, vmcs12_pool); + + return 0; +} + +static void clear_vmcs12_launched(struct list_head *vmcs12_pool, + unsigned long vmcs12_pa) +{ + struct vmcs12_list *iter; + struct list_head *pos; + + list_for_each(pos, vmcs12_pool) { + iter = list_entry(pos, struct vmcs12_list, node); + if (vmcs12_pa == iter->vmcs12_pa) { + list_del(&iter->node); + kfree(iter); + break; + } + } +} + static u64 construct_eptp(unsigned long root_hpa); static void kvm_cpu_vmxon(u64 addr); static void kvm_cpu_vmxoff(void); @@ -5111,6 +5169,18 @@ static void nested_free_all_saved_vmcss(struct vcpu_vmx *vmx) } /* + * Free the vmcs12 list. + */ +static void nested_free_vmcs12_list(struct vcpu_vmx *vmx) +{ + struct vmcs12_list *item, *n; + list_for_each_entry_safe(item, n, &vmx->nested.vmcs12_pool, node) { + list_del(&item->node); + kfree(item); + } +} + +/* * Emulate the VMXON instruction. * Currently, we just remember that VMX is active, and do not save or even * inspect the argument to VMXON (the so-called "VMXON pointer") because we @@ -5207,6 +5277,7 @@ static void free_nested(struct vcpu_vmx *vmx) } nested_free_all_saved_vmcss(vmx); + nested_free_vmcs12_list(vmx); } /* Emulate the VMXOFF instruction */ @@ -5359,7 +5430,7 @@ static int handle_vmclear(struct kvm_vcpu *vcpu) return 1; } vmcs12 = kmap(page); - vmcs12->launch_state = 0; + clear_vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)); kunmap(page); nested_release_page(page); @@ -6467,6 +6538,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id) vmx->nested.current_vmptr = -1ull; vmx->nested.current_vmcs12 = NULL; + INIT_LIST_HEAD(&vmx->nested.vmcs12_pool); return &vmx->vcpu; @@ -6846,6 +6918,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) struct vcpu_vmx *vmx = to_vmx(vcpu); int cpu; struct loaded_vmcs *vmcs02; + int is_launched; if (!nested_vmx_check_permission(vcpu) || !nested_vmx_check_vmcs12(vcpu)) @@ -6864,7 +6937,9 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) * for misconfigurations which will anyway be caught by the processor * when using the merged vmcs02. */ - if (vmcs12->launch_state == launch) { + is_launched = + vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)); + if (is_launched == launch) { nested_vmx_failValid(vcpu, launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS : VMXERR_VMRESUME_NONLAUNCHED_VMCS); @@ -6953,7 +7028,8 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) vcpu->cpu = cpu; put_cpu(); - vmcs12->launch_state = 1; + if (set_vmcs12_launched(&vmx->nested.vmcs12_pool, __pa(vmcs12)) < 0) + return -ENOMEM; prepare_vmcs02(vcpu);
The launch state is not a member in the VMCS area, use a separate variable (list) to store it instead. Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> --- arch/x86/kvm/vmx.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 81 insertions(+), 5 deletions(-)