All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Avi Kivity <avi@qumranet.com>
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@osdl.org>, Linus Torvalds <torvalds@osdl.org>
Subject: [patch, try#2] kvm: fix GFP_KERNEL allocation in atomic section in kvm_dev_ioctl_create_vcpu()
Date: Thu, 28 Dec 2006 14:08:33 +0100	[thread overview]
Message-ID: <20061228130833.GA555@elte.hu> (raw)
In-Reply-To: <20061228125544.GA31207@elte.hu>


* Ingo Molnar <mingo@elte.hu> wrote:

> > Yes it does.  It calls nonpaging_init_context() which calls 
> > vmx_set_cr3() which promptly trashes address space of the VM that 
> > previously ran on that vcpu (or, if there were none, logs a vmwrite 
> > error).
> 
> ok, i missed that. Nevertheless the problem of the nonatomic alloc 
> remains. I guess a kvm_mmu_init() needs to be split into 
> kvm_mmu_create() and kvm_mmu_setup()?

the patch below implements this fix. Lightly tested on 32-bit/VMX on 
2.6.20-rc2-rt2 but seems to have done the trick.

	Ingo

-------------------->
Subject: [patch] kvm: fix GFP_KERNEL allocation in atomic section in kvm_dev_ioctl_create_vcpu()
From: Ingo Molnar <mingo@elte.hu>

fix an GFP_KERNEL allocation in atomic section: 
kvm_dev_ioctl_create_vcpu() called kvm_mmu_init(), which calls 
alloc_pages(), while holding the vcpu.

The fix is to set up the MMU state in two phases: kvm_mmu_create() and 
kvm_mmu_setup().

(NOTE: free_vcpus does an kvm_mmu_destroy() call so there's no need
 for any extra teardown branch on allocation/init failure here.)

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/kvm/kvm.h      |    3 ++-
 drivers/kvm/kvm_main.c |   10 ++++++----
 drivers/kvm/mmu.c      |   24 +++++++++---------------
 3 files changed, 17 insertions(+), 20 deletions(-)

Index: linux/drivers/kvm/kvm.h
===================================================================
--- linux.orig/drivers/kvm/kvm.h
+++ linux/drivers/kvm/kvm.h
@@ -319,7 +319,8 @@ int kvm_init_arch(struct kvm_arch_ops *o
 void kvm_exit_arch(void);
 
 void kvm_mmu_destroy(struct kvm_vcpu *vcpu);
-int kvm_mmu_init(struct kvm_vcpu *vcpu);
+int kvm_mmu_create(struct kvm_vcpu *vcpu);
+int kvm_mmu_setup(struct kvm_vcpu *vcpu);
 
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
Index: linux/drivers/kvm/kvm_main.c
===================================================================
--- linux.orig/drivers/kvm/kvm_main.c
+++ linux/drivers/kvm/kvm_main.c
@@ -522,12 +522,14 @@ static int kvm_dev_ioctl_create_vcpu(str
 	if (r < 0)
 		goto out_free_vcpus;
 
-	kvm_arch_ops->vcpu_load(vcpu);
+	r = kvm_mmu_create(vcpu);
+	if (r < 0)
+		goto out_free_vcpus;
 
-	r = kvm_arch_ops->vcpu_setup(vcpu);
+	kvm_arch_ops->vcpu_load(vcpu);
+	r = kvm_mmu_setup(vcpu);
 	if (r >= 0)
-		r = kvm_mmu_init(vcpu);
-
+		r = kvm_arch_ops->vcpu_setup(vcpu);
 	vcpu_put(vcpu);
 
 	if (r < 0)
Index: linux/drivers/kvm/mmu.c
===================================================================
--- linux.orig/drivers/kvm/mmu.c
+++ linux/drivers/kvm/mmu.c
@@ -639,28 +639,22 @@ error_1:
 	return -ENOMEM;
 }
 
-int kvm_mmu_init(struct kvm_vcpu *vcpu)
+int kvm_mmu_create(struct kvm_vcpu *vcpu)
 {
-	int r;
-
 	ASSERT(vcpu);
 	ASSERT(!VALID_PAGE(vcpu->mmu.root_hpa));
 	ASSERT(list_empty(&vcpu->free_pages));
 
-	r = alloc_mmu_pages(vcpu);
-	if (r)
-		goto out;
-
-	r = init_kvm_mmu(vcpu);
-	if (r)
-		goto out_free_pages;
+	return alloc_mmu_pages(vcpu);
+}
 
-	return 0;
+int kvm_mmu_setup(struct kvm_vcpu *vcpu)
+{
+	ASSERT(vcpu);
+	ASSERT(!VALID_PAGE(vcpu->mmu.root_hpa));
+	ASSERT(!list_empty(&vcpu->free_pages));
 
-out_free_pages:
-	free_mmu_pages(vcpu);
-out:
-	return r;
+	return init_kvm_mmu(vcpu);
 }
 
 void kvm_mmu_destroy(struct kvm_vcpu *vcpu)

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
To: Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
Cc: kvm-devel
	<kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>,
	Andrew Morton <akpm-3NddpPZAyC0@public.gmane.org>,
	Linus Torvalds <torvalds-3NddpPZAyC0@public.gmane.org>,
	linux-kernel
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: [patch, try#2] kvm: fix GFP_KERNEL allocation in atomic section in kvm_dev_ioctl_create_vcpu()
Date: Thu, 28 Dec 2006 14:08:33 +0100	[thread overview]
Message-ID: <20061228130833.GA555@elte.hu> (raw)
In-Reply-To: <20061228125544.GA31207-X9Un+BFzKDI@public.gmane.org>


* Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org> wrote:

> > Yes it does.  It calls nonpaging_init_context() which calls 
> > vmx_set_cr3() which promptly trashes address space of the VM that 
> > previously ran on that vcpu (or, if there were none, logs a vmwrite 
> > error).
> 
> ok, i missed that. Nevertheless the problem of the nonatomic alloc 
> remains. I guess a kvm_mmu_init() needs to be split into 
> kvm_mmu_create() and kvm_mmu_setup()?

the patch below implements this fix. Lightly tested on 32-bit/VMX on 
2.6.20-rc2-rt2 but seems to have done the trick.

	Ingo

-------------------->
Subject: [patch] kvm: fix GFP_KERNEL allocation in atomic section in kvm_dev_ioctl_create_vcpu()
From: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>

fix an GFP_KERNEL allocation in atomic section: 
kvm_dev_ioctl_create_vcpu() called kvm_mmu_init(), which calls 
alloc_pages(), while holding the vcpu.

The fix is to set up the MMU state in two phases: kvm_mmu_create() and 
kvm_mmu_setup().

(NOTE: free_vcpus does an kvm_mmu_destroy() call so there's no need
 for any extra teardown branch on allocation/init failure here.)

Signed-off-by: Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>
---
 drivers/kvm/kvm.h      |    3 ++-
 drivers/kvm/kvm_main.c |   10 ++++++----
 drivers/kvm/mmu.c      |   24 +++++++++---------------
 3 files changed, 17 insertions(+), 20 deletions(-)

Index: linux/drivers/kvm/kvm.h
===================================================================
--- linux.orig/drivers/kvm/kvm.h
+++ linux/drivers/kvm/kvm.h
@@ -319,7 +319,8 @@ int kvm_init_arch(struct kvm_arch_ops *o
 void kvm_exit_arch(void);
 
 void kvm_mmu_destroy(struct kvm_vcpu *vcpu);
-int kvm_mmu_init(struct kvm_vcpu *vcpu);
+int kvm_mmu_create(struct kvm_vcpu *vcpu);
+int kvm_mmu_setup(struct kvm_vcpu *vcpu);
 
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
Index: linux/drivers/kvm/kvm_main.c
===================================================================
--- linux.orig/drivers/kvm/kvm_main.c
+++ linux/drivers/kvm/kvm_main.c
@@ -522,12 +522,14 @@ static int kvm_dev_ioctl_create_vcpu(str
 	if (r < 0)
 		goto out_free_vcpus;
 
-	kvm_arch_ops->vcpu_load(vcpu);
+	r = kvm_mmu_create(vcpu);
+	if (r < 0)
+		goto out_free_vcpus;
 
-	r = kvm_arch_ops->vcpu_setup(vcpu);
+	kvm_arch_ops->vcpu_load(vcpu);
+	r = kvm_mmu_setup(vcpu);
 	if (r >= 0)
-		r = kvm_mmu_init(vcpu);
-
+		r = kvm_arch_ops->vcpu_setup(vcpu);
 	vcpu_put(vcpu);
 
 	if (r < 0)
Index: linux/drivers/kvm/mmu.c
===================================================================
--- linux.orig/drivers/kvm/mmu.c
+++ linux/drivers/kvm/mmu.c
@@ -639,28 +639,22 @@ error_1:
 	return -ENOMEM;
 }
 
-int kvm_mmu_init(struct kvm_vcpu *vcpu)
+int kvm_mmu_create(struct kvm_vcpu *vcpu)
 {
-	int r;
-
 	ASSERT(vcpu);
 	ASSERT(!VALID_PAGE(vcpu->mmu.root_hpa));
 	ASSERT(list_empty(&vcpu->free_pages));
 
-	r = alloc_mmu_pages(vcpu);
-	if (r)
-		goto out;
-
-	r = init_kvm_mmu(vcpu);
-	if (r)
-		goto out_free_pages;
+	return alloc_mmu_pages(vcpu);
+}
 
-	return 0;
+int kvm_mmu_setup(struct kvm_vcpu *vcpu)
+{
+	ASSERT(vcpu);
+	ASSERT(!VALID_PAGE(vcpu->mmu.root_hpa));
+	ASSERT(!list_empty(&vcpu->free_pages));
 
-out_free_pages:
-	free_mmu_pages(vcpu);
-out:
-	return r;
+	return init_kvm_mmu(vcpu);
 }
 
 void kvm_mmu_destroy(struct kvm_vcpu *vcpu)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

  reply	other threads:[~2006-12-28 13:11 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-28 10:07 [PATCH 0/8] KVM updates for 2.6.20-rc2 Avi Kivity
2006-12-28 10:08 ` [PATCH 1/8] KVM: Use boot_cpu_data instead of current_cpu_data Avi Kivity
2006-12-28 10:09 ` [PATCH 2/8] KVM: Simplify is_long_mode() Avi Kivity
     [not found] ` <45939755.7010603-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 10:10   ` [PATCH 3/8] KVM: Initialize kvm_arch_ops on unload Avi Kivity
2006-12-28 10:33   ` [PATCH 0/8] KVM updates for 2.6.20-rc2 Ingo Molnar
     [not found]     ` <20061228103345.GA4708-X9Un+BFzKDI@public.gmane.org>
2006-12-28 11:04       ` Avi Kivity
     [not found]         ` <4593A4B7.2070404-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 11:23           ` Ingo Molnar
     [not found]             ` <20061228112356.GA14386-X9Un+BFzKDI@public.gmane.org>
2006-12-28 12:21               ` Avi Kivity
2006-12-28 13:15               ` Ingo Molnar
2006-12-28 11:30           ` Ingo Molnar
     [not found]             ` <20061228113038.GA16190-X9Un+BFzKDI@public.gmane.org>
2006-12-28 12:32               ` Avi Kivity
     [not found]                 ` <4593B948.5090009-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 13:37                   ` Ingo Molnar
     [not found]                     ` <20061228133746.GC3392-X9Un+BFzKDI@public.gmane.org>
2006-12-28 13:49                       ` Avi Kivity
     [not found]                         ` <4593CB61.5050709-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 13:50                           ` Ingo Molnar
     [not found]                             ` <20061228135020.GA7606-X9Un+BFzKDI@public.gmane.org>
2006-12-28 13:58                               ` Avi Kivity
     [not found]                                 ` <4593CD74.6060202-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 14:07                                   ` Ingo Molnar
     [not found]                                     ` <20061228140742.GA10033-X9Un+BFzKDI@public.gmane.org>
2006-12-28 14:18                                       ` Avi Kivity
     [not found]                                         ` <4593D243.1030301-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 15:01                                           ` Ingo Molnar
     [not found]                                             ` <20061228150104.GB16057-X9Un+BFzKDI@public.gmane.org>
2006-12-28 15:09                                               ` Avi Kivity
     [not found]                                                 ` <4593DE1D.8010701-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 15:11                                                   ` Ingo Molnar
     [not found]                                                     ` <20061228151159.GA20279-X9Un+BFzKDI@public.gmane.org>
2006-12-28 15:25                                                       ` Avi Kivity
     [not found]                                                         ` <4593E1E3.2020800-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2006-12-28 15:28                                                           ` Avi Kivity
2006-12-28 10:11 ` [PATCH 4/8] KVM: Implement a few system configuration msrs Avi Kivity
2006-12-28 10:11   ` Avi Kivity
2007-01-01  0:07   ` Ingo Oeser
2007-01-01  0:07     ` Ingo Oeser
2007-01-01  8:20     ` Avi Kivity
2007-01-01  8:20       ` Avi Kivity
2006-12-28 10:12 ` [PATCH 5/8] KVM: Move common msr handling to arch independent code Avi Kivity
2006-12-28 10:13 ` [PATCH 6/8] KVM: More msr misery Avi Kivity
2006-12-28 10:14 ` [PATCH 7/8] KVM: Rename some msrs Avi Kivity
2006-12-28 10:14   ` Avi Kivity
2006-12-28 10:15 ` [PATCH 8/8] KVM: Fix oops on oom Avi Kivity
2006-12-28 10:15   ` Avi Kivity
2006-12-28 12:42 ` [patch] kvm: fix GFP_KERNEL allocation in atomic section in kvm_dev_ioctl_create_vcpu() Ingo Molnar
2006-12-28 12:42   ` Ingo Molnar
2006-12-28 12:56   ` Avi Kivity
2006-12-28 12:55     ` Ingo Molnar
2006-12-28 13:08       ` Ingo Molnar [this message]
2006-12-28 13:08         ` [patch, try#2] " Ingo Molnar
2006-12-28 13:14         ` Avi Kivity
2006-12-28 13:23           ` Ingo Molnar
2006-12-28 13:30             ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061228130833.GA555@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@osdl.org \
    --cc=avi@qumranet.com \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.