Re: [Xen-devel] [PATCH for-next v2 07/10] x86/domain: factor out pv_domain_destroy

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Apr 26, 2017 at 06:32:46AM -0600, Jan Beulich wrote:
> >>> On 25.04.17 at 15:52, <wei.liu2@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -536,6 +536,16 @@ static bool emulation_flags_ok(const struct domain *d, 
> > uint32_t emflags)
> >      return true;
> >  }
> >  
> > +static void pv_domain_destroy(struct domain *d)
> > +{
> > +    destroy_perdomain_mapping(d, GDT_LDT_VIRT_START,
> > +                              GDT_LDT_MBYTES << (20 - PAGE_SHIFT));
> 
> What use is this, considering both calling paths have ...
> 
> > @@ -712,10 +722,8 @@ int arch_domain_create(struct domain *d, unsigned int 
> > domcr_flags,
> >          paging_final_teardown(d);
> >      free_perdomain_mappings(d);
> 
> ... this and ...
> 
> >      if ( is_pv_domain(d) )
> > -    {
> > -        xfree(d->arch.pv_domain.cpuidmasks);
> > -        free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> > -    }
> > +        pv_domain_destroy(d);
> > +
> >      return rc;
> >  }
> >  
> > @@ -735,10 +743,7 @@ void arch_domain_destroy(struct domain *d)
> >  
> >      free_perdomain_mappings(d);
> 
> ... this?
> 

Yes, I know. But IMO it would be far clearer we have matching allocation
and deallocation.

We still need free_perdomain_mappings to have it free the top-level page
table.

Wei.

> Jan
> 
> >      if ( is_pv_domain(d) )
> > -    {
> > -        free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> > -        xfree(d->arch.pv_domain.cpuidmasks);
> > -    }
> > +        pv_domain_destroy(d);
> >  
> >      free_xenheap_page(d->shared_info);
> >      cleanup_domain_irq_mapping(d);
> > -- 
> > 2.11.0
> 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel