On Tue, Feb 04, 2020 at 10:32:47AM +0100, Jan Beulich wrote:
> On 03.02.2020 18:37, Roger Pau Monne wrote:
> > @@ -182,6 +192,11 @@ void nvmx_vcpu_destroy(struct vcpu *v)
> > free_domheap_page(v->arch.hvm.vmx.vmwrite_bitmap);
> > v->arch.hvm.vmx.vmwrite_bitmap = NULL;
> > }
> > + if ( nvmx->msr_merged )
> > + {
> > + free_domheap_page(nvmx->msr_merged);
> > + nvmx->msr_merged = NULL;
> > + }
>
> Can this not be done ...
>
> > }
> >
> > void nvmx_domain_relinquish_resources(struct domain *d)
>
> ... in this function, thus happening earlier upon domain
> cleanup, and leaving less resources allocated in case a domain
> ends up as zombie (due to another bug elsewhere)? Actually -
> aren't you extending an existing bug this way? When
> nestedhvm_vcpu_initialise() fails, nestedhvm_vcpu_destroy()
> won't be called afaict.
nestedhvm_vcpu_destroy will be called by hvm_vcpu_initialise (the
caller of nestedhvm_vcpu_initialise) AFAICT.
> Hence nvmx_vcpu_initialise() not
> cleaning up after itself in case of failure looks to be a
> memory leak. As of b3344bb1cae0 any such will be taken care
> of implicitly as long as the freeing happens on the
> relinquish-resources paths.
I can move the new addition to nvmx_domain_relinquish_resources, I've
originally added it to nvmx_vcpu_destroy because that's where other
pages are also freed.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
