Re: [Xen-devel] [PATCH v4 4/7] x86 / vmx: move teardown from domain_destroy()...

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 28 January 2020 08:15
> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Jun Nakajima <jun.nakajima@xxxxxxxxx>;
> Kevin Tian <kevin.tian@xxxxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>
> Subject: Re: [PATCH v4 4/7] x86 / vmx: move teardown from
> domain_destroy()...
> 
> On 24.01.2020 16:31, Paul Durrant wrote:
> > ... to domain_relinquish_resources().
> >
> > The teardown code frees the APICv page. This does not need to be done
> late
> > so do it in domain_relinquish_resources() rather than domain_destroy().
> 
> For the normal domain destruction path this is fine. For the error path
> of domain_create(), however, this will leak the page, as in this case
> hvm_domain_relinquish_resources() won't be called.

Well it's really arch_domain_create() that's at fault but, yes that needs 
fixing.

> I'm afraid there
> already is a similar issue with e.g. viridian_domain_deinit(). I guess
> I'll make a patch.
> 

Ok, thanks.

  Paul

> Jan
> 
> > Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> > ---
> > Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Wei Liu <wl@xxxxxxx>
> > Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> > Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> >
> > v4:
> >   - New in v4 (disaggregated from v3 patch #3)
> > ---
> >  xen/arch/x86/hvm/vmx/vmx.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> > index b262d38a7c..606f3dc2eb 100644
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -419,7 +419,7 @@ static int vmx_domain_initialise(struct domain *d)
> >      return 0;
> >  }
> >
> > -static void vmx_domain_destroy(struct domain *d)
> > +static void vmx_domain_relinquish_resources(struct domain *d)
> >  {
> >      if ( !has_vlapic(d) )
> >          return;
> > @@ -2240,7 +2240,7 @@ static struct hvm_function_table __initdata
> vmx_function_table = {
> >      .cpu_up_prepare       = vmx_cpu_up_prepare,
> >      .cpu_dead             = vmx_cpu_dead,
> >      .domain_initialise    = vmx_domain_initialise,
> > -    .domain_destroy       = vmx_domain_destroy,
> > +    .domain_relinquish_resources = vmx_domain_relinquish_resources,
> >      .vcpu_initialise      = vmx_vcpu_initialise,
> >      .vcpu_destroy         = vmx_vcpu_destroy,
> >      .save_cpu_ctxt        = vmx_save_vmcs_ctxt,
> >

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel