* [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c
@ 2015-03-04 2:13 Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 1/5] target-i386: Move topology.h to include/hw/i386 Eduardo Habkost
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Andreas Färber, Paolo Bonzini
This series removes the APIC ID initialization code from x86_cpu_initfn()
(getting us one step closer to making object_new() of X86CPU have no dependency
on cpu_exec_init() and other global QEMU state), and moves the APIC ID
compatibility logic from target-i386/cpu.c to hw/i386/pc.c.
Changes v3 -> v4:
* Don't set apic_id = cpu_index on *-user, as it was undoned by cpu_copy()
anyway (but keep requiring apic-id to be explicitly set on softmmu)
Changes v2 -> v3:
* Removed patches:
* [PATCH v2 2/9] target-i386: Rename cpu_x86_init() to cpu_x86_init_user()
* [PATCH v2 3/9] target-i386: Eliminate cpu_init() function
* [PATCH v2 4/9] target-i386: Simplify error handling on cpu_x86_init_user()
* Redone patch "target-i386: Set APIC ID using cpu_index on CONFIG_USER"
using the existing error handling style
Changes v1 -> v2:
* Move topology.h to include/hw/i386 instead of hw/i386
* Instead of adding an apic_id_set field, make apic_id a int64_t field
and set it to -1 by default
* Check for cpu_init() errors on linux-user
Eduardo Habkost (5):
target-i386: Move topology.h to include/hw/i386
target-i386: Remove unused APIC ID default code
target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id
target-i386: Move APIC ID compatibility code to pc.c
target-i386: Require APIC ID to be explicitly set before CPU realize
hw/i386/pc.c | 35 +++++++++++++++++
{target-i386 => include/hw/i386}/topology.h | 6 +--
target-i386/cpu-qom.h | 1 +
target-i386/cpu.c | 60 ++++++++---------------------
target-i386/cpu.h | 1 -
target-i386/kvm.c | 2 +-
tests/Makefile | 2 -
tests/test-x86-cpuid.c | 2 +-
8 files changed, 58 insertions(+), 51 deletions(-)
rename {target-i386 => include/hw/i386}/topology.h (97%)
--
2.1.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v4 1/5] target-i386: Move topology.h to include/hw/i386
2015-03-04 2:13 [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c Eduardo Habkost
@ 2015-03-04 2:13 ` Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code Eduardo Habkost
` (3 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Andreas Färber, Paolo Bonzini
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 <pbonzini@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
{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 07a6c5f..9c6f3a9 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 d543e2b..8fc5727 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 307035c..7d4b96d 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 8d9f96a..6cd20d4 100644
--- a/tests/test-x86-cpuid.c
+++ b/tests/test-x86-cpuid.c
@@ -24,7 +24,7 @@
#include <glib.h>
-#include "topology.h"
+#include "hw/i386/topology.h"
static void test_topo_bits(void)
{
--
2.1.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code
2015-03-04 2:13 [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 1/5] target-i386: Move topology.h to include/hw/i386 Eduardo Habkost
@ 2015-03-04 2:13 ` Eduardo Habkost
2015-03-05 13:43 ` Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id Eduardo Habkost
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Andreas Färber, Paolo Bonzini
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.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
target-i386/cpu.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8fc5727..05cac57 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2923,7 +2923,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);
--
2.1.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id
2015-03-04 2:13 [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 1/5] target-i386: Move topology.h to include/hw/i386 Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code Eduardo Habkost
@ 2015-03-04 2:13 ` Eduardo Habkost
2015-03-04 14:20 ` Andreas Färber
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 5/5] target-i386: Require APIC ID to be explicitly set before CPU realize Eduardo Habkost
4 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Andreas Färber, Paolo Bonzini
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 <pbonzini@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
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 b557b61..4a6f48a 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 05cac57..1fb1df4 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 */
@@ -2272,7 +2272,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) {
@@ -2721,7 +2722,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";
@@ -2740,7 +2740,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;
@@ -2936,9 +2936,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 478450c..38bedc2 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 40d6a14..27fe2be 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
--
2.1.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c
2015-03-04 2:13 [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c Eduardo Habkost
` (2 preceding siblings ...)
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id Eduardo Habkost
@ 2015-03-04 2:13 ` Eduardo Habkost
2015-03-05 0:32 ` Andreas Färber
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 5/5] target-i386: Require APIC ID to be explicitly set before CPU realize Eduardo Habkost
4 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Andreas Färber, Paolo Bonzini
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 <pbonzini@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
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 b229856..8c3c470 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.
+ */
+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 1fb1df4..49a5e58 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"
@@ -2844,39 +2843,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);
--
2.1.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v4 5/5] target-i386: Require APIC ID to be explicitly set before CPU realize
2015-03-04 2:13 [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c Eduardo Habkost
` (3 preceding siblings ...)
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c Eduardo Habkost
@ 2015-03-04 2:13 ` Eduardo Habkost
2015-03-05 0:22 ` Andreas Färber
4 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 2:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Andreas Färber, Paolo Bonzini
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).
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
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 4a6f48a..31a0c1e 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 49a5e58..45700fb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2779,6 +2779,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;
}
@@ -2890,6 +2895,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 */
--
2.1.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id Eduardo Habkost
@ 2015-03-04 14:20 ` Andreas Färber
2015-03-04 15:02 ` Eduardo Habkost
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2015-03-04 14:20 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Paolo Bonzini
Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
> The field doesn't need to be inside CPUState, and it is not specific for
You mean CPUX86State or CPUArchState, I guess. :)
> the CPUID instruction, so move and rename it.
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Could you please use Class::field syntax for consistency when you apply?
Cheers,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id
2015-03-04 14:20 ` Andreas Färber
@ 2015-03-04 15:02 ` Eduardo Habkost
0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-04 15:02 UTC (permalink / raw)
To: Andreas Färber; +Cc: Gu Zheng, Igor Mammedov, qemu-devel, Paolo Bonzini
On Wed, Mar 04, 2015 at 03:20:55PM +0100, Andreas Färber wrote:
> Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
> > The field doesn't need to be inside CPUState, and it is not specific for
>
> You mean CPUX86State or CPUArchState, I guess. :)
>
> > the CPUID instruction, so move and rename it.
> >
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > Reviewed-by: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> Could you please use Class::field syntax for consistency when you apply?
Will fix both issues. Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 5/5] target-i386: Require APIC ID to be explicitly set before CPU realize
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 5/5] target-i386: Require APIC ID to be explicitly set before CPU realize Eduardo Habkost
@ 2015-03-05 0:22 ` Andreas Färber
0 siblings, 0 replies; 18+ messages in thread
From: Andreas Färber @ 2015-03-05 0:22 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Paolo Bonzini
Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
> 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).
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> target-i386/cpu-qom.h | 2 +-
> target-i386/cpu.c | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
Reviewed-by: Andreas Färber <afaerber@suse.de>
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c Eduardo Habkost
@ 2015-03-05 0:32 ` Andreas Färber
2015-03-05 13:37 ` Eduardo Habkost
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2015-03-05 0:32 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel; +Cc: Gu Zheng, Igor Mammedov, Paolo Bonzini
Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
> 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 <pbonzini@redhat.com>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> 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 b229856..8c3c470 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.
> + */
> +uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
Looking a bit closer here, as I am poking around its call site for the
socket modeling, can't this be made static while at it? (If so, don't
forget to drop the prototype.)
Regards,
Andreas
> +{
> + 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
[snip]
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c
2015-03-05 0:32 ` Andreas Färber
@ 2015-03-05 13:37 ` Eduardo Habkost
2015-03-05 15:47 ` Andreas Färber
0 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-05 13:37 UTC (permalink / raw)
To: Andreas Färber; +Cc: Gu Zheng, Igor Mammedov, qemu-devel, Paolo Bonzini
On Thu, Mar 05, 2015 at 01:32:02AM +0100, Andreas Färber wrote:
> Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
> > 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 <pbonzini@redhat.com>
> > Reviewed-by: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > 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 b229856..8c3c470 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.
> > + */
> > +uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
>
> Looking a bit closer here, as I am poking around its call site for the
> socket modeling, can't this be made static while at it? (If so, don't
> forget to drop the prototype.)
Yes, I'll make it static. I assume it's OK to do it before committing
instead of resending the series because of that?
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code Eduardo Habkost
@ 2015-03-05 13:43 ` Eduardo Habkost
2015-03-05 13:46 ` Paolo Bonzini
2015-03-05 18:35 ` Andreas Färber
0 siblings, 2 replies; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-05 13:43 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Riku Voipio, Paolo Bonzini, Gu Zheng,
Igor Mammedov, Andreas Färber
On Tue, Mar 03, 2015 at 11:13:41PM -0300, Eduardo Habkost wrote:
> 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.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
This patch is holding the rest of the series, so a Reviewed-by or
Acked-by would be welcome.
This change removes the 254-CPU limit from {i386,x86_64}-linux-user that
Peter and I discussed previously.
> ---
> target-i386/cpu.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 8fc5727..05cac57 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2923,7 +2923,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);
>
> --
> 2.1.0
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code
2015-03-05 13:43 ` Eduardo Habkost
@ 2015-03-05 13:46 ` Paolo Bonzini
2015-03-05 18:35 ` Andreas Färber
1 sibling, 0 replies; 18+ messages in thread
From: Paolo Bonzini @ 2015-03-05 13:46 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: Gu Zheng, Igor Mammedov, Riku Voipio, Andreas Färber, Peter Maydell
On 05/03/2015 14:43, Eduardo Habkost wrote:
> On Tue, Mar 03, 2015 at 11:13:41PM -0300, Eduardo Habkost wrote:
>> 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.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> This patch is holding the rest of the series, so a Reviewed-by or
> Acked-by would be welcome.
>
> This change removes the 254-CPU limit from {i386,x86_64}-linux-user that
> Peter and I discussed previously.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
>> ---
>> target-i386/cpu.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 8fc5727..05cac57 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -2923,7 +2923,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);
>>
>> --
>> 2.1.0
>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c
2015-03-05 13:37 ` Eduardo Habkost
@ 2015-03-05 15:47 ` Andreas Färber
2015-03-06 15:29 ` Eduardo Habkost
0 siblings, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2015-03-05 15:47 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Gu Zheng, Igor Mammedov, qemu-devel, Paolo Bonzini
Am 05.03.2015 um 14:37 schrieb Eduardo Habkost:
> On Thu, Mar 05, 2015 at 01:32:02AM +0100, Andreas Färber wrote:
>> Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
>>> 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 <pbonzini@redhat.com>
>>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>> ---
>>> 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 b229856..8c3c470 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
[...]
>>> @@ -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.
>>> + */
>>> +uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
>>
>> Looking a bit closer here, as I am poking around its call site for the
>> socket modeling, can't this be made static while at it? (If so, don't
>> forget to drop the prototype.)
>
> Yes, I'll make it static. I assume it's OK to do it before committing
> instead of resending the series because of that?
For me, sure. If you want to go safe or be verbose, you can post the
diff as a reply when you apply.
Cheers,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code
2015-03-05 13:43 ` Eduardo Habkost
2015-03-05 13:46 ` Paolo Bonzini
@ 2015-03-05 18:35 ` Andreas Färber
2015-03-05 18:46 ` Eduardo Habkost
1 sibling, 1 reply; 18+ messages in thread
From: Andreas Färber @ 2015-03-05 18:35 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
Cc: Gu Zheng, Peter Maydell, Igor Mammedov, Riku Voipio, Paolo Bonzini
Am 05.03.2015 um 14:43 schrieb Eduardo Habkost:
> On Tue, Mar 03, 2015 at 11:13:41PM -0300, Eduardo Habkost wrote:
>> 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.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> This patch is holding the rest of the series, so a Reviewed-by or
> Acked-by would be welcome.
>
> This change removes the 254-CPU limit from {i386,x86_64}-linux-user that
> Peter and I discussed previously.
Reviewed-by: Andreas Färber <afaerber@suse.de>
Are you going to send a new pull for the 2 plus these 5 now?
Thanks,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code
2015-03-05 18:35 ` Andreas Färber
@ 2015-03-05 18:46 ` Eduardo Habkost
2015-03-06 20:27 ` Eduardo Habkost
0 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-05 18:46 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, Riku Voipio, qemu-devel, Paolo Bonzini, Gu Zheng,
Igor Mammedov
On Thu, Mar 05, 2015 at 07:35:17PM +0100, Andreas Färber wrote:
> Am 05.03.2015 um 14:43 schrieb Eduardo Habkost:
> > On Tue, Mar 03, 2015 at 11:13:41PM -0300, Eduardo Habkost wrote:
> >> 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.
> >>
> >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >
> > This patch is holding the rest of the series, so a Reviewed-by or
> > Acked-by would be welcome.
> >
> > This change removes the 254-CPU limit from {i386,x86_64}-linux-user that
> > Peter and I discussed previously.
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
> Are you going to send a new pull for the 2 plus these 5 now?
Yes. I plan to send a pull request tomorrow.
(If we get reviews in time, the pull request may include the
instance_init series as well)
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c
2015-03-05 15:47 ` Andreas Färber
@ 2015-03-06 15:29 ` Eduardo Habkost
0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-06 15:29 UTC (permalink / raw)
To: Andreas Färber; +Cc: Gu Zheng, Igor Mammedov, qemu-devel, Paolo Bonzini
On Thu, Mar 05, 2015 at 04:47:53PM +0100, Andreas Färber wrote:
> Am 05.03.2015 um 14:37 schrieb Eduardo Habkost:
> > On Thu, Mar 05, 2015 at 01:32:02AM +0100, Andreas Färber wrote:
> >> Am 04.03.2015 um 03:13 schrieb Eduardo Habkost:
> >>> 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 <pbonzini@redhat.com>
> >>> Reviewed-by: Andreas Färber <afaerber@suse.de>
> >>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> >>> ---
> >>> 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 b229856..8c3c470 100644
> >>> --- a/hw/i386/pc.c
> >>> +++ b/hw/i386/pc.c
> [...]
> >>> @@ -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.
> >>> + */
> >>> +uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
> >>
> >> Looking a bit closer here, as I am poking around its call site for the
> >> socket modeling, can't this be made static while at it? (If so, don't
> >> forget to drop the prototype.)
> >
> > Yes, I'll make it static. I assume it's OK to do it before committing
> > instead of resending the series because of that?
>
> For me, sure. If you want to go safe or be verbose, you can post the
> diff as a reply when you apply.
Applied with fixup:
---
hw/i386/pc.c | 2 +-
target-i386/cpu.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8c3c470..ed54d93 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -646,7 +646,7 @@ void enable_compat_apic_id_mode(void)
* 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)
+static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
{
uint32_t correct_id;
static bool warned;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 38bedc2..0638d24 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
--
2.1.0
--
Eduardo
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code
2015-03-05 18:46 ` Eduardo Habkost
@ 2015-03-06 20:27 ` Eduardo Habkost
0 siblings, 0 replies; 18+ messages in thread
From: Eduardo Habkost @ 2015-03-06 20:27 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, Riku Voipio, qemu-devel, Paolo Bonzini, Gu Zheng,
Igor Mammedov
On Thu, Mar 05, 2015 at 03:46:13PM -0300, Eduardo Habkost wrote:
> On Thu, Mar 05, 2015 at 07:35:17PM +0100, Andreas Färber wrote:
> > Am 05.03.2015 um 14:43 schrieb Eduardo Habkost:
> > > On Tue, Mar 03, 2015 at 11:13:41PM -0300, Eduardo Habkost wrote:
> > >> 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.
> > >>
> > >> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > >
> > > This patch is holding the rest of the series, so a Reviewed-by or
> > > Acked-by would be welcome.
> > >
> > > This change removes the 254-CPU limit from {i386,x86_64}-linux-user that
> > > Peter and I discussed previously.
> >
> > Reviewed-by: Andreas Färber <afaerber@suse.de>
> >
> > Are you going to send a new pull for the 2 plus these 5 now?
>
> Yes. I plan to send a pull request tomorrow.
>
> (If we get reviews in time, the pull request may include the
> instance_init series as well)
I just learned from another thread that Peter will be back to work on
March 10th, so I will submit the pull request on Monday.
--
Eduardo
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-03-06 20:29 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-04 2:13 [Qemu-devel] [PATCH v4 0/5] target-i386: Simplify APIC ID initialization, move compat code to pc.c Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 1/5] target-i386: Move topology.h to include/hw/i386 Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 2/5] target-i386: Remove unused APIC ID default code Eduardo Habkost
2015-03-05 13:43 ` Eduardo Habkost
2015-03-05 13:46 ` Paolo Bonzini
2015-03-05 18:35 ` Andreas Färber
2015-03-05 18:46 ` Eduardo Habkost
2015-03-06 20:27 ` Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 3/5] target-i386: Move CPUX86State.cpuid_apic_id to X86CPU.apic_id Eduardo Habkost
2015-03-04 14:20 ` Andreas Färber
2015-03-04 15:02 ` Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 4/5] target-i386: Move APIC ID compatibility code to pc.c Eduardo Habkost
2015-03-05 0:32 ` Andreas Färber
2015-03-05 13:37 ` Eduardo Habkost
2015-03-05 15:47 ` Andreas Färber
2015-03-06 15:29 ` Eduardo Habkost
2015-03-04 2:13 ` [Qemu-devel] [PATCH v4 5/5] target-i386: Require APIC ID to be explicitly set before CPU realize Eduardo Habkost
2015-03-05 0:22 ` Andreas Färber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.