From a3c2ca7d29ff39cd5b06e48fa1e42c91d05ebd36 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 15:48:55 -0200 Subject: 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 | 42 ++++++++++++++---------------------------- 1 file changed, 14 insertions(+), 28 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) -{ - const char **p = &featureset[31]; - char *q, *b, bit; - int nc; - - 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; +static void listflags(FILE *f, fprintf_function print, const char **featureset) +{ + int bit; + bool first = true; + + 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"); } } -- cgit v1.2.3 From 08e1a1e5a175ecbfdb761db5a62090498f736969 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 15:57:50 -0200 Subject: 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 */ } } -- cgit v1.2.3 From 8a3f75b39db7d2c891d1e91dde5180d3ff9e10f7 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:41:06 -0200 Subject: 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 --- include/hw/i386/topology.h | 134 +++++++++++++++++++++++++++++++++++++++++++++ target-i386/cpu.c | 2 +- target-i386/topology.h | 134 --------------------------------------------- tests/Makefile | 2 - tests/test-x86-cpuid.c | 2 +- 5 files changed, 136 insertions(+), 138 deletions(-) create mode 100644 include/hw/i386/topology.h delete mode 100644 target-i386/topology.h diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h new file mode 100644 index 0000000000..9c6f3a937a --- /dev/null +++ b/include/hw/i386/topology.h @@ -0,0 +1,134 @@ +/* + * x86 CPU topology data structures and functions + * + * Copyright (c) 2012 Red Hat Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#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: + * Intel® 64 Architecture Processor Topology Enumeration + * http://software.intel.com/en-us/articles/intel-64-architecture-processor-topology-enumeration/ + * + * This code should be compatible with AMD's "Extended Method" described at: + * AMD CPUID Specification (Publication #25481) + * Section 3: Multiple Core Calcuation + * as long as: + * nr_threads is set to 1; + * OFFSET_IDX is assumed to be 0; + * CPUID Fn8000_0008_ECX[ApicIdCoreIdSize[3:0]] is set to apicid_core_width(). + */ + +#include +#include + +#include "qemu/bitops.h" + +/* APIC IDs can be 32-bit, but beware: APIC IDs > 255 require x2APIC support + */ +typedef uint32_t apic_id_t; + +/* Return the bit width needed for 'count' IDs + */ +static unsigned apicid_bitwidth_for_count(unsigned count) +{ + g_assert(count >= 1); + count -= 1; + return count ? 32 - clz32(count) : 0; +} + +/* Bit width of the SMT_ID (thread ID) field on the APIC ID + */ +static inline unsigned apicid_smt_width(unsigned nr_cores, unsigned nr_threads) +{ + return apicid_bitwidth_for_count(nr_threads); +} + +/* Bit width of the Core_ID field + */ +static inline unsigned apicid_core_width(unsigned nr_cores, unsigned nr_threads) +{ + return apicid_bitwidth_for_count(nr_cores); +} + +/* Bit offset of the Core_ID field + */ +static inline unsigned apicid_core_offset(unsigned nr_cores, + unsigned nr_threads) +{ + return apicid_smt_width(nr_cores, nr_threads); +} + +/* Bit offset of the Pkg_ID (socket ID) field + */ +static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads) +{ + return apicid_core_offset(nr_cores, nr_threads) + + apicid_core_width(nr_cores, nr_threads); +} + +/* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID + * + * The caller must make sure core_id < nr_cores and smt_id < nr_threads. + */ +static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores, + unsigned nr_threads, + unsigned pkg_id, + unsigned core_id, + unsigned smt_id) +{ + return (pkg_id << apicid_pkg_offset(nr_cores, nr_threads)) | + (core_id << apicid_core_offset(nr_cores, nr_threads)) | + smt_id; +} + +/* Calculate thread/core/package IDs for a specific topology, + * based on (contiguous) CPU index + */ +static inline void x86_topo_ids_from_idx(unsigned nr_cores, + unsigned nr_threads, + unsigned cpu_index, + unsigned *pkg_id, + unsigned *core_id, + unsigned *smt_id) +{ + unsigned core_index = cpu_index / nr_threads; + *smt_id = cpu_index % nr_threads; + *core_id = core_index % nr_cores; + *pkg_id = core_index / nr_cores; +} + +/* Make APIC ID for the CPU 'cpu_index' + * + * 'cpu_index' is a sequential, contiguous ID for the CPU. + */ +static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores, + unsigned nr_threads, + unsigned cpu_index) +{ + unsigned pkg_id, core_id, smt_id; + x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, + &pkg_id, &core_id, &smt_id); + return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id, smt_id); +} + +#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/target-i386/topology.h b/target-i386/topology.h deleted file mode 100644 index 07a6c5fb55..0000000000 --- a/target-i386/topology.h +++ /dev/null @@ -1,134 +0,0 @@ -/* - * x86 CPU topology data structures and functions - * - * Copyright (c) 2012 Red Hat Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * 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 - -/* This file implements the APIC-ID-based CPU topology enumeration logic, - * documented at the following document: - * Intel® 64 Architecture Processor Topology Enumeration - * http://software.intel.com/en-us/articles/intel-64-architecture-processor-topology-enumeration/ - * - * This code should be compatible with AMD's "Extended Method" described at: - * AMD CPUID Specification (Publication #25481) - * Section 3: Multiple Core Calcuation - * as long as: - * nr_threads is set to 1; - * OFFSET_IDX is assumed to be 0; - * CPUID Fn8000_0008_ECX[ApicIdCoreIdSize[3:0]] is set to apicid_core_width(). - */ - -#include -#include - -#include "qemu/bitops.h" - -/* APIC IDs can be 32-bit, but beware: APIC IDs > 255 require x2APIC support - */ -typedef uint32_t apic_id_t; - -/* Return the bit width needed for 'count' IDs - */ -static unsigned apicid_bitwidth_for_count(unsigned count) -{ - g_assert(count >= 1); - count -= 1; - return count ? 32 - clz32(count) : 0; -} - -/* Bit width of the SMT_ID (thread ID) field on the APIC ID - */ -static inline unsigned apicid_smt_width(unsigned nr_cores, unsigned nr_threads) -{ - return apicid_bitwidth_for_count(nr_threads); -} - -/* Bit width of the Core_ID field - */ -static inline unsigned apicid_core_width(unsigned nr_cores, unsigned nr_threads) -{ - return apicid_bitwidth_for_count(nr_cores); -} - -/* Bit offset of the Core_ID field - */ -static inline unsigned apicid_core_offset(unsigned nr_cores, - unsigned nr_threads) -{ - return apicid_smt_width(nr_cores, nr_threads); -} - -/* Bit offset of the Pkg_ID (socket ID) field - */ -static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads) -{ - return apicid_core_offset(nr_cores, nr_threads) + - apicid_core_width(nr_cores, nr_threads); -} - -/* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID - * - * The caller must make sure core_id < nr_cores and smt_id < nr_threads. - */ -static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores, - unsigned nr_threads, - unsigned pkg_id, - unsigned core_id, - unsigned smt_id) -{ - return (pkg_id << apicid_pkg_offset(nr_cores, nr_threads)) | - (core_id << apicid_core_offset(nr_cores, nr_threads)) | - smt_id; -} - -/* Calculate thread/core/package IDs for a specific topology, - * based on (contiguous) CPU index - */ -static inline void x86_topo_ids_from_idx(unsigned nr_cores, - unsigned nr_threads, - unsigned cpu_index, - unsigned *pkg_id, - unsigned *core_id, - unsigned *smt_id) -{ - unsigned core_index = cpu_index / nr_threads; - *smt_id = cpu_index % nr_threads; - *core_id = core_index % nr_cores; - *pkg_id = core_index / nr_cores; -} - -/* Make APIC ID for the CPU 'cpu_index' - * - * 'cpu_index' is a sequential, contiguous ID for the CPU. - */ -static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores, - unsigned nr_threads, - unsigned cpu_index) -{ - unsigned pkg_id, core_id, smt_id; - x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, - &pkg_id, &core_id, &smt_id); - return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id, smt_id); -} - -#endif /* TARGET_I386_TOPOLOGY_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) { -- cgit v1.2.3 From 644dba250a3ed04079792f0d6cc918fb1483e6a5 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:05:20 -0200 Subject: 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; } -- cgit v1.2.3 From 15258d46baef5f8265ad5f1002905664cf58f051 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:07:01 -0200 Subject: 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 -- cgit v1.2.3 From 18b0e4e77142ace948497a053bd5b56c1b849592 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Fri, 19 Dec 2014 14:51:00 -0200 Subject: 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) -- cgit v1.2.3 From 9e9d3863adcbd1ffeca30f240f49805b00ba0d87 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:20:10 -0200 Subject: 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 -- cgit v1.2.3 From 696da41b1b741f6056e52c572e05abd790637be1 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 16:48:51 -0200 Subject: 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); -- cgit v1.2.3 From 9c235e83f1c3437be6ca45755909efb745c10deb Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:28:45 -0200 Subject: 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; -- cgit v1.2.3 From e1356dd70aef11425883dd4d2885f1d208eb9d57 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:31:11 -0200 Subject: 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); -- cgit v1.2.3 From de13197a38cf45c990802661a057f64a05426cbc Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:43:35 -0200 Subject: 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); -- cgit v1.2.3