Re: [PATCH v3] x86/HVM: Fix teardown ordering in hvm_vcpu_destroy()

["Jan Beulich" <JBeulich@suse.com>]

>>> On 10.01.17 at 15:26, <andrew.cooper3@citrix.com> wrote:
> On 10/01/17 14:15, Andrew Cooper wrote:
>> On 10/01/17 14:03, Suravee Suthikulpanit wrote:
>>> The order of destroy function calls in hvm_vcpu_destroy() should be
>>> the reverse of init calls in hvm_vcpu_initialise().
>>>
>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> and queued.
> 
> Wait no.

Which clearly suggests that the earlier R-b-s should have been
dropped too.

> The order in vcpu_initialise is
> 
> hvm_vcpu_cacheattr_init()
> vlapic_init()
> hvm_funcs.vcpu_initialise()
> softirq_tasklet_init()
> setup_compat_arg_xlat()
> 
> Therefore, moving the tasklet_kill() is wrong.
> 
> The overall delta should be:
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1626,12 +1626,12 @@ void hvm_vcpu_destroy(struct vcpu *v)
>      free_compat_arg_xlat(v);
>  
>      tasklet_kill(&v->arch.hvm_vcpu.assert_evtchn_irq_tasklet);
> -    hvm_vcpu_cacheattr_destroy(v);
> +    hvm_funcs.vcpu_destroy(v);
>  
>      if ( is_hvm_vcpu(v) )
>          vlapic_destroy(v);
>  
> -    hvm_funcs.vcpu_destroy(v);
> +    hvm_vcpu_cacheattr_destroy(v);
>  }
>  
> IIRC.
> 
> If you agree, I will fold this correction in while committing.

This variant is
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


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