From a3c2ca7d29ff39cd5b06e48fa1e42c91d05ebd36 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 15:48:55 -0200 Subject: [PATCH 01/11] target-i386: Simplify listflags() function listflags() had lots of unnecessary complexity. Instead of printing to a buffer that will be immediately printed, simply call the printing function directly. Also, remove the fbits and flags arguments that were always set to the same value. Also, there's no need to list the flags in reverse order. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3a9b32ef7d..39d2fda885 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1911,34 +1911,19 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, } } -/* generate a composite string into buf of all cpuid names in featureset - * selected by fbits. indicate truncation at bufsize in the event of overflow. - * if flags, suppress names undefined in featureset. +/* Print all cpuid feature names in featureset */ -static void listflags(char *buf, int bufsize, uint32_t fbits, - const char **featureset, uint32_t flags) +static void listflags(FILE *f, fprintf_function print, const char **featureset) { - const char **p = &featureset[31]; - char *q, *b, bit; - int nc; + int bit; + bool first = true; - b = 4 <= bufsize ? buf + (bufsize -= 3) - 1 : NULL; - *buf = '\0'; - for (q = buf, bit = 31; fbits && bufsize; --p, fbits &= ~(1 << bit), --bit) - if (fbits & 1 << bit && (*p || !flags)) { - if (*p) - nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p); - else - nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit); - if (bufsize <= nc) { - if (b) { - memcpy(b, "...", sizeof("...")); - } - return; - } - q += nc; - bufsize -= nc; + for (bit = 0; bit < 32; bit++) { + if (featureset[bit]) { + print(f, "%s%s", first ? "" : " ", featureset[bit]); + first = false; } + } } /* generate CPU information. */ @@ -1963,8 +1948,9 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf) for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) { FeatureWordInfo *fw = &feature_word_info[i]; - listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1); - (*cpu_fprintf)(f, " %s\n", buf); + (*cpu_fprintf)(f, " "); + listflags(f, cpu_fprintf, fw->feat_names); + (*cpu_fprintf)(f, "\n"); } } From 08e1a1e5a175ecbfdb761db5a62090498f736969 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 15:57:50 -0200 Subject: [PATCH 02/11] target-i386: Eliminate unnecessary get_cpuid_vendor() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function was used in only two places. In one of them, the function made the code less readable by requiring temporary te[bcd]x variables. In the other one we can simply inline the existing code. Reviewed-by: Andreas Färber Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 39d2fda885..8c44e5b2da 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2214,14 +2214,6 @@ void x86_cpudef_setup(void) } } -static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, - uint32_t *ecx, uint32_t *edx) -{ - *ebx = env->cpuid_vendor1; - *edx = env->cpuid_vendor2; - *ecx = env->cpuid_vendor3; -} - void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) @@ -2255,7 +2247,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, switch(index) { case 0: *eax = env->cpuid_level; - get_cpuid_vendor(env, ebx, ecx, edx); + *ebx = env->cpuid_vendor1; + *edx = env->cpuid_vendor2; + *ecx = env->cpuid_vendor3; break; case 1: *eax = env->cpuid_version; @@ -2448,11 +2442,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, * So dont set it here for Intel to make Linux guests happy. */ if (cs->nr_cores * cs->nr_threads > 1) { - uint32_t tebx, tecx, tedx; - get_cpuid_vendor(env, &tebx, &tecx, &tedx); - if (tebx != CPUID_VENDOR_INTEL_1 || - tedx != CPUID_VENDOR_INTEL_2 || - tecx != CPUID_VENDOR_INTEL_3) { + if (env->cpuid_vendor1 != CPUID_VENDOR_INTEL_1 || + env->cpuid_vendor2 != CPUID_VENDOR_INTEL_2 || + env->cpuid_vendor3 != CPUID_VENDOR_INTEL_3) { *ecx |= 1 << 1; /* CmpLegacy bit */ } } From 8a3f75b39db7d2c891d1e91dde5180d3ff9e10f7 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:41:06 -0200 Subject: [PATCH 03/11] target-i386: Move topology.h to include/hw/i386 This will allow the PC code to use the header, and lets us eliminate the QEMU_INCLUDES hack inside tests/Makefile. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- {target-i386 => include/hw/i386}/topology.h | 6 +++--- target-i386/cpu.c | 2 +- tests/Makefile | 2 -- tests/test-x86-cpuid.c | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) rename {target-i386 => include/hw/i386}/topology.h (97%) diff --git a/target-i386/topology.h b/include/hw/i386/topology.h similarity index 97% rename from target-i386/topology.h rename to include/hw/i386/topology.h index 07a6c5fb55..9c6f3a937a 100644 --- a/target-i386/topology.h +++ b/include/hw/i386/topology.h @@ -21,8 +21,8 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ -#ifndef TARGET_I386_TOPOLOGY_H -#define TARGET_I386_TOPOLOGY_H +#ifndef HW_I386_TOPOLOGY_H +#define HW_I386_TOPOLOGY_H /* This file implements the APIC-ID-based CPU topology enumeration logic, * documented at the following document: @@ -131,4 +131,4 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores, return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id, smt_id); } -#endif /* TARGET_I386_TOPOLOGY_H */ +#endif /* HW_I386_TOPOLOGY_H */ diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8c44e5b2da..356f97ea14 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -25,7 +25,7 @@ #include "sysemu/kvm.h" #include "sysemu/cpus.h" #include "kvm_i386.h" -#include "topology.h" +#include "hw/i386/topology.h" #include "qemu/option.h" #include "qemu/config-file.h" diff --git a/tests/Makefile b/tests/Makefile index 307035c26c..7d4b96d4fd 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -239,8 +239,6 @@ $(test-obj-y): QEMU_INCLUDES += -Itests QEMU_CFLAGS += -I$(SRC_PATH)/tests qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o -tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386 - tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a tests/check-qstring$(EXESUF): tests/check-qstring.o libqemuutil.a tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c index 8d9f96a113..6cd20d4a23 100644 --- a/tests/test-x86-cpuid.c +++ b/tests/test-x86-cpuid.c @@ -24,7 +24,7 @@ #include -#include "topology.h" +#include "hw/i386/topology.h" static void test_topo_bits(void) { From 644dba250a3ed04079792f0d6cc918fb1483e6a5 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:05:20 -0200 Subject: [PATCH 04/11] target-i386: Rename cpu_x86_init() to cpu_x86_init_user() The function is used only for CONFIG_USER, so make its purpose clear. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 2 +- target-i386/cpu.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 356f97ea14..8f18556960 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2135,7 +2135,7 @@ out: return cpu; } -X86CPU *cpu_x86_init(const char *cpu_model) +X86CPU *cpu_x86_init_user(const char *cpu_model) { Error *error = NULL; X86CPU *cpu; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 478450cfb6..41d7f0ac6e 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -982,7 +982,7 @@ typedef struct CPUX86State { #include "cpu-qom.h" -X86CPU *cpu_x86_init(const char *cpu_model); +X86CPU *cpu_x86_init_user(const char *cpu_model); X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge, Error **errp); int cpu_x86_exec(CPUX86State *s); @@ -1173,7 +1173,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_user(cpu_model); if (cpu == NULL) { return NULL; } From 15258d46baef5f8265ad5f1002905664cf58f051 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:07:01 -0200 Subject: [PATCH 05/11] target-i386: Eliminate cpu_init() function Instead of putting extra logic inside cpu.h, just do everything inside cpu_x86_init_user(). Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 6 +++--- target-i386/cpu.h | 12 +++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8f18556960..aee4d3e7ce 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2135,7 +2135,7 @@ out: return cpu; } -X86CPU *cpu_x86_init_user(const char *cpu_model) +CPUX86State *cpu_x86_init_user(const char *cpu_model) { Error *error = NULL; X86CPU *cpu; @@ -2153,10 +2153,10 @@ out: error_free(error); if (cpu != NULL) { object_unref(OBJECT(cpu)); - cpu = NULL; } + return NULL; } - return cpu; + return &cpu->env; } static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 41d7f0ac6e..d5bd74e16b 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -982,7 +982,6 @@ typedef struct CPUX86State { #include "cpu-qom.h" -X86CPU *cpu_x86_init_user(const char *cpu_model); X86CPU *cpu_x86_create(const char *cpu_model, DeviceState *icc_bridge, Error **errp); int cpu_x86_exec(CPUX86State *s); @@ -1171,14 +1170,9 @@ uint64_t cpu_get_tsc(CPUX86State *env); # define PHYS_ADDR_MASK 0xfffffffffLL # endif -static inline CPUX86State *cpu_init(const char *cpu_model) -{ - X86CPU *cpu = cpu_x86_init_user(cpu_model); - if (cpu == NULL) { - return NULL; - } - return &cpu->env; -} +/* CPU creation function for *-user */ +CPUX86State *cpu_x86_init_user(const char *cpu_model); +#define cpu_init cpu_x86_init_user #define cpu_exec cpu_x86_exec #define cpu_gen_code cpu_x86_gen_code From 18b0e4e77142ace948497a053bd5b56c1b849592 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 19 Dec 2014 14:51:00 -0200 Subject: [PATCH 06/11] target-i386: Simplify error handling on cpu_x86_init_user() Isolate error handling path from the "if (error)" checks. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index aee4d3e7ce..f6a7671f0d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2142,21 +2142,23 @@ CPUX86State *cpu_x86_init_user(const char *cpu_model) cpu = cpu_x86_create(cpu_model, NULL, &error); if (error) { - goto out; + goto error; } object_property_set_bool(OBJECT(cpu), true, "realized", &error); - -out: if (error) { - error_report("%s", error_get_pretty(error)); - error_free(error); - if (cpu != NULL) { - object_unref(OBJECT(cpu)); - } - return NULL; + goto error; } + return &cpu->env; + +error: + error_report("%s", error_get_pretty(error)); + error_free(error); + if (cpu != NULL) { + object_unref(OBJECT(cpu)); + } + return NULL; } static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data) From 9e9d3863adcbd1ffeca30f240f49805b00ba0d87 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:20:10 -0200 Subject: [PATCH 07/11] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id The field doesn't need to be inside CPUState, and it is not specific for the CPUID instruction, so move and rename it. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu-qom.h | 1 + target-i386/cpu.c | 17 ++++++++--------- target-i386/cpu.h | 1 - target-i386/kvm.c | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index b557b619cf..4a6f48a8db 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -93,6 +93,7 @@ typedef struct X86CPU { bool expose_kvm; bool migratable; bool host_features; + uint32_t apic_id; /* if true the CPUID code directly forward host cache leaves to the guest */ bool cache_info_passthrough; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index f6a7671f0d..e8cd7b38ba 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1690,7 +1690,7 @@ 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; + int64_t value = cpu->apic_id; visit_type_int(v, &value, name, errp); } @@ -1723,11 +1723,11 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque, return; } - if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) { + if ((value != cpu->apic_id) && cpu_exists(value)) { error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value); return; } - cpu->env.cpuid_apic_id = value; + cpu->apic_id = value; } /* Generic getter for "feature-words" and "filtered-features" properties */ @@ -2255,7 +2255,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 1: *eax = env->cpuid_version; - *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */ + *ebx = (cpu->apic_id << 24) | + 8 << 8; /* CLFLUSH size in quad words, Linux wants it. */ *ecx = env->features[FEAT_1_ECX]; *edx = env->features[FEAT_1_EDX]; if (cs->nr_cores * cs->nr_threads > 1) { @@ -2702,7 +2703,6 @@ static void mce_init(X86CPU *cpu) #ifndef CONFIG_USER_ONLY static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) { - CPUX86State *env = &cpu->env; DeviceState *dev = DEVICE(cpu); APICCommonState *apic; const char *apic_type = "apic"; @@ -2721,7 +2721,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) object_property_add_child(OBJECT(cpu), "apic", OBJECT(cpu->apic_state), NULL); - qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id); + qdev_prop_set_uint8(cpu->apic_state, "id", cpu->apic_id); /* TODO: convert to link<> */ apic = APIC_COMMON(cpu->apic_state); apic->cpu = cpu; @@ -2904,7 +2904,7 @@ static void x86_cpu_initfn(Object *obj) NULL, NULL, (void *)cpu->filtered_features, NULL); cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; - env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index); + cpu->apic_id = x86_cpu_apic_id_from_index(cs->cpu_index); x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort); @@ -2918,9 +2918,8 @@ static void x86_cpu_initfn(Object *obj) static int64_t x86_cpu_get_arch_id(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); - CPUX86State *env = &cpu->env; - return env->cpuid_apic_id; + return cpu->apic_id; } static bool x86_cpu_get_paging_enabled(const CPUState *cs) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index d5bd74e16b..f0e6ca4843 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -944,7 +944,6 @@ typedef struct CPUX86State { uint32_t cpuid_version; FeatureWordArray features; uint32_t cpuid_model[12]; - uint32_t cpuid_apic_id; /* MTRRs */ uint64_t mtrr_fixed[11]; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 40d6a14c85..27fe2be653 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -430,7 +430,7 @@ static void cpu_update_state(void *opaque, int running, RunState state) unsigned long kvm_arch_vcpu_id(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); - return cpu->env.cpuid_apic_id; + return cpu->apic_id; } #ifndef KVM_CPUID_SIGNATURE_NEXT From 696da41b1b741f6056e52c572e05abd790637be1 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 16:48:51 -0200 Subject: [PATCH 08/11] linux-user: Check for cpu_init() errors This was the only caller of cpu_init() that was not checking for NULL yet. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- linux-user/main.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/linux-user/main.c b/linux-user/main.c index d92702a734..111c1ffafc 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3453,10 +3453,17 @@ CPUArchState *cpu_copy(CPUArchState *env) { CPUState *cpu = ENV_GET_CPU(env); CPUArchState *new_env = cpu_init(cpu_model); - CPUState *new_cpu = ENV_GET_CPU(new_env); + CPUState *new_cpu; CPUBreakpoint *bp; CPUWatchpoint *wp; + if (!new_env) { + fprintf(stderr, "cpu_copy: Failed to create new CPU\n"); + exit(1); + } + + new_cpu = ENV_GET_CPU(new_env); + /* Reset non arch specific state */ cpu_reset(new_cpu); From 9c235e83f1c3437be6ca45755909efb745c10deb Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:28:45 -0200 Subject: [PATCH 09/11] target-i386: Set APIC ID using cpu_index on CONFIG_USER The PC CPU initialization code already sets apic-id based on the CPU topology, and CONFIG_USER doesn't need the topology-based APIC ID calculation code. Make CONFIG_USER set apic-id before realizing the CPU (just like PC already does), so we can simplify x86_cpu_initfn later. As there is no CPU topology configuration in CONFIG_USER, just use cpu_index as the APIC ID. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index e8cd7b38ba..3e0c39c8bd 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2145,6 +2145,12 @@ CPUX86State *cpu_x86_init_user(const char *cpu_model) goto error; } + object_property_set_int(OBJECT(cpu), CPU(cpu)->cpu_index, "apic-id", + &error); + if (error) { + goto error; + } + object_property_set_bool(OBJECT(cpu), true, "realized", &error); if (error) { goto error; From e1356dd70aef11425883dd4d2885f1d208eb9d57 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:31:11 -0200 Subject: [PATCH 10/11] target-i386: Require APIC ID to be explicitly set before CPU realize Instead of setting APIC ID automatically when creating a X86CPU, require the property to be set before realizing the object (which all callers of cpu_x86_create() already do). Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu-qom.h | 2 +- target-i386/cpu.c | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 4a6f48a8db..31a0c1e776 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -93,7 +93,7 @@ typedef struct X86CPU { bool expose_kvm; bool migratable; bool host_features; - uint32_t apic_id; + int64_t apic_id; /* if true the CPUID code directly forward host cache leaves to the guest */ bool cache_info_passthrough; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3e0c39c8bd..54422f33ee 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2767,6 +2767,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) Error *local_err = NULL; static bool ht_warned; + if (cpu->apic_id < 0) { + error_setg(errp, "apic-id property was not initialized properly"); + return; + } + if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) { env->cpuid_level = 7; } @@ -2910,7 +2915,7 @@ static void x86_cpu_initfn(Object *obj) NULL, NULL, (void *)cpu->filtered_features, NULL); cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; - cpu->apic_id = x86_cpu_apic_id_from_index(cs->cpu_index); + cpu->apic_id = -1; x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort); From de13197a38cf45c990802661a057f64a05426cbc Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:43:35 -0200 Subject: [PATCH 11/11] target-i386: Move APIC ID compatibility code to pc.c The APIC ID compatibility code is required only for PC, and now that x86_cpu_initfn() doesn't use x86_cpu_apic_id_from_index() anymore, that code can be moved to pc.c. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- hw/i386/pc.c | 35 +++++++++++++++++++++++++++++++++++ target-i386/cpu.c | 34 ---------------------------------- 2 files changed, 35 insertions(+), 34 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index c7af6aae01..2519297890 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -25,6 +25,8 @@ #include "hw/i386/pc.h" #include "hw/char/serial.h" #include "hw/i386/apic.h" +#include "hw/i386/topology.h" +#include "sysemu/cpus.h" #include "hw/block/fdc.h" #include "hw/ide.h" #include "hw/pci/pci.h" @@ -628,6 +630,39 @@ bool e820_get_entry(int idx, uint32_t type, uint64_t *address, uint64_t *length) return false; } +/* Enables contiguous-apic-ID mode, for compatibility */ +static bool compat_apic_id_mode; + +void enable_compat_apic_id_mode(void) +{ + compat_apic_id_mode = true; +} + +/* Calculates initial APIC ID for a specific CPU index + * + * Currently we need to be able to calculate the APIC ID from the CPU index + * alone (without requiring a CPU object), as the QEMU<->Seabios interfaces have + * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC ID of + * all CPUs up to max_cpus. + */ +uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index) +{ + uint32_t correct_id; + static bool warned; + + correct_id = x86_apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index); + if (compat_apic_id_mode) { + if (cpu_index != correct_id && !warned) { + error_report("APIC IDs set in compatibility mode, " + "CPU topology won't match the configuration"); + warned = true; + } + return cpu_index; + } else { + return correct_id; + } +} + /* Calculates the limit to CPU APIC ID values * * This function returns the limit for the APIC ID value, so that all diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 54422f33ee..70fd6db72f 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -25,7 +25,6 @@ #include "sysemu/kvm.h" #include "sysemu/cpus.h" #include "kvm_i386.h" -#include "hw/i386/topology.h" #include "qemu/option.h" #include "qemu/config-file.h" @@ -2836,39 +2835,6 @@ out: } } -/* Enables contiguous-apic-ID mode, for compatibility */ -static bool compat_apic_id_mode; - -void enable_compat_apic_id_mode(void) -{ - compat_apic_id_mode = true; -} - -/* Calculates initial APIC ID for a specific CPU index - * - * Currently we need to be able to calculate the APIC ID from the CPU index - * alone (without requiring a CPU object), as the QEMU<->Seabios interfaces have - * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC ID of - * all CPUs up to max_cpus. - */ -uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index) -{ - uint32_t correct_id; - static bool warned; - - correct_id = x86_apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index); - if (compat_apic_id_mode) { - if (cpu_index != correct_id && !warned) { - error_report("APIC IDs set in compatibility mode, " - "CPU topology won't match the configuration"); - warned = true; - } - return cpu_index; - } else { - return correct_id; - } -} - static void x86_cpu_initfn(Object *obj) { CPUState *cs = CPU(obj);