Re: [Xen-devel] [PATCH for-next 2/8] x86/domain: factor out pv_vcpu_initialise

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Apr 24, 2017 at 03:57:42AM -0600, Jan Beulich wrote:
> >>> On 10.04.17 at 15:27, <wei.liu2@xxxxxxxxxx> wrote:
> > +int vcpu_initialise(struct vcpu *v)
> > +{
> > +    struct domain *d = v->domain;
> > +    int rc;
> > +
> > +    v->arch.flags = TF_kernel_mode;
> > +
> > +    rc = mapcache_vcpu_init(v);
> > +    if ( rc )
> > +        return rc;
> > +
> > +    if ( !is_idle_domain(d) )
> > +    {
> > +        paging_vcpu_init(v);
> > +
> > +        if ( (rc = vcpu_init_fpu(v)) != 0 )
> > +            return rc;
> > +
> > +        vmce_init_vcpu(v);
> > +    }
> > +    else if ( (rc = xstate_alloc_save_area(v)) != 0 )
> > +        return rc;
> > +
> > +    spin_lock_init(&v->arch.vpmu.vpmu_lock);
> > +
> > +    if ( is_hvm_domain(d) )
> > +        rc = hvm_vcpu_initialise(v);
> > +    else
> > +        rc = pv_vcpu_initialise(v);
> > +
> >      if ( rc )
> >      {
> >          vcpu_destroy_fpu(v);
> 
> Below here v->arch.pv_vcpu.trap_ctxt is being freed, which now also
> belongs into the PV function.
> 

After looking more closely into the original code, I think there is at
least one issue that the perdomain mapping is  not destroyed in the
error path.

Note there is no memory leak because eventually all perdomain mappings
will be freed in arch_domain_destroy. But I think we should make the
code look less bad while we are at it.  It is probably going to be more
code churn coming.

Wei.

> Jan
> 

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