From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [patch] remove vcpu_info array v5 Date: Tue, 04 Nov 2008 13:54:58 +0200 Message-ID: <49103812.1070104@redhat.com> References: <4909C00F.8050704@sgi.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Anthony Liguori , Hollis Blanchard , "kvm@vger.kernel.org" , "kvm-ia64@vger.kernel.org" , Glauber de Oliveira Costa To: Jes Sorensen Return-path: Received: from mx2.redhat.com ([66.187.237.31]:54153 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751382AbYKDLzF (ORCPT ); Tue, 4 Nov 2008 06:55:05 -0500 In-Reply-To: <4909C00F.8050704@sgi.com> Sender: kvm-owner@vger.kernel.org List-ID: Jes Sorensen wrote: > > So here we go, trying to catch up on the PCI passthrough patch revision > number, here's v5 of the struct vcpu_info patch. > > In the end I decided to merge the contents of struct vcpu_info directly > into CPU_COMMON as it was silly to create new files just to remove them > in the next patch again. > > This boots for me on ia64 and builds on x86_64, obviously it is 100% > perfect ! > > Comments, bug reports, or upstream merge, welcome :-) > Merge vcpu_info into CPUState. > > Move contents of struct vcpu_info directly into CPU_COMMON. Rename > struct qemu_kvm_work_item to qemu_work_item as it really isn't KVM > specific. > > Well, it is actually kvm specific, since qemu doesn't have vccpu threads. > -int kvm_run(kvm_context_t kvm, int vcpu) > +int kvm_run(kvm_context_t kvm, int vcpu, void *env) > That's a little ugly -- passing two arguments which describe the vcpu. How about passing env to kvm_create_vcpu() instead? > #define CPU_TEMP_BUF_NLONGS 128 > #define CPU_COMMON \ > struct TranslationBlock *current_tb; /* currently executing TB */ \ > @@ -200,6 +204,15 @@ > /* user data */ \ > void *opaque; \ > \ > - const char *cpu_model_str; > + const char *cpu_model_str; \ > + \ > + int sipi_needed; \ > + int init; \ > + pthread_t thread; \ > + int signalled; \ > + int stop; \ > + int stopped; \ > + int created; \ > + struct qemu_work_item *queued_work_first, *queued_work_last; > struct kvm_stuff { ... } and put that in as a member, so it's easier to remember this is a kvm addition. > > #if defined(TARGET_I386) || defined(TARGET_X86_64) > +#ifdef USE_KVM > +static CPUState *qemu_kvm_cpu_env(int index) > +{ > + CPUState *penv; > + > + penv = first_cpu; > + > + while (penv) { > + if (penv->cpu_index == index) > + return penv; > + penv = (CPUState *)penv->next_cpu; > + } > + > + return NULL; > +} > +#endif > This can't be the best way of determining the env pointer from a cpu number. > @@ -143,4 +143,12 @@ > void cpu_save(QEMUFile *f, void *opaque); > int cpu_load(QEMUFile *f, void *opaque, int version_id); > > +/* work queue */ > +struct qemu_work_item { > + struct qemu_kvm_work_item *next; > Missed rename here. > + void (*func)(void *data); > + void *data; > + int done; > +}; > + > Please keep this in kvm specific files. > @@ -28,7 +28,6 @@ > #include > #include > > -#define bool _Bool > why? Please separate into a patch that does the kvm_run int vcpu -> void *vcpu conversion, and then the vcpu_info -> env conversion. -- error compiling committee.c: too many arguments to function