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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.01.2020 09:22, Durrant, Paul wrote:
>> -----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.

Why arch_domain_create()? For HVM domains hvm_domain_initialise()
is the last thing tried, and hence no further cleanup is needed
(assuming hvm_domain_initialise() cleans up in case of failure
after itself). It's failures encountered by domain_create() after
arch_domain_create() has succeeded which are a problem here. In
this case arch_domain_destroy() will be called, but nothing down
from there has called hvm_domain_relinquish_resources() so far.

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

Which turned out to take more time because of other issues I've
found in the course, but I think I now have something I can
actually test.

Jan

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