From 869b7649b534b51dd9e6ddbbdeb7397e083f527f Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:41:06 -0200 Subject: [PATCH 1/7] target-i386: Move topology.h to include/hw/i386 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Andreas Färber 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 d543e2b537..8fc5727168 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 8c3329e50ad74245acbea89bdaa8af12ecf4972c Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 15:48:55 -0200 Subject: [PATCH 2/7] 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 8fc5727168..80e9b9dba8 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 5eb2f7a4df03b53f7eaf56d2dd53d75328909826 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Feb 2015 15:57:50 -0200 Subject: [PATCH 3/7] 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 80e9b9dba8..2f3a4504ea 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2213,14 +2213,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) @@ -2254,7 +2246,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; @@ -2447,11 +2441,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 0f4b210e504d7db42b96882e94481f444e420fe3 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Tue, 3 Mar 2015 22:54:03 -0300 Subject: [PATCH 4/7] target-i386: Remove unused APIC ID default code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing apic_id = cpu_index code has no visible effect: the PC code already initializes the APIC ID according to the topology on pc_new_cpu(), and linux-user memcpy()s the CPU state (including cpuid_apic_id) on cpu_copy(). Remove the dead code and simply let APIC ID to to be 0 by default. This doesn't change behavior of PC because apic-id is already explicitly set, and doesn't affect linux-user because APIC ID was already always 0. Reviewed-by: Paolo Bonzini Reviewed-by: Andreas Färber Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 2f3a4504ea..3e1a0e7822 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2901,7 +2901,6 @@ 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); x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort); From 7e72a45c99fccfe5586d7c4e2f7441f28e24e450 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:20:10 -0200 Subject: [PATCH 5/7] target-i386: Move CPUX86State::cpuid_apic_id to X86CPU::apic_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The field doesn't need to be inside CPUX86State, and it is not specific for the CPUID instruction, so move and rename it. Reviewed-by: Paolo Bonzini Reviewed-by: Andreas Färber Signed-off-by: Eduardo Habkost --- target-i386/cpu-qom.h | 1 + target-i386/cpu.c | 15 +++++++-------- target-i386/cpu.h | 1 - target-i386/kvm.c | 2 +- 4 files changed, 9 insertions(+), 10 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 3e1a0e7822..6dd74f073e 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 */ @@ -2252,7 +2252,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) { @@ -2699,7 +2700,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"; @@ -2718,7 +2718,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; @@ -2914,9 +2914,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 478450cfb6..38bedc2c79 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 54a402930ac6d1a9d6d402229ae8ba8bef7e598e Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:43:35 -0200 Subject: [PATCH 6/7] target-i386: Move APIC ID compatibility code to pc.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Andreas Färber Signed-off-by: Eduardo Habkost --- hw/i386/pc.c | 35 +++++++++++++++++++++++++++++++++++ target-i386/cpu.c | 34 ---------------------------------- target-i386/cpu.h | 1 - 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 79eaad5fec..b5b2aadb52 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" @@ -629,6 +631,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. + */ +static 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 6dd74f073e..110716e84c 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" @@ -2822,39 +2821,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); diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 38bedc2c79..0638d24a88 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1328,7 +1328,6 @@ void x86_cpu_compat_kvm_no_autodisable(FeatureWord w, uint32_t features); /* Return name of 32-bit register, from a R_* constant */ const char *get_register_name_32(unsigned int reg); -uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index); void enable_compat_apic_id_mode(void); #define APIC_DEFAULT_ADDRESS 0xfee00000 From 9886e834f47adabdbfd54ab606788ce7326e6779 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost Date: Thu, 18 Dec 2014 23:31:11 -0200 Subject: [PATCH 7/7] target-i386: Require APIC ID to be explicitly set before CPU realize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On softmuu, instead of setting APIC ID automatically when creating a X86CPU, require the property to be set before realizing the object (which is already done by the CPU creation code on PC). Keep apic_id = 0 by default on *-user so it can simply create a new CPU object and realize it without extra steps (so target-i386 will be able to use cpu_generic_init() eventually). Reviewed-by: Andreas Färber Signed-off-by: Eduardo Habkost --- target-i386/cpu-qom.h | 2 +- target-i386/cpu.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) 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 110716e84c..50907d0bf1 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2757,6 +2757,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; } @@ -2868,6 +2873,11 @@ static void x86_cpu_initfn(Object *obj) cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; +#ifndef CONFIG_USER_ONLY + /* Any code creating new X86CPU objects have to set apic-id explicitly */ + cpu->apic_id = -1; +#endif + x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort); /* init various static tables used in TCG mode */