On Tue, Nov 4, 2008 at 9:54 AM, Avi Kivity <avi@xxxxxxxxxx> wrote:
> 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.
By far it is not. But at least it does not depend on any kvm-specific
data, and works
for both qemu and kvm. So it's better to be this way.
If we change this scheme to a better one, like a hash table, then
it'll improve both qemu and kvm.
>> @@ -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
>
>
--
Glauber Costa.
"Free as in Freedom"
http://glommer.net
"The less confident you are, the more serious you have to act."
--
To unsubscribe from this list: send the line "unsubscribe kvm-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
