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 07:29:34AM -0600, Jan Beulich wrote:
> >>> On 26.04.17 at 14:56, <wei.liu2@xxxxxxxxxx> wrote:
> > 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.
> 
> Well, in the case here it's really pointless, but if want to keep it I
> don't mind as long as ...
> 
> > We still need free_perdomain_mappings to have it free the top-level page
> > table.
> 
> ... you then at least switch their order, so that
> free_perdomain_mappings() happens last.
> 

OK. This is a good point.

> Jan
> 

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