Patchwork [08/19] cpu: introduce get_arch_id() method and override it for target-i386

login
register
mail settings
Submitter Igor Mammedov
Date April 11, 2013, 2:51 p.m.
Message ID <1365691918-30594-9-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/235809/
State New
Headers show

Comments

Igor Mammedov - April 11, 2013, 2:51 p.m.
get_arch_id() adds possibility for generic code to get guest visible
CPI id without accessing CPUArchState. If target doesn't override it,
it will return cpu_index.

Override it on target-i386 to return APIC ID.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 * it will be used later by new cpu_exists() generic function and
   acpi_piix.
 * s/cpu_firmware_id/cpu_arch_id/
---
 include/qom/cpu.h |  2 ++
 qom/cpu.c         |  6 ++++++
 target-i386/cpu.c | 10 ++++++++++
 3 files changed, 18 insertions(+)
Eduardo Habkost - April 11, 2013, 7:04 p.m.
On Thu, Apr 11, 2013 at 04:51:47PM +0200, Igor Mammedov wrote:
> get_arch_id() adds possibility for generic code to get guest visible
> CPI id without accessing CPUArchState. If target doesn't override it,
> it will return cpu_index.
> 
> Override it on target-i386 to return APIC ID.

The code implementing the method looks OK, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

But I still need to understand better why exactly i386 needs to use it
and make the externally-visible hotplug interface based on APIC IDs if
all other architectures are going to be happy using CPU indexes to do
the same thing.

> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  * it will be used later by new cpu_exists() generic function and
>    acpi_piix.
>  * s/cpu_firmware_id/cpu_arch_id/
> ---
>  include/qom/cpu.h |  2 ++
>  qom/cpu.c         |  6 ++++++
>  target-i386/cpu.c | 10 ++++++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 3664a1b..b376416 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -45,6 +45,7 @@ typedef struct CPUState CPUState;
>   * instantiatable CPU type.
>   * @reset: Callback to reset the #CPUState to its initial state.
>   * @do_interrupt: Callback for interrupt handling.
> + * @get_arch_id: Callback for getting architecture depended CPU ID
>   * @vmsd: State description for migration.
>   *
>   * Represents a CPU family or model.
> @@ -58,6 +59,7 @@ typedef struct CPUClass {
>  
>      void (*reset)(CPUState *cpu);
>      void (*do_interrupt)(CPUState *cpu);
> +    int64_t (*get_arch_id)(CPUState *cpu);
>  
>      const struct VMStateDescription *vmsd;
>  } CPUClass;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index fe85960..90cbd77 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -77,6 +77,11 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>      }
>  }
>  
> +static int64_t cpu_common_get_arch_id(CPUState *cpu)
> +{
> +    return cpu->cpu_index;
> +}
> +
>  static void cpu_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -84,6 +89,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>  
>      k->class_by_name = cpu_common_class_by_name;
>      k->reset = cpu_common_reset;
> +    k->get_arch_id = cpu_common_get_arch_id;
>      dc->realize = cpu_common_realizefn;
>      dc->no_user = 1;
>  }
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index c28436c..9cca031 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2272,6 +2272,14 @@ static void x86_cpu_initfn(Object *obj)
>      }
>  }
>  
> +static int64_t x86_cpu_get_arch_id(CPUState *cpu)
> +{
> +    X86CPU *x86cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86cpu->env;
> +
> +    return env->cpuid_apic_id;
> +}
> +
>  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  {
>      X86CPUClass *xcc = X86_CPU_CLASS(oc);
> @@ -2286,6 +2294,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>  
>      cc->do_interrupt = x86_cpu_do_interrupt;
>      cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
> +
> +    cc->get_arch_id = x86_cpu_get_arch_id;
>  }
>  
>  static const TypeInfo x86_cpu_type_info = {
> -- 
> 1.8.2
>
Igor Mammedov - April 12, 2013, 10:31 a.m.
On Thu, 11 Apr 2013 16:04:26 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Thu, Apr 11, 2013 at 04:51:47PM +0200, Igor Mammedov wrote:
> > get_arch_id() adds possibility for generic code to get guest visible
> > CPI id without accessing CPUArchState. If target doesn't override it,
> > it will return cpu_index.
> > 
> > Override it on target-i386 to return APIC ID.
> 
> The code implementing the method looks OK, so:
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> But I still need to understand better why exactly i386 needs to use it
> and make the externally-visible hotplug interface based on APIC IDs if
> all other architectures are going to be happy using CPU indexes to do
> the same thing.
get_arch_id() was prompted by 10/19 which is used not only by target-i386.
so it was introduced to make acpi_piix4 CPU hotplug more generic. And proved
to useful in a number of other places.

> 
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  * it will be used later by new cpu_exists() generic function and
> >    acpi_piix.
> >  * s/cpu_firmware_id/cpu_arch_id/
> > ---
> >  include/qom/cpu.h |  2 ++
> >  qom/cpu.c         |  6 ++++++
> >  target-i386/cpu.c | 10 ++++++++++
> >  3 files changed, 18 insertions(+)
> > 
> > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > index 3664a1b..b376416 100644
> > --- a/include/qom/cpu.h
> > +++ b/include/qom/cpu.h
> > @@ -45,6 +45,7 @@ typedef struct CPUState CPUState;
> >   * instantiatable CPU type.
> >   * @reset: Callback to reset the #CPUState to its initial state.
> >   * @do_interrupt: Callback for interrupt handling.
> > + * @get_arch_id: Callback for getting architecture depended CPU ID
> >   * @vmsd: State description for migration.
> >   *
> >   * Represents a CPU family or model.
> > @@ -58,6 +59,7 @@ typedef struct CPUClass {
> >  
> >      void (*reset)(CPUState *cpu);
> >      void (*do_interrupt)(CPUState *cpu);
> > +    int64_t (*get_arch_id)(CPUState *cpu);
> >  
> >      const struct VMStateDescription *vmsd;
> >  } CPUClass;
> > diff --git a/qom/cpu.c b/qom/cpu.c
> > index fe85960..90cbd77 100644
> > --- a/qom/cpu.c
> > +++ b/qom/cpu.c
> > @@ -77,6 +77,11 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
> >      }
> >  }
> >  
> > +static int64_t cpu_common_get_arch_id(CPUState *cpu)
> > +{
> > +    return cpu->cpu_index;
> > +}
> > +
> >  static void cpu_class_init(ObjectClass *klass, void *data)
> >  {
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -84,6 +89,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
> >  
> >      k->class_by_name = cpu_common_class_by_name;
> >      k->reset = cpu_common_reset;
> > +    k->get_arch_id = cpu_common_get_arch_id;
> >      dc->realize = cpu_common_realizefn;
> >      dc->no_user = 1;
> >  }
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index c28436c..9cca031 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -2272,6 +2272,14 @@ static void x86_cpu_initfn(Object *obj)
> >      }
> >  }
> >  
> > +static int64_t x86_cpu_get_arch_id(CPUState *cpu)
> > +{
> > +    X86CPU *x86cpu = X86_CPU(cpu);
> > +    CPUX86State *env = &x86cpu->env;
> > +
> > +    return env->cpuid_apic_id;
> > +}
> > +
> >  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> >  {
> >      X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > @@ -2286,6 +2294,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> >  
> >      cc->do_interrupt = x86_cpu_do_interrupt;
> >      cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
> > +
> > +    cc->get_arch_id = x86_cpu_get_arch_id;
> >  }
> >  
> >  static const TypeInfo x86_cpu_type_info = {
> > -- 
> > 1.8.2
> > 
> 
> -- 
> Eduardo
Eduardo Habkost - April 12, 2013, 1:47 p.m.
On Fri, Apr 12, 2013 at 12:31:22PM +0200, Igor Mammedov wrote:
> On Thu, 11 Apr 2013 16:04:26 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Thu, Apr 11, 2013 at 04:51:47PM +0200, Igor Mammedov wrote:
> > > get_arch_id() adds possibility for generic code to get guest visible
> > > CPI id without accessing CPUArchState. If target doesn't override it,
> > > it will return cpu_index.
> > > 
> > > Override it on target-i386 to return APIC ID.
> > 
> > The code implementing the method looks OK, so:
> > 
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > 
> > But I still need to understand better why exactly i386 needs to use it
> > and make the externally-visible hotplug interface based on APIC IDs if
> > all other architectures are going to be happy using CPU indexes to do
> > the same thing.
> get_arch_id() was prompted by 10/19 which is used not only by target-i386.
> so it was introduced to make acpi_piix4 CPU hotplug more generic. And proved
> to useful in a number of other places.

OK, so get_arch_id() is useful for other code too, not just for the
external cpu-add interface. It makes lots of sense to use it on
acpi_piix4, yes.

(My question about making the cpu-add interface based on arch IDs
instead of arch-independent CPU indexes is being discussed under patch
18/19).

> 
> > 
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  * it will be used later by new cpu_exists() generic function and
> > >    acpi_piix.
> > >  * s/cpu_firmware_id/cpu_arch_id/
> > > ---
> > >  include/qom/cpu.h |  2 ++
> > >  qom/cpu.c         |  6 ++++++
> > >  target-i386/cpu.c | 10 ++++++++++
> > >  3 files changed, 18 insertions(+)
> > > 
> > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > > index 3664a1b..b376416 100644
> > > --- a/include/qom/cpu.h
> > > +++ b/include/qom/cpu.h
> > > @@ -45,6 +45,7 @@ typedef struct CPUState CPUState;
> > >   * instantiatable CPU type.
> > >   * @reset: Callback to reset the #CPUState to its initial state.
> > >   * @do_interrupt: Callback for interrupt handling.
> > > + * @get_arch_id: Callback for getting architecture depended CPU ID
> > >   * @vmsd: State description for migration.
> > >   *
> > >   * Represents a CPU family or model.
> > > @@ -58,6 +59,7 @@ typedef struct CPUClass {
> > >  
> > >      void (*reset)(CPUState *cpu);
> > >      void (*do_interrupt)(CPUState *cpu);
> > > +    int64_t (*get_arch_id)(CPUState *cpu);
> > >  
> > >      const struct VMStateDescription *vmsd;
> > >  } CPUClass;
> > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > index fe85960..90cbd77 100644
> > > --- a/qom/cpu.c
> > > +++ b/qom/cpu.c
> > > @@ -77,6 +77,11 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
> > >      }
> > >  }
> > >  
> > > +static int64_t cpu_common_get_arch_id(CPUState *cpu)
> > > +{
> > > +    return cpu->cpu_index;
> > > +}
> > > +
> > >  static void cpu_class_init(ObjectClass *klass, void *data)
> > >  {
> > >      DeviceClass *dc = DEVICE_CLASS(klass);
> > > @@ -84,6 +89,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
> > >  
> > >      k->class_by_name = cpu_common_class_by_name;
> > >      k->reset = cpu_common_reset;
> > > +    k->get_arch_id = cpu_common_get_arch_id;
> > >      dc->realize = cpu_common_realizefn;
> > >      dc->no_user = 1;
> > >  }
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index c28436c..9cca031 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -2272,6 +2272,14 @@ static void x86_cpu_initfn(Object *obj)
> > >      }
> > >  }
> > >  
> > > +static int64_t x86_cpu_get_arch_id(CPUState *cpu)
> > > +{
> > > +    X86CPU *x86cpu = X86_CPU(cpu);
> > > +    CPUX86State *env = &x86cpu->env;
> > > +
> > > +    return env->cpuid_apic_id;
> > > +}
> > > +
> > >  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> > >  {
> > >      X86CPUClass *xcc = X86_CPU_CLASS(oc);
> > > @@ -2286,6 +2294,8 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> > >  
> > >      cc->do_interrupt = x86_cpu_do_interrupt;
> > >      cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
> > > +
> > > +    cc->get_arch_id = x86_cpu_get_arch_id;
> > >  }
> > >  
> > >  static const TypeInfo x86_cpu_type_info = {
> > > -- 
> > > 1.8.2
> > > 
> > 
> > -- 
> > Eduardo
> 
> 
> -- 
> Regards,
>   Igor
Andreas Färber - April 15, 2013, 3:24 p.m.
Am 11.04.2013 16:51, schrieb Igor Mammedov:
> get_arch_id() adds possibility for generic code to get guest visible
> CPI id without accessing CPUArchState. If target doesn't override it,
> it will return cpu_index.
> 
> Override it on target-i386 to return APIC ID.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  * it will be used later by new cpu_exists() generic function and
>    acpi_piix.
>  * s/cpu_firmware_id/cpu_arch_id/
> ---
>  include/qom/cpu.h |  2 ++
>  qom/cpu.c         |  6 ++++++
>  target-i386/cpu.c | 10 ++++++++++
>  3 files changed, 18 insertions(+)

I'm still unclear about this ID topic, but please don't add QOM methods
with arch_ in the name. The arch is expressed through subclasses.

Andreas
Igor Mammedov - April 15, 2013, 3:34 p.m.
On Mon, 15 Apr 2013 17:24:36 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 11.04.2013 16:51, schrieb Igor Mammedov:
> > get_arch_id() adds possibility for generic code to get guest visible
> > CPI id without accessing CPUArchState. If target doesn't override it,
> > it will return cpu_index.
> > 
> > Override it on target-i386 to return APIC ID.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  * it will be used later by new cpu_exists() generic function and
> >    acpi_piix.
> >  * s/cpu_firmware_id/cpu_arch_id/
> > ---
> >  include/qom/cpu.h |  2 ++
> >  qom/cpu.c         |  6 ++++++
> >  target-i386/cpu.c | 10 ++++++++++
> >  3 files changed, 18 insertions(+)
> 
> I'm still unclear about this ID topic, but please don't add QOM methods
> with arch_ in the name. The arch is expressed through subclasses.

arch is there to explicitly show that returned ID is defined by target and
not generic code. This method then allow to implement cpu_exists() and helps
to init present cpus bitmap in  acpi_piix4.

Would be  cpu_id() acceptable?

> 
> Andreas
>
Andreas Färber - April 15, 2013, 3:42 p.m.
Am 15.04.2013 17:34, schrieb Igor Mammedov:
> On Mon, 15 Apr 2013 17:24:36 +0200
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 11.04.2013 16:51, schrieb Igor Mammedov:
>>> get_arch_id() adds possibility for generic code to get guest visible
>>> CPI id without accessing CPUArchState. If target doesn't override it,
>>> it will return cpu_index.
>>>
>>> Override it on target-i386 to return APIC ID.
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>>  * it will be used later by new cpu_exists() generic function and
>>>    acpi_piix.
>>>  * s/cpu_firmware_id/cpu_arch_id/
>>> ---
>>>  include/qom/cpu.h |  2 ++
>>>  qom/cpu.c         |  6 ++++++
>>>  target-i386/cpu.c | 10 ++++++++++
>>>  3 files changed, 18 insertions(+)
>>
>> I'm still unclear about this ID topic, but please don't add QOM methods
>> with arch_ in the name. The arch is expressed through subclasses.
> 
> arch is there to explicitly show that returned ID is defined by target and
> not generic code. This method then allow to implement cpu_exists() and helps
> to init present cpus bitmap in  acpi_piix4.
> 
> Would be  cpu_id() acceptable?

Certainly.

Andreas
Eduardo Habkost - April 15, 2013, 3:47 p.m.
On Mon, Apr 15, 2013 at 05:24:36PM +0200, Andreas Färber wrote:
> Am 11.04.2013 16:51, schrieb Igor Mammedov:
> > get_arch_id() adds possibility for generic code to get guest visible
> > CPI id without accessing CPUArchState. If target doesn't override it,
> > it will return cpu_index.
> > 
> > Override it on target-i386 to return APIC ID.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  * it will be used later by new cpu_exists() generic function and
> >    acpi_piix.
> >  * s/cpu_firmware_id/cpu_arch_id/
> > ---
> >  include/qom/cpu.h |  2 ++
> >  qom/cpu.c         |  6 ++++++
> >  target-i386/cpu.c | 10 ++++++++++
> >  3 files changed, 18 insertions(+)
> 
> I'm still unclear about this ID topic, but please don't add QOM methods
> with arch_ in the name. The arch is expressed through subclasses.

The naming seemed appropriate to me as it is an abstract method that is
expected to have arch-specific implementations and arch-specific
meaning.

On the other hand, "firmware_id" looked clearer to me (as it was very
specific about the meaning/usefulness of the return value), I don't know
what was the original reason to change from firmware_id to arch_id.

Other than "arch_id" and "firmware_id , I don't know how we could name
that method. It's not "just an ID", but a special kind of ID, so I
believe just "cpu_id" wouldn't work.
Igor Mammedov - April 15, 2013, 8:34 p.m.
On Mon, 15 Apr 2013 12:47:52 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Apr 15, 2013 at 05:24:36PM +0200, Andreas Färber wrote:
> > Am 11.04.2013 16:51, schrieb Igor Mammedov:
> > > get_arch_id() adds possibility for generic code to get guest visible
> > > CPI id without accessing CPUArchState. If target doesn't override it,
> > > it will return cpu_index.
> > > 
> > > Override it on target-i386 to return APIC ID.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  * it will be used later by new cpu_exists() generic function and
> > >    acpi_piix.
> > >  * s/cpu_firmware_id/cpu_arch_id/
> > > ---
> > >  include/qom/cpu.h |  2 ++
> > >  qom/cpu.c         |  6 ++++++
> > >  target-i386/cpu.c | 10 ++++++++++
> > >  3 files changed, 18 insertions(+)
> > 
> > I'm still unclear about this ID topic, but please don't add QOM methods
> > with arch_ in the name. The arch is expressed through subclasses.
> 
> The naming seemed appropriate to me as it is an abstract method that is
> expected to have arch-specific implementations and arch-specific
> meaning.
> 
> On the other hand, "firmware_id" looked clearer to me (as it was very
> specific about the meaning/usefulness of the return value), I don't know
> what was the original reason to change from firmware_id to arch_id.
Objection here
http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg01332.html

In case of x86, it might be firmware defined id, but in general case it might
be not the case, since it could be hardwired into CPU. So get_arch_id()
seems more appropriate.

> 
> Other than "arch_id" and "firmware_id , I don't know how we could name
> that method. It's not "just an ID", but a special kind of ID, so I
> believe just "cpu_id" wouldn't work.
> 
> -- 
> Eduardo
>

Patch

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3664a1b..b376416 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -45,6 +45,7 @@  typedef struct CPUState CPUState;
  * instantiatable CPU type.
  * @reset: Callback to reset the #CPUState to its initial state.
  * @do_interrupt: Callback for interrupt handling.
+ * @get_arch_id: Callback for getting architecture depended CPU ID
  * @vmsd: State description for migration.
  *
  * Represents a CPU family or model.
@@ -58,6 +59,7 @@  typedef struct CPUClass {
 
     void (*reset)(CPUState *cpu);
     void (*do_interrupt)(CPUState *cpu);
+    int64_t (*get_arch_id)(CPUState *cpu);
 
     const struct VMStateDescription *vmsd;
 } CPUClass;
diff --git a/qom/cpu.c b/qom/cpu.c
index fe85960..90cbd77 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -77,6 +77,11 @@  static void cpu_common_realizefn(DeviceState *dev, Error **errp)
     }
 }
 
+static int64_t cpu_common_get_arch_id(CPUState *cpu)
+{
+    return cpu->cpu_index;
+}
+
 static void cpu_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -84,6 +89,7 @@  static void cpu_class_init(ObjectClass *klass, void *data)
 
     k->class_by_name = cpu_common_class_by_name;
     k->reset = cpu_common_reset;
+    k->get_arch_id = cpu_common_get_arch_id;
     dc->realize = cpu_common_realizefn;
     dc->no_user = 1;
 }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c28436c..9cca031 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2272,6 +2272,14 @@  static void x86_cpu_initfn(Object *obj)
     }
 }
 
+static int64_t x86_cpu_get_arch_id(CPUState *cpu)
+{
+    X86CPU *x86cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86cpu->env;
+
+    return env->cpuid_apic_id;
+}
+
 static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 {
     X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -2286,6 +2294,8 @@  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     cc->do_interrupt = x86_cpu_do_interrupt;
     cpu_class_set_vmsd(cc, &vmstate_x86_cpu);
+
+    cc->get_arch_id = x86_cpu_get_arch_id;
 }
 
 static const TypeInfo x86_cpu_type_info = {