Patchwork [qom-cpu,v2,1/7] target-i386: Inline APIC cpu_env property setting

login
register
mail settings
Submitter Andreas Färber
Date Oct. 12, 2012, 1:26 a.m.
Message ID <1350005203-15405-2-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/191033/
State New
Headers show

Comments

Andreas Färber - Oct. 12, 2012, 1:26 a.m.
This prepares for changing the variable type from void*.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Igor Mammedov <imammedo@redhat.com>
---
 hw/apic_common.c  |    1 -
 target-i386/cpu.c |    5 ++++-
 2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
Igor Mammedov - Oct. 15, 2012, 11:22 p.m.
On Fri, 12 Oct 2012 03:26:37 +0200
Andreas Färber <afaerber@suse.de> wrote:

> This prepares for changing the variable type from void*.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/apic_common.c  |    1 -
>  target-i386/cpu.c |    5 ++++-
>  2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
> 
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index 371f95d..a26a631 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -368,7 +368,6 @@ static const VMStateDescription vmstate_apic_common = {
>  
>  static Property apic_properties_common[] = {
>      DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> -    DEFINE_PROP_PTR("cpu_env", APICCommonState, cpu_env),
>      DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
>                      true),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index e307b4e..0cce910 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -41,6 +41,7 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "hw/xen.h"
>  #include "hw/sysbus.h"
> +#include "hw/apic_internal.h"
>  #endif
>  
>  /* feature flags taken from "Intel Processor Identification and the CPUID
> @@ -1884,6 +1885,7 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>  #ifndef CONFIG_USER_ONLY
>      static int apic_mapped;
>      CPUX86State *env = &cpu->env;
> +    APICCommonState *apic;
>      const char *apic_type = "apic";
>  
>      if (kvm_irqchip_in_kernel()) {
> @@ -1902,7 +1904,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>                                OBJECT(env->apic_state), NULL);
>      qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
>      /* TODO: convert to link<> */
> -    qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
> +    apic = APIC_COMMON(env->apic_state);
> +    apic->cpu_env = env;
>  
>      if (qdev_init(env->apic_state)) {
>          error_setg(errp, "APIC device '%s' could not be initialized",
> -- 
> 1.7.10.4
> 
Reviewed-By: Igor Mammedov <imammedo@redhat.com>
Andreas Färber - Oct. 16, 2012, 4:02 p.m.
Am 12.10.2012 03:26, schrieb Andreas Färber:
> This prepares for changing the variable type from void*.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Igor Mammedov <imammedo@redhat.com>

Paolo, are you happy with getting rid of the pointer property this way?

Jan, are you okay with accessing APICCommonState in target-i386/cpu.c as
an interim solution?

Andreas

> ---
>  hw/apic_common.c  |    1 -
>  target-i386/cpu.c |    5 ++++-
>  2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
> 
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index 371f95d..a26a631 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -368,7 +368,6 @@ static const VMStateDescription vmstate_apic_common = {
>  
>  static Property apic_properties_common[] = {
>      DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
> -    DEFINE_PROP_PTR("cpu_env", APICCommonState, cpu_env),
>      DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
>                      true),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index e307b4e..0cce910 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -41,6 +41,7 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "hw/xen.h"
>  #include "hw/sysbus.h"
> +#include "hw/apic_internal.h"
>  #endif
>  
>  /* feature flags taken from "Intel Processor Identification and the CPUID
> @@ -1884,6 +1885,7 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>  #ifndef CONFIG_USER_ONLY
>      static int apic_mapped;
>      CPUX86State *env = &cpu->env;
> +    APICCommonState *apic;
>      const char *apic_type = "apic";
>  
>      if (kvm_irqchip_in_kernel()) {
> @@ -1902,7 +1904,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>                                OBJECT(env->apic_state), NULL);
>      qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
>      /* TODO: convert to link<> */
> -    qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
> +    apic = APIC_COMMON(env->apic_state);
> +    apic->cpu_env = env;
>  
>      if (qdev_init(env->apic_state)) {
>          error_setg(errp, "APIC device '%s' could not be initialized",
>
Paolo Bonzini - Oct. 16, 2012, 4:04 p.m.
Il 16/10/2012 18:02, Andreas Färber ha scritto:
> Am 12.10.2012 03:26, schrieb Andreas Färber:
>> This prepares for changing the variable type from void*.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Cc: Igor Mammedov <imammedo@redhat.com>
> 
> Paolo, are you happy with getting rid of the pointer property this way?

I wouldn't call it "getting rid of the pointer property", more like
sweeping it under a rug... :)  But it is okay with the TODO comment.

Paolo

> Jan, are you okay with accessing APICCommonState in target-i386/cpu.c as
> an interim solution?
> 
> Andreas
> 
>> ---
>>  hw/apic_common.c  |    1 -
>>  target-i386/cpu.c |    5 ++++-
>>  2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>>
>> diff --git a/hw/apic_common.c b/hw/apic_common.c
>> index 371f95d..a26a631 100644
>> --- a/hw/apic_common.c
>> +++ b/hw/apic_common.c
>> @@ -368,7 +368,6 @@ static const VMStateDescription vmstate_apic_common = {
>>  
>>  static Property apic_properties_common[] = {
>>      DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
>> -    DEFINE_PROP_PTR("cpu_env", APICCommonState, cpu_env),
>>      DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
>>                      true),
>>      DEFINE_PROP_END_OF_LIST(),
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index e307b4e..0cce910 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -41,6 +41,7 @@
>>  #ifndef CONFIG_USER_ONLY
>>  #include "hw/xen.h"
>>  #include "hw/sysbus.h"
>> +#include "hw/apic_internal.h"
>>  #endif
>>  
>>  /* feature flags taken from "Intel Processor Identification and the CPUID
>> @@ -1884,6 +1885,7 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>>  #ifndef CONFIG_USER_ONLY
>>      static int apic_mapped;
>>      CPUX86State *env = &cpu->env;
>> +    APICCommonState *apic;
>>      const char *apic_type = "apic";
>>  
>>      if (kvm_irqchip_in_kernel()) {
>> @@ -1902,7 +1904,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>>                                OBJECT(env->apic_state), NULL);
>>      qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
>>      /* TODO: convert to link<> */
>> -    qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
>> +    apic = APIC_COMMON(env->apic_state);
>> +    apic->cpu_env = env;
>>  
>>      if (qdev_init(env->apic_state)) {
>>          error_setg(errp, "APIC device '%s' could not be initialized",
>>
>
Jan Kiszka - Oct. 17, 2012, 12:50 p.m.
On 2012-10-16 18:02, Andreas Färber wrote:
> Am 12.10.2012 03:26, schrieb Andreas Färber:
>> This prepares for changing the variable type from void*.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Cc: Igor Mammedov <imammedo@redhat.com>
> 
> Paolo, are you happy with getting rid of the pointer property this way?
> 
> Jan, are you okay with accessing APICCommonState in target-i386/cpu.c as
> an interim solution?

Yes, I don't see it as a problem.

Jan

> 
> Andreas
> 
>> ---
>>  hw/apic_common.c  |    1 -
>>  target-i386/cpu.c |    5 ++++-
>>  2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>>
>> diff --git a/hw/apic_common.c b/hw/apic_common.c
>> index 371f95d..a26a631 100644
>> --- a/hw/apic_common.c
>> +++ b/hw/apic_common.c
>> @@ -368,7 +368,6 @@ static const VMStateDescription vmstate_apic_common = {
>>  
>>  static Property apic_properties_common[] = {
>>      DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
>> -    DEFINE_PROP_PTR("cpu_env", APICCommonState, cpu_env),
>>      DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
>>                      true),
>>      DEFINE_PROP_END_OF_LIST(),
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index e307b4e..0cce910 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -41,6 +41,7 @@
>>  #ifndef CONFIG_USER_ONLY
>>  #include "hw/xen.h"
>>  #include "hw/sysbus.h"
>> +#include "hw/apic_internal.h"
>>  #endif
>>  
>>  /* feature flags taken from "Intel Processor Identification and the CPUID
>> @@ -1884,6 +1885,7 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>>  #ifndef CONFIG_USER_ONLY
>>      static int apic_mapped;
>>      CPUX86State *env = &cpu->env;
>> +    APICCommonState *apic;
>>      const char *apic_type = "apic";
>>  
>>      if (kvm_irqchip_in_kernel()) {
>> @@ -1902,7 +1904,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
>>                                OBJECT(env->apic_state), NULL);
>>      qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
>>      /* TODO: convert to link<> */
>> -    qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
>> +    apic = APIC_COMMON(env->apic_state);
>> +    apic->cpu_env = env;
>>  
>>      if (qdev_init(env->apic_state)) {
>>          error_setg(errp, "APIC device '%s' could not be initialized",
>>
>

Patch

diff --git a/hw/apic_common.c b/hw/apic_common.c
index 371f95d..a26a631 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -368,7 +368,6 @@  static const VMStateDescription vmstate_apic_common = {
 
 static Property apic_properties_common[] = {
     DEFINE_PROP_UINT8("id", APICCommonState, id, -1),
-    DEFINE_PROP_PTR("cpu_env", APICCommonState, cpu_env),
     DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control, VAPIC_ENABLE_BIT,
                     true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e307b4e..0cce910 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -41,6 +41,7 @@ 
 #ifndef CONFIG_USER_ONLY
 #include "hw/xen.h"
 #include "hw/sysbus.h"
+#include "hw/apic_internal.h"
 #endif
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
@@ -1884,6 +1885,7 @@  static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
 #ifndef CONFIG_USER_ONLY
     static int apic_mapped;
     CPUX86State *env = &cpu->env;
+    APICCommonState *apic;
     const char *apic_type = "apic";
 
     if (kvm_irqchip_in_kernel()) {
@@ -1902,7 +1904,8 @@  static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
                               OBJECT(env->apic_state), NULL);
     qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
     /* TODO: convert to link<> */
-    qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
+    apic = APIC_COMMON(env->apic_state);
+    apic->cpu_env = env;
 
     if (qdev_init(env->apic_state)) {
         error_setg(errp, "APIC device '%s' could not be initialized",