Skip to contentSkip to conversation

Handle cpu_model in copy_cpu()

PATCH In review Kirill A. Shutemov 2 participants 3 messages Nov 24, 2007 – Nov 25, 2007

lore
Share & tools
lore

Kirill A. Shutemov

author
#0 lore raw

PATCH 1/1 Handle cpu_model in copy_cpu()

copy_cpu() has been broken since cpu_model added to parameters list of cpu_init(). This patch fix copy_cpu() by storing cpu_model string in CPUState structure on cpu_init and use this string in copy_cpu().

---
 cpu-defs.h               |    4 +++-
 exec.c                   |    7 +------
 target-arm/helper.c      |    1 +
 target-i386/helper2.c    |    1 +
 target-m68k/helper.c     |    4 +++-
 target-mips/translate.c  |    1 +
 target-ppc/helper.c      |    1 +
 target-sparc/translate.c |    1 +
 8 files changed, 12 insertions(+), 8 deletions(-)
cpu-defs.h +3 -1
index 139dca2..9c96532 100644
@@ -146,6 +146,8 @@ typedef struct CPUTLBEntry {
     void *next_cpu; /* next CPU sharing TB cache */                     \
     int cpu_index; /* CPU index (informative) */                        \
     /* user data */                                                     \
-    void *opaque;
+    void *opaque;                                                       \
+                                                                        \
+    char cpu_model_str[32];
 
 #endif
exec.c +1 -6
index 046e967..3fe340a 100644
@@ -1317,9 +1317,7 @@ void cpu_abort(CPUState *env, const char *fmt, ...)
 
 CPUState *cpu_copy(CPUState *env)
 {
-#if 0
-    /* XXX: broken, must be handled by each CPU */
-    CPUState *new_env = cpu_init();
+    CPUState *new_env = cpu_init(env->cpu_model_str);
     /* preserve chaining and index */
     CPUState *next_cpu = new_env->next_cpu;
     int cpu_index = new_env->cpu_index;

@@ -1327,9 +1325,6 @@ CPUState *cpu_copy(CPUState *env)
     new_env->next_cpu = next_cpu;
     new_env->cpu_index = cpu_index;
     return new_env;
-#else
-    return NULL;
-#endif
 }
 
 #if !defined(CONFIG_USER_ONLY)
target-arm/helper.c +1 -0
index 038025d..ba0c503 100644
@@ -182,6 +182,7 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    strcpy(env->cpu_model_str, cpu_model);
     env->cp15.c0_cpuid = id;
     cpu_reset(env);
     return env;
target-i386/helper2.c +1 -0
index 67658e2..0d4a046 100644
@@ -99,6 +99,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    strcpy(env->cpu_model_str, cpu_model);
 
     /* init various static tables */
     if (!inited) {
target-m68k/helper.c +3 -1
index f6b0cd6..316dae5 100644
@@ -126,11 +126,13 @@ CPUM68KState *cpu_m68k_init(const char *cpu_model)
         return NULL;
     cpu_exec_init(env);
 
+    strcpy(env->cpu_model_str, cpu_model);
+
     if (cpu_m68k_set_model(env, cpu_model) < 0) {
         cpu_m68k_close(env);
         return NULL;
     }
-        
+
     cpu_reset(env);
     return env;
 }
target-mips/translate.c +1 -0
index 55e6290..d8b70ff 100644
@@ -6754,6 +6754,7 @@ CPUMIPSState *cpu_mips_init (const char *cpu_model)
     env->cpu_model = def;
 
     cpu_exec_init(env);
+    strcpy(env->cpu_model_str, cpu_model);
     cpu_reset(env);
     return env;
 }
target-ppc/helper.c +1 -0
index cb9b778..8bfd97d 100644
@@ -2987,6 +2987,7 @@ CPUPPCState *cpu_ppc_init (const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    strcpy(env->cpu_model_str, cpu_model);
     cpu_ppc_register_internal(env, def);
     cpu_ppc_reset(env);
     return env;
target-sparc/translate.c +1 -0
index 1e373ce..891adf4 100644
@@ -3506,6 +3506,7 @@ CPUSPARCState *cpu_sparc_init(const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    strcpy(env->cpu_model_str, cpu_model);
     env->version = def->iu_version;
     env->fsr = def->fpu_version;
 #if !defined(TARGET_SPARC64)

--
Regards, Kirill A. Shutemov
+ Belarus, Minsk
+ Velesys LLC, http://www.velesys.com/
+ ALT Linux Team, http://www.altlinux.com/

#1 lore raw
Quoted 3 lines · Kirill A. Shutemov

copy_cpu() has been broken since cpu_model added to parameters list of cpu_init(). This patch fix copy_cpu() by storing cpu_model string in CPUState structure on cpu_init and use this string in copy_cpu().

Please excuse my layman's question: why does each cpu need its own copy of the string, and not just point to the same const char *cpu_model?

The reason I'm asking is that

Quoted 1 line

+ char cpu_model_str[32];

might be too short for the feature-reach i386 cpu model string, and that

Quoted 1 line

+ strcpy(env->cpu_model_str, cpu_model);

is not safe when this is mind.

Regards,
Dan.

author
#2 lore raw
Quoted 7 lines · Dan Kenigsberg
Quoted 3 lines · Kirill A. Shutemov

copy_cpu() has been broken since cpu_model added to parameters list of cpu_init(). This patch fix copy_cpu() by storing cpu_model string in CPUState structure on cpu_init and use this string in copy_cpu().

Please excuse my layman's question: why does each cpu need its own copy of the string, and not just point to the same const char *cpu_model?

It's a good idea. Thanks. Updated patch:

cpu-defs.h +3 -1
index 139dca2..5e0f046 100644
@@ -146,6 +146,8 @@ typedef struct CPUTLBEntry {
     void *next_cpu; /* next CPU sharing TB cache */                     \
     int cpu_index; /* CPU index (informative) */                        \
     /* user data */                                                     \
-    void *opaque;
+    void *opaque;                                                       \
+                                                                        \
+    const char *cpu_model_str;
 
 #endif
exec.c +1 -6
index 046e967..3fe340a 100644
@@ -1317,9 +1317,7 @@ void cpu_abort(CPUState *env, const char *fmt, ...)
 
 CPUState *cpu_copy(CPUState *env)
 {
-#if 0
-    /* XXX: broken, must be handled by each CPU */
-    CPUState *new_env = cpu_init();
+    CPUState *new_env = cpu_init(env->cpu_model_str);
     /* preserve chaining and index */
     CPUState *next_cpu = new_env->next_cpu;
     int cpu_index = new_env->cpu_index;

@@ -1327,9 +1325,6 @@ CPUState *cpu_copy(CPUState *env)
     new_env->next_cpu = next_cpu;
     new_env->cpu_index = cpu_index;
     return new_env;
-#else
-    return NULL;
-#endif
 }
 
 #if !defined(CONFIG_USER_ONLY)
target-arm/helper.c +1 -0
index 038025d..a27602d 100644
@@ -182,6 +182,7 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    env->cpu_model_str = cpu_model;
     env->cp15.c0_cpuid = id;
     cpu_reset(env);
     return env;
target-i386/helper2.c +1 -0
index 67658e2..968d9dd 100644
@@ -99,6 +99,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    env->cpu_model_str = cpu_model;
 
     /* init various static tables */
     if (!inited) {
target-m68k/helper.c +3 -1
index f6b0cd6..c639648 100644
@@ -126,11 +126,13 @@ CPUM68KState *cpu_m68k_init(const char *cpu_model)
         return NULL;
     cpu_exec_init(env);
 
+    env->cpu_model_str = cpu_model;
+
     if (cpu_m68k_set_model(env, cpu_model) < 0) {
         cpu_m68k_close(env);
         return NULL;
     }
-        
+
     cpu_reset(env);
     return env;
 }
target-mips/translate.c +1 -0
index 55e6290..374eebd 100644
@@ -6754,6 +6754,7 @@ CPUMIPSState *cpu_mips_init (const char *cpu_model)
     env->cpu_model = def;
 
     cpu_exec_init(env);
+    env->cpu_model_str = cpu_model;
     cpu_reset(env);
     return env;
 }
target-ppc/helper.c +1 -0
index cb9b778..dbbde12 100644
@@ -2987,6 +2987,7 @@ CPUPPCState *cpu_ppc_init (const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    env->cpu_model_str = cpu_model;
     cpu_ppc_register_internal(env, def);
     cpu_ppc_reset(env);
     return env;
target-sparc/translate.c +1 -0
index 1e373ce..a7e46fb 100644
@@ -3506,6 +3506,7 @@ CPUSPARCState *cpu_sparc_init(const char *cpu_model)
     if (!env)
         return NULL;
     cpu_exec_init(env);
+    env->cpu_model_str = cpu_model;
     env->version = def->iu_version;
     env->fsr = def->fpu_version;
 #if !defined(TARGET_SPARC64)
Signature · 4 lines
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + Velesys LLC, http://www.velesys.com/
 + ALT Linux Team, http://www.altlinux.com/