Re: [Xen-devel] [PATCH v3 1/2] nvmx: implement support for MSR bitmaps

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel