* [PATCH v2] kvm: call kvm_arch_destroy_vm if vm creation fails
@ 2019-10-23 20:32 Jim Mattson
2019-10-24 0:05 ` Sean Christopherson
0 siblings, 1 reply; 4+ messages in thread
From: Jim Mattson @ 2019-10-23 20:32 UTC (permalink / raw)
To: kvm, Paolo Bonzini, Sean Christopherson; +Cc: John Sperbeck, Jim Mattson
From: John Sperbeck <jsperbeck@google.com>
In kvm_create_vm(), if we've successfully called kvm_arch_init_vm(), but
then fail later in the function, we need to call kvm_arch_destroy_vm()
so that it can do any necessary cleanup (like freeing memory).
Fixes: 44a95dae1d229a ("KVM: x86: Detect and Initialize AVIC support")
Signed-off-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
v1 -> v2: Call kvm_arch_destroy_vm before refcount_set
virt/kvm/kvm_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index fd68fbe0a75d2..c1a1cc2aa7a80 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -645,7 +645,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
r = kvm_arch_init_vm(kvm, type);
if (r)
- goto out_err_no_disable;
+ goto out_err_no_arch_destroy_vm;
r = hardware_enable_all();
if (r)
@@ -697,11 +697,13 @@ static struct kvm *kvm_create_vm(unsigned long type)
out_err_no_srcu:
hardware_disable_all();
out_err_no_disable:
+ kvm_arch_destroy_vm(kvm);
refcount_set(&kvm->users_count, 0);
for (i = 0; i < KVM_NR_BUSES; i++)
kfree(kvm_get_bus(kvm, i));
for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
kvm_free_memslots(kvm, __kvm_memslots(kvm, i));
+out_err_no_arch_destroy_vm:
kvm_arch_free_vm(kvm);
mmdrop(current->mm);
return ERR_PTR(r);
--
2.24.0.rc0.303.g954a862665-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] kvm: call kvm_arch_destroy_vm if vm creation fails
2019-10-23 20:32 [PATCH v2] kvm: call kvm_arch_destroy_vm if vm creation fails Jim Mattson
@ 2019-10-24 0:05 ` Sean Christopherson
2019-10-24 1:18 ` Junaid Shahid
0 siblings, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2019-10-24 0:05 UTC (permalink / raw)
To: Jim Mattson; +Cc: kvm, Paolo Bonzini, John Sperbeck
On Wed, Oct 23, 2019 at 01:32:14PM -0700, Jim Mattson wrote:
> From: John Sperbeck <jsperbeck@google.com>
>
> In kvm_create_vm(), if we've successfully called kvm_arch_init_vm(), but
> then fail later in the function, we need to call kvm_arch_destroy_vm()
> so that it can do any necessary cleanup (like freeing memory).
>
> Fixes: 44a95dae1d229a ("KVM: x86: Detect and Initialize AVIC support")
> Signed-off-by: John Sperbeck <jsperbeck@google.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
> v1 -> v2: Call kvm_arch_destroy_vm before refcount_set
>
> virt/kvm/kvm_main.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index fd68fbe0a75d2..c1a1cc2aa7a80 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -645,7 +645,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>
> r = kvm_arch_init_vm(kvm, type);
> if (r)
> - goto out_err_no_disable;
> + goto out_err_no_arch_destroy_vm;
>
> r = hardware_enable_all();
> if (r)
> @@ -697,11 +697,13 @@ static struct kvm *kvm_create_vm(unsigned long type)
> out_err_no_srcu:
> hardware_disable_all();
> out_err_no_disable:
> + kvm_arch_destroy_vm(kvm);
> refcount_set(&kvm->users_count, 0);
> for (i = 0; i < KVM_NR_BUSES; i++)
> kfree(kvm_get_bus(kvm, i));
> for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
> kvm_free_memslots(kvm, __kvm_memslots(kvm, i));
Side topic, the loops to free the buses and memslots belong higher up,
the arrays aren't initialized until after hardware_enable(). Probably
doesn't harm anything but it's a waste of cycles. I'll send a patch.
> +out_err_no_arch_destroy_vm:
> kvm_arch_free_vm(kvm);
> mmdrop(current->mm);
> return ERR_PTR(r);
> --
> 2.24.0.rc0.303.g954a862665-goog
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] kvm: call kvm_arch_destroy_vm if vm creation fails
2019-10-24 0:05 ` Sean Christopherson
@ 2019-10-24 1:18 ` Junaid Shahid
2019-10-24 2:31 ` Sean Christopherson
0 siblings, 1 reply; 4+ messages in thread
From: Junaid Shahid @ 2019-10-24 1:18 UTC (permalink / raw)
To: Sean Christopherson, Jim Mattson; +Cc: kvm, Paolo Bonzini, John Sperbeck
[Plain-text resend]
On 10/23/19 5:05 PM, Sean Christopherson wrote:
>
> Side topic, the loops to free the buses and memslots belong higher up,
> the arrays aren't initialized until after hardware_enable(). Probably
> doesn't harm anything but it's a waste of cycles. I'll send a patch.
>
Aren't the x86_set_memory_region() calls inside kvm_arch_destroy_vm() going to be problematic if hardware_enable_all() fails? Perhaps we should move the memslots allocation before kvm_arch_init_vm(), or check for NULL memslots in kvm_arch_destroy_vm().
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] kvm: call kvm_arch_destroy_vm if vm creation fails
2019-10-24 1:18 ` Junaid Shahid
@ 2019-10-24 2:31 ` Sean Christopherson
0 siblings, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2019-10-24 2:31 UTC (permalink / raw)
To: Junaid Shahid; +Cc: Jim Mattson, kvm, Paolo Bonzini, John Sperbeck
On Wed, Oct 23, 2019 at 06:18:35PM -0700, Junaid Shahid wrote:
> [Plain-text resend]
>
> On 10/23/19 5:05 PM, Sean Christopherson wrote:
> >
> > Side topic, the loops to free the buses and memslots belong higher up,
> > the arrays aren't initialized until after hardware_enable(). Probably
> > doesn't harm anything but it's a waste of cycles. I'll send a patch.
> >
>
> Aren't the x86_set_memory_region() calls inside kvm_arch_destroy_vm() going
> to be problematic if hardware_enable_all() fails? Perhaps we should move the
> memslots allocation before kvm_arch_init_vm(), or check for NULL memslots in
> kvm_arch_destroy_vm().
Oof, that does appear to be the case. Initializing memslots and buses
before calling into arch code seems like the way to go.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-24 2:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-23 20:32 [PATCH v2] kvm: call kvm_arch_destroy_vm if vm creation fails Jim Mattson
2019-10-24 0:05 ` Sean Christopherson
2019-10-24 1:18 ` Junaid Shahid
2019-10-24 2:31 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).