Patchwork [RFC,11/18] target-i386: cpu_x86_init: allow APIC ID to be set by caller

login
register
mail settings
Submitter Eduardo Habkost
Date Oct. 3, 2012, 1:29 p.m.
Message ID <1349270954-4657-12-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/188787/
State New
Headers show

Comments

Eduardo Habkost - Oct. 3, 2012, 1:29 p.m.
This allows callers of cpu_x86_init() to override the APIC ID, in case
it needs to build a specific cores/threads topology.

Because *-user doesn't have any concept of CPU topology, we do not
require every caller to specify an APIC ID. So a negative value will
indicate that the CPU index can be used as APIC ID, and *-user will use
that mode.

By now, all the callers use the default behavior, that's using the CPU
index as APIC ID, but later the PC code will be changed to calculate the
APIC IDs according to CPU topology.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/pc.c              |  2 +-
 target-i386/cpu.h    |  4 ++--
 target-i386/helper.c | 15 +++++++++++++--
 3 files changed, 16 insertions(+), 5 deletions(-)
Igor Mammedov - Oct. 4, 2012, 12:47 p.m.
On Wed,  3 Oct 2012 10:29:07 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This allows callers of cpu_x86_init() to override the APIC ID, in case
> it needs to build a specific cores/threads topology.
> 
> Because *-user doesn't have any concept of CPU topology, we do not
> require every caller to specify an APIC ID. So a negative value will
> indicate that the CPU index can be used as APIC ID, and *-user will use
> that mode.
> 
> By now, all the callers use the default behavior, that's using the CPU
> index as APIC ID, but later the PC code will be changed to calculate the
> APIC IDs according to CPU topology.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/pc.c              |  2 +-
>  target-i386/cpu.h    |  4 ++--
>  target-i386/helper.c | 15 +++++++++++++--
>  3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 3cf56de..0915a9b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -908,7 +908,7 @@ static X86CPU *pc_new_cpu(PC *pc, const char *cpu_model)
>      X86CPU *cpu;
>      CPUX86State *env;
>  
> -    cpu = cpu_x86_init(cpu_model);
> +    cpu = cpu_x86_init(cpu_model, -1);
>      if (cpu == NULL) {
>          fprintf(stderr, "Unable to find x86 CPU definition\n");
>          exit(1);
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index f37e80b..3b6445c 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -874,7 +874,7 @@ typedef struct CPUX86State {
>  
>  #include "cpu-qom.h"
>  
> -X86CPU *cpu_x86_init(const char *cpu_model);
> +X86CPU *cpu_x86_init(const char *cpu_model, long apic_id);
>  int cpu_x86_exec(CPUX86State *s);
>  void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>  void x86_cpudef_setup(void);
> @@ -1050,7 +1050,7 @@ uint64_t cpu_get_tsc(CPUX86State *env);
>  
>  static inline CPUX86State *cpu_init(const char *cpu_model)
>  {
> -    X86CPU *cpu = cpu_x86_init(cpu_model);
> +    X86CPU *cpu = cpu_x86_init(cpu_model, -1);
>      if (cpu == NULL) {
>          return NULL;
>      }
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 70a9f72..423d8da 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1239,7 +1239,15 @@ int cpu_x86_get_descr_debug(CPUX86State *env,
> unsigned int selector, return 1;
>  }
>  
> -X86CPU *cpu_x86_init(const char *cpu_model)
> +/**
> + * cpu_x86_init:
> + *
> + * Creates and initializes a X86CPU object.
> + *
> + * @apic_id: sets a specific APIC ID for the CPU. If negative, the CPU
> index
> + *           will be used as APIC ID.
> + */
> +X86CPU *cpu_x86_init(const char *cpu_model, long apic_id)
Risking to being slapped again:

We are trying to move from cpu_model and probably cpu_init itself,
once CPU could be created & initialized just as any QOM object. So
maybe cpu_x86_init() could be left alone just as it is now for
using in *-linux-user and softmmu part could done in pc_cpus_init().

That will allow to play with machine specific properties only for softmmu
instead of additional ifdef-enery in cpu_x86_init() since apic is not part
of *-linux-user.
Additionally it would allow to set parent of CPU at board level right
after CPU is created and at realize time set back-link<> from apic to
CPU.

>  {
>      X86CPU *cpu;
>      CPUX86State *env;
> @@ -1249,7 +1257,10 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>      env = &cpu->env;
>      env->cpu_model_str = cpu_model;
>  
> -    if (cpu_x86_register(cpu, cpu_model, env->cpu_index) < 0) {
> +    if (apic_id < 0) {
> +        apic_id = env->cpu_index;
> +    }
> +    if (cpu_x86_register(cpu, cpu_model, apic_id) < 0) {
>          object_delete(OBJECT(cpu));
>          return NULL;
>      }

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 3cf56de..0915a9b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -908,7 +908,7 @@  static X86CPU *pc_new_cpu(PC *pc, const char *cpu_model)
     X86CPU *cpu;
     CPUX86State *env;
 
-    cpu = cpu_x86_init(cpu_model);
+    cpu = cpu_x86_init(cpu_model, -1);
     if (cpu == NULL) {
         fprintf(stderr, "Unable to find x86 CPU definition\n");
         exit(1);
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f37e80b..3b6445c 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -874,7 +874,7 @@  typedef struct CPUX86State {
 
 #include "cpu-qom.h"
 
-X86CPU *cpu_x86_init(const char *cpu_model);
+X86CPU *cpu_x86_init(const char *cpu_model, long apic_id);
 int cpu_x86_exec(CPUX86State *s);
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf);
 void x86_cpudef_setup(void);
@@ -1050,7 +1050,7 @@  uint64_t cpu_get_tsc(CPUX86State *env);
 
 static inline CPUX86State *cpu_init(const char *cpu_model)
 {
-    X86CPU *cpu = cpu_x86_init(cpu_model);
+    X86CPU *cpu = cpu_x86_init(cpu_model, -1);
     if (cpu == NULL) {
         return NULL;
     }
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 70a9f72..423d8da 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1239,7 +1239,15 @@  int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
     return 1;
 }
 
-X86CPU *cpu_x86_init(const char *cpu_model)
+/**
+ * cpu_x86_init:
+ *
+ * Creates and initializes a X86CPU object.
+ *
+ * @apic_id: sets a specific APIC ID for the CPU. If negative, the CPU index
+ *           will be used as APIC ID.
+ */
+X86CPU *cpu_x86_init(const char *cpu_model, long apic_id)
 {
     X86CPU *cpu;
     CPUX86State *env;
@@ -1249,7 +1257,10 @@  X86CPU *cpu_x86_init(const char *cpu_model)
     env = &cpu->env;
     env->cpu_model_str = cpu_model;
 
-    if (cpu_x86_register(cpu, cpu_model, env->cpu_index) < 0) {
+    if (apic_id < 0) {
+        apic_id = env->cpu_index;
+    }
+    if (cpu_x86_register(cpu, cpu_model, apic_id) < 0) {
         object_delete(OBJECT(cpu));
         return NULL;
     }