* [PATCH v2] KVM: Clean up vm creation and release @ 2010-11-09 16:02 Jan Kiszka 2010-11-09 21:01 ` Alexander Graf 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2010-11-09 16:02 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti Cc: kvm, kvm-ppc, kvm-ia64@vger.kernel.org, Alexander Graf, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390 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 <jan.kiszka@siemens.com> --- 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 *) { - 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; diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 985d825..bade533 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -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 */ diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2044302..fc29223 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -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, diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index bcf71c7..2d63f2c 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -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); diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f7aa0e8..339dd43 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -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); } -- 1.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] KVM: Clean up vm creation and release 2010-11-09 16:02 [PATCH v2] KVM: Clean up vm creation and release Jan Kiszka @ 2010-11-09 21:01 ` Alexander Graf 2010-11-10 9:17 ` Avi Kivity 0 siblings, 1 reply; 9+ messages in thread From: Alexander Graf @ 2010-11-09 21:01 UTC (permalink / raw) To: Jan Kiszka Cc: Avi Kivity, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64@vger.kernel.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390 On 09.11.2010, at 17:02, Jan Kiszka wrote: > 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 <jan.kiszka@siemens.com> > --- > > 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 *) Eh, no. This doesn't compile :). Apart from that, the code seems to work on ppc64. Tested-by: Alexander Graf <agraf@suse.de> Alex ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] KVM: Clean up vm creation and release 2010-11-09 21:01 ` Alexander Graf @ 2010-11-10 9:17 ` Avi Kivity [not found] ` <4CDA633C.7020109-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2010-11-10 9:17 UTC (permalink / raw) To: Alexander Graf Cc: Jan Kiszka, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64@vger.kernel.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390 On 11/09/2010 11:01 PM, Alexander Graf wrote: > On 09.11.2010, at 17:02, Jan Kiszka wrote: > > > 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<jan.kiszka@siemens.com> > > --- > > > > 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 *) > > Eh, no. This doesn't compile :). What's the problem? lack of a formal argument? > Apart from that, the code seems to work on ppc64. > > > Tested-by: Alexander Graf<agraf@suse.de> > Seems my ppc builder has died, so thanks for the test. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <4CDA633C.7020109-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2] KVM: Clean up vm creation and release [not found] ` <4CDA633C.7020109-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2010-11-10 9:30 ` Jan Kiszka [not found] ` <4CDA6640.1070200-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2010-11-10 9:30 UTC (permalink / raw) To: Avi Kivity Cc: Alexander Graf, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Am 10.11.2010 10:17, Avi Kivity wrote: > On 11/09/2010 11:01 PM, Alexander Graf wrote: >> On 09.11.2010, at 17:02, Jan Kiszka wrote: >> >>> 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<jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> >>> --- >>> >>> 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 *) >> >> 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 -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <4CDA6640.1070200-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH v2] KVM: Clean up vm creation and release [not found] ` <4CDA6640.1070200-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> @ 2010-11-10 9:45 ` Avi Kivity [not found] ` <4CDA69B5.1080704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2010-11-10 9:45 UTC (permalink / raw) To: Jan Kiszka Cc: Alexander Graf, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 11/10/2010 11:30 AM, Jan Kiszka wrote: > Am 10.11.2010 10:17, Avi Kivity wrote: > > On 11/09/2010 11:01 PM, Alexander Graf wrote: > >> On 09.11.2010, at 17:02, Jan Kiszka wrote: > >> > >>> 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<jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> > >>> --- > >>> > >>> 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 *) > >> > >> 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: > 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. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <4CDA69B5.1080704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2] KVM: Clean up vm creation and release [not found] ` <4CDA69B5.1080704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2010-11-10 10:18 ` Jan Kiszka [not found] ` <4CDA715A.5020709-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2010-11-10 10:18 UTC (permalink / raw) To: Avi Kivity Cc: Alexander Graf, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Am 10.11.2010 10:45, Avi Kivity wrote: > On 11/10/2010 11:30 AM, Jan Kiszka wrote: >> Am 10.11.2010 10:17, Avi Kivity wrote: >>> On 11/09/2010 11:01 PM, Alexander Graf wrote: >>>> On 09.11.2010, at 17:02, Jan Kiszka wrote: >>>> >>>>> 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<jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> >>>>> --- >>>>> >>>>> 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 *) >>>> >>>> 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: > >> 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 -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <4CDA715A.5020709-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH v2] KVM: Clean up vm creation and release [not found] ` <4CDA715A.5020709-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> @ 2010-11-10 10:21 ` Avi Kivity [not found] ` <4CDA722F.9050804-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Avi Kivity @ 2010-11-10 10:21 UTC (permalink / raw) To: Jan Kiszka Cc: Alexander Graf, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 11/10/2010 12:18 PM, Jan Kiszka wrote: > Am 10.11.2010 10:45, Avi Kivity wrote: > > On 11/10/2010 11:30 AM, Jan Kiszka wrote: > >> Am 10.11.2010 10:17, Avi Kivity wrote: > >>> On 11/09/2010 11:01 PM, Alexander Graf wrote: > >>>> On 09.11.2010, at 17:02, Jan Kiszka wrote: > >>>> > >>>>> 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<jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> > >>>>> --- > >>>>> > >>>>> 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 *) > >>>> > >>>> 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: > > > >> 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. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <4CDA722F.9050804-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2] KVM: Clean up vm creation and release [not found] ` <4CDA722F.9050804-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2010-11-10 12:57 ` Alexander Graf 2010-11-10 13:48 ` Avi Kivity 1 sibling, 0 replies; 9+ messages in thread From: Alexander Graf @ 2010-11-10 12:57 UTC (permalink / raw) To: Avi Kivity Cc: Jan Kiszka, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 10.11.2010, at 11:21, Avi Kivity wrote: > On 11/10/2010 12:18 PM, Jan Kiszka wrote: >> Am 10.11.2010 10:45, Avi Kivity wrote: >> > On 11/10/2010 11:30 AM, Jan Kiszka wrote: >> >> Am 10.11.2010 10:17, Avi Kivity wrote: >> >>> On 11/09/2010 11:01 PM, Alexander Graf wrote: >> >>>> On 09.11.2010, at 17:02, Jan Kiszka wrote: >> >>>> >> >>>>> 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<jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org> >> >>>>> --- >> >>>>> >> >>>>> 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 *) >> >>>> >> >>>> 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: >> > >> >> 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. Yes, that was the problem. Sorry for not being clear about it, I thought it was obvious :). Alex ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] KVM: Clean up vm creation and release [not found] ` <4CDA722F.9050804-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2010-11-10 12:57 ` Alexander Graf @ 2010-11-10 13:48 ` Avi Kivity 1 sibling, 0 replies; 9+ messages in thread From: Avi Kivity @ 2010-11-10 13:48 UTC (permalink / raw) To: Jan Kiszka Cc: Alexander Graf, Marcelo Tosatti, kvm, kvm-ppc, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Zhang, Xiantao, Carsten Otte, Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 11/10/2010 12:21 PM, Avi Kivity wrote: >> 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. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-11-10 13:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-09 16:02 [PATCH v2] KVM: Clean up vm creation and release Jan Kiszka
2010-11-09 21:01 ` Alexander Graf
2010-11-10 9:17 ` Avi Kivity
[not found] ` <4CDA633C.7020109-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-11-10 9:30 ` Jan Kiszka
[not found] ` <4CDA6640.1070200-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>
2010-11-10 9:45 ` Avi Kivity
[not found] ` <4CDA69B5.1080704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-11-10 10:18 ` Jan Kiszka
[not found] ` <4CDA715A.5020709-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>
2010-11-10 10:21 ` Avi Kivity
[not found] ` <4CDA722F.9050804-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-11-10 12:57 ` Alexander Graf
2010-11-10 13:48 ` Avi Kivity
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox