[Qemu-devel] [PATCH 0 of 2] Fix PowerPC KVM build breaks

[Qemu-devel] [PATCH 0 of 2] Fix PowerPC KVM build breaks

[Hollis Blanchard]

Hi Anthony, please apply these two patches to fix the PowerPC KVM build.

-Hollis

[PATCH 0 of 2] Fix PowerPC KVM build breaks

[Hollis Blanchard]

Hi Anthony, please apply these two patches to fix the PowerPC KVM build.

-Hollis

[Qemu-devel] [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Hollis Blanchard]

Unbreaks PowerPC and S390 KVM builds.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

diff --git a/kvm-all.c b/kvm-all.c
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -207,26 +207,6 @@ err:
     return ret;
 }
 
-int kvm_put_mp_state(CPUState *env)
-{
-    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
-
-    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
-}
-
-int kvm_get_mp_state(CPUState *env)
-{
-    struct kvm_mp_state mp_state;
-    int ret;
-
-    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
-    if (ret < 0) {
-        return ret;
-    }
-    env->mp_state = mp_state.mp_state;
-    return 0;
-}
-
 /*
  * dirty pages logging control
  */
diff --git a/kvm.h b/kvm.h
--- a/kvm.h
+++ b/kvm.h
@@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, 
 
 int kvm_vcpu_ioctl(CPUState *env, int type, ...);
 
-int kvm_get_mp_state(CPUState *env);
-int kvm_put_mp_state(CPUState *env);
-
 /* Arch specific hooks */
 
 int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
     return 0;
 }
 
+static int kvm_put_mp_state(CPUState *env)
+{
+    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
+
+    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
+}
+
+static int kvm_get_mp_state(CPUState *env)
+{
+    struct kvm_mp_state mp_state;
+    int ret;
+
+    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
+    if (ret < 0) {
+        return ret;
+    }
+    env->mp_state = mp_state.mp_state;
+    return 0;
+}
+
 int kvm_arch_put_registers(CPUState *env)
 {
     int ret;

[PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Hollis Blanchard]

Unbreaks PowerPC and S390 KVM builds.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

diff --git a/kvm-all.c b/kvm-all.c
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -207,26 +207,6 @@ err:
     return ret;
 }
 
-int kvm_put_mp_state(CPUState *env)
-{
-    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
-
-    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
-}
-
-int kvm_get_mp_state(CPUState *env)
-{
-    struct kvm_mp_state mp_state;
-    int ret;
-
-    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
-    if (ret < 0) {
-        return ret;
-    }
-    env->mp_state = mp_state.mp_state;
-    return 0;
-}
-
 /*
  * dirty pages logging control
  */
diff --git a/kvm.h b/kvm.h
--- a/kvm.h
+++ b/kvm.h
@@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, 
 
 int kvm_vcpu_ioctl(CPUState *env, int type, ...);
 
-int kvm_get_mp_state(CPUState *env);
-int kvm_put_mp_state(CPUState *env);
-
 /* Arch specific hooks */
 
 int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
     return 0;
 }
 
+static int kvm_put_mp_state(CPUState *env)
+{
+    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
+
+    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
+}
+
+static int kvm_get_mp_state(CPUState *env)
+{
+    struct kvm_mp_state mp_state;
+    int ret;
+
+    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
+    if (ret < 0) {
+        return ret;
+    }
+    env->mp_state = mp_state.mp_state;
+    return 0;
+}
+
 int kvm_arch_put_registers(CPUState *env)
 {
     int ret;

[Qemu-devel] [PATCH 2 of 2] kvm ppc: Remove unused label

[Hollis Blanchard]

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

diff --git a/target-ppc/kvm_ppc.c b/target-ppc/kvm_ppc.c
--- a/target-ppc/kvm_ppc.c
+++ b/target-ppc/kvm_ppc.c
@@ -52,7 +52,6 @@ close:
     fclose(f);
 free:
     free(path);
-out:
     return ret;
 }
 

[PATCH 2 of 2] kvm ppc: Remove unused label

[Hollis Blanchard]

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

diff --git a/target-ppc/kvm_ppc.c b/target-ppc/kvm_ppc.c
--- a/target-ppc/kvm_ppc.c
+++ b/target-ppc/kvm_ppc.c
@@ -52,7 +52,6 @@ close:
     fclose(f);
 free:
     free(path);
-out:
     return ret;
 }
 

[Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]

Hollis Blanchard wrote:
> Unbreaks PowerPC and S390 KVM builds.

What breaks precisely?

Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
shared with ia64 - one day. We could still move things back then, but
maybe we can handle the build issues already in place, specifically as
qemu-kvm is carrying this in generic code since ages.

Jan

> 
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> 
> diff --git a/kvm-all.c b/kvm-all.c
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -207,26 +207,6 @@ err:
>      return ret;
>  }
>  
> -int kvm_put_mp_state(CPUState *env)
> -{
> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
> -
> -    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
> -}
> -
> -int kvm_get_mp_state(CPUState *env)
> -{
> -    struct kvm_mp_state mp_state;
> -    int ret;
> -
> -    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
> -    if (ret < 0) {
> -        return ret;
> -    }
> -    env->mp_state = mp_state.mp_state;
> -    return 0;
> -}
> -
>  /*
>   * dirty pages logging control
>   */
> diff --git a/kvm.h b/kvm.h
> --- a/kvm.h
> +++ b/kvm.h
> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, 
>  
>  int kvm_vcpu_ioctl(CPUState *env, int type, ...);
>  
> -int kvm_get_mp_state(CPUState *env);
> -int kvm_put_mp_state(CPUState *env);
> -
>  /* Arch specific hooks */
>  
>  int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
>      return 0;
>  }
>  
> +static int kvm_put_mp_state(CPUState *env)
> +{
> +    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
> +
> +    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
> +}
> +
> +static int kvm_get_mp_state(CPUState *env)
> +{
> +    struct kvm_mp_state mp_state;
> +    int ret;
> +
> +    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    env->mp_state = mp_state.mp_state;
> +    return 0;
> +}
> +
>  int kvm_arch_put_registers(CPUState *env)
>  {
>      int ret;



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2271 bytes --]

Hollis Blanchard wrote:
> Unbreaks PowerPC and S390 KVM builds.

What breaks precisely?

Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
shared with ia64 - one day. We could still move things back then, but
maybe we can handle the build issues already in place, specifically as
qemu-kvm is carrying this in generic code since ages.

Jan

> 
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> 
> diff --git a/kvm-all.c b/kvm-all.c
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -207,26 +207,6 @@ err:
>      return ret;
>  }
>  
> -int kvm_put_mp_state(CPUState *env)
> -{
> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
> -
> -    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
> -}
> -
> -int kvm_get_mp_state(CPUState *env)
> -{
> -    struct kvm_mp_state mp_state;
> -    int ret;
> -
> -    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
> -    if (ret < 0) {
> -        return ret;
> -    }
> -    env->mp_state = mp_state.mp_state;
> -    return 0;
> -}
> -
>  /*
>   * dirty pages logging control
>   */
> diff --git a/kvm.h b/kvm.h
> --- a/kvm.h
> +++ b/kvm.h
> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, 
>  
>  int kvm_vcpu_ioctl(CPUState *env, int type, ...);
>  
> -int kvm_get_mp_state(CPUState *env);
> -int kvm_put_mp_state(CPUState *env);
> -
>  /* Arch specific hooks */
>  
>  int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
>      return 0;
>  }
>  
> +static int kvm_put_mp_state(CPUState *env)
> +{
> +    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
> +
> +    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
> +}
> +
> +static int kvm_get_mp_state(CPUState *env)
> +{
> +    struct kvm_mp_state mp_state;
> +    int ret;
> +
> +    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +    env->mp_state = mp_state.mp_state;
> +    return 0;
> +}
> +
>  int kvm_arch_put_registers(CPUState *env)
>  {
>      int ret;



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Alexander Graf]

On 09.11.2009, at 23:12, Jan Kiszka wrote:

> Hollis Blanchard wrote:
>> Unbreaks PowerPC and S390 KVM builds.
>
> What breaks precisely?
>
> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
> shared with ia64 - one day. We could still move things back then, but
> maybe we can handle the build issues already in place, specifically as
> qemu-kvm is carrying this in generic code since ages.

The "generic" code tries to access env->mp_state which doesn't exist  
in PPC's env because it's only defined in the X86 env.

I honestly don't care where to move things. And I don't think anyone  
else should. Either move it to CPU_COMMON if you think it's generic or  
move the code to the platform specific part.

We've been going through this at least 2 times in previous threads.  
But qemu HEAD has been broken for PPC kvm since MP states got  
introduced and it's no fun talking against walls on this.


Alex

Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Alexander Graf]

On 09.11.2009, at 23:12, Jan Kiszka wrote:

> Hollis Blanchard wrote:
>> Unbreaks PowerPC and S390 KVM builds.
>
> What breaks precisely?
>
> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
> shared with ia64 - one day. We could still move things back then, but
> maybe we can handle the build issues already in place, specifically as
> qemu-kvm is carrying this in generic code since ages.

The "generic" code tries to access env->mp_state which doesn't exist  
in PPC's env because it's only defined in the X86 env.

I honestly don't care where to move things. And I don't think anyone  
else should. Either move it to CPU_COMMON if you think it's generic or  
move the code to the platform specific part.

We've been going through this at least 2 times in previous threads.  
But qemu HEAD has been broken for PPC kvm since MP states got  
introduced and it's no fun talking against walls on this.


Alex

[Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Hollis Blanchard]

This is the technique Anthony requested this morning, rather than
relocating mp_state. To be honest, I don't care which way it's fixed;
this is at least the 3rd patch to solve this seemingly trivial
problem.

/home/hollisb/source/qemu-fresh.hg/kvm-all.c: In function 'kvm_put_mp_state':
/home/hollisb/source/qemu-fresh.hg/kvm-all.c:212: error: 'struct
CPUPPCState' has no member named 'mp_state'

-Hollis

On Mon, Nov 9, 2009 at 2:12 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Hollis Blanchard wrote:
>> Unbreaks PowerPC and S390 KVM builds.
>
> What breaks precisely?
>
> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
> shared with ia64 - one day. We could still move things back then, but
> maybe we can handle the build issues already in place, specifically as
> qemu-kvm is carrying this in generic code since ages.
>
> Jan
>
>>
>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -207,26 +207,6 @@ err:
>>      return ret;
>>  }
>>
>> -int kvm_put_mp_state(CPUState *env)
>> -{
>> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>> -
>> -    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>> -}
>> -
>> -int kvm_get_mp_state(CPUState *env)
>> -{
>> -    struct kvm_mp_state mp_state;
>> -    int ret;
>> -
>> -    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>> -    if (ret < 0) {
>> -        return ret;
>> -    }
>> -    env->mp_state = mp_state.mp_state;
>> -    return 0;
>> -}
>> -
>>  /*
>>   * dirty pages logging control
>>   */
>> diff --git a/kvm.h b/kvm.h
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type,
>>
>>  int kvm_vcpu_ioctl(CPUState *env, int type, ...);
>>
>> -int kvm_get_mp_state(CPUState *env);
>> -int kvm_put_mp_state(CPUState *env);
>> -
>>  /* Arch specific hooks */
>>
>>  int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
>>      return 0;
>>  }
>>
>> +static int kvm_put_mp_state(CPUState *env)
>> +{
>> +    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>> +
>> +    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>> +}
>> +
>> +static int kvm_get_mp_state(CPUState *env)
>> +{
>> +    struct kvm_mp_state mp_state;
>> +    int ret;
>> +
>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    env->mp_state = mp_state.mp_state;
>> +    return 0;
>> +}
>> +
>>  int kvm_arch_put_registers(CPUState *env)
>>  {
>>      int ret;
>
>
>

Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific

[Hollis Blanchard]

This is the technique Anthony requested this morning, rather than
relocating mp_state. To be honest, I don't care which way it's fixed;
this is at least the 3rd patch to solve this seemingly trivial
problem.

/home/hollisb/source/qemu-fresh.hg/kvm-all.c: In function 'kvm_put_mp_state':
/home/hollisb/source/qemu-fresh.hg/kvm-all.c:212: error: 'struct
CPUPPCState' has no member named 'mp_state'

-Hollis

On Mon, Nov 9, 2009 at 2:12 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Hollis Blanchard wrote:
>> Unbreaks PowerPC and S390 KVM builds.
>
> What breaks precisely?
>
> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
> shared with ia64 - one day. We could still move things back then, but
> maybe we can handle the build issues already in place, specifically as
> qemu-kvm is carrying this in generic code since ages.
>
> Jan
>
>>
>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -207,26 +207,6 @@ err:
>>      return ret;
>>  }
>>
>> -int kvm_put_mp_state(CPUState *env)
>> -{
>> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>> -
>> -    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>> -}
>> -
>> -int kvm_get_mp_state(CPUState *env)
>> -{
>> -    struct kvm_mp_state mp_state;
>> -    int ret;
>> -
>> -    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>> -    if (ret < 0) {
>> -        return ret;
>> -    }
>> -    env->mp_state = mp_state.mp_state;
>> -    return 0;
>> -}
>> -
>>  /*
>>   * dirty pages logging control
>>   */
>> diff --git a/kvm.h b/kvm.h
>> --- a/kvm.h
>> +++ b/kvm.h
>> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type,
>>
>>  int kvm_vcpu_ioctl(CPUState *env, int type, ...);
>>
>> -int kvm_get_mp_state(CPUState *env);
>> -int kvm_put_mp_state(CPUState *env);
>> -
>>  /* Arch specific hooks */
>>
>>  int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
>>      return 0;
>>  }
>>
>> +static int kvm_put_mp_state(CPUState *env)
>> +{
>> +    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>> +
>> +    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>> +}
>> +
>> +static int kvm_get_mp_state(CPUState *env)
>> +{
>> +    struct kvm_mp_state mp_state;
>> +    int ret;
>> +
>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +    env->mp_state = mp_state.mp_state;
>> +    return 0;
>> +}
>> +
>>  int kvm_arch_put_registers(CPUState *env)
>>  {
>>      int ret;
>
>
>

[Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

Hollis Blanchard wrote:
> This is the technique Anthony requested this morning, rather than
> relocating mp_state.

Then let's move it. There is not that much code to share anyway.

> To be honest, I don't care which way it's fixed;
> this is at least the 3rd patch to solve this seemingly trivial
> problem.

That's indeed unfortunate, but I think to remember that not all of these
attempts were clean.

Jan

> 
> /home/hollisb/source/qemu-fresh.hg/kvm-all.c: In function 'kvm_put_mp_state':
> /home/hollisb/source/qemu-fresh.hg/kvm-all.c:212: error: 'struct
> CPUPPCState' has no member named 'mp_state'
> 
> -Hollis
> 
> On Mon, Nov 9, 2009 at 2:12 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Hollis Blanchard wrote:
>>> Unbreaks PowerPC and S390 KVM builds.
>> What breaks precisely?
>>
>> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
>> shared with ia64 - one day. We could still move things back then, but
>> maybe we can handle the build issues already in place, specifically as
>> qemu-kvm is carrying this in generic code since ages.
>>
>> Jan
>>
>>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -207,26 +207,6 @@ err:
>>>      return ret;
>>>  }
>>>
>>> -int kvm_put_mp_state(CPUState *env)
>>> -{
>>> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>>> -
>>> -    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>>> -}
>>> -
>>> -int kvm_get_mp_state(CPUState *env)
>>> -{
>>> -    struct kvm_mp_state mp_state;
>>> -    int ret;
>>> -
>>> -    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>>> -    if (ret < 0) {
>>> -        return ret;
>>> -    }
>>> -    env->mp_state = mp_state.mp_state;
>>> -    return 0;
>>> -}
>>> -
>>>  /*
>>>   * dirty pages logging control
>>>   */
>>> diff --git a/kvm.h b/kvm.h
>>> --- a/kvm.h
>>> +++ b/kvm.h
>>> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type,
>>>
>>>  int kvm_vcpu_ioctl(CPUState *env, int type, ...);
>>>
>>> -int kvm_get_mp_state(CPUState *env);
>>> -int kvm_put_mp_state(CPUState *env);
>>> -
>>>  /* Arch specific hooks */
>>>
>>>  int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> --- a/target-i386/kvm.c
>>> +++ b/target-i386/kvm.c
>>> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
>>>      return 0;
>>>  }
>>>
>>> +static int kvm_put_mp_state(CPUState *env)
>>> +{
>>> +    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>>> +
>>> +    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>>> +}
>>> +
>>> +static int kvm_get_mp_state(CPUState *env)
>>> +{
>>> +    struct kvm_mp_state mp_state;
>>> +    int ret;
>>> +
>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>>> +    if (ret < 0) {
>>> +        return ret;
>>> +    }
>>> +    env->mp_state = mp_state.mp_state;
>>> +    return 0;
>>> +}
>>> +
>>>  int kvm_arch_put_registers(CPUState *env)
>>>  {
>>>      int ret;
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

Hollis Blanchard wrote:
> This is the technique Anthony requested this morning, rather than
> relocating mp_state.

Then let's move it. There is not that much code to share anyway.

> To be honest, I don't care which way it's fixed;
> this is at least the 3rd patch to solve this seemingly trivial
> problem.

That's indeed unfortunate, but I think to remember that not all of these
attempts were clean.

Jan

> 
> /home/hollisb/source/qemu-fresh.hg/kvm-all.c: In function 'kvm_put_mp_state':
> /home/hollisb/source/qemu-fresh.hg/kvm-all.c:212: error: 'struct
> CPUPPCState' has no member named 'mp_state'
> 
> -Hollis
> 
> On Mon, Nov 9, 2009 at 2:12 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Hollis Blanchard wrote:
>>> Unbreaks PowerPC and S390 KVM builds.
>> What breaks precisely?
>>
>> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
>> shared with ia64 - one day. We could still move things back then, but
>> maybe we can handle the build issues already in place, specifically as
>> qemu-kvm is carrying this in generic code since ages.
>>
>> Jan
>>
>>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -207,26 +207,6 @@ err:
>>>      return ret;
>>>  }
>>>
>>> -int kvm_put_mp_state(CPUState *env)
>>> -{
>>> -    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>>> -
>>> -    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>>> -}
>>> -
>>> -int kvm_get_mp_state(CPUState *env)
>>> -{
>>> -    struct kvm_mp_state mp_state;
>>> -    int ret;
>>> -
>>> -    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>>> -    if (ret < 0) {
>>> -        return ret;
>>> -    }
>>> -    env->mp_state = mp_state.mp_state;
>>> -    return 0;
>>> -}
>>> -
>>>  /*
>>>   * dirty pages logging control
>>>   */
>>> diff --git a/kvm.h b/kvm.h
>>> --- a/kvm.h
>>> +++ b/kvm.h
>>> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type,
>>>
>>>  int kvm_vcpu_ioctl(CPUState *env, int type, ...);
>>>
>>> -int kvm_get_mp_state(CPUState *env);
>>> -int kvm_put_mp_state(CPUState *env);
>>> -
>>>  /* Arch specific hooks */
>>>
>>>  int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> --- a/target-i386/kvm.c
>>> +++ b/target-i386/kvm.c
>>> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env)
>>>      return 0;
>>>  }
>>>
>>> +static int kvm_put_mp_state(CPUState *env)
>>> +{
>>> +    struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
>>> +
>>> +    return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state);
>>> +}
>>> +
>>> +static int kvm_get_mp_state(CPUState *env)
>>> +{
>>> +    struct kvm_mp_state mp_state;
>>> +    int ret;
>>> +
>>> +    ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state);
>>> +    if (ret < 0) {
>>> +        return ret;
>>> +    }
>>> +    env->mp_state = mp_state.mp_state;
>>> +    return 0;
>>> +}
>>> +
>>>  int kvm_arch_put_registers(CPUState *env)
>>>  {
>>>      int ret;
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code

[Anthony Liguori]

Jan Kiszka wrote:
> Hollis Blanchard wrote:
>   
>> Unbreaks PowerPC and S390 KVM builds.
>>     
>
> What breaks precisely?
>
> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
> shared with ia64 - one day. We could still move things back then, but
> maybe we can handle the build issues already in place, specifically as
> qemu-kvm is carrying this in generic code since ages.
>   

mp_state is pretty specific to the in-kernel apic.  ia64 may share 
because it shares an interrupt controller but that does not make it 
generic.  That just makes two architectures have a common field.

I don't think there's anything wrong with kvm exposing this as a common 
ioctl but from a qemu perspective, it makes no sense as a common cpu 
field.  It only makes sense when using kvm and when using an in-kernel 
apic.  That's very architecture specific from an qemu perspective.

-- 
Regards,

Anthony Liguori

Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific

[Anthony Liguori]

Jan Kiszka wrote:
> Hollis Blanchard wrote:
>   
>> Unbreaks PowerPC and S390 KVM builds.
>>     
>
> What breaks precisely?
>
> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be
> shared with ia64 - one day. We could still move things back then, but
> maybe we can handle the build issues already in place, specifically as
> qemu-kvm is carrying this in generic code since ages.
>   

mp_state is pretty specific to the in-kernel apic.  ia64 may share 
because it shares an interrupt controller but that does not make it 
generic.  That just makes two architectures have a common field.

I don't think there's anything wrong with kvm exposing this as a common 
ioctl but from a qemu perspective, it makes no sense as a common cpu 
field.  It only makes sense when using kvm and when using an in-kernel 
apic.  That's very architecture specific from an qemu perspective.

-- 
Regards,

Anthony Liguori