Re: [patch] remove vcpu_info array v5

Avi Kivity
2008-11-04T11:55:05Z

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