The previous order of relinquish resource is: relinquish_domain_resources() -> vcpu_destroy() -> nvmx_vcpu_destroy(). However some L1 resources like nv_vvmcx and io_bitmaps are free in nvmx_vcpu_destroy(), therefore the relinquish_domain_resources() will not reduce the refcnt of the domain to 0, therefore the latter vcpu release functions will not be called. To fix this issue, we need to release the nv_vvmcx and io_bitmaps in relinquish_domain_resources(). Besides, after destroy the nested vcpu, we need to switch the vmx->vmcs back to the L1 and let the vcpu_destroy() logic to free the L1 VMCS page. Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> --- xen/arch/x86/hvm/hvm.c | 3 +++ xen/arch/x86/hvm/vmx/vmx.c | 3 ++- xen/arch/x86/hvm/vmx/vvmx.c | 11 +++++++++++ xen/include/asm-x86/hvm/hvm.h | 1 + xen/include/asm-x86/hvm/vmx/vvmx.h | 1 + 5 files changed, 18 insertions(+), 1 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 7f8a025..0576a24 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -561,6 +561,9 @@ int hvm_domain_initialise(struct domain *d) void hvm_domain_relinquish_resources(struct domain *d) { + if ( hvm_funcs.nhvm_domain_relinquish_resources ) + hvm_funcs.nhvm_domain_relinquish_resources(d); + hvm_destroy_ioreq_page(d, &d->arch.hvm_domain.ioreq); hvm_destroy_ioreq_page(d, &d->arch.hvm_domain.buf_ioreq); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index ffb86c1..3ea7012 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1547,7 +1547,8 @@ static struct hvm_function_table __read_mostly vmx_function_table = { .nhvm_vcpu_asid = nvmx_vcpu_asid, .nhvm_vmcx_guest_intercepts_trap = nvmx_intercepts_exception, .nhvm_vcpu_vmexit_trap = nvmx_vmexit_trap, - .nhvm_intr_blocked = nvmx_intr_blocked + .nhvm_intr_blocked = nvmx_intr_blocked, + .nhvm_domain_relinquish_resources = nvmx_domain_relinquish_resources }; struct hvm_function_table * __init start_vmx(void) diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index 2e0b79d..1f610eb 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -57,6 +57,9 @@ void nvmx_vcpu_destroy(struct vcpu *v) { struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); + if ( nvcpu->nv_n1vmcx ) + v->arch.hvm_vmx.vmcs = nvcpu->nv_n1vmcx; + nvmx_purge_vvmcs(v); if ( nvcpu->nv_n2vmcx ) { __vmpclear(virt_to_maddr(nvcpu->nv_n2vmcx)); @@ -65,6 +68,14 @@ void nvmx_vcpu_destroy(struct vcpu *v) } } +void nvmx_domain_relinquish_resources(struct domain *d) +{ + struct vcpu *v; + + for_each_vcpu ( d, v ) + nvmx_purge_vvmcs(v); +} + int nvmx_vcpu_reset(struct vcpu *v) { return 0; diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 7243c4e..3592a8c 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -179,6 +179,7 @@ struct hvm_function_table { bool_t (*nhvm_vmcx_hap_enabled)(struct vcpu *v); enum hvm_intblk (*nhvm_intr_blocked)(struct vcpu *v); + void (*nhvm_domain_relinquish_resources)(struct domain *d); }; extern struct hvm_function_table hvm_funcs; diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h b/xen/include/asm-x86/hvm/vmx/vvmx.h index 995f9f4..bbc34e7 100644 --- a/xen/include/asm-x86/hvm/vmx/vvmx.h +++ b/xen/include/asm-x86/hvm/vmx/vvmx.h @@ -96,6 +96,7 @@ uint32_t nvmx_vcpu_asid(struct vcpu *v); enum hvm_intblk nvmx_intr_blocked(struct vcpu *v); int nvmx_intercepts_exception(struct vcpu *v, unsigned int trap, int error_code); +void nvmx_domain_relinquish_resources(struct domain *d); int nvmx_handle_vmxon(struct cpu_user_regs *regs); int nvmx_handle_vmxoff(struct cpu_user_regs *regs); -- 1.7.1
Keir Fraser
2012-Aug-24 08:45 UTC
Re: [PATCH] nvmx: fix resource relinquish for nested VMX
On 23/08/2012 04:11, "Dongxiao Xu" <dongxiao.xu@intel.com> wrote:> The previous order of relinquish resource is: > relinquish_domain_resources() -> vcpu_destroy() -> nvmx_vcpu_destroy(). > However some L1 resources like nv_vvmcx and io_bitmaps are free in > nvmx_vcpu_destroy(), therefore the relinquish_domain_resources() > will not reduce the refcnt of the domain to 0, therefore the latter > vcpu release functions will not be called. > > To fix this issue, we need to release the nv_vvmcx and io_bitmaps in > relinquish_domain_resources(). > > Besides, after destroy the nested vcpu, we need to switch the vmx->vmcs > back to the L1 and let the vcpu_destroy() logic to free the L1 VMCS page. > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>Couple of comments below.> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 2e0b79d..1f610eb 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -57,6 +57,9 @@ void nvmx_vcpu_destroy(struct vcpu *v) > { > struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); > > + if ( nvcpu->nv_n1vmcx ) > + v->arch.hvm_vmx.vmcs = nvcpu->nv_n1vmcx;Okay, this undoes the fork in nvmx_handle_vmxon()? A small code comment to explain that would be handy.> nvmx_purge_vvmcs(v);This call of nvmx_purge_vvmcs() is no longer needed, and should be removed? -- Keir> if ( nvcpu->nv_n2vmcx ) { > __vmpclear(virt_to_maddr(nvcpu->nv_n2vmcx)); > @@ -65,6 +68,14 @@ void nvmx_vcpu_destroy(struct vcpu *v) > } > } > > +void nvmx_domain_relinquish_resources(struct domain *d) > +{ > + struct vcpu *v; > + > + for_each_vcpu ( d, v ) > + nvmx_purge_vvmcs(v); > +} > + > int nvmx_vcpu_reset(struct vcpu *v) > { > return 0; > diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h > index 7243c4e..3592a8c 100644 > --- a/xen/include/asm-x86/hvm/hvm.h > +++ b/xen/include/asm-x86/hvm/hvm.h > @@ -179,6 +179,7 @@ struct hvm_function_table { > bool_t (*nhvm_vmcx_hap_enabled)(struct vcpu *v); > > enum hvm_intblk (*nhvm_intr_blocked)(struct vcpu *v); > + void (*nhvm_domain_relinquish_resources)(struct domain *d); > }; > > extern struct hvm_function_table hvm_funcs; > diff --git a/xen/include/asm-x86/hvm/vmx/vvmx.h > b/xen/include/asm-x86/hvm/vmx/vvmx.h > index 995f9f4..bbc34e7 100644 > --- a/xen/include/asm-x86/hvm/vmx/vvmx.h > +++ b/xen/include/asm-x86/hvm/vmx/vvmx.h > @@ -96,6 +96,7 @@ uint32_t nvmx_vcpu_asid(struct vcpu *v); > enum hvm_intblk nvmx_intr_blocked(struct vcpu *v); > int nvmx_intercepts_exception(struct vcpu *v, > unsigned int trap, int error_code); > +void nvmx_domain_relinquish_resources(struct domain *d); > > int nvmx_handle_vmxon(struct cpu_user_regs *regs); > int nvmx_handle_vmxoff(struct cpu_user_regs *regs);
Xu, Dongxiao
2012-Aug-27 00:55 UTC
Re: [PATCH] nvmx: fix resource relinquish for nested VMX
> -----Original Message----- > From: Keir Fraser [mailto:keir.xen@gmail.com] On Behalf Of Keir Fraser > Sent: Friday, August 24, 2012 4:46 PM > To: Xu, Dongxiao; xen-devel@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] nvmx: fix resource relinquish for nested VMX > > On 23/08/2012 04:11, "Dongxiao Xu" <dongxiao.xu@intel.com> wrote: > > > The previous order of relinquish resource is: > > relinquish_domain_resources() -> vcpu_destroy() -> nvmx_vcpu_destroy(). > > However some L1 resources like nv_vvmcx and io_bitmaps are free in > > nvmx_vcpu_destroy(), therefore the relinquish_domain_resources() will > > not reduce the refcnt of the domain to 0, therefore the latter vcpu > > release functions will not be called. > > > > To fix this issue, we need to release the nv_vvmcx and io_bitmaps in > > relinquish_domain_resources(). > > > > Besides, after destroy the nested vcpu, we need to switch the > > vmx->vmcs back to the L1 and let the vcpu_destroy() logic to free the L1 > VMCS page. > > > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com> > > Couple of comments below. > > > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > > index 2e0b79d..1f610eb 100644 > > --- a/xen/arch/x86/hvm/vmx/vvmx.c > > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > > @@ -57,6 +57,9 @@ void nvmx_vcpu_destroy(struct vcpu *v) { > > struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); > > > > + if ( nvcpu->nv_n1vmcx ) > > + v->arch.hvm_vmx.vmcs = nvcpu->nv_n1vmcx; > > Okay, this undoes the fork in nvmx_handle_vmxon()? A small code comment to > explain that would be handy.Consider the following case: When the vcpu is representing the L2 guest, therefore the v->arch.hvm_vmx.vmcs points to the L2''s VMCS (as known as the shadow VMCS, nvcpu->nv_n2vmcx), and at this time, user destroy the L1 guest by "xl destroy", we need to set the v->arch.hvm_vmx.vmcs back to L1''s VMCS, otherwise, L2''s VMCS will be free twice and keep L1''s VMCS un-freed. I will add a comment the code.> > > nvmx_purge_vvmcs(v); > > This call of nvmx_purge_vvmcs() is no longer needed, and should be removed?Yes, this could be removed. I will send out a new version. Thanks, Dongxiao> > -- Keir > > > if ( nvcpu->nv_n2vmcx ) { > > __vmpclear(virt_to_maddr(nvcpu->nv_n2vmcx)); > > @@ -65,6 +68,14 @@ void nvmx_vcpu_destroy(struct vcpu *v) > > } > > } > > > > +void nvmx_domain_relinquish_resources(struct domain *d) { > > + struct vcpu *v; > > + > > + for_each_vcpu ( d, v ) > > + nvmx_purge_vvmcs(v); > > +} > > + > > int nvmx_vcpu_reset(struct vcpu *v) > > { > > return 0; > > diff --git a/xen/include/asm-x86/hvm/hvm.h > > b/xen/include/asm-x86/hvm/hvm.h index 7243c4e..3592a8c 100644 > > --- a/xen/include/asm-x86/hvm/hvm.h > > +++ b/xen/include/asm-x86/hvm/hvm.h > > @@ -179,6 +179,7 @@ struct hvm_function_table { > > bool_t (*nhvm_vmcx_hap_enabled)(struct vcpu *v); > > > > enum hvm_intblk (*nhvm_intr_blocked)(struct vcpu *v); > > + void (*nhvm_domain_relinquish_resources)(struct domain *d); > > }; > > > > extern struct hvm_function_table hvm_funcs; diff --git > > a/xen/include/asm-x86/hvm/vmx/vvmx.h > > b/xen/include/asm-x86/hvm/vmx/vvmx.h > > index 995f9f4..bbc34e7 100644 > > --- a/xen/include/asm-x86/hvm/vmx/vvmx.h > > +++ b/xen/include/asm-x86/hvm/vmx/vvmx.h > > @@ -96,6 +96,7 @@ uint32_t nvmx_vcpu_asid(struct vcpu *v); enum > > hvm_intblk nvmx_intr_blocked(struct vcpu *v); int > > nvmx_intercepts_exception(struct vcpu *v, > > unsigned int trap, int error_code); > > +void nvmx_domain_relinquish_resources(struct domain *d); > > > > int nvmx_handle_vmxon(struct cpu_user_regs *regs); int > > nvmx_handle_vmxoff(struct cpu_user_regs *regs); >
Maybe Matching Threads
- [PATCH v3 0/4] nested vmx: enable VMCS shadowing feature
- [PATCH v2 0/3] nested vmx bug fixes
- [PATCH] Nested VMX: Expose unrestricted guest feature to guest
- [PATCH v3] x86/nhvm: properly clean up after failure to set up all vCPU-s
- [PATCH 00/11] Add virtual EPT support Xen.