Patchwork [10/16] target-i386: introduce apic-id property

login
register
mail settings
Submitter Igor Mammedov
Date April 15, 2013, 10:12 p.m.
Message ID <1366063976-4909-11-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/236747/
State New
Headers show

Comments

Igor Mammedov - April 15, 2013, 10:12 p.m.
... and use it from board level to set APIC ID for CPUs
it creates.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
Note:
  * pc_new_cpu() function will be reused later in CPU hot-plug hook.

v3:
  * user error_propagate() in property setter
v2:
  * use generic cpu_exists() instead of custom one
  * make apic-id dynamic property, so it won't be possible to use it
    as global property, since it's instance specific
---
 hw/i386/pc.c      | 25 ++++++++++++++++++++++++-
 target-i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)
Michael S. Tsirkin - April 22, 2013, 9:49 a.m.
On Tue, Apr 16, 2013 at 12:12:50AM +0200, Igor Mammedov wrote:
> ... and use it from board level to set APIC ID for CPUs
> it creates.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> Note:
>   * pc_new_cpu() function will be reused later in CPU hot-plug hook.
> 
> v3:
>   * user error_propagate() in property setter
> v2:
>   * use generic cpu_exists() instead of custom one
>   * make apic-id dynamic property, so it won't be possible to use it
>     as global property, since it's instance specific
> ---
>  hw/i386/pc.c      | 25 ++++++++++++++++++++++++-
>  target-i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index dc1a78b..cb57878 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -889,9 +889,29 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>      }
>  }
>  
> +static void pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
> +{
> +    X86CPU *cpu;
> +
> +    cpu = cpu_x86_create(cpu_model, errp);
> +    if (!cpu) {
> +        return;
> +    }
> +
> +    object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
> +    object_property_set_bool(OBJECT(cpu), true, "realized", errp);
> +
> +    if (error_is_set(errp)) {
> +        if (cpu != NULL) {
> +            object_unref(OBJECT(cpu));
> +        }
> +    }
> +}
> +
>  void pc_cpus_init(const char *cpu_model)
>  {
>      int i;
> +    Error *error = NULL;
>  
>      /* init CPUs */
>      if (cpu_model == NULL) {
> @@ -903,7 +923,10 @@ void pc_cpus_init(const char *cpu_model)
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        if (!cpu_x86_init(cpu_model)) {
> +        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> +        if (error) {
> +            fprintf(stderr, "%s\n", error_get_pretty(error));
> +            error_free(error);
>              exit(1);
>          }
>      }
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index a415fa3..6d6c527 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1271,6 +1271,45 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
>      cpu->env.tsc_khz = value / 1000;
>  }
>  
> +static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
> +                                  const char *name, Error **errp)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    int64_t value = cpu->env.cpuid_apic_id;
> +
> +    visit_type_int(v, &value, name, errp);
> +}
> +
> +static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
> +                                  const char *name, Error **errp)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    const int64_t min = 0;
> +    const int64_t max = UINT32_MAX;
> +    Error *error = NULL;
> +    int64_t value;
> +
> +    visit_type_int(v, &value, name, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +    if (value < min || value > max) {
> +        error_setg(&error, "Property %s.%s doesn't take value %" PRId64
> +                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
> +                   object_get_typename(obj), name, value, min, max);
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    if (cpu_exists(value)) {
> +        error_setg(&error, "CPU with APIC ID %" PRIi64 " exists", value);
> +        error_propagate(errp, error);
> +        return;
> +    }
> +    cpu->env.cpuid_apic_id = value;
> +}
> +
>  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>  {
>      x86_def_t *def;
> @@ -2259,6 +2298,9 @@ static void x86_cpu_initfn(Object *obj)
>      object_property_add(obj, "tsc-frequency", "int",
>                          x86_cpuid_get_tsc_freq,
>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> +    object_property_add(obj, "apic-id", "int",
> +                        x86_cpuid_get_apic_id,
> +                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
>  
>      env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
>  
> -- 
> 1.8.2
>
Andreas Färber - April 22, 2013, 2:05 p.m.
Am 16.04.2013 00:12, schrieb Igor Mammedov:
> ... and use it from board level to set APIC ID for CPUs
> it creates.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> Note:
>   * pc_new_cpu() function will be reused later in CPU hot-plug hook.

Suggest as main commit message to avoid the "...":

The property is used from board level to set APIC ID for CPUs it
creates. Do so in a new pc_new_cpu() helper, to be reused for hot-plug.

> 
> v3:
>   * user error_propagate() in property setter
> v2:
>   * use generic cpu_exists() instead of custom one
>   * make apic-id dynamic property, so it won't be possible to use it
>     as global property, since it's instance specific
> ---
>  hw/i386/pc.c      | 25 ++++++++++++++++++++++++-
>  target-i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index dc1a78b..cb57878 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -889,9 +889,29 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>      }
>  }
>  
> +static void pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
> +{
> +    X86CPU *cpu;
> +
> +    cpu = cpu_x86_create(cpu_model, errp);
> +    if (!cpu) {
> +        return;
> +    }
> +
> +    object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
> +    object_property_set_bool(OBJECT(cpu), true, "realized", errp);
> +
> +    if (error_is_set(errp)) {

Please use a local Error* variable with error_propagate, otherwise this
is fragile.

> +        if (cpu != NULL) {
> +            object_unref(OBJECT(cpu));
> +        }
> +    }
> +}
> +
>  void pc_cpus_init(const char *cpu_model)
>  {
>      int i;
> +    Error *error = NULL;
>  
>      /* init CPUs */
>      if (cpu_model == NULL) {
> @@ -903,7 +923,10 @@ void pc_cpus_init(const char *cpu_model)
>      }
>  
>      for (i = 0; i < smp_cpus; i++) {
> -        if (!cpu_x86_init(cpu_model)) {
> +        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> +        if (error) {
> +            fprintf(stderr, "%s\n", error_get_pretty(error));
> +            error_free(error);
>              exit(1);
>          }
>      }
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index a415fa3..6d6c527 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1271,6 +1271,45 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
>      cpu->env.tsc_khz = value / 1000;
>  }
>  
> +static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
> +                                  const char *name, Error **errp)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    int64_t value = cpu->env.cpuid_apic_id;
> +
> +    visit_type_int(v, &value, name, errp);
> +}
> +
> +static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
> +                                  const char *name, Error **errp)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    const int64_t min = 0;
> +    const int64_t max = UINT32_MAX;
> +    Error *error = NULL;
> +    int64_t value;
> +
> +    visit_type_int(v, &value, name, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }

> +    if (value < min || value > max) {
> +        error_setg(&error, "Property %s.%s doesn't take value %" PRId64
> +                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
> +                   object_get_typename(obj), name, value, min, max);
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    if (cpu_exists(value)) {
> +        error_setg(&error, "CPU with APIC ID %" PRIi64 " exists", value);
> +        error_propagate(errp, error);

You could do both error_setg()s directly on errp.

> +        return;
> +    }
> +    cpu->env.cpuid_apic_id = value;
> +}
> +
>  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>  {
>      x86_def_t *def;
> @@ -2259,6 +2298,9 @@ static void x86_cpu_initfn(Object *obj)
>      object_property_add(obj, "tsc-frequency", "int",
>                          x86_cpuid_get_tsc_freq,
>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> +    object_property_add(obj, "apic-id", "int",
> +                        x86_cpuid_get_apic_id,
> +                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
>  
>      env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
>  

Otherwise I like this x86-specific version now!

Andreas
Igor Mammedov - April 22, 2013, 4:30 p.m.
On Mon, 22 Apr 2013 16:05:34 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 16.04.2013 00:12, schrieb Igor Mammedov:
> > ... and use it from board level to set APIC ID for CPUs
> > it creates.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> > Note:
> >   * pc_new_cpu() function will be reused later in CPU hot-plug hook.
> 
> Suggest as main commit message to avoid the "...":
> 
> The property is used from board level to set APIC ID for CPUs it
> creates. Do so in a new pc_new_cpu() helper, to be reused for hot-plug.
I'll do on next respin.

> > 
> > v3:
> >   * user error_propagate() in property setter
> > v2:
> >   * use generic cpu_exists() instead of custom one
> >   * make apic-id dynamic property, so it won't be possible to use it
> >     as global property, since it's instance specific
> > ---
> >  hw/i386/pc.c      | 25 ++++++++++++++++++++++++-
> >  target-i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index dc1a78b..cb57878 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -889,9 +889,29 @@ void pc_acpi_smi_interrupt(void *opaque, int irq,
> > int level) }
> >  }
> >  
> > +static void pc_new_cpu(const char *cpu_model, int64_t apic_id, Error
> > **errp) +{
> > +    X86CPU *cpu;
> > +
> > +    cpu = cpu_x86_create(cpu_model, errp);
> > +    if (!cpu) {
> > +        return;
> > +    }
> > +
> > +    object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
> > +    object_property_set_bool(OBJECT(cpu), true, "realized", errp);
> > +
> > +    if (error_is_set(errp)) {
> 
> Please use a local Error* variable with error_propagate, otherwise this
> is fragile.
done

> 
> > +        if (cpu != NULL) {
> > +            object_unref(OBJECT(cpu));
> > +        }
> > +    }
> > +}
> > +
> >  void pc_cpus_init(const char *cpu_model)
> >  {
> >      int i;
> > +    Error *error = NULL;
> >  
> >      /* init CPUs */
> >      if (cpu_model == NULL) {
> > @@ -903,7 +923,10 @@ void pc_cpus_init(const char *cpu_model)
> >      }
> >  
> >      for (i = 0; i < smp_cpus; i++) {
> > -        if (!cpu_x86_init(cpu_model)) {
> > +        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> > +        if (error) {
> > +            fprintf(stderr, "%s\n", error_get_pretty(error));
> > +            error_free(error);
> >              exit(1);
> >          }
> >      }
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index a415fa3..6d6c527 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1271,6 +1271,45 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
> > Visitor *v, void *opaque, cpu->env.tsc_khz = value / 1000;
> >  }
> >  
> > +static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
> > +                                  const char *name, Error **errp)
> > +{
> > +    X86CPU *cpu = X86_CPU(obj);
> > +    int64_t value = cpu->env.cpuid_apic_id;
> > +
> > +    visit_type_int(v, &value, name, errp);
> > +}
> > +
> > +static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
> > +                                  const char *name, Error **errp)
> > +{
> > +    X86CPU *cpu = X86_CPU(obj);
> > +    const int64_t min = 0;
> > +    const int64_t max = UINT32_MAX;
> > +    Error *error = NULL;
> > +    int64_t value;
> > +
> > +    visit_type_int(v, &value, name, &error);
> > +    if (error) {
> > +        error_propagate(errp, error);
> > +        return;
> > +    }
> 
> > +    if (value < min || value > max) {
> > +        error_setg(&error, "Property %s.%s doesn't take value %" PRId64
> > +                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
> > +                   object_get_typename(obj), name, value, min, max);
> > +        error_propagate(errp, error);
> > +        return;
> > +    }
> > +
> > +    if (cpu_exists(value)) {
> > +        error_setg(&error, "CPU with APIC ID %" PRIi64 " exists", value);
> > +        error_propagate(errp, error);
> 
> You could do both error_setg()s directly on errp.
it was so before, but I was requested to use local error here:
http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg01606.html

> 
> > +        return;
> > +    }
> > +    cpu->env.cpuid_apic_id = value;
> > +}
> > +
> >  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> >  {
> >      x86_def_t *def;
> > @@ -2259,6 +2298,9 @@ static void x86_cpu_initfn(Object *obj)
> >      object_property_add(obj, "tsc-frequency", "int",
> >                          x86_cpuid_get_tsc_freq,
> >                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> > +    object_property_add(obj, "apic-id", "int",
> > +                        x86_cpuid_get_apic_id,
> > +                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
> >  
> >      env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
> >  
> 
> Otherwise I like this x86-specific version now!
> 
> Andreas
>
Andreas Färber - April 26, 2013, 4:35 p.m.
Am 22.04.2013 18:30, schrieb Igor Mammedov:
> On Mon, 22 Apr 2013 16:05:34 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 16.04.2013 00:12, schrieb Igor Mammedov:
>>> ... and use it from board level to set APIC ID for CPUs
>>> it creates.
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>> Note:
>>>   * pc_new_cpu() function will be reused later in CPU hot-plug hook.
>>
>> Suggest as main commit message to avoid the "...":
>>
>> The property is used from board level to set APIC ID for CPUs it
>> creates. Do so in a new pc_new_cpu() helper, to be reused for hot-plug.
> I'll do on next respin.
> 
>>>
>>> v3:
>>>   * user error_propagate() in property setter
>>> v2:
>>>   * use generic cpu_exists() instead of custom one
>>>   * make apic-id dynamic property, so it won't be possible to use it
>>>     as global property, since it's instance specific
>>> ---
>>>  hw/i386/pc.c      | 25 ++++++++++++++++++++++++-
>>>  target-i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 66 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index dc1a78b..cb57878 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -889,9 +889,29 @@ void pc_acpi_smi_interrupt(void *opaque, int irq,
>>> int level) }
>>>  }
>>>  
>>> +static void pc_new_cpu(const char *cpu_model, int64_t apic_id, Error
>>> **errp) +{
>>> +    X86CPU *cpu;
>>> +
>>> +    cpu = cpu_x86_create(cpu_model, errp);
>>> +    if (!cpu) {
>>> +        return;
>>> +    }
>>> +
>>> +    object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
>>> +    object_property_set_bool(OBJECT(cpu), true, "realized", errp);
>>> +
>>> +    if (error_is_set(errp)) {
>>
>> Please use a local Error* variable with error_propagate, otherwise this
>> is fragile.
> done
> 
>>
>>> +        if (cpu != NULL) {
>>> +            object_unref(OBJECT(cpu));
>>> +        }
>>> +    }
>>> +}
>>> +
>>>  void pc_cpus_init(const char *cpu_model)
>>>  {
>>>      int i;
>>> +    Error *error = NULL;
>>>  
>>>      /* init CPUs */
>>>      if (cpu_model == NULL) {
>>> @@ -903,7 +923,10 @@ void pc_cpus_init(const char *cpu_model)
>>>      }
>>>  
>>>      for (i = 0; i < smp_cpus; i++) {
>>> -        if (!cpu_x86_init(cpu_model)) {
>>> +        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
>>> +        if (error) {
>>> +            fprintf(stderr, "%s\n", error_get_pretty(error));
>>> +            error_free(error);
>>>              exit(1);
>>>          }
>>>      }
>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>> index a415fa3..6d6c527 100644
>>> --- a/target-i386/cpu.c
>>> +++ b/target-i386/cpu.c
>>> @@ -1271,6 +1271,45 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
>>> Visitor *v, void *opaque, cpu->env.tsc_khz = value / 1000;
>>>  }
>>>  
>>> +static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
>>> +                                  const char *name, Error **errp)
>>> +{
>>> +    X86CPU *cpu = X86_CPU(obj);
>>> +    int64_t value = cpu->env.cpuid_apic_id;
>>> +
>>> +    visit_type_int(v, &value, name, errp);
>>> +}
>>> +
>>> +static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
>>> +                                  const char *name, Error **errp)
>>> +{
>>> +    X86CPU *cpu = X86_CPU(obj);
>>> +    const int64_t min = 0;
>>> +    const int64_t max = UINT32_MAX;
>>> +    Error *error = NULL;
>>> +    int64_t value;
>>> +
>>> +    visit_type_int(v, &value, name, &error);
>>> +    if (error) {
>>> +        error_propagate(errp, error);
>>> +        return;
>>> +    }
>>
>>> +    if (value < min || value > max) {
>>> +        error_setg(&error, "Property %s.%s doesn't take value %" PRId64
>>> +                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
>>> +                   object_get_typename(obj), name, value, min, max);
>>> +        error_propagate(errp, error);
>>> +        return;
>>> +    }
>>> +
>>> +    if (cpu_exists(value)) {
>>> +        error_setg(&error, "CPU with APIC ID %" PRIi64 " exists", value);
>>> +        error_propagate(errp, error);
>>
>> You could do both error_setg()s directly on errp.
> it was so before, but I was requested to use local error here:
> http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg01606.html

The only thing Paolo requested there (and which I +1) is not to do
error_is_set(errp). That is independent of error_setg() though, which is
followed by a return, so has no functional difference.

Andreas

> 
>>
>>> +        return;
>>> +    }
>>> +    cpu->env.cpuid_apic_id = value;
>>> +}
>>> +
>>>  static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
>>>  {
>>>      x86_def_t *def;
>>> @@ -2259,6 +2298,9 @@ static void x86_cpu_initfn(Object *obj)
>>>      object_property_add(obj, "tsc-frequency", "int",
>>>                          x86_cpuid_get_tsc_freq,
>>>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
>>> +    object_property_add(obj, "apic-id", "int",
>>> +                        x86_cpuid_get_apic_id,
>>> +                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
>>>  
>>>      env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
>>>  
>>
>> Otherwise I like this x86-specific version now!
>>
>> Andreas
>>
> 
>

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index dc1a78b..cb57878 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -889,9 +889,29 @@  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
+static void pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
+{
+    X86CPU *cpu;
+
+    cpu = cpu_x86_create(cpu_model, errp);
+    if (!cpu) {
+        return;
+    }
+
+    object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
+    object_property_set_bool(OBJECT(cpu), true, "realized", errp);
+
+    if (error_is_set(errp)) {
+        if (cpu != NULL) {
+            object_unref(OBJECT(cpu));
+        }
+    }
+}
+
 void pc_cpus_init(const char *cpu_model)
 {
     int i;
+    Error *error = NULL;
 
     /* init CPUs */
     if (cpu_model == NULL) {
@@ -903,7 +923,10 @@  void pc_cpus_init(const char *cpu_model)
     }
 
     for (i = 0; i < smp_cpus; i++) {
-        if (!cpu_x86_init(cpu_model)) {
+        pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
+        if (error) {
+            fprintf(stderr, "%s\n", error_get_pretty(error));
+            error_free(error);
             exit(1);
         }
     }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a415fa3..6d6c527 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1271,6 +1271,45 @@  static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
     cpu->env.tsc_khz = value / 1000;
 }
 
+static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    int64_t value = cpu->env.cpuid_apic_id;
+
+    visit_type_int(v, &value, name, errp);
+}
+
+static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
+                                  const char *name, Error **errp)
+{
+    X86CPU *cpu = X86_CPU(obj);
+    const int64_t min = 0;
+    const int64_t max = UINT32_MAX;
+    Error *error = NULL;
+    int64_t value;
+
+    visit_type_int(v, &value, name, &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+    if (value < min || value > max) {
+        error_setg(&error, "Property %s.%s doesn't take value %" PRId64
+                   " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
+                   object_get_typename(obj), name, value, min, max);
+        error_propagate(errp, error);
+        return;
+    }
+
+    if (cpu_exists(value)) {
+        error_setg(&error, "CPU with APIC ID %" PRIi64 " exists", value);
+        error_propagate(errp, error);
+        return;
+    }
+    cpu->env.cpuid_apic_id = value;
+}
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
 {
     x86_def_t *def;
@@ -2259,6 +2298,9 @@  static void x86_cpu_initfn(Object *obj)
     object_property_add(obj, "tsc-frequency", "int",
                         x86_cpuid_get_tsc_freq,
                         x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
+    object_property_add(obj, "apic-id", "int",
+                        x86_cpuid_get_apic_id,
+                        x86_cpuid_set_apic_id, NULL, NULL, NULL);
 
     env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);