[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/HVM: re-order error path of hvm_domain_initialise()
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Thu, 28 Jan 2021 16:51:51 +0000
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uO2erlpow8qcwr79nJAH50HDQvP8iT2gZvzll8QmW+o=; b=C92QrTI/vYxwDyvSKLEnPVY1J8DD1ngaAbHzetbIWXrq7ShbPkc1eSJryJBDYgiWarr5thgQADJR32ixVDtUqZNcP9eIatiNqB2kao1i2YB1fjkS03GvzMdvU2p/K9loWSKFqTnuJLgKdne0chpj5f4O36prEdCtnA2mtDZnB7r23/WWtHxNIDyay7FCMIlHbkKozwvy2xmKGEIYRgifVq2/ArGRoCMpOGgphUyWcMscEoSj90nnYaIMVeIZnZ4TeIJMXtaSLyQylRjUj3dfy3b266obZ0MIxcap7PaT+E3LzSYZEJd9kDXXIV5p3utlYudrFOs3bRdt0Qb6ln9Oiw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kQ8eeHqiLGlPHXVKNo+DTkTxC5U7vEXYzF5l6lw85iCdTit4tq0JrpkJX3AA63M5FPAwBF3ymEMtR3+eaf3hJxr2dYusMA8GJ8N0onjeuYFlP8fvLy9KdEkiptgd2t6Sko8aA6Z8nyQInRZ6hp0Y9KyxCmsbQ3dUMT1+eKI22lz1r9lMh+EHS1Z6lvoJRRcg9OgQwxFrot9VEJjtfxzN7oHuRcwHGEs5gY0cWCeXze/0nFxbcg/v46iXd0D2itlEtMke7eF4ulPvpTl58+JJ6LMLF65hT4YddCp3/zV6sgOaFVIu8yP79s5gCdUtS7C6oYR4KdjCmTPeOB1UfBOt4w==
- Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Delivery-date: Thu, 28 Jan 2021 16:52:16 +0000
- Ironport-sdr: JS1D0Je09n/FxX1uvhNeK245E2GC2e/VG94lA0IpGGriv9ytwDNl65+rP29RnJ9TRNWQg2IFy4 kI3s9dxKdmo4uhC/sD7q137roosXB6JnKj6CcS1WMndNGMoueKyr1IAef52OGqcW5ZyFUFan8P e00hQVfR3wTwglU7WGoMaaflIRHz1URdKce3XTZX2ESppafLwtAhZ/jkaoctm501lR+eko/KgG NHH2BCUZvVzDDBGNUkEvCge7PHgSftD0iWmIcNA6X1kaOmM/jv2AVMBaJWYaVnDjLrcIRVcDxB 4OI=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 28/01/2021 14:40, Jan Beulich wrote:
> hvm_destroy_all_ioreq_servers(), called from
> hvm_domain_relinquish_resources(), invokes relocate_portio_handler(),
> which uses d->arch.hvm.io_handler. Defer freeing of this array
> accordingly on the error path of hvm_domain_initialise().
>
> Similarly rtc_deinit() requires d->arch.hvm.pl_time to still be around,
> or else an armed timer structure would get freed, and that timer never
> get killed.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> We may want to consider moving the other two XFREE()s later as well,
> if only to be on the safe side.
Wherever possible, I want to move stuff like this into the idempotent
domain_teardown()/_domain_destroy() logic, although I suspect you want
this suitable for backport as well?
This won't be the last ordering bug we find.
Also, I suspect this one is mixed up in the complexity of
arch_domain_destroy() which needs aligning carefully between x86 and ARM
before it can be switched.
> For vRTC I question the calling of check_update_timer() from rtc_init():
> I would consider it more reasonable to do so immediately before the
> guest gets first launched, especially if a guest remains paused for a
> while after creation.
That does look suspect. (And yes - it can take minutes of wallclock
time to construct large guests, given the unintentional behaviour
introduced by the idle scrubbing logic.)
Honestly, its the kind of thing which shouldn't be firing until turned
on by HVMLoader. Maybe we're too late to fix that...
~Andrew
|
