Re: [patch] remove vcpu_info array v5

[Avi Kivity <avi@redhat.com>]

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 <tm>!
>
> 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 <sys/syscall.h>
>  #include <sys/mman.h>
>  
> -#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