KVM: Clean up vm creation and release

PATCH In review Jan Kiszka 6 participants 9 messages Nov 9, 2010 – Nov 10, 2010 5 messages not in the archive

Subsystems (7)

x86 KVM PowerPC s390 +3 more subsystems

lore

Share & tools

lore

Jan Kiszka

author

2010-11-09 16:02 UTC #0 lore raw

PATCH v2 1/1 KVM: Clean up vm creation and release

IA64 support forces us to abstract the allocation of the kvm structure. But instead of mixing this up with arch-specific initialization and doing the same on destruction, split both steps. This allows to move generic destruction calls into generic code.

It also fixes error clean-up on failures of kvm_create_vm for IA64.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- Fixed s390 conversion and added linux/slab.h
as remarked by Christian (thanks!)

 arch/ia64/include/asm/kvm_host.h |    4 ++++
 arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
 arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
 arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
 arch/x86/kvm/x86.c               |   12 ++----------
 include/linux/kvm_host.h         |   15 ++++++++++++++-
 virt/kvm/kvm_main.c              |   19 +++++++++++++------
 7 files changed, 49 insertions(+), 72 deletions(-)

arch/ia64/include/asm/kvm_host.h +4 -0

index 2f229e5..2689ee5 100644
@@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu);
 int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
 void kvm_sal_emul(struct kvm_vcpu *vcpu);
 
+#define __KVM_HAVE_ARCH_VM_ALLOC 1
+struct kvm *kvm_arch_alloc_vm(void);
+void kvm_arch_free_vm(struct kvm *kvm);
+
 #endif /* __ASSEMBLY__*/
 
 #endif

arch/ia64/kvm/kvm-ia64.c +7 ~~-21~~

index f56a631..48a48bd 100644
@@ -749,7 +749,7 @@ out:
 	return r;
 }
 
-static struct kvm *kvm_alloc_kvm(void)
+struct kvm *kvm_arch_alloc_vm(void)
 {
 
 	struct kvm *kvm;

@@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
 	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));
 
 	if (!vm_base)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
 	kvm = (struct kvm *)(vm_base +

@@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
 #define GUEST_PHYSICAL_RR4	0x2739
 #define VMM_INIT_RR		0x1660
 
-static void kvm_init_vm(struct kvm *kvm)
+int kvm_arch_init_vm(struct kvm *kvm)
 {
 	BUG_ON(!kvm);
 
+	kvm->arch.is_sn2 = ia64_platform_is("sn2");
+
 	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
 	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
 	kvm->arch.vmm_init_rr = VMM_INIT_RR;

@@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)
 
 	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
 	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
-}
-
-struct  kvm *kvm_arch_create_vm(void)
-{
-	struct kvm *kvm = kvm_alloc_kvm();
-
-	if (IS_ERR(kvm))
-		return ERR_PTR(-ENOMEM);
-
-	kvm->arch.is_sn2 = ia64_platform_is("sn2");
-
-	kvm_init_vm(kvm);
-
-	return kvm;
 
+	return 0;
 }
 
 static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,

@@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
-static void free_kvm(struct kvm *kvm)
+void kvm_arch_free_vm(struct kvm *kvm)
 {
 	unsigned long vm_base = kvm->arch.vm_base;

@@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 #endif
 	kfree(kvm->arch.vioapic);
 	kvm_release_vm_pages(kvm);
-	kvm_free_physmem(kvm);
-	cleanup_srcu_struct(&kvm->srcu);
-	free_kvm(kvm);
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)

arch/powerpc/kvm/powerpc.c +3 ~~-17~~

index 38f756f..ce3dd65 100644
@@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = kvmppc_core_check_processor_compat();
 }
 
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)
 {
-	struct kvm *kvm;
-
-	kvm = kzalloc(sizeof(struct kvm), GFP_KERNEL);
-	if (!kvm)
-		return ERR_PTR(-ENOMEM);
-
-	return kvm;
+	return 0;
 }
 
-static void kvmppc_free_vcpus(struct kvm *kvm)
+void kvm_arch_destroy_vm(struct kvm *kvm)
 {
 	unsigned int i;
 	struct kvm_vcpu *vcpu;

@@ -176,14 +170,6 @@ void kvm_arch_sync_events(struct kvm *kvm)
 {
 }
 
-void kvm_arch_destroy_vm(struct kvm *kvm)
-{
-	kvmppc_free_vcpus(kvm);
-	kvm_free_physmem(kvm);
-	cleanup_srcu_struct(&kvm->srcu);
-	kfree(kvm);
-}
-
 int kvm_dev_ioctl_check_extension(long ext)
 {
 	int r;

arch/s390/kvm/kvm-s390.c +6 ~~-17~~

index 985d825..bade533 100644
@@ -164,24 +164,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	return r;
 }
 
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *kvm)
 {
-	struct kvm *kvm;
 	int rc;
 	char debug_name[16];
 
 	rc = s390_enable_sie();
 	if (rc)
-		goto out_nokvm;
-
-	rc = -ENOMEM;
-	kvm = kzalloc(sizeof(struct kvm), GFP_KERNEL);
-	if (!kvm)
-		goto out_nokvm;
+		goto out_err;
 
 	kvm->arch.sca = (struct sca_block *) get_zeroed_page(GFP_KERNEL);
 	if (!kvm->arch.sca)
-		goto out_nosca;
+		goto out_err;
 
 	sprintf(debug_name, "kvm-%u", current->pid);

@@ -195,13 +189,11 @@ struct kvm *kvm_arch_create_vm(void)
 	debug_register_view(kvm->arch.dbf, &debug_sprintf_view);
 	VM_EVENT(kvm, 3, "%s", "vm created");
 
-	return kvm;
+	return 0;
 out_nodbf:
 	free_page((unsigned long)(kvm->arch.sca));
-out_nosca:
-	kfree(kvm);
-out_nokvm:
-	return ERR_PTR(rc);
+out_err:
+	return rc;
 }
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)

@@ -240,11 +232,8 @@ void kvm_arch_sync_events(struct kvm *kvm)
 void kvm_arch_destroy_vm(struct kvm *kvm)
 {
 	kvm_free_vcpus(kvm);
-	kvm_free_physmem(kvm);
 	free_page((unsigned long)(kvm->arch.sca));
 	debug_unregister(kvm->arch.dbf);
-	cleanup_srcu_struct(&kvm->srcu);
-	kfree(kvm);
 }
 
 /* Section: vcpu related */

arch/x86/kvm/x86.c +2 ~~-10~~

index 2044302..fc29223 100644
@@ -5962,13 +5962,8 @@ void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 	free_page((unsigned long)vcpu->arch.pio_data);
 }
 
-struct  kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *kvm)
 {
-	struct kvm *kvm = kzalloc(sizeof(struct kvm), GFP_KERNEL);
-
-	if (!kvm)
-		return ERR_PTR(-ENOMEM);
-
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 	INIT_LIST_HEAD(&kvm->arch.assigned_dev_head);

@@ -5977,7 +5972,7 @@ struct  kvm *kvm_arch_create_vm(void)
 
 	spin_lock_init(&kvm->arch.tsc_write_lock);
 
-	return kvm;
+	return 0;
 }
 
 static void kvm_unload_vcpu_mmu(struct kvm_vcpu *vcpu)

@@ -6022,13 +6017,10 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	kfree(kvm->arch.vpic);
 	kfree(kvm->arch.vioapic);
 	kvm_free_vcpus(kvm);
-	kvm_free_physmem(kvm);
 	if (kvm->arch.apic_access_page)
 		put_page(kvm->arch.apic_access_page);
 	if (kvm->arch.ept_identity_pagetable)
 		put_page(kvm->arch.ept_identity_pagetable);
-	cleanup_srcu_struct(&kvm->srcu);
-	kfree(kvm);
 }
 
 int kvm_arch_prepare_memory_region(struct kvm *kvm,

include/linux/kvm_host.h +14 -1

index bcf71c7..2d63f2c 100644
@@ -16,6 +16,7 @@
 #include <linux/mm.h>
 #include <linux/preempt.h>
 #include <linux/msi.h>
+#include <linux/slab.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>

@@ -441,7 +442,19 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
 
 void kvm_free_physmem(struct kvm *kvm);
 
-struct  kvm *kvm_arch_create_vm(void);
+#ifndef __KVM_HAVE_ARCH_VM_ALLOC
+static inline struct kvm *kvm_arch_alloc_vm(void)
+{
+	return kzalloc(sizeof(struct kvm), GFP_KERNEL);
+}
+
+static inline void kvm_arch_free_vm(struct kvm *kvm)
+{
+	kfree(kvm);
+}
+#endif
+
+int kvm_arch_init_vm(struct kvm *kvm);
 void kvm_arch_destroy_vm(struct kvm *kvm);
 void kvm_free_all_assigned_devices(struct kvm *kvm);
 void kvm_arch_sync_events(struct kvm *kvm);

virt/kvm/kvm_main.c +13 -6

index f7aa0e8..339dd43 100644
@@ -383,11 +383,15 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
 
 static struct kvm *kvm_create_vm(void)
 {
-	int r = 0, i;
-	struct kvm *kvm = kvm_arch_create_vm();
+	int r, i;
+	struct kvm *kvm = kvm_arch_alloc_vm();
 
-	if (IS_ERR(kvm))
-		goto out;
+	if (!kvm)
+		return ERR_PTR(-ENOMEM);
+
+	r = kvm_arch_init_vm(kvm);
+	if (r)
+		goto out_err_nodisable;
 
 	r = hardware_enable_all();
 	if (r)

@@ -427,7 +431,7 @@ static struct kvm *kvm_create_vm(void)
 	spin_lock(&kvm_lock);
 	list_add(&kvm->vm_list, &vm_list);
 	spin_unlock(&kvm_lock);
-out:
+
 	return kvm;
 
 out_err:

@@ -438,7 +442,7 @@ out_err_nodisable:
 	for (i = 0; i < KVM_NR_BUSES; i++)
 		kfree(kvm->buses[i]);
 	kfree(kvm->memslots);
-	kfree(kvm);
+	kvm_arch_free_vm(kvm);
 	return ERR_PTR(r);
 }

@@ -512,6 +516,9 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	kvm_arch_flush_shadow(kvm);
 #endif
 	kvm_arch_destroy_vm(kvm);
+	kvm_free_physmem(kvm);
+	cleanup_srcu_struct(&kvm->srcu);
+	kvm_arch_free_vm(kvm);
 	hardware_disable_all();
 	mmdrop(mm);
 }

Signature · 1 line

1.7.1

Alexander Graf

Tested-by on v2 1/1

2010-11-09 21:01 UTC #1 lore raw

Quoted 121 lines · Jan Kiszka

It also fixes error clean-up on failures of kvm_create_vm for IA64.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- Fixed s390 conversion and added linux/slab.h
as remarked by Christian (thanks!)

arch/ia64/include/asm/kvm_host.h |    4 ++++
arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
arch/x86/kvm/x86.c               |   12 ++----------
include/linux/kvm_host.h         |   15 ++++++++++++++-
virt/kvm/kvm_main.c              |   19 +++++++++++++------
7 files changed, 49 insertions(+), 72 deletions(-)

arch/ia64/include/asm/kvm_host.h +4 -0

index 2f229e5..2689ee5 100644
@@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu);
 int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
 void kvm_sal_emul(struct kvm_vcpu *vcpu);
 
+#define __KVM_HAVE_ARCH_VM_ALLOC 1
+struct kvm *kvm_arch_alloc_vm(void);
+void kvm_arch_free_vm(struct kvm *kvm);
+
 #endif /* __ASSEMBLY__*/
 
 #endif

arch/ia64/kvm/kvm-ia64.c +7 ~~-21~~

index f56a631..48a48bd 100644
@@ -749,7 +749,7 @@ out:
 	return r;
 }
 
-static struct kvm *kvm_alloc_kvm(void)
+struct kvm *kvm_arch_alloc_vm(void)
 {
 
 	struct kvm *kvm;

@@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
 	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));
 
 	if (!vm_base)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
 	kvm = (struct kvm *)(vm_base +

@@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
 #define GUEST_PHYSICAL_RR4	0x2739
 #define VMM_INIT_RR		0x1660
 
-static void kvm_init_vm(struct kvm *kvm)
+int kvm_arch_init_vm(struct kvm *kvm)
 {
 	BUG_ON(!kvm);
 
+	kvm->arch.is_sn2 = ia64_platform_is("sn2");
+
 	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
 	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
 	kvm->arch.vmm_init_rr = VMM_INIT_RR;

@@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)
 
 	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
 	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
-}
-
-struct  kvm *kvm_arch_create_vm(void)
-{
-	struct kvm *kvm = kvm_alloc_kvm();
-
-	if (IS_ERR(kvm))
-		return ERR_PTR(-ENOMEM);
-
-	kvm->arch.is_sn2 = ia64_platform_is("sn2");
-
-	kvm_init_vm(kvm);
-
-	return kvm;
 
+	return 0;
 }
 
 static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,

@@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
-static void free_kvm(struct kvm *kvm)
+void kvm_arch_free_vm(struct kvm *kvm)
 {
 	unsigned long vm_base = kvm->arch.vm_base;

@@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 #endif
 	kfree(kvm->arch.vioapic);
 	kvm_release_vm_pages(kvm);
-	kvm_free_physmem(kvm);
-	cleanup_srcu_struct(&kvm->srcu);
-	free_kvm(kvm);
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)

arch/powerpc/kvm/powerpc.c +0 -0

index 38f756f..ce3dd65 100644

arch/powerpc/kvm/powerpc.c void kvm_arch_check_processor_compat(void *rtn) · line 148

Quoted 5 lines

arch/powerpc/kvm/powerpc.c +1 -1

@@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = kvmppc_core_check_processor_compat();
 }
 
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)

Eh, no. This doesn't compile :).

Apart from that, the code seems to work on ppc64.

Tested-by Alexander Graf <[email protected]>

Alex

Avi Kivity

2010-11-10 09:17 UTC #2 lore raw

Quoted 126 lines · Alexander Graf

Quoted 124 lines · Jan Kiszka

It also fixes error clean-up on failures of kvm_create_vm for IA64.

Signed-off-by Jan Kiszka<[email protected]>

---

Changes in v2:
- Fixed s390 conversion and added linux/slab.h
as remarked by Christian (thanks!)

 arch/ia64/include/asm/kvm_host.h |    4 ++++
 arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
 arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
 arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
 arch/x86/kvm/x86.c               |   12 ++----------
 include/linux/kvm_host.h         |   15 ++++++++++++++-
 virt/kvm/kvm_main.c              |   19 +++++++++++++------
 7 files changed, 49 insertions(+), 72 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h index 2f229e5..2689ee5 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu); int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run); void kvm_sal_emul(struct kvm_vcpu *vcpu);

+#define __KVM_HAVE_ARCH_VM_ALLOC 1
+struct kvm *kvm_arch_alloc_vm(void);
+void kvm_arch_free_vm(struct kvm *kvm);
+
#endif /* __ASSEMBLY__*/

#endif
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index f56a631..48a48bd 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -749,7 +749,7 @@ out:

(quoted diff) +0 -0

 	return r;
 }
 
 -static struct kvm *kvm_alloc_kvm(void)
 +struct kvm *kvm_arch_alloc_vm(void)
 {
 
 	struct kvm *kvm;
 @@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
 	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));
 
 	if (!vm_base)
 -		return ERR_PTR(-ENOMEM);
 +		return NULL;
 
 	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
 	kvm = (struct kvm *)(vm_base +
 @@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
 #define GUEST_PHYSICAL_RR4	0x2739
 #define VMM_INIT_RR		0x1660
 
 -static void kvm_init_vm(struct kvm *kvm)
 +int kvm_arch_init_vm(struct kvm *kvm)
 {
 	BUG_ON(!kvm);
 
 +	kvm->arch.is_sn2 = ia64_platform_is("sn2");
 +
 	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
 	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
 	kvm->arch.vmm_init_rr = VMM_INIT_RR;
 @@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)
 
 	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
 	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID,&kvm->arch.irq_sources_bitmap);
 -}
 -
 -struct  kvm *kvm_arch_create_vm(void)
 -{
 -	struct kvm *kvm = kvm_alloc_kvm();
 -
 -	if (IS_ERR(kvm))
 -		return ERR_PTR(-ENOMEM);
 -
 -	kvm->arch.is_sn2 = ia64_platform_is("sn2");
 -
 -	kvm_init_vm(kvm);
 -
 -	return kvm;
 
 +	return 0;
 }
 
 static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,
 @@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
 -static void free_kvm(struct kvm *kvm)
 +void kvm_arch_free_vm(struct kvm *kvm)
 {
 	unsigned long vm_base = kvm->arch.vm_base;
 
 @@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 #endif
 	kfree(kvm->arch.vioapic);
 	kvm_release_vm_pages(kvm);
 -	kvm_free_physmem(kvm);
 -	cleanup_srcu_struct(&kvm->srcu);
 -	free_kvm(kvm);
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
 index 38f756f..ce3dd65 100644
 --- a/arch/powerpc/kvm/powerpc.c
 +++ b/arch/powerpc/kvm/powerpc.c
 @@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = kvmppc_core_check_processor_compat();
 }

Quoted 4 lines

Quoted 2 lines
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)
Eh, no. This doesn't compile :).

What's the problem? lack of a formal argument?

Quoted 5 lines

Apart from that, the code seems to work on ppc64.
Tested-by Alexander Graf<[email protected]>

Seems my ppc builder has died, so thanks for the test.

Signature · 1 line

error compiling committee.c: too many arguments to function

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jan Kiszka

2010-11-10 09:30 UTC #4 lore raw

Quoted 127 lines · Avi Kivity

Quoted 126 lines · Alexander Graf

Quoted 124 lines · Jan Kiszka

It also fixes error clean-up on failures of kvm_create_vm for IA64.

Signed-off-by Jan Kiszka<[email protected]>

---

Changes in v2:
- Fixed s390 conversion and added linux/slab.h
as remarked by Christian (thanks!)

 arch/ia64/include/asm/kvm_host.h |    4 ++++
 arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
 arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
 arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
 arch/x86/kvm/x86.c               |   12 ++----------
 include/linux/kvm_host.h         |   15 ++++++++++++++-
 virt/kvm/kvm_main.c              |   19 +++++++++++++------
 7 files changed, 49 insertions(+), 72 deletions(-)

+#define __KVM_HAVE_ARCH_VM_ALLOC 1
+struct kvm *kvm_arch_alloc_vm(void);
+void kvm_arch_free_vm(struct kvm *kvm);
+
#endif /* __ASSEMBLY__*/

#endif
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index f56a631..48a48bd 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -749,7 +749,7 @@ out:

(quoted diff) +0 -0

 	return r;
 }
 
 -static struct kvm *kvm_alloc_kvm(void)
 +struct kvm *kvm_arch_alloc_vm(void)
 {
 
 	struct kvm *kvm;
 @@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
 	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));
 
 	if (!vm_base)
 -		return ERR_PTR(-ENOMEM);
 +		return NULL;
 
 	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
 	kvm = (struct kvm *)(vm_base +
 @@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
 #define GUEST_PHYSICAL_RR4	0x2739
 #define VMM_INIT_RR		0x1660
 
 -static void kvm_init_vm(struct kvm *kvm)
 +int kvm_arch_init_vm(struct kvm *kvm)
 {
 	BUG_ON(!kvm);
 
 +	kvm->arch.is_sn2 = ia64_platform_is("sn2");
 +
 	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
 	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
 	kvm->arch.vmm_init_rr = VMM_INIT_RR;
 @@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)
 
 	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
 	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID,&kvm->arch.irq_sources_bitmap);
 -}
 -
 -struct  kvm *kvm_arch_create_vm(void)
 -{
 -	struct kvm *kvm = kvm_alloc_kvm();
 -
 -	if (IS_ERR(kvm))
 -		return ERR_PTR(-ENOMEM);
 -
 -	kvm->arch.is_sn2 = ia64_platform_is("sn2");
 -
 -	kvm_init_vm(kvm);
 -
 -	return kvm;
 
 +	return 0;
 }
 
 static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,
 @@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 	return -EINVAL;
 }
 
 -static void free_kvm(struct kvm *kvm)
 +void kvm_arch_free_vm(struct kvm *kvm)
 {
 	unsigned long vm_base = kvm->arch.vm_base;
 
 @@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 #endif
 	kfree(kvm->arch.vioapic);
 	kvm_release_vm_pages(kvm);
 -	kvm_free_physmem(kvm);
 -	cleanup_srcu_struct(&kvm->srcu);
 -	free_kvm(kvm);
 }
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
 index 38f756f..ce3dd65 100644
 --- a/arch/powerpc/kvm/powerpc.c
 +++ b/arch/powerpc/kvm/powerpc.c
 @@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
 	*(int *)rtn = kvmppc_core_check_processor_compat();
 }

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)
Eh, no. This doesn't compile :).
What's the problem? lack of a formal argument?

The unnamed argument is referenced as 'kvm' in this function. Will you fix this up on merge or should I resend?

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2010-11-10 09:45 UTC #6 lore raw

Quoted 128 lines · Jan Kiszka

Quoted 127 lines · Avi Kivity

Quoted 126 lines · Alexander Graf

Quoted 124 lines · Jan Kiszka

It also fixes error clean-up on failures of kvm_create_vm for IA64.

Signed-off-by Jan Kiszka<[email protected]>

---

Changes in v2:
- Fixed s390 conversion and added linux/slab.h
as remarked by Christian (thanks!)

  arch/ia64/include/asm/kvm_host.h |    4 ++++
  arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
  arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
  arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
  arch/x86/kvm/x86.c               |   12 ++----------
  include/linux/kvm_host.h         |   15 ++++++++++++++-
  virt/kvm/kvm_main.c              |   19 +++++++++++++------
  7 files changed, 49 insertions(+), 72 deletions(-)

  diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
  index 2f229e5..2689ee5 100644
  --- a/arch/ia64/include/asm/kvm_host.h
  +++ b/arch/ia64/include/asm/kvm_host.h
  @@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu);
  int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
  void kvm_sal_emul(struct kvm_vcpu *vcpu);

  +#define __KVM_HAVE_ARCH_VM_ALLOC 1
  +struct kvm *kvm_arch_alloc_vm(void);
  +void kvm_arch_free_vm(struct kvm *kvm);
  +
  #endif /* __ASSEMBLY__*/

  #endif
  diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
  index f56a631..48a48bd 100644
  --- a/arch/ia64/kvm/kvm-ia64.c
  +++ b/arch/ia64/kvm/kvm-ia64.c
  @@ -749,7 +749,7 @@ out:
  	return r;
  }

  -static struct kvm *kvm_alloc_kvm(void)
  +struct kvm *kvm_arch_alloc_vm(void)
  {

  	struct kvm *kvm;
  @@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
  	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));

  	if (!vm_base)
  -		return ERR_PTR(-ENOMEM);
  +		return NULL;

  	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
  	kvm = (struct kvm *)(vm_base +
  @@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
  #define GUEST_PHYSICAL_RR4	0x2739
  #define VMM_INIT_RR		0x1660

  -static void kvm_init_vm(struct kvm *kvm)
  +int kvm_arch_init_vm(struct kvm *kvm)
  {
  	BUG_ON(!kvm);

  +	kvm->arch.is_sn2 = ia64_platform_is("sn2");
  +
  	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
  	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
  	kvm->arch.vmm_init_rr = VMM_INIT_RR;
  @@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)

  	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
  	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID,&kvm->arch.irq_sources_bitmap);
  -}
  -
  -struct  kvm *kvm_arch_create_vm(void)
  -{
  -	struct kvm *kvm = kvm_alloc_kvm();
  -
  -	if (IS_ERR(kvm))
  -		return ERR_PTR(-ENOMEM);
  -
  -	kvm->arch.is_sn2 = ia64_platform_is("sn2");
  -
  -	kvm_init_vm(kvm);
  -
  -	return kvm;

  +	return 0;
  }

static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,
@@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
return -EINVAL;
}

  -static void free_kvm(struct kvm *kvm)
  +void kvm_arch_free_vm(struct kvm *kvm)
  {
  	unsigned long vm_base = kvm->arch.vm_base;

  @@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
  #endif
  	kfree(kvm->arch.vioapic);
  	kvm_release_vm_pages(kvm);
  -	kvm_free_physmem(kvm);
  -	cleanup_srcu_struct(&kvm->srcu);
  -	free_kvm(kvm);
  }

  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
  diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
  index 38f756f..ce3dd65 100644
  --- a/arch/powerpc/kvm/powerpc.c
  +++ b/arch/powerpc/kvm/powerpc.c
  @@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
  	*(int *)rtn = kvmppc_core_check_processor_compat();
  }

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)
Eh, no. This doesn't compile :).
What's the problem? lack of a formal argument?
The unnamed argument is referenced as 'kvm' in this function.

It isn't referenced:

Quoted 8 lines

int kvm_arch_init_vm(struct kvm *)
{
    return 0;
}
Will you
fix this up on merge or should I resend?

I'll fix it, as soon as I understand it.

Signature · 1 line

error compiling committee.c: too many arguments to function

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jan Kiszka

2010-11-10 10:18 UTC #8 lore raw

Quoted 132 lines · Avi Kivity

Quoted 131 lines · Jan Kiszka

Quoted 130 lines · Avi Kivity

Quoted 129 lines · Alexander Graf

Quoted 126 lines · Jan Kiszka

It also fixes error clean-up on failures of kvm_create_vm for IA64.

Signed-off-by Jan Kiszka<[email protected]>

---

Changes in v2:
- Fixed s390 conversion and added linux/slab.h
as remarked by Christian (thanks!)

  arch/ia64/include/asm/kvm_host.h |    4 ++++
  arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
  arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
  arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
  arch/x86/kvm/x86.c               |   12 ++----------
  include/linux/kvm_host.h         |   15 ++++++++++++++-
  virt/kvm/kvm_main.c              |   19 +++++++++++++------
  7 files changed, 49 insertions(+), 72 deletions(-)

  diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
  index 2f229e5..2689ee5 100644
  --- a/arch/ia64/include/asm/kvm_host.h
  +++ b/arch/ia64/include/asm/kvm_host.h
  @@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu);
  int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
  void kvm_sal_emul(struct kvm_vcpu *vcpu);

  +#define __KVM_HAVE_ARCH_VM_ALLOC 1
  +struct kvm *kvm_arch_alloc_vm(void);
  +void kvm_arch_free_vm(struct kvm *kvm);
  +
  #endif /* __ASSEMBLY__*/

  #endif
  diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
  index f56a631..48a48bd 100644
  --- a/arch/ia64/kvm/kvm-ia64.c
  +++ b/arch/ia64/kvm/kvm-ia64.c
  @@ -749,7 +749,7 @@ out:
  	return r;
  }

  -static struct kvm *kvm_alloc_kvm(void)
  +struct kvm *kvm_arch_alloc_vm(void)
  {

  	struct kvm *kvm;
  @@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
  	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));

  	if (!vm_base)
  -		return ERR_PTR(-ENOMEM);
  +		return NULL;

  	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
  	kvm = (struct kvm *)(vm_base +
  @@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
  #define GUEST_PHYSICAL_RR4	0x2739
  #define VMM_INIT_RR		0x1660

  -static void kvm_init_vm(struct kvm *kvm)
  +int kvm_arch_init_vm(struct kvm *kvm)
  {
  	BUG_ON(!kvm);

  +	kvm->arch.is_sn2 = ia64_platform_is("sn2");
  +
  	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
  	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
  	kvm->arch.vmm_init_rr = VMM_INIT_RR;
  @@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)

  	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
  	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID,&kvm->arch.irq_sources_bitmap);
  -}
  -
  -struct  kvm *kvm_arch_create_vm(void)
  -{
  -	struct kvm *kvm = kvm_alloc_kvm();
  -
  -	if (IS_ERR(kvm))
  -		return ERR_PTR(-ENOMEM);
  -
  -	kvm->arch.is_sn2 = ia64_platform_is("sn2");
  -
  -	kvm_init_vm(kvm);
  -
  -	return kvm;

  +	return 0;
  }

static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,
@@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
return -EINVAL;
}

  -static void free_kvm(struct kvm *kvm)
  +void kvm_arch_free_vm(struct kvm *kvm)
  {
  	unsigned long vm_base = kvm->arch.vm_base;

  @@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
  #endif
  	kfree(kvm->arch.vioapic);
  	kvm_release_vm_pages(kvm);
  -	kvm_free_physmem(kvm);
  -	cleanup_srcu_struct(&kvm->srcu);
  -	free_kvm(kvm);
  }

  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
  diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
  index 38f756f..ce3dd65 100644
  --- a/arch/powerpc/kvm/powerpc.c
  +++ b/arch/powerpc/kvm/powerpc.c
  @@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
  	*(int *)rtn = kvmppc_core_check_processor_compat();
  }

-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)

Quoted 13 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Eh, no. This doesn't compile :).
What's the problem? lack of a formal argument?
The unnamed argument is referenced as 'kvm' in this function.
It isn't referenced:
Quoted 4 lines
int kvm_arch_init_vm(struct kvm *)
{
    return 0;
}

Indeed. Anyway, this wasn't intended (even more if some compiler versions/settings dislike it).

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

A message that is not in the archive (sent off-list or not yet received): [email protected]

Avi Kivity

2010-11-10 10:21 UTC #10 lore raw

Quoted 141 lines · Jan Kiszka

Quoted 140 lines · Avi Kivity

Quoted 128 lines · Jan Kiszka

Quoted 127 lines · Avi Kivity

Quoted 126 lines · Alexander Graf

Quoted 124 lines · Jan Kiszka

   IA64 support forces us to abstract the allocation of the kvm structure.
   But instead of mixing this up with arch-specific initialization and
   doing the same on destruction, split both steps. This allows to move
   generic destruction calls into generic code.

   It also fixes error clean-up on failures of kvm_create_vm for IA64.

   Signed-off-by: Jan Kiszka<[email protected]>
   ---

   Changes in v2:
   - Fixed s390 conversion and added linux/slab.h
     as remarked by Christian (thanks!)

   arch/ia64/include/asm/kvm_host.h |    4 ++++
   arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
   arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
   arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
   arch/x86/kvm/x86.c               |   12 ++----------
   include/linux/kvm_host.h         |   15 ++++++++++++++-
   virt/kvm/kvm_main.c              |   19 +++++++++++++------
   7 files changed, 49 insertions(+), 72 deletions(-)

   diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
   index 2f229e5..2689ee5 100644
   --- a/arch/ia64/include/asm/kvm_host.h
   +++ b/arch/ia64/include/asm/kvm_host.h
   @@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu);
   int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
   void kvm_sal_emul(struct kvm_vcpu *vcpu);

   +#define __KVM_HAVE_ARCH_VM_ALLOC 1
   +struct kvm *kvm_arch_alloc_vm(void);
   +void kvm_arch_free_vm(struct kvm *kvm);
   +
   #endif /* __ASSEMBLY__*/

   #endif
   diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
   index f56a631..48a48bd 100644
   --- a/arch/ia64/kvm/kvm-ia64.c
   +++ b/arch/ia64/kvm/kvm-ia64.c
   @@ -749,7 +749,7 @@ out:
   	return r;
   }

   -static struct kvm *kvm_alloc_kvm(void)
   +struct kvm *kvm_arch_alloc_vm(void)
   {

   	struct kvm *kvm;
   @@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
   	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));

   	if (!vm_base)
   -		return ERR_PTR(-ENOMEM);
   +		return NULL;

   	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
   	kvm = (struct kvm *)(vm_base +
   @@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
   #define GUEST_PHYSICAL_RR4	0x2739
   #define VMM_INIT_RR		0x1660

   -static void kvm_init_vm(struct kvm *kvm)
   +int kvm_arch_init_vm(struct kvm *kvm)
   {
   	BUG_ON(!kvm);

   +	kvm->arch.is_sn2 = ia64_platform_is("sn2");
   +
   	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
   	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
   	kvm->arch.vmm_init_rr = VMM_INIT_RR;
   @@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)

   	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
   	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID,&kvm->arch.irq_sources_bitmap);
   -}
   -
   -struct  kvm *kvm_arch_create_vm(void)
   -{
   -	struct kvm *kvm = kvm_alloc_kvm();
   -
   -	if (IS_ERR(kvm))
   -		return ERR_PTR(-ENOMEM);
   -
   -	kvm->arch.is_sn2 = ia64_platform_is("sn2");
   -
   -	kvm_init_vm(kvm);
   -
   -	return kvm;

   +	return 0;
   }

   static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,
   @@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
   	return -EINVAL;
   }

   -static void free_kvm(struct kvm *kvm)
   +void kvm_arch_free_vm(struct kvm *kvm)
   {
   	unsigned long vm_base = kvm->arch.vm_base;

   @@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
   #endif
   	kfree(kvm->arch.vioapic);
   	kvm_release_vm_pages(kvm);
   -	kvm_free_physmem(kvm);
   -	cleanup_srcu_struct(&kvm->srcu);
   -	free_kvm(kvm);
   }

   void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
   diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
   index 38f756f..ce3dd65 100644
   --- a/arch/powerpc/kvm/powerpc.c
   +++ b/arch/powerpc/kvm/powerpc.c
   @@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
   	*(int *)rtn = kvmppc_core_check_processor_compat();
   }

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)
Eh, no. This doesn't compile :).
What's the problem? lack of a formal argument?
The unnamed argument is referenced as 'kvm' in this function.

It isn't referenced:

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
int kvm_arch_init_vm(struct kvm *)
 {
     return 0;
}
Indeed. Anyway, this wasn't intended (even more if some compiler versions/settings dislike it).

Ok. I'll add the name and hope that this was the problem.

Signature · 1 line

error compiling committee.c: too many arguments to function

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alexander Graf

2010-11-10 12:57 UTC #12 lore raw

Quoted 148 lines · Avi Kivity

Quoted 147 lines · Jan Kiszka

Quoted 145 lines · Avi Kivity

Quoted 128 lines · Jan Kiszka

Quoted 127 lines · Avi Kivity

Quoted 126 lines · Alexander Graf

Quoted 124 lines · Jan Kiszka

   IA64 support forces us to abstract the allocation of the kvm structure.
   But instead of mixing this up with arch-specific initialization and
   doing the same on destruction, split both steps. This allows to move
   generic destruction calls into generic code.

   It also fixes error clean-up on failures of kvm_create_vm for IA64.

   Signed-off-by: Jan Kiszka<[email protected]>
   ---

   Changes in v2:
   - Fixed s390 conversion and added linux/slab.h
     as remarked by Christian (thanks!)

   arch/ia64/include/asm/kvm_host.h |    4 ++++
   arch/ia64/kvm/kvm-ia64.c         |   28 +++++++---------------------
   arch/powerpc/kvm/powerpc.c       |   20 +++-----------------
   arch/s390/kvm/kvm-s390.c         |   23 ++++++-----------------
   arch/x86/kvm/x86.c               |   12 ++----------
   include/linux/kvm_host.h         |   15 ++++++++++++++-
   virt/kvm/kvm_main.c              |   19 +++++++++++++------
   7 files changed, 49 insertions(+), 72 deletions(-)

   diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
   index 2f229e5..2689ee5 100644
   --- a/arch/ia64/include/asm/kvm_host.h
   +++ b/arch/ia64/include/asm/kvm_host.h
   @@ -590,6 +590,10 @@ int kvm_emulate_halt(struct kvm_vcpu *vcpu);
   int kvm_pal_emul(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run);
   void kvm_sal_emul(struct kvm_vcpu *vcpu);

   +#define __KVM_HAVE_ARCH_VM_ALLOC 1
   +struct kvm *kvm_arch_alloc_vm(void);
   +void kvm_arch_free_vm(struct kvm *kvm);
   +
   #endif /* __ASSEMBLY__*/

   #endif
   diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
   index f56a631..48a48bd 100644
   --- a/arch/ia64/kvm/kvm-ia64.c
   +++ b/arch/ia64/kvm/kvm-ia64.c
   @@ -749,7 +749,7 @@ out:
   	return r;
   }

   -static struct kvm *kvm_alloc_kvm(void)
   +struct kvm *kvm_arch_alloc_vm(void)
   {

   	struct kvm *kvm;
   @@ -760,7 +760,7 @@ static struct kvm *kvm_alloc_kvm(void)
   	vm_base = __get_free_pages(GFP_KERNEL, get_order(KVM_VM_DATA_SIZE));

   	if (!vm_base)
   -		return ERR_PTR(-ENOMEM);
   +		return NULL;

   	memset((void *)vm_base, 0, KVM_VM_DATA_SIZE);
   	kvm = (struct kvm *)(vm_base +
   @@ -806,10 +806,12 @@ static void kvm_build_io_pmt(struct kvm *kvm)
   #define GUEST_PHYSICAL_RR4	0x2739
   #define VMM_INIT_RR		0x1660

   -static void kvm_init_vm(struct kvm *kvm)
   +int kvm_arch_init_vm(struct kvm *kvm)
   {
   	BUG_ON(!kvm);

   +	kvm->arch.is_sn2 = ia64_platform_is("sn2");
   +
   	kvm->arch.metaphysical_rr0 = GUEST_PHYSICAL_RR0;
   	kvm->arch.metaphysical_rr4 = GUEST_PHYSICAL_RR4;
   	kvm->arch.vmm_init_rr = VMM_INIT_RR;
   @@ -823,21 +825,8 @@ static void kvm_init_vm(struct kvm *kvm)

   	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
   	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID,&kvm->arch.irq_sources_bitmap);
   -}
   -
   -struct  kvm *kvm_arch_create_vm(void)
   -{
   -	struct kvm *kvm = kvm_alloc_kvm();
   -
   -	if (IS_ERR(kvm))
   -		return ERR_PTR(-ENOMEM);
   -
   -	kvm->arch.is_sn2 = ia64_platform_is("sn2");
   -
   -	kvm_init_vm(kvm);
   -
   -	return kvm;

   +	return 0;
   }

   static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm,
   @@ -1357,7 +1346,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
   	return -EINVAL;
   }

   -static void free_kvm(struct kvm *kvm)
   +void kvm_arch_free_vm(struct kvm *kvm)
   {
   	unsigned long vm_base = kvm->arch.vm_base;

   @@ -1399,9 +1388,6 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
   #endif
   	kfree(kvm->arch.vioapic);
   	kvm_release_vm_pages(kvm);
   -	kvm_free_physmem(kvm);
   -	cleanup_srcu_struct(&kvm->srcu);
   -	free_kvm(kvm);
   }

   void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
   diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
   index 38f756f..ce3dd65 100644
   --- a/arch/powerpc/kvm/powerpc.c
   +++ b/arch/powerpc/kvm/powerpc.c
   @@ -145,18 +145,12 @@ void kvm_arch_check_processor_compat(void *rtn)
   	*(int *)rtn = kvmppc_core_check_processor_compat();
   }

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_init_vm(struct kvm *)
Eh, no. This doesn't compile :).
What's the problem? lack of a formal argument?
The unnamed argument is referenced as 'kvm' in this function.

It isn't referenced:

Quoted 4 lines

int kvm_arch_init_vm(struct kvm *)
 {
     return 0;
}

Quoted 4 lines

Quoted 2 lines
Indeed. Anyway, this wasn't intended (even more if some compiler versions/settings dislike it).
Ok. I'll add the name and hope that this was the problem.

Yes, that was the problem. Sorry for not being clear about it, I thought it was obvious :).

Alex

Avi Kivity

2010-11-10 13:48 UTC #13 lore raw

In reply to a message not in the archive

Quoted 6 lines · Avi Kivity

Quoted 2 lines
Indeed. Anyway, this wasn't intended (even more if some compiler versions/settings dislike it).
Ok. I'll add the name and hope that this was the problem.

Interestingly, ia64 and powerpc fail the build for unrelated reasons.

Signature · 1 line

error compiling committee.c: too many arguments to function