* [Qemu-devel] [RFC V2 01/10] cpu: introduce CpuTopoInfo structure for argument simplification
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 02/10] qom/cpu: move register_vmstate to common CPUClass.realizefn Gu Zheng
` (10 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
target-i386/topology.h | 33 +++++++++++++++++----------------
1 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/target-i386/topology.h b/target-i386/topology.h
index 07a6c5f..e9ff89c 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -47,6 +47,12 @@
*/
typedef uint32_t apic_id_t;
+typedef struct X86CPUTopoInfo {
+ unsigned pkg_id;
+ unsigned core_id;
+ unsigned smt_id;
+} X86CPUTopoInfo;
+
/* Return the bit width needed for 'count' IDs
*/
static unsigned apicid_bitwidth_for_count(unsigned count)
@@ -92,13 +98,11 @@ static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned 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)
+ const X86CPUTopoInfo *topo)
{
- return (pkg_id << apicid_pkg_offset(nr_cores, nr_threads)) |
- (core_id << apicid_core_offset(nr_cores, nr_threads)) |
- smt_id;
+ return (topo->pkg_id << apicid_pkg_offset(nr_cores, nr_threads)) |
+ (topo->core_id << apicid_core_offset(nr_cores, nr_threads)) |
+ topo->smt_id;
}
/* Calculate thread/core/package IDs for a specific topology,
@@ -107,14 +111,12 @@ static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
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)
+ X86CPUTopoInfo *topo)
{
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;
+ topo->smt_id = cpu_index % nr_threads;
+ topo->core_id = core_index % nr_cores;
+ topo->pkg_id = core_index / nr_cores;
}
/* Make APIC ID for the CPU 'cpu_index'
@@ -125,10 +127,9 @@ 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);
+ X86CPUTopoInfo topo;
+ x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, &topo);
+ return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
}
#endif /* TARGET_I386_TOPOLOGY_H */
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [RFC V2 02/10] qom/cpu: move register_vmstate to common CPUClass.realizefn
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 01/10] cpu: introduce CpuTopoInfo structure for argument simplification Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-09-09 12:17 ` Igor Mammedov
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 03/10] cpu: add device_add foo-x86_64-cpu support Gu Zheng
` (9 subsequent siblings)
11 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn,
apic vmstate register into x86_cpu_apic_realize. And use the
cc->get_arch_id as the instance id that suggested by Igor to
fix the migration issue.
Besides, also use cc->get_arch_id as the cpu index of HMP/QMP command output to
fix the index duplicated issue when hotadd a hot-removed cpu.
v2:
-fix the cpu index duplicated issue in the QMP/HMP command output.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
cpus.c | 4 +++-
exec.c | 32 +++++++++++++++++++-------------
hw/intc/apic_common.c | 3 +--
include/hw/i386/apic_internal.h | 2 ++
include/qom/cpu.h | 2 ++
monitor.c | 4 +++-
qom/cpu.c | 2 ++
target-i386/cpu.c | 10 ++++++++--
8 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/cpus.c b/cpus.c
index 2b5c0bd..cce2744 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1394,6 +1394,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
{
CpuInfoList *head = NULL, *cur_item = NULL;
CPUState *cpu;
+ CPUClass *cc;
CPU_FOREACH(cpu) {
CpuInfoList *info;
@@ -1411,11 +1412,12 @@ CpuInfoList *qmp_query_cpus(Error **errp)
CPUMIPSState *env = &mips_cpu->env;
#endif
+ cc = CPU_GET_CLASS(cpu);
cpu_synchronize_state(cpu);
info = g_malloc0(sizeof(*info));
info->value = g_malloc0(sizeof(*info->value));
- info->value->CPU = cpu->cpu_index;
+ info->value->CPU = cc->get_arch_id(cpu);
info->value->current = (cpu == first_cpu);
info->value->halted = cpu->halted;
info->value->thread_id = cpu->thread_id;
diff --git a/exec.c b/exec.c
index 5f9857c..c514492 100644
--- a/exec.c
+++ b/exec.c
@@ -473,10 +473,28 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
}
#endif
+void cpu_vmstate_register(CPUState *cpu)
+{
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ int cpu_index = cc->get_arch_id(cpu);
+
+ if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+ vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
+ }
+#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
+ register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
+ cpu_save, cpu_load, cpu->env_ptr);
+ assert(cc->vmsd == NULL);
+ assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
+#endif
+ if (cc->vmsd != NULL) {
+ vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
+ }
+}
+
void cpu_exec_init(CPUArchState *env)
{
CPUState *cpu = ENV_GET_CPU(env);
- CPUClass *cc = CPU_GET_CLASS(cpu);
CPUState *some_cpu;
int cpu_index;
@@ -499,18 +517,6 @@ void cpu_exec_init(CPUArchState *env)
#if defined(CONFIG_USER_ONLY)
cpu_list_unlock();
#endif
- if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
- vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
- }
-#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
- register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
- cpu_save, cpu_load, env);
- assert(cc->vmsd == NULL);
- assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
-#endif
- if (cc->vmsd != NULL) {
- vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
- }
}
#if defined(TARGET_HAS_ICE)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index ce3d903..029f67d 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -345,7 +345,7 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
return 0;
}
-static const VMStateDescription vmstate_apic_common = {
+const VMStateDescription vmstate_apic_common = {
.name = "apic",
.version_id = 3,
.minimum_version_id = 3,
@@ -391,7 +391,6 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
DeviceClass *dc = DEVICE_CLASS(klass);
- dc->vmsd = &vmstate_apic_common;
dc->reset = apic_reset_common;
dc->props = apic_properties_common;
idc->realize = apic_common_realize;
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 83e2a42..2c91609 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -23,6 +23,7 @@
#include "exec/memory.h"
#include "hw/cpu/icc_bus.h"
#include "qemu/timer.h"
+#include "migration/vmstate.h"
/* APIC Local Vector Table */
#define APIC_LVT_TIMER 0
@@ -136,6 +137,7 @@ typedef struct VAPICState {
} QEMU_PACKED VAPICState;
extern bool apic_report_tpr_access;
+extern const VMStateDescription vmstate_apic_common;
void apic_report_irq_delivered(int delivered);
bool apic_next_timer(APICCommonState *s, int64_t current_time);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 1aafbf5..bc32c9a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -549,6 +549,8 @@ void cpu_interrupt(CPUState *cpu, int mask);
#endif /* USER_ONLY */
+void cpu_vmstate_register(CPUState *cpu);
+
#ifdef CONFIG_SOFTMMU
static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
bool is_write, bool is_exec,
diff --git a/monitor.c b/monitor.c
index 34cee74..55fd06c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1032,7 +1032,9 @@ static CPUArchState *mon_get_cpu(void)
int monitor_get_cpu_index(void)
{
CPUState *cpu = ENV_GET_CPU(mon_get_cpu());
- return cpu->cpu_index;
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+
+ return cc->get_arch_id(cpu);
}
static void do_info_registers(Monitor *mon, const QDict *qdict)
diff --git a/qom/cpu.c b/qom/cpu.c
index b32dd0a..bf16590 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -301,6 +301,8 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cpu = CPU(dev);
+ cpu_vmstate_register(cpu);
+
if (dev->hotplugged) {
cpu_synchronize_post_init(cpu);
notifier_list_notify(&cpu_added_notifiers, dev);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 217500c..2aa2b31 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2662,11 +2662,17 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
{
- if (cpu->apic_state == NULL) {
+ DeviceState *apic_state = cpu->apic_state;
+ CPUClass *cc = CPU_GET_CLASS(CPU(cpu));
+
+ if (apic_state == NULL) {
return;
}
- if (qdev_init(cpu->apic_state)) {
+ vmstate_register(0, cc->get_arch_id(CPU(cpu)),
+ &vmstate_apic_common, apic_state);
+
+ if (qdev_init(apic_state)) {
error_setg(errp, "APIC device '%s' could not be initialized",
object_get_typename(OBJECT(cpu->apic_state)));
return;
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 02/10] qom/cpu: move register_vmstate to common CPUClass.realizefn
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 02/10] qom/cpu: move register_vmstate to common CPUClass.realizefn Gu Zheng
@ 2014-09-09 12:17 ` Igor Mammedov
2014-09-10 2:38 ` Gu Zheng
0 siblings, 1 reply; 52+ messages in thread
From: Igor Mammedov @ 2014-09-09 12:17 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:34 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn,
> apic vmstate register into x86_cpu_apic_realize. And use the
> cc->get_arch_id as the instance id that suggested by Igor to
> fix the migration issue.
> Besides, also use cc->get_arch_id as the cpu index of HMP/QMP command output to
> fix the index duplicated issue when hotadd a hot-removed cpu.
>
> v2:
> -fix the cpu index duplicated issue in the QMP/HMP command output.
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Please split CPU vmstate/APIC vmstate/QMP-monitor changes into separate patches.
> ---
> cpus.c | 4 +++-
> exec.c | 32 +++++++++++++++++++-------------
> hw/intc/apic_common.c | 3 +--
> include/hw/i386/apic_internal.h | 2 ++
> include/qom/cpu.h | 2 ++
> monitor.c | 4 +++-
> qom/cpu.c | 2 ++
> target-i386/cpu.c | 10 ++++++++--
> 8 files changed, 40 insertions(+), 19 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 2b5c0bd..cce2744 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1394,6 +1394,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
> {
> CpuInfoList *head = NULL, *cur_item = NULL;
> CPUState *cpu;
> + CPUClass *cc;
>
> CPU_FOREACH(cpu) {
> CpuInfoList *info;
> @@ -1411,11 +1412,12 @@ CpuInfoList *qmp_query_cpus(Error **errp)
> CPUMIPSState *env = &mips_cpu->env;
> #endif
>
> + cc = CPU_GET_CLASS(cpu);
> cpu_synchronize_state(cpu);
>
> info = g_malloc0(sizeof(*info));
> info->value = g_malloc0(sizeof(*info->value));
> - info->value->CPU = cpu->cpu_index;
> + info->value->CPU = cc->get_arch_id(cpu);
> info->value->current = (cpu == first_cpu);
> info->value->halted = cpu->halted;
> info->value->thread_id = cpu->thread_id;
> diff --git a/exec.c b/exec.c
> index 5f9857c..c514492 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -473,10 +473,28 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
> }
> #endif
>
> +void cpu_vmstate_register(CPUState *cpu)
> +{
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> + int cpu_index = cc->get_arch_id(cpu);
> +
> + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> + vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
> + }
> +#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
> + register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
> + cpu_save, cpu_load, cpu->env_ptr);
> + assert(cc->vmsd == NULL);
> + assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
> +#endif
> + if (cc->vmsd != NULL) {
> + vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
> + }
> +}
> +
> void cpu_exec_init(CPUArchState *env)
> {
> CPUState *cpu = ENV_GET_CPU(env);
> - CPUClass *cc = CPU_GET_CLASS(cpu);
> CPUState *some_cpu;
> int cpu_index;
>
> @@ -499,18 +517,6 @@ void cpu_exec_init(CPUArchState *env)
> #if defined(CONFIG_USER_ONLY)
> cpu_list_unlock();
> #endif
> - if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> - vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
> - }
> -#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
> - register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
> - cpu_save, cpu_load, env);
> - assert(cc->vmsd == NULL);
> - assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
> -#endif
> - if (cc->vmsd != NULL) {
> - vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
> - }
> }
>
> #if defined(TARGET_HAS_ICE)
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index ce3d903..029f67d 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -345,7 +345,7 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
> return 0;
> }
>
> -static const VMStateDescription vmstate_apic_common = {
> +const VMStateDescription vmstate_apic_common = {
> .name = "apic",
> .version_id = 3,
> .minimum_version_id = 3,
> @@ -391,7 +391,6 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
> ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->vmsd = &vmstate_apic_common;
> dc->reset = apic_reset_common;
> dc->props = apic_properties_common;
> idc->realize = apic_common_realize;
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 83e2a42..2c91609 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -23,6 +23,7 @@
> #include "exec/memory.h"
> #include "hw/cpu/icc_bus.h"
> #include "qemu/timer.h"
> +#include "migration/vmstate.h"
>
> /* APIC Local Vector Table */
> #define APIC_LVT_TIMER 0
> @@ -136,6 +137,7 @@ typedef struct VAPICState {
> } QEMU_PACKED VAPICState;
>
> extern bool apic_report_tpr_access;
> +extern const VMStateDescription vmstate_apic_common;
>
> void apic_report_irq_delivered(int delivered);
> bool apic_next_timer(APICCommonState *s, int64_t current_time);
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 1aafbf5..bc32c9a 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -549,6 +549,8 @@ void cpu_interrupt(CPUState *cpu, int mask);
>
> #endif /* USER_ONLY */
>
> +void cpu_vmstate_register(CPUState *cpu);
> +
> #ifdef CONFIG_SOFTMMU
> static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
> bool is_write, bool is_exec,
> diff --git a/monitor.c b/monitor.c
> index 34cee74..55fd06c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1032,7 +1032,9 @@ static CPUArchState *mon_get_cpu(void)
> int monitor_get_cpu_index(void)
> {
> CPUState *cpu = ENV_GET_CPU(mon_get_cpu());
> - return cpu->cpu_index;
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> + return cc->get_arch_id(cpu);
> }
>
> static void do_info_registers(Monitor *mon, const QDict *qdict)
> diff --git a/qom/cpu.c b/qom/cpu.c
> index b32dd0a..bf16590 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -301,6 +301,8 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
> {
> CPUState *cpu = CPU(dev);
>
> + cpu_vmstate_register(cpu);
> +
> if (dev->hotplugged) {
> cpu_synchronize_post_init(cpu);
> notifier_list_notify(&cpu_added_notifiers, dev);
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 217500c..2aa2b31 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2662,11 +2662,17 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>
> static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> {
> - if (cpu->apic_state == NULL) {
> + DeviceState *apic_state = cpu->apic_state;
> + CPUClass *cc = CPU_GET_CLASS(CPU(cpu));
> +
> + if (apic_state == NULL) {
> return;
> }
>
> - if (qdev_init(cpu->apic_state)) {
> + vmstate_register(0, cc->get_arch_id(CPU(cpu)),
> + &vmstate_apic_common, apic_state);
Why have you dropped generic dc->vmsd = &vmstate_apic_common from above
and opencoded it here?
> +
> + if (qdev_init(apic_state)) {
> error_setg(errp, "APIC device '%s' could not be initialized",
> object_get_typename(OBJECT(cpu->apic_state)));
> return;
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 02/10] qom/cpu: move register_vmstate to common CPUClass.realizefn
2014-09-09 12:17 ` Igor Mammedov
@ 2014-09-10 2:38 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-10 2:38 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
Hi Igor,
On 09/09/2014 08:17 PM, Igor Mammedov wrote:
> On Thu, 28 Aug 2014 11:36:34 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>
>> Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn,
>> apic vmstate register into x86_cpu_apic_realize. And use the
>> cc->get_arch_id as the instance id that suggested by Igor to
>> fix the migration issue.
>> Besides, also use cc->get_arch_id as the cpu index of HMP/QMP command output to
>> fix the index duplicated issue when hotadd a hot-removed cpu.
>>
>> v2:
>> -fix the cpu index duplicated issue in the QMP/HMP command output.
>>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Please split CPU vmstate/APIC vmstate/QMP-monitor changes into separate patches.
Got it, it does need to be restructured.
>
>> ---
>> cpus.c | 4 +++-
>> exec.c | 32 +++++++++++++++++++-------------
>> hw/intc/apic_common.c | 3 +--
>> include/hw/i386/apic_internal.h | 2 ++
>> include/qom/cpu.h | 2 ++
>> monitor.c | 4 +++-
>> qom/cpu.c | 2 ++
>> target-i386/cpu.c | 10 ++++++++--
>> 8 files changed, 40 insertions(+), 19 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 2b5c0bd..cce2744 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -1394,6 +1394,7 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>> {
>> CpuInfoList *head = NULL, *cur_item = NULL;
>> CPUState *cpu;
>> + CPUClass *cc;
>>
>> CPU_FOREACH(cpu) {
>> CpuInfoList *info;
>> @@ -1411,11 +1412,12 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>> CPUMIPSState *env = &mips_cpu->env;
>> #endif
>>
>> + cc = CPU_GET_CLASS(cpu);
>> cpu_synchronize_state(cpu);
>>
>> info = g_malloc0(sizeof(*info));
>> info->value = g_malloc0(sizeof(*info->value));
>> - info->value->CPU = cpu->cpu_index;
>> + info->value->CPU = cc->get_arch_id(cpu);
>> info->value->current = (cpu == first_cpu);
>> info->value->halted = cpu->halted;
>> info->value->thread_id = cpu->thread_id;
>> diff --git a/exec.c b/exec.c
>> index 5f9857c..c514492 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -473,10 +473,28 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
>> }
>> #endif
>>
>> +void cpu_vmstate_register(CPUState *cpu)
>> +{
>> + CPUClass *cc = CPU_GET_CLASS(cpu);
>> + int cpu_index = cc->get_arch_id(cpu);
>> +
>> + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
>> + vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
>> + }
>> +#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
>> + register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
>> + cpu_save, cpu_load, cpu->env_ptr);
>> + assert(cc->vmsd == NULL);
>> + assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
>> +#endif
>> + if (cc->vmsd != NULL) {
>> + vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
>> + }
>> +}
>> +
>> void cpu_exec_init(CPUArchState *env)
>> {
>> CPUState *cpu = ENV_GET_CPU(env);
>> - CPUClass *cc = CPU_GET_CLASS(cpu);
>> CPUState *some_cpu;
>> int cpu_index;
>>
>> @@ -499,18 +517,6 @@ void cpu_exec_init(CPUArchState *env)
>> #if defined(CONFIG_USER_ONLY)
>> cpu_list_unlock();
>> #endif
>> - if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
>> - vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu);
>> - }
>> -#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
>> - register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
>> - cpu_save, cpu_load, env);
>> - assert(cc->vmsd == NULL);
>> - assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
>> -#endif
>> - if (cc->vmsd != NULL) {
>> - vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
>> - }
>> }
>>
>> #if defined(TARGET_HAS_ICE)
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index ce3d903..029f67d 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -345,7 +345,7 @@ static int apic_dispatch_post_load(void *opaque, int version_id)
>> return 0;
>> }
>>
>> -static const VMStateDescription vmstate_apic_common = {
>> +const VMStateDescription vmstate_apic_common = {
>> .name = "apic",
>> .version_id = 3,
>> .minimum_version_id = 3,
>> @@ -391,7 +391,6 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
>> ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass);
>> DeviceClass *dc = DEVICE_CLASS(klass);
>>
>> - dc->vmsd = &vmstate_apic_common;
>> dc->reset = apic_reset_common;
>> dc->props = apic_properties_common;
>> idc->realize = apic_common_realize;
>> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
>> index 83e2a42..2c91609 100644
>> --- a/include/hw/i386/apic_internal.h
>> +++ b/include/hw/i386/apic_internal.h
>> @@ -23,6 +23,7 @@
>> #include "exec/memory.h"
>> #include "hw/cpu/icc_bus.h"
>> #include "qemu/timer.h"
>> +#include "migration/vmstate.h"
>>
>> /* APIC Local Vector Table */
>> #define APIC_LVT_TIMER 0
>> @@ -136,6 +137,7 @@ typedef struct VAPICState {
>> } QEMU_PACKED VAPICState;
>>
>> extern bool apic_report_tpr_access;
>> +extern const VMStateDescription vmstate_apic_common;
>>
>> void apic_report_irq_delivered(int delivered);
>> bool apic_next_timer(APICCommonState *s, int64_t current_time);
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 1aafbf5..bc32c9a 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -549,6 +549,8 @@ void cpu_interrupt(CPUState *cpu, int mask);
>>
>> #endif /* USER_ONLY */
>>
>> +void cpu_vmstate_register(CPUState *cpu);
>> +
>> #ifdef CONFIG_SOFTMMU
>> static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
>> bool is_write, bool is_exec,
>> diff --git a/monitor.c b/monitor.c
>> index 34cee74..55fd06c 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1032,7 +1032,9 @@ static CPUArchState *mon_get_cpu(void)
>> int monitor_get_cpu_index(void)
>> {
>> CPUState *cpu = ENV_GET_CPU(mon_get_cpu());
>> - return cpu->cpu_index;
>> + CPUClass *cc = CPU_GET_CLASS(cpu);
>> +
>> + return cc->get_arch_id(cpu);
>> }
>>
>> static void do_info_registers(Monitor *mon, const QDict *qdict)
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index b32dd0a..bf16590 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -301,6 +301,8 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>> {
>> CPUState *cpu = CPU(dev);
>>
>> + cpu_vmstate_register(cpu);
>> +
>> if (dev->hotplugged) {
>> cpu_synchronize_post_init(cpu);
>> notifier_list_notify(&cpu_added_notifiers, dev);
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 217500c..2aa2b31 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -2662,11 +2662,17 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>>
>> static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>> {
>> - if (cpu->apic_state == NULL) {
>> + DeviceState *apic_state = cpu->apic_state;
>> + CPUClass *cc = CPU_GET_CLASS(CPU(cpu));
>> +
>> + if (apic_state == NULL) {
>> return;
>> }
>>
>> - if (qdev_init(cpu->apic_state)) {
>> + vmstate_register(0, cc->get_arch_id(CPU(cpu)),
>> + &vmstate_apic_common, apic_state);
> Why have you dropped generic dc->vmsd = &vmstate_apic_common from above
> and opencoded it here?
Because if dc->vmsd was set, it will be registered with a sequence instance
id in device_set_realized, which will break the instance rule (cc->get_arch_id),
so we hacked it here.
Any better solution is welcome.
Thanks,
Gu
>
>
>> +
>> + if (qdev_init(apic_state)) {
>> error_setg(errp, "APIC device '%s' could not be initialized",
>> object_get_typename(OBJECT(cpu->apic_state)));
>> return;
>
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [RFC V2 03/10] cpu: add device_add foo-x86_64-cpu support
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 01/10] cpu: introduce CpuTopoInfo structure for argument simplification Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 02/10] qom/cpu: move register_vmstate to common CPUClass.realizefn Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-09-09 12:44 ` Igor Mammedov
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 04/10] x86: add x86_cpu_unrealizefn() for cpu apic remove Gu Zheng
` (8 subsequent siblings)
11 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Add support to device_add foo-x86_64-cpu, and additional checks of
apic id are added into x86_cpuid_set_apic_id() and x86_cpu_apic_create()
for duplicate. Besides, in order to support "device/device_add foo-x86_64-cpu"
which without specified apic id, we add a new function get_free_apic_id() to
provide the first free apid id each time to avoid apic id duplicate.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
include/qom/cpu.h | 1 +
qdev-monitor.c | 1 +
target-i386/cpu.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++-
target-i386/topology.h | 18 ++++++++++++++
4 files changed, 80 insertions(+), 1 deletions(-)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index bc32c9a..2fc00ef 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -292,6 +292,7 @@ struct CPUState {
QTAILQ_HEAD(CPUTailQ, CPUState);
extern struct CPUTailQ cpus;
#define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node)
+#define CPU_REMOVE(cpu) QTAILQ_REMOVE(&cpus, cpu, node)
#define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node)
#define CPU_FOREACH_SAFE(cpu, next_cpu) \
QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index fb9ee24..1aa446d 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -24,6 +24,7 @@
#include "qmp-commands.h"
#include "sysemu/arch_init.h"
#include "qemu/config-file.h"
+#include "qom/object_interfaces.h"
/*
* Aliases were a bad idea from the start. Let's keep them
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2aa2b31..5255ddb 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -49,6 +49,7 @@
#include "hw/i386/apic_internal.h"
#endif
+#include "qom/object_interfaces.h"
/* Cache topology CPUID constants: */
@@ -1634,6 +1635,7 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
const int64_t max = UINT32_MAX;
Error *error = NULL;
int64_t value;
+ X86CPUTopoInfo topo;
if (dev->realized) {
error_setg(errp, "Attempt to set property '%s' on '%s' after "
@@ -1653,6 +1655,19 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
return;
}
+ if (value > x86_cpu_apic_id_from_index(max_cpus - 1)) {
+ error_setg(errp, "CPU with APIC ID %" PRIi64
+ " is more than MAX APIC ID limits", value);
+ return;
+ }
+
+ x86_topo_ids_from_apic_id(smp_cores, smp_threads, value, &topo);
+ if (topo.smt_id >= smp_threads || topo.core_id >= smp_cores) {
+ error_setg(errp, "CPU with APIC ID %" PRIi64 " does not match "
+ "topology configuration.", value);
+ return;
+ }
+
if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) {
error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value);
return;
@@ -2096,12 +2111,21 @@ out:
return cpu;
}
+static void x86_cpu_cpudef_instance_init(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+
+ dev->hotplugged = true;
+}
+
static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
{
X86CPUDefinition *cpudef = data;
X86CPUClass *xcc = X86_CPU_CLASS(oc);
+ DeviceClass *dc = DEVICE_CLASS(oc);
xcc->cpu_def = cpudef;
+ dc->cannot_instantiate_with_device_add_yet = false;
}
static void x86_register_cpudef_type(X86CPUDefinition *def)
@@ -2110,6 +2134,8 @@ static void x86_register_cpudef_type(X86CPUDefinition *def)
TypeInfo ti = {
.name = typename,
.parent = TYPE_X86_CPU,
+ .instance_size = sizeof(X86CPU),
+ .instance_init = x86_cpu_cpudef_instance_init,
.class_init = x86_cpu_cpudef_class_init,
.class_data = def,
};
@@ -2652,6 +2678,14 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
return;
}
+ if (env->cpuid_apic_id > x86_cpu_apic_id_from_index(max_cpus - 1)) {
+ error_setg(errp, "CPU with APIC ID %" PRIi32
+ " is more than MAX APIC ID:%" PRIi32,
+ env->cpuid_apic_id,
+ x86_cpu_apic_id_from_index(max_cpus - 1));
+ return;
+ }
+
object_property_add_child(OBJECT(cpu), "apic",
OBJECT(cpu->apic_state), NULL);
qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id);
@@ -2777,6 +2811,21 @@ uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
}
}
+static uint32_t get_free_apic_id(void)
+{
+ int i;
+
+ for (i = 0; i < max_cpus; i++) {
+ uint32_t id = x86_cpu_apic_id_from_index(i);
+
+ if (!cpu_exists(id)) {
+ return id;
+ }
+ }
+
+ return x86_cpu_apic_id_from_index(max_cpus);
+}
+
static void x86_cpu_initfn(Object *obj)
{
CPUState *cs = CPU(obj);
@@ -2784,7 +2833,9 @@ static void x86_cpu_initfn(Object *obj)
X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
CPUX86State *env = &cpu->env;
static int inited;
+ uint32_t value;
+ value = get_free_apic_id();
cs->env_ptr = env;
cpu_exec_init(env);
@@ -2823,7 +2874,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);
+ env->cpuid_apic_id = value;
x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
@@ -2837,6 +2888,13 @@ static void x86_cpu_initfn(Object *obj)
}
}
+static void x86_cpu_finalizefn(Object *obj)
+{
+ CPUState *cs = CPU(obj);
+
+ CPU_REMOVE(cs);
+}
+
static int64_t x86_cpu_get_arch_id(CPUState *cs)
{
X86CPU *cpu = X86_CPU(cs);
@@ -2937,6 +2995,7 @@ static const TypeInfo x86_cpu_type_info = {
.parent = TYPE_CPU,
.instance_size = sizeof(X86CPU),
.instance_init = x86_cpu_initfn,
+ .instance_finalize = x86_cpu_finalizefn,
.abstract = true,
.class_size = sizeof(X86CPUClass),
.class_init = x86_cpu_common_class_init,
diff --git a/target-i386/topology.h b/target-i386/topology.h
index e9ff89c..dcb4988 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -132,4 +132,22 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
}
+/* Calculate CPU topology based on CPU APIC ID.
+ */
+static inline void x86_topo_ids_from_apic_id(unsigned nr_cores,
+ unsigned nr_threads,
+ apic_id_t apic_id,
+ X86CPUTopoInfo *topo)
+{
+ unsigned offset_mask;
+ topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads);
+
+ offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1;
+ topo->core_id = (apic_id & offset_mask)
+ >> apicid_core_offset(nr_cores, nr_threads);
+
+ offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1;
+ topo->smt_id = apic_id & offset_mask;
+}
+
#endif /* TARGET_I386_TOPOLOGY_H */
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 03/10] cpu: add device_add foo-x86_64-cpu support
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 03/10] cpu: add device_add foo-x86_64-cpu support Gu Zheng
@ 2014-09-09 12:44 ` Igor Mammedov
2014-09-10 3:37 ` Gu Zheng
0 siblings, 1 reply; 52+ messages in thread
From: Igor Mammedov @ 2014-09-09 12:44 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:35 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>
> Add support to device_add foo-x86_64-cpu, and additional checks of
> apic id are added into x86_cpuid_set_apic_id() and x86_cpu_apic_create()
> for duplicate. Besides, in order to support "device/device_add foo-x86_64-cpu"
> which without specified apic id, we add a new function get_free_apic_id() to
> provide the first free apid id each time to avoid apic id duplicate.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> include/qom/cpu.h | 1 +
> qdev-monitor.c | 1 +
> target-i386/cpu.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++-
> target-i386/topology.h | 18 ++++++++++++++
> 4 files changed, 80 insertions(+), 1 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index bc32c9a..2fc00ef 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -292,6 +292,7 @@ struct CPUState {
> QTAILQ_HEAD(CPUTailQ, CPUState);
> extern struct CPUTailQ cpus;
> #define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node)
> +#define CPU_REMOVE(cpu) QTAILQ_REMOVE(&cpus, cpu, node)
> #define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node)
> #define CPU_FOREACH_SAFE(cpu, next_cpu) \
> QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu)
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index fb9ee24..1aa446d 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -24,6 +24,7 @@
> #include "qmp-commands.h"
> #include "sysemu/arch_init.h"
> #include "qemu/config-file.h"
> +#include "qom/object_interfaces.h"
>
> /*
> * Aliases were a bad idea from the start. Let's keep them
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 2aa2b31..5255ddb 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -49,6 +49,7 @@
> #include "hw/i386/apic_internal.h"
> #endif
>
> +#include "qom/object_interfaces.h"
This probably belongs to another patch.
>
> /* Cache topology CPUID constants: */
>
> @@ -1634,6 +1635,7 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
> const int64_t max = UINT32_MAX;
> Error *error = NULL;
> int64_t value;
> + X86CPUTopoInfo topo;
>
> if (dev->realized) {
> error_setg(errp, "Attempt to set property '%s' on '%s' after "
> @@ -1653,6 +1655,19 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
> return;
> }
>
> + if (value > x86_cpu_apic_id_from_index(max_cpus - 1)) {
> + error_setg(errp, "CPU with APIC ID %" PRIi64
> + " is more than MAX APIC ID limits", value);
> + return;
> + }
> +
> + x86_topo_ids_from_apic_id(smp_cores, smp_threads, value, &topo);
> + if (topo.smt_id >= smp_threads || topo.core_id >= smp_cores) {
> + error_setg(errp, "CPU with APIC ID %" PRIi64 " does not match "
> + "topology configuration.", value);
> + return;
> + }
> +
> if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) {
> error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value);
> return;
> @@ -2096,12 +2111,21 @@ out:
> return cpu;
> }
>
> +static void x86_cpu_cpudef_instance_init(Object *obj)
> +{
> + DeviceState *dev = DEVICE(obj);
> +
> + dev->hotplugged = true;
> +}
looks unnecessary, see device_initfn() which already does above.
> +
> static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
> {
> X86CPUDefinition *cpudef = data;
> X86CPUClass *xcc = X86_CPU_CLASS(oc);
> + DeviceClass *dc = DEVICE_CLASS(oc);
>
> xcc->cpu_def = cpudef;
> + dc->cannot_instantiate_with_device_add_yet = false;
> }
>
> static void x86_register_cpudef_type(X86CPUDefinition *def)
> @@ -2110,6 +2134,8 @@ static void x86_register_cpudef_type(X86CPUDefinition *def)
> TypeInfo ti = {
> .name = typename,
> .parent = TYPE_X86_CPU,
> + .instance_size = sizeof(X86CPU),
> + .instance_init = x86_cpu_cpudef_instance_init,
this hunk is not needed, providing x86_cpu_cpudef_instance_init() is not necessary.
> .class_init = x86_cpu_cpudef_class_init,
> .class_data = def,
> };
> @@ -2652,6 +2678,14 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
> return;
> }
>
> + if (env->cpuid_apic_id > x86_cpu_apic_id_from_index(max_cpus - 1)) {
> + error_setg(errp, "CPU with APIC ID %" PRIi32
> + " is more than MAX APIC ID:%" PRIi32,
> + env->cpuid_apic_id,
> + x86_cpu_apic_id_from_index(max_cpus - 1));
> + return;
> + }
use property apic-id here that has checks instead of duplicating them.
> +
> object_property_add_child(OBJECT(cpu), "apic",
> OBJECT(cpu->apic_state), NULL);
> qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id);
> @@ -2777,6 +2811,21 @@ uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
> }
> }
>
> +static uint32_t get_free_apic_id(void)
> +{
> + int i;
> +
> + for (i = 0; i < max_cpus; i++) {
> + uint32_t id = x86_cpu_apic_id_from_index(i);
> +
> + if (!cpu_exists(id)) {
> + return id;
> + }
> + }
> +
> + return x86_cpu_apic_id_from_index(max_cpus);
> +}
> +
> static void x86_cpu_initfn(Object *obj)
> {
> CPUState *cs = CPU(obj);
> @@ -2784,7 +2833,9 @@ static void x86_cpu_initfn(Object *obj)
> X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
> CPUX86State *env = &cpu->env;
> static int inited;
> + uint32_t value;
s/value/apic_id/ ???
>
> + value = get_free_apic_id();
What if there isn't any free APIC ID it the time it's called?
Could you just assign default broadcast value here 0xFFFFFFFF
and do actual APIC ID allocation at realize time if it still has
default value.
> cs->env_ptr = env;
> cpu_exec_init(env);
>
> @@ -2823,7 +2874,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);
> + env->cpuid_apic_id = value;
>
> x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
>
> @@ -2837,6 +2888,13 @@ static void x86_cpu_initfn(Object *obj)
> }
> }
>
> +static void x86_cpu_finalizefn(Object *obj)
> +{
> + CPUState *cs = CPU(obj);
> +
> + CPU_REMOVE(cs);
> +}
It might be better to split off device_del support into a separate patch.
> static int64_t x86_cpu_get_arch_id(CPUState *cs)
> {
> X86CPU *cpu = X86_CPU(cs);
> @@ -2937,6 +2995,7 @@ static const TypeInfo x86_cpu_type_info = {
> .parent = TYPE_CPU,
> .instance_size = sizeof(X86CPU),
> .instance_init = x86_cpu_initfn,
> + .instance_finalize = x86_cpu_finalizefn,
> .abstract = true,
> .class_size = sizeof(X86CPUClass),
> .class_init = x86_cpu_common_class_init,
> diff --git a/target-i386/topology.h b/target-i386/topology.h
> index e9ff89c..dcb4988 100644
> --- a/target-i386/topology.h
> +++ b/target-i386/topology.h
> @@ -132,4 +132,22 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
> return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
> }
>
> +/* Calculate CPU topology based on CPU APIC ID.
> + */
> +static inline void x86_topo_ids_from_apic_id(unsigned nr_cores,
> + unsigned nr_threads,
> + apic_id_t apic_id,
> + X86CPUTopoInfo *topo)
> +{
> + unsigned offset_mask;
> + topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads);
> +
> + offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1;
> + topo->core_id = (apic_id & offset_mask)
> + >> apicid_core_offset(nr_cores, nr_threads);
> +
> + offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1;
> + topo->smt_id = apic_id & offset_mask;
> +}
> +
> #endif /* TARGET_I386_TOPOLOGY_H */
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 03/10] cpu: add device_add foo-x86_64-cpu support
2014-09-09 12:44 ` Igor Mammedov
@ 2014-09-10 3:37 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-10 3:37 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
Hi Igor,
On 09/09/2014 08:44 PM, Igor Mammedov wrote:
> On Thu, 28 Aug 2014 11:36:35 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>
>> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>>
>> Add support to device_add foo-x86_64-cpu, and additional checks of
>> apic id are added into x86_cpuid_set_apic_id() and x86_cpu_apic_create()
>> for duplicate. Besides, in order to support "device/device_add foo-x86_64-cpu"
>> which without specified apic id, we add a new function get_free_apic_id() to
>> provide the first free apid id each time to avoid apic id duplicate.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>> include/qom/cpu.h | 1 +
>> qdev-monitor.c | 1 +
>> target-i386/cpu.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++-
>> target-i386/topology.h | 18 ++++++++++++++
>> 4 files changed, 80 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index bc32c9a..2fc00ef 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -292,6 +292,7 @@ struct CPUState {
>> QTAILQ_HEAD(CPUTailQ, CPUState);
>> extern struct CPUTailQ cpus;
>> #define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node)
>> +#define CPU_REMOVE(cpu) QTAILQ_REMOVE(&cpus, cpu, node)
>> #define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node)
>> #define CPU_FOREACH_SAFE(cpu, next_cpu) \
>> QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu)
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index fb9ee24..1aa446d 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -24,6 +24,7 @@
>> #include "qmp-commands.h"
>> #include "sysemu/arch_init.h"
>> #include "qemu/config-file.h"
>> +#include "qom/object_interfaces.h"
>>
>> /*
>> * Aliases were a bad idea from the start. Let's keep them
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 2aa2b31..5255ddb 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -49,6 +49,7 @@
>> #include "hw/i386/apic_internal.h"
>> #endif
>>
>> +#include "qom/object_interfaces.h"
> This probably belongs to another patch.
Yes, It's a typo here.
>
>>
>> /* Cache topology CPUID constants: */
>>
>> @@ -1634,6 +1635,7 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
>> const int64_t max = UINT32_MAX;
>> Error *error = NULL;
>> int64_t value;
>> + X86CPUTopoInfo topo;
>>
>> if (dev->realized) {
>> error_setg(errp, "Attempt to set property '%s' on '%s' after "
>> @@ -1653,6 +1655,19 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
>> return;
>> }
>>
>> + if (value > x86_cpu_apic_id_from_index(max_cpus - 1)) {
>> + error_setg(errp, "CPU with APIC ID %" PRIi64
>> + " is more than MAX APIC ID limits", value);
>> + return;
>> + }
>> +
>> + x86_topo_ids_from_apic_id(smp_cores, smp_threads, value, &topo);
>> + if (topo.smt_id >= smp_threads || topo.core_id >= smp_cores) {
>> + error_setg(errp, "CPU with APIC ID %" PRIi64 " does not match "
>> + "topology configuration.", value);
>> + return;
>> + }
>> +
>> if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) {
>> error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value);
>> return;
>> @@ -2096,12 +2111,21 @@ out:
>> return cpu;
>> }
>>
>> +static void x86_cpu_cpudef_instance_init(Object *obj)
>> +{
>> + DeviceState *dev = DEVICE(obj);
>> +
>> + dev->hotplugged = true;
>> +}
> looks unnecessary, see device_initfn() which already does above.
You are right, we do not need the x86_cpu_cpudef_instance_init, device_initfn
will do the job for us.
>
>
>> +
>> static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data)
>> {
>> X86CPUDefinition *cpudef = data;
>> X86CPUClass *xcc = X86_CPU_CLASS(oc);
>> + DeviceClass *dc = DEVICE_CLASS(oc);
>>
>> xcc->cpu_def = cpudef;
>> + dc->cannot_instantiate_with_device_add_yet = false;
>> }
>>
>> static void x86_register_cpudef_type(X86CPUDefinition *def)
>> @@ -2110,6 +2134,8 @@ static void x86_register_cpudef_type(X86CPUDefinition *def)
>> TypeInfo ti = {
>> .name = typename,
>> .parent = TYPE_X86_CPU,
>> + .instance_size = sizeof(X86CPU),
>> + .instance_init = x86_cpu_cpudef_instance_init,
> this hunk is not needed, providing x86_cpu_cpudef_instance_init() is not necessary.
>
>> .class_init = x86_cpu_cpudef_class_init,
>> .class_data = def,
>> };
>> @@ -2652,6 +2678,14 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
>> return;
>> }
>>
>> + if (env->cpuid_apic_id > x86_cpu_apic_id_from_index(max_cpus - 1)) {
>> + error_setg(errp, "CPU with APIC ID %" PRIi32
>> + " is more than MAX APIC ID:%" PRIi32,
>> + env->cpuid_apic_id,
>> + x86_cpu_apic_id_from_index(max_cpus - 1));
>> + return;
>> + }
> use property apic-id here that has checks instead of duplicating them.
Reasonable, I will fix it.
>
>> +
>> object_property_add_child(OBJECT(cpu), "apic",
>> OBJECT(cpu->apic_state), NULL);
>> qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id);
>> @@ -2777,6 +2811,21 @@ uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
>> }
>> }
>>
>> +static uint32_t get_free_apic_id(void)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < max_cpus; i++) {
>> + uint32_t id = x86_cpu_apic_id_from_index(i);
>> +
>> + if (!cpu_exists(id)) {
>> + return id;
>> + }
>> + }
>> +
>> + return x86_cpu_apic_id_from_index(max_cpus);
>> +}
>> +
>> static void x86_cpu_initfn(Object *obj)
>> {
>> CPUState *cs = CPU(obj);
>> @@ -2784,7 +2833,9 @@ static void x86_cpu_initfn(Object *obj)
>> X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
>> CPUX86State *env = &cpu->env;
>> static int inited;
>> + uint32_t value;
> s/value/apic_id/ ???
>
>>
>> + value = get_free_apic_id();
> What if there isn't any free APIC ID it the time it's called?
It will return a invalid id (x86_cpu_apic_id_from_index(max_cpus)),
and let the following check rejects it.
>
> Could you just assign default broadcast value here 0xFFFFFFFF
> and do actual APIC ID allocation at realize time if it still has
> default value.
Sounds reasonable, will try this way.
>
>> cs->env_ptr = env;
>> cpu_exec_init(env);
>>
>> @@ -2823,7 +2874,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);
>> + env->cpuid_apic_id = value;
>>
>> x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
>>
>> @@ -2837,6 +2888,13 @@ static void x86_cpu_initfn(Object *obj)
>> }
>> }
>>
>> +static void x86_cpu_finalizefn(Object *obj)
>> +{
>> + CPUState *cs = CPU(obj);
>> +
>> + CPU_REMOVE(cs);
>> +}
> It might be better to split off device_del support into a separate patch.
Yes, It should be moved to "device_del" part.
Thanks,
Gu
>
>> static int64_t x86_cpu_get_arch_id(CPUState *cs)
>> {
>> X86CPU *cpu = X86_CPU(cs);
>> @@ -2937,6 +2995,7 @@ static const TypeInfo x86_cpu_type_info = {
>> .parent = TYPE_CPU,
>> .instance_size = sizeof(X86CPU),
>> .instance_init = x86_cpu_initfn,
>> + .instance_finalize = x86_cpu_finalizefn,
>> .abstract = true,
>> .class_size = sizeof(X86CPUClass),
>> .class_init = x86_cpu_common_class_init,
>> diff --git a/target-i386/topology.h b/target-i386/topology.h
>> index e9ff89c..dcb4988 100644
>> --- a/target-i386/topology.h
>> +++ b/target-i386/topology.h
>> @@ -132,4 +132,22 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
>> return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
>> }
>>
>> +/* Calculate CPU topology based on CPU APIC ID.
>> + */
>> +static inline void x86_topo_ids_from_apic_id(unsigned nr_cores,
>> + unsigned nr_threads,
>> + apic_id_t apic_id,
>> + X86CPUTopoInfo *topo)
>> +{
>> + unsigned offset_mask;
>> + topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads);
>> +
>> + offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1;
>> + topo->core_id = (apic_id & offset_mask)
>> + >> apicid_core_offset(nr_cores, nr_threads);
>> +
>> + offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1;
>> + topo->smt_id = apic_id & offset_mask;
>> +}
>> +
>> #endif /* TARGET_I386_TOPOLOGY_H */
>
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [RFC V2 04/10] x86: add x86_cpu_unrealizefn() for cpu apic remove
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (2 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 03/10] cpu: add device_add foo-x86_64-cpu support Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-09-09 13:58 ` Igor Mammedov
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 05/10] i386: add cpu device_del support Gu Zheng
` (7 subsequent siblings)
11 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Implement x86_cpu_unrealizefn() for corresponding x86_cpu_realizefn(),
which is mostly used to clean the apic related allocation and vmstates
at here.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
hw/i386/kvm/apic.c | 8 +++++++
hw/intc/apic.c | 10 ++++++++
hw/intc/apic_common.c | 23 +++++++++++++++++++-
include/hw/cpu/icc_bus.h | 1 +
include/hw/i386/apic_internal.h | 1 +
target-i386/cpu-qom.h | 1 +
target-i386/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++
7 files changed, 88 insertions(+), 1 deletions(-)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index e873b50..c95fb8a 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -183,11 +183,19 @@ static void kvm_apic_realize(DeviceState *dev, Error **errp)
}
}
+static void kvm_apic_unrealize(DeviceState *dev, Error **errp)
+{
+ APICCommonState *s = APIC_COMMON(dev);
+
+ object_unparent(OBJECT(&s->io_memory));
+}
+
static void kvm_apic_class_init(ObjectClass *klass, void *data)
{
APICCommonClass *k = APIC_COMMON_CLASS(klass);
k->realize = kvm_apic_realize;
+ k->unrealize = kvm_apic_unrealize;
k->set_base = kvm_apic_set_base;
k->set_tpr = kvm_apic_set_tpr;
k->get_tpr = kvm_apic_get_tpr;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 03ff9e9..6d38965 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -885,11 +885,21 @@ static void apic_realize(DeviceState *dev, Error **errp)
msi_supported = true;
}
+static void apic_unrealize(DeviceState *dev, Error **errp)
+{
+ APICCommonState *s = APIC_COMMON(dev);
+
+ object_unparent(OBJECT(&s->io_memory));
+ timer_free(s->timer);
+ local_apics[s->idx] = NULL;
+}
+
static void apic_class_init(ObjectClass *klass, void *data)
{
APICCommonClass *k = APIC_COMMON_CLASS(klass);
k->realize = apic_realize;
+ k->unrealize = apic_unrealize;
k->set_base = apic_set_base;
k->set_tpr = apic_set_tpr;
k->get_tpr = apic_get_tpr;
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 029f67d..8d17be1 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -289,12 +289,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
return 0;
}
+static int apic_no;
+
static void apic_common_realize(DeviceState *dev, Error **errp)
{
APICCommonState *s = APIC_COMMON(dev);
APICCommonClass *info;
static DeviceState *vapic;
- static int apic_no;
static bool mmio_registered;
if (apic_no >= MAX_APICS) {
@@ -324,6 +325,25 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
}
+static void apic_common_unrealize(DeviceState *dev, Error **errp)
+{
+ APICCommonState *s = APIC_COMMON(dev);
+ APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
+
+ if (apic_no <= 0) {
+ error_setg(errp, "%s exit failed.",
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+ apic_no--;
+
+ info->unrealize(dev, errp);
+
+ if (apic_report_tpr_access && info->enable_tpr_reporting) {
+ info->enable_tpr_reporting(s, false);
+ }
+}
+
static void apic_dispatch_pre_save(void *opaque)
{
APICCommonState *s = APIC_COMMON(opaque);
@@ -394,6 +414,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
dc->reset = apic_reset_common;
dc->props = apic_properties_common;
idc->realize = apic_common_realize;
+ idc->unrealize = apic_common_unrealize;
/*
* Reason: APIC and CPU need to be wired up by
* x86_cpu_apic_create()
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index 98a979f..75ed309 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -67,6 +67,7 @@ typedef struct ICCDeviceClass {
/*< public >*/
DeviceRealize realize;
+ DeviceUnrealize unrealize;
} ICCDeviceClass;
#define TYPE_ICC_DEVICE "icc-device"
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 2c91609..6c9e390 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -82,6 +82,7 @@ typedef struct APICCommonClass
ICCDeviceClass parent_class;
DeviceRealize realize;
+ DeviceUnrealize unrealize;
void (*set_base)(APICCommonState *s, uint64_t val);
void (*set_tpr)(APICCommonState *s, uint8_t val);
uint8_t (*get_tpr)(APICCommonState *s);
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 71a1b97..2239105 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -65,6 +65,7 @@ typedef struct X86CPUClass {
bool kvm_required;
DeviceRealize parent_realize;
+ DeviceUnrealize parent_unrealize;
void (*parent_reset)(CPUState *cpu);
} X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5255ddb..72a94a6 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2712,10 +2712,32 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
return;
}
}
+
+static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
+{
+ Error *local_err = NULL;
+
+ if (cpu->apic_state == NULL) {
+ return;
+ }
+
+ object_property_set_bool(OBJECT(cpu->apic_state),
+ false, "realized", &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ vmstate_unregister(NULL, &vmstate_apic_common, cpu->apic_state);
+ object_unparent(OBJECT(cpu->apic_state));
+}
#else
static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
{
}
+static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
+{
+}
#endif
static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
@@ -2778,6 +2800,27 @@ out:
}
}
+static void x86_cpu_unrealizefn(DeviceState *dev, Error **errp)
+{
+ X86CPU *cpu = X86_CPU(dev);
+ CPUClass *cc = CPU_GET_CLASS(dev);
+ Error *local_err = NULL;
+
+ if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
+ vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
+ }
+
+ if (cc->vmsd != NULL) {
+ vmstate_unregister(NULL, cc->vmsd, cpu);
+ }
+
+ x86_cpu_apic_unrealize(cpu, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
+ }
+}
+
/* Enables contiguous-apic-ID mode, for compatibility */
static bool compat_apic_id_mode;
@@ -2957,7 +3000,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
xcc->parent_realize = dc->realize;
+ xcc->parent_unrealize = dc->unrealize;
dc->realize = x86_cpu_realizefn;
+ dc->unrealize = x86_cpu_unrealizefn;
dc->bus_type = TYPE_ICC_BUS;
dc->props = x86_cpu_properties;
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 04/10] x86: add x86_cpu_unrealizefn() for cpu apic remove
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 04/10] x86: add x86_cpu_unrealizefn() for cpu apic remove Gu Zheng
@ 2014-09-09 13:58 ` Igor Mammedov
2014-09-11 3:06 ` Gu Zheng
0 siblings, 1 reply; 52+ messages in thread
From: Igor Mammedov @ 2014-09-09 13:58 UTC (permalink / raw)
To: Gu Zheng
Cc: Eduardo, qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:36 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>
> Implement x86_cpu_unrealizefn() for corresponding x86_cpu_realizefn(),
> which is mostly used to clean the apic related allocation and vmstates
> at here.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> hw/i386/kvm/apic.c | 8 +++++++
> hw/intc/apic.c | 10 ++++++++
> hw/intc/apic_common.c | 23 +++++++++++++++++++-
> include/hw/cpu/icc_bus.h | 1 +
> include/hw/i386/apic_internal.h | 1 +
> target-i386/cpu-qom.h | 1 +
> target-i386/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++
> 7 files changed, 88 insertions(+), 1 deletions(-)
>
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index e873b50..c95fb8a 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -183,11 +183,19 @@ static void kvm_apic_realize(DeviceState *dev, Error **errp)
> }
> }
>
> +static void kvm_apic_unrealize(DeviceState *dev, Error **errp)
> +{
> + APICCommonState *s = APIC_COMMON(dev);
> +
> + object_unparent(OBJECT(&s->io_memory));
> +}
> +
what will disable/destroy in kernel APIC?
> static void kvm_apic_class_init(ObjectClass *klass, void *data)
> {
> APICCommonClass *k = APIC_COMMON_CLASS(klass);
>
> k->realize = kvm_apic_realize;
> + k->unrealize = kvm_apic_unrealize;
> k->set_base = kvm_apic_set_base;
> k->set_tpr = kvm_apic_set_tpr;
> k->get_tpr = kvm_apic_get_tpr;
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 03ff9e9..6d38965 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -885,11 +885,21 @@ static void apic_realize(DeviceState *dev, Error **errp)
> msi_supported = true;
> }
>
> +static void apic_unrealize(DeviceState *dev, Error **errp)
> +{
> + APICCommonState *s = APIC_COMMON(dev);
> +
> + object_unparent(OBJECT(&s->io_memory));
> + timer_free(s->timer);
> + local_apics[s->idx] = NULL;
> +}
> +
> static void apic_class_init(ObjectClass *klass, void *data)
> {
> APICCommonClass *k = APIC_COMMON_CLASS(klass);
>
> k->realize = apic_realize;
> + k->unrealize = apic_unrealize;
> k->set_base = apic_set_base;
> k->set_tpr = apic_set_tpr;
> k->get_tpr = apic_get_tpr;
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 029f67d..8d17be1 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -289,12 +289,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
> return 0;
> }
>
> +static int apic_no;
> +
some apic code still assumes that index in local_apics[] is APIC ID, so
apic_no and related stuff won't work with arbitrary CPU hotadd (i.e. arbitrary APIC ID)
to make it work the APICCommonState.idx should be dropped altogether and
replaced with apic_id.
I addition intc/apic.c is designed for APIC ID 8-bit max, we probably should add
some assert there so it wouldn't break silently when APIC ID goes above this
maximum.
> static void apic_common_realize(DeviceState *dev, Error **errp)
> {
> APICCommonState *s = APIC_COMMON(dev);
> APICCommonClass *info;
> static DeviceState *vapic;
> - static int apic_no;
> static bool mmio_registered;
>
> if (apic_no >= MAX_APICS) {
> @@ -324,6 +325,25 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>
> }
>
> +static void apic_common_unrealize(DeviceState *dev, Error **errp)
> +{
> + APICCommonState *s = APIC_COMMON(dev);
> + APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
> +
> + if (apic_no <= 0) {
> + error_setg(errp, "%s exit failed.",
> + object_get_typename(OBJECT(dev)));
> + return;
> + }
> + apic_no--;
> +
> + info->unrealize(dev, errp);
> +
> + if (apic_report_tpr_access && info->enable_tpr_reporting) {
> + info->enable_tpr_reporting(s, false);
> + }
what about unrealizing vapic?
> +}
> +
> static void apic_dispatch_pre_save(void *opaque)
> {
> APICCommonState *s = APIC_COMMON(opaque);
> @@ -394,6 +414,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
> dc->reset = apic_reset_common;
> dc->props = apic_properties_common;
> idc->realize = apic_common_realize;
> + idc->unrealize = apic_common_unrealize;
> /*
> * Reason: APIC and CPU need to be wired up by
> * x86_cpu_apic_create()
> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> index 98a979f..75ed309 100644
> --- a/include/hw/cpu/icc_bus.h
> +++ b/include/hw/cpu/icc_bus.h
> @@ -67,6 +67,7 @@ typedef struct ICCDeviceClass {
> /*< public >*/
>
> DeviceRealize realize;
> + DeviceUnrealize unrealize;
> } ICCDeviceClass;
>
> #define TYPE_ICC_DEVICE "icc-device"
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 2c91609..6c9e390 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -82,6 +82,7 @@ typedef struct APICCommonClass
> ICCDeviceClass parent_class;
>
> DeviceRealize realize;
> + DeviceUnrealize unrealize;
> void (*set_base)(APICCommonState *s, uint64_t val);
> void (*set_tpr)(APICCommonState *s, uint8_t val);
> uint8_t (*get_tpr)(APICCommonState *s);
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index 71a1b97..2239105 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -65,6 +65,7 @@ typedef struct X86CPUClass {
> bool kvm_required;
>
> DeviceRealize parent_realize;
> + DeviceUnrealize parent_unrealize;
> void (*parent_reset)(CPUState *cpu);
> } X86CPUClass;
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 5255ddb..72a94a6 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2712,10 +2712,32 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> return;
> }
> }
> +
> +static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
> +{
> + Error *local_err = NULL;
> +
> + if (cpu->apic_state == NULL) {
> + return;
> + }
> +
> + object_property_set_bool(OBJECT(cpu->apic_state),
> + false, "realized", &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + vmstate_unregister(NULL, &vmstate_apic_common, cpu->apic_state);
this should be done by device_unrealize when APIC is being unrealized.
> + object_unparent(OBJECT(cpu->apic_state));
> +}
> #else
> static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
> {
> }
> +static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
> +{
> +}
> #endif
>
> static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> @@ -2778,6 +2800,27 @@ out:
> }
> }
>
> +static void x86_cpu_unrealizefn(DeviceState *dev, Error **errp)
> +{
> + X86CPU *cpu = X86_CPU(dev);
> + CPUClass *cc = CPU_GET_CLASS(dev);
> + Error *local_err = NULL;
> +
> + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> + vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> + }
> +
> + if (cc->vmsd != NULL) {
> + vmstate_unregister(NULL, cc->vmsd, cpu);
> + }
I don't recall which variant x86cpu uses but it probably should be
one of above
or even better, make device_set_realized()->vmstate_[un]register*() work
wit x86cpu if possible.
> +
> + x86_cpu_apic_unrealize(cpu, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +}
> +
> /* Enables contiguous-apic-ID mode, for compatibility */
> static bool compat_apic_id_mode;
>
> @@ -2957,7 +3000,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> xcc->parent_realize = dc->realize;
> + xcc->parent_unrealize = dc->unrealize;
> dc->realize = x86_cpu_realizefn;
> + dc->unrealize = x86_cpu_unrealizefn;
> dc->bus_type = TYPE_ICC_BUS;
> dc->props = x86_cpu_properties;
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 04/10] x86: add x86_cpu_unrealizefn() for cpu apic remove
2014-09-09 13:58 ` Igor Mammedov
@ 2014-09-11 3:06 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-11 3:06 UTC (permalink / raw)
To: Igor Mammedov
Cc: Eduardo, qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
Hi Igor,
On 09/09/2014 09:58 PM, Igor Mammedov wrote:
> On Thu, 28 Aug 2014 11:36:36 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>
>> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>>
>> Implement x86_cpu_unrealizefn() for corresponding x86_cpu_realizefn(),
>> which is mostly used to clean the apic related allocation and vmstates
>> at here.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>> hw/i386/kvm/apic.c | 8 +++++++
>> hw/intc/apic.c | 10 ++++++++
>> hw/intc/apic_common.c | 23 +++++++++++++++++++-
>> include/hw/cpu/icc_bus.h | 1 +
>> include/hw/i386/apic_internal.h | 1 +
>> target-i386/cpu-qom.h | 1 +
>> target-i386/cpu.c | 45 +++++++++++++++++++++++++++++++++++++++
>> 7 files changed, 88 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
>> index e873b50..c95fb8a 100644
>> --- a/hw/i386/kvm/apic.c
>> +++ b/hw/i386/kvm/apic.c
>> @@ -183,11 +183,19 @@ static void kvm_apic_realize(DeviceState *dev, Error **errp)
>> }
>> }
>>
>> +static void kvm_apic_unrealize(DeviceState *dev, Error **errp)
>> +{
>> + APICCommonState *s = APIC_COMMON(dev);
>> +
>> + object_unparent(OBJECT(&s->io_memory));
>> +}
>> +
> what will disable/destroy in kernel APIC?
As we parked kvm cpu, is this step still seriously needed?
>
>> static void kvm_apic_class_init(ObjectClass *klass, void *data)
>> {
>> APICCommonClass *k = APIC_COMMON_CLASS(klass);
>>
>> k->realize = kvm_apic_realize;
>> + k->unrealize = kvm_apic_unrealize;
>> k->set_base = kvm_apic_set_base;
>> k->set_tpr = kvm_apic_set_tpr;
>> k->get_tpr = kvm_apic_get_tpr;
>> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
>> index 03ff9e9..6d38965 100644
>> --- a/hw/intc/apic.c
>> +++ b/hw/intc/apic.c
>> @@ -885,11 +885,21 @@ static void apic_realize(DeviceState *dev, Error **errp)
>> msi_supported = true;
>> }
>>
>> +static void apic_unrealize(DeviceState *dev, Error **errp)
>> +{
>> + APICCommonState *s = APIC_COMMON(dev);
>> +
>> + object_unparent(OBJECT(&s->io_memory));
>> + timer_free(s->timer);
>> + local_apics[s->idx] = NULL;
>> +}
>> +
>> static void apic_class_init(ObjectClass *klass, void *data)
>> {
>> APICCommonClass *k = APIC_COMMON_CLASS(klass);
>>
>> k->realize = apic_realize;
>> + k->unrealize = apic_unrealize;
>> k->set_base = apic_set_base;
>> k->set_tpr = apic_set_tpr;
>> k->get_tpr = apic_get_tpr;
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
>> index 029f67d..8d17be1 100644
>> --- a/hw/intc/apic_common.c
>> +++ b/hw/intc/apic_common.c
>> @@ -289,12 +289,13 @@ static int apic_load_old(QEMUFile *f, void *opaque, int version_id)
>> return 0;
>> }
>>
>> +static int apic_no;
>> +
> some apic code still assumes that index in local_apics[] is APIC ID, so
> apic_no and related stuff won't work with arbitrary CPU hotadd (i.e. arbitrary APIC ID)
> to make it work the APICCommonState.idx should be dropped altogether and
> replaced with apic_id.
If so, we absolutely need to change this.
>
> I addition intc/apic.c is designed for APIC ID 8-bit max, we probably should add
> some assert there so it wouldn't break silently when APIC ID goes above this
> maximum.
OK.
>
>> static void apic_common_realize(DeviceState *dev, Error **errp)
>> {
>> APICCommonState *s = APIC_COMMON(dev);
>> APICCommonClass *info;
>> static DeviceState *vapic;
>> - static int apic_no;
>> static bool mmio_registered;
>>
>> if (apic_no >= MAX_APICS) {
>> @@ -324,6 +325,25 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
>>
>> }
>>
>> +static void apic_common_unrealize(DeviceState *dev, Error **errp)
>> +{
>> + APICCommonState *s = APIC_COMMON(dev);
>> + APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>> +
>> + if (apic_no <= 0) {
>> + error_setg(errp, "%s exit failed.",
>> + object_get_typename(OBJECT(dev)));
>> + return;
>> + }
>> + apic_no--;
>> +
>> + info->unrealize(dev, errp);
>> +
>> + if (apic_report_tpr_access && info->enable_tpr_reporting) {
>> + info->enable_tpr_reporting(s, false);
>> + }
> what about unrealizing vapic?
IMO, we should not unrealize the vapic, because all apics share one vapic.
>
>
>> +}
>> +
>> static void apic_dispatch_pre_save(void *opaque)
>> {
>> APICCommonState *s = APIC_COMMON(opaque);
>> @@ -394,6 +414,7 @@ static void apic_common_class_init(ObjectClass *klass, void *data)
>> dc->reset = apic_reset_common;
>> dc->props = apic_properties_common;
>> idc->realize = apic_common_realize;
>> + idc->unrealize = apic_common_unrealize;
>> /*
>> * Reason: APIC and CPU need to be wired up by
>> * x86_cpu_apic_create()
>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
>> index 98a979f..75ed309 100644
>> --- a/include/hw/cpu/icc_bus.h
>> +++ b/include/hw/cpu/icc_bus.h
>> @@ -67,6 +67,7 @@ typedef struct ICCDeviceClass {
>> /*< public >*/
>>
>> DeviceRealize realize;
>> + DeviceUnrealize unrealize;
>> } ICCDeviceClass;
>>
>> #define TYPE_ICC_DEVICE "icc-device"
>> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
>> index 2c91609..6c9e390 100644
>> --- a/include/hw/i386/apic_internal.h
>> +++ b/include/hw/i386/apic_internal.h
>> @@ -82,6 +82,7 @@ typedef struct APICCommonClass
>> ICCDeviceClass parent_class;
>>
>> DeviceRealize realize;
>> + DeviceUnrealize unrealize;
>> void (*set_base)(APICCommonState *s, uint64_t val);
>> void (*set_tpr)(APICCommonState *s, uint8_t val);
>> uint8_t (*get_tpr)(APICCommonState *s);
>> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
>> index 71a1b97..2239105 100644
>> --- a/target-i386/cpu-qom.h
>> +++ b/target-i386/cpu-qom.h
>> @@ -65,6 +65,7 @@ typedef struct X86CPUClass {
>> bool kvm_required;
>>
>> DeviceRealize parent_realize;
>> + DeviceUnrealize parent_unrealize;
>> void (*parent_reset)(CPUState *cpu);
>> } X86CPUClass;
>>
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 5255ddb..72a94a6 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -2712,10 +2712,32 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>> return;
>> }
>> }
>> +
>> +static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
>> +{
>> + Error *local_err = NULL;
>> +
>> + if (cpu->apic_state == NULL) {
>> + return;
>> + }
>> +
>> + object_property_set_bool(OBJECT(cpu->apic_state),
>> + false, "realized", &local_err);
>> + if (local_err != NULL) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> +
>> + vmstate_unregister(NULL, &vmstate_apic_common, cpu->apic_state);
> this should be done by device_unrealize when APIC is being unrealized.
Yes, I will fix it.
>
>> + object_unparent(OBJECT(cpu->apic_state));
>> +}
>> #else
>> static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>> {
>> }
>> +static void x86_cpu_apic_unrealize(X86CPU *cpu, Error **errp)
>> +{
>> +}
>> #endif
>>
>> static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>> @@ -2778,6 +2800,27 @@ out:
>> }
>> }
>>
>> +static void x86_cpu_unrealizefn(DeviceState *dev, Error **errp)
>> +{
>> + X86CPU *cpu = X86_CPU(dev);
>> + CPUClass *cc = CPU_GET_CLASS(dev);
>> + Error *local_err = NULL;
>> +
>> + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
>> + vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
>> + }
>> +
>> + if (cc->vmsd != NULL) {
>> + vmstate_unregister(NULL, cc->vmsd, cpu);
>> + }
> I don't recall which variant x86cpu uses but it probably should be
> one of above
> or even better, make device_set_realized()->vmstate_[un]register*() work
> wit x86cpu if possible.
I'll try this way, thanks.
Regards,
Gu
>
>> +
>> + x86_cpu_apic_unrealize(cpu, &local_err);
>> + if (local_err != NULL) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> +}
>> +
>> /* Enables contiguous-apic-ID mode, for compatibility */
>> static bool compat_apic_id_mode;
>>
>> @@ -2957,7 +3000,9 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
>> DeviceClass *dc = DEVICE_CLASS(oc);
>>
>> xcc->parent_realize = dc->realize;
>> + xcc->parent_unrealize = dc->unrealize;
>> dc->realize = x86_cpu_realizefn;
>> + dc->unrealize = x86_cpu_unrealizefn;
>> dc->bus_type = TYPE_ICC_BUS;
>> dc->props = x86_cpu_properties;
>>
>
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [RFC V2 05/10] i386: add cpu device_del support
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (3 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 04/10] x86: add x86_cpu_unrealizefn() for cpu apic remove Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-09-09 14:11 ` Igor Mammedov
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 06/10] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Gu Zheng
` (6 subsequent siblings)
11 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
In order to add cpu(i386) device_del support, introduce the cpu hot
unplug hanlde(x86_cpu_unplug) which will trigger the unrealize routine,
and just register it to the cpu class unplug handle.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
target-i386/cpu.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 72a94a6..dc5153e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2993,6 +2993,27 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_END_OF_LIST()
};
+static int x86_cpu_unplug(DeviceState *dev)
+{
+ CPUState *cpu = CPU(dev);
+ X86CPUClass *xcc;
+ Error *err = NULL;
+
+ if (cpu == first_cpu) {
+ error_report("Can not remove the first cpu!\n");
+ return -1;
+ }
+
+ xcc = X86_CPU_GET_CLASS(DEVICE(cpu));
+ xcc->parent_unrealize(DEVICE(cpu), &err);
+ if (err != NULL) {
+ error_report("%s", error_get_pretty(err));
+ error_free(err);
+ return -1;
+ }
+ return 0;
+}
+
static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
{
X86CPUClass *xcc = X86_CPU_CLASS(oc);
@@ -3005,6 +3026,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
dc->unrealize = x86_cpu_unrealizefn;
dc->bus_type = TYPE_ICC_BUS;
dc->props = x86_cpu_properties;
+ dc->unplug = x86_cpu_unplug;
xcc->parent_reset = cc->reset;
cc->reset = x86_cpu_reset;
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 05/10] i386: add cpu device_del support
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 05/10] i386: add cpu device_del support Gu Zheng
@ 2014-09-09 14:11 ` Igor Mammedov
0 siblings, 0 replies; 52+ messages in thread
From: Igor Mammedov @ 2014-09-09 14:11 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:37 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> In order to add cpu(i386) device_del support, introduce the cpu hot
> unplug hanlde(x86_cpu_unplug) which will trigger the unrealize routine,
> and just register it to the cpu class unplug handle.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> target-i386/cpu.c | 22 ++++++++++++++++++++++
> 1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 72a94a6..dc5153e 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2993,6 +2993,27 @@ static Property x86_cpu_properties[] = {
> DEFINE_PROP_END_OF_LIST()
> };
>
> +static int x86_cpu_unplug(DeviceState *dev)
> +{
> + CPUState *cpu = CPU(dev);
> + X86CPUClass *xcc;
> + Error *err = NULL;
> +
> + if (cpu == first_cpu) {
> + error_report("Can not remove the first cpu!\n");
> + return -1;
> + }
it might be too late error out at this time since, guest already got and
handled unplug request. This check should be done at unplug request time
instead of when CPU is destroyed.
We can try to prevent guest to eject first_cpu by not declaring
_EJ0 ACPI method for it.
> +
> + xcc = X86_CPU_GET_CLASS(DEVICE(cpu));
> + xcc->parent_unrealize(DEVICE(cpu), &err);
Why is it not just setting 'realize' property of device to 'false'?
> + if (err != NULL) {
> + error_report("%s", error_get_pretty(err));
> + error_free(err);
> + return -1;
> + }
> + return 0;
> +}
> +
> static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> {
> X86CPUClass *xcc = X86_CPU_CLASS(oc);
> @@ -3005,6 +3026,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
> dc->unrealize = x86_cpu_unrealizefn;
> dc->bus_type = TYPE_ICC_BUS;
> dc->props = x86_cpu_properties;
> + dc->unplug = x86_cpu_unplug;
>
> xcc->parent_reset = cc->reset;
> cc->reset = x86_cpu_reset;
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [RFC V2 06/10] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier'
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (4 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 05/10] i386: add cpu device_del support Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 07/10] qom cpu: add UNPLUG cpu notify support Gu Zheng
` (5 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' for
adding remove vcpu notifier support.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
hw/acpi/ich9.c | 8 ++++----
hw/acpi/piix4.c | 10 +++++-----
hw/i386/pc.c | 2 +-
include/hw/acpi/ich9.h | 2 +-
include/sysemu/sysemu.h | 2 +-
qom/cpu.c | 10 +++++-----
6 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 7b14bbb..804f774 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -209,9 +209,9 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
acpi_pm1_evt_power_down(&pm->acpi_regs);
}
-static void ich9_cpu_added_req(Notifier *n, void *opaque)
+static void ich9_cpu_hotplug_req(Notifier *n, void *opaque)
{
- ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
+ ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_hotplug_notifier);
assert(pm != NULL);
AcpiCpuHotplug_add(&pm->acpi_regs.gpe, &pm->gpe_cpu, CPU(opaque));
@@ -246,8 +246,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
&pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
- pm->cpu_added_notifier.notify = ich9_cpu_added_req;
- qemu_register_cpu_added_notifier(&pm->cpu_added_notifier);
+ pm->cpu_hotplug_notifier.notify = ich9_cpu_hotplug_req;
+ qemu_register_cpu_hotplug_notifier(&pm->cpu_hotplug_notifier);
if (pm->acpi_memory_hotplug.is_enabled) {
acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..5cd6300 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -83,7 +83,7 @@ typedef struct PIIX4PMState {
uint8_t s4_val;
AcpiCpuHotplug gpe_cpu;
- Notifier cpu_added_notifier;
+ Notifier cpu_hotplug_notifier;
MemHotplugState acpi_memory_hotplug;
} PIIX4PMState;
@@ -544,9 +544,9 @@ static const MemoryRegionOps piix4_gpe_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static void piix4_cpu_added_req(Notifier *n, void *opaque)
+static void piix4_cpu_hotplug(Notifier *n, void *opaque)
{
- PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
+ PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
assert(s != NULL);
AcpiCpuHotplug_add(&s->ar.gpe, &s->gpe_cpu, CPU(opaque));
@@ -565,8 +565,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu,
PIIX4_CPU_HOTPLUG_IO_BASE);
- s->cpu_added_notifier.notify = piix4_cpu_added_req;
- qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
+ s->cpu_hotplug_notifier.notify = piix4_cpu_hotplug;
+ qemu_register_cpu_hotplug_notifier(&s->cpu_hotplug_notifier);
if (s->acpi_memory_hotplug.is_enabled) {
acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8fa8d2f..1b26985 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -413,7 +413,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
/* init CPU hotplug notifier */
cpu_hotplug_cb.rtc_state = s;
cpu_hotplug_cb.cpu_added_notifier.notify = rtc_notify_cpu_added;
- qemu_register_cpu_added_notifier(&cpu_hotplug_cb.cpu_added_notifier);
+ qemu_register_cpu_hotplug_notifier(&cpu_hotplug_cb.cpu_added_notifier);
if (set_boot_dev(s, boot_device)) {
exit(1);
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 7e42448..bbb5d2c 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -47,7 +47,7 @@ typedef struct ICH9LPCPMRegs {
Notifier powerdown_notifier;
AcpiCpuHotplug gpe_cpu;
- Notifier cpu_added_notifier;
+ Notifier cpu_hotplug_notifier;
MemHotplugState acpi_memory_hotplug;
} ICH9LPCPMRegs;
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..6810f70 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -184,7 +184,7 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
void drive_hot_add(Monitor *mon, const QDict *qdict);
/* CPU hotplug */
-void qemu_register_cpu_added_notifier(Notifier *notifier);
+void qemu_register_cpu_hotplug_notifier(Notifier *notifier);
/* pcie aer error injection */
void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
diff --git a/qom/cpu.c b/qom/cpu.c
index bf16590..add92b1 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -108,12 +108,12 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
}
/* CPU hot-plug notifiers */
-static NotifierList cpu_added_notifiers =
- NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
+static NotifierList cpu_hotplug_notifiers =
+ NOTIFIER_LIST_INITIALIZER(cpu_hotplug_notifiers);
-void qemu_register_cpu_added_notifier(Notifier *notifier)
+void qemu_register_cpu_hotplug_notifier(Notifier *notifier)
{
- notifier_list_add(&cpu_added_notifiers, notifier);
+ notifier_list_add(&cpu_hotplug_notifiers, notifier);
}
void cpu_reset_interrupt(CPUState *cpu, int mask)
@@ -305,7 +305,7 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
if (dev->hotplugged) {
cpu_synchronize_post_init(cpu);
- notifier_list_notify(&cpu_added_notifiers, dev);
+ notifier_list_notify(&cpu_hotplug_notifiers, dev);
cpu_resume(cpu);
}
}
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [RFC V2 07/10] qom cpu: add UNPLUG cpu notify support
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (5 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 06/10] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 08/10] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c Gu Zheng
` (4 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
Introduce a common cpu hotplug notifier(CPUNotifier)
to support UNPLUG cpu notify.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
hw/acpi/cpu_hotplug.c | 15 +++++++++++----
hw/acpi/ich9.c | 5 ++++-
hw/acpi/piix4.c | 11 +++++++----
include/hw/acpi/cpu_hotplug.h | 13 ++++++++++++-
qom/cpu.c | 7 ++++++-
5 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 2ad83a0..56cb316 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -36,15 +36,22 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
},
};
-void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
+void AcpiCpuHotplug_handle(ACPIGPE *gpe, AcpiCpuHotplug *g,
+ CPUNotifier *notify)
{
- CPUClass *k = CPU_GET_CLASS(cpu);
+ CPUClass *k = CPU_GET_CLASS(notify->dev);
+ HotplugEventType type = notify->type;
int64_t cpu_id;
*gpe->sts = *gpe->sts | ACPI_CPU_HOTPLUG_STATUS;
- cpu_id = k->get_arch_id(CPU(cpu));
+ cpu_id = k->get_arch_id(CPU(notify->dev));
g_assert((cpu_id / 8) < ACPI_GPE_PROC_LEN);
- g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
+
+ if (type == PLUG) {
+ g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
+ } else {
+ g->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
+ }
}
void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 804f774..2db491d 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -212,9 +212,12 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
static void ich9_cpu_hotplug_req(Notifier *n, void *opaque)
{
ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_hotplug_notifier);
+ CPUNotifier *notifier = (CPUNotifier *)opaque;
assert(pm != NULL);
- AcpiCpuHotplug_add(&pm->acpi_regs.gpe, &pm->gpe_cpu, CPU(opaque));
+
+ AcpiCpuHotplug_handle(&pm->acpi_regs.gpe, &pm->gpe_cpu, notifier);
+
acpi_update_sci(&pm->acpi_regs, pm->irq);
}
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 5cd6300..cc15b60 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -93,7 +93,7 @@ typedef struct PIIX4PMState {
#define PIIX4_PM(obj) \
OBJECT_CHECK(PIIX4PMState, (obj), TYPE_PIIX4_PM)
-static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
+static void piix4_acpi_system_hotplug_init(MemoryRegion *parent,
PCIBus *bus, PIIX4PMState *s);
#define ACPI_ENABLE 0xf1
@@ -465,7 +465,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
qemu_add_machine_init_done_notifier(&s->machine_ready);
qemu_register_reset(piix4_reset, s);
- piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, s);
+ piix4_acpi_system_hotplug_init(pci_address_space_io(dev), dev->bus, s);
piix4_pm_add_propeties(s);
return 0;
@@ -547,13 +547,16 @@ static const MemoryRegionOps piix4_gpe_ops = {
static void piix4_cpu_hotplug(Notifier *n, void *opaque)
{
PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
+ CPUNotifier *notifier = (CPUNotifier *)opaque;
assert(s != NULL);
- AcpiCpuHotplug_add(&s->ar.gpe, &s->gpe_cpu, CPU(opaque));
+
+ AcpiCpuHotplug_handle(&s->ar.gpe, &s->gpe_cpu, notifier);
+
acpi_update_sci(&s->ar, s->irq);
}
-static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
+static void piix4_acpi_system_hotplug_init(MemoryRegion *parent,
PCIBus *bus, PIIX4PMState *s)
{
memory_region_init_io(&s->io_gpe, OBJECT(s), &piix4_gpe_ops, s,
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 9e5d30c..4fe0066 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -15,12 +15,23 @@
#include "hw/acpi/acpi.h"
#include "hw/acpi/pc-hotplug.h"
+typedef enum {
+ PLUG,
+ UNPLUG,
+} HotplugEventType;
+
+typedef struct CPUNotifier {
+ DeviceState *dev;
+ HotplugEventType type;
+} CPUNotifier;
+
typedef struct AcpiCpuHotplug {
MemoryRegion io;
uint8_t sts[ACPI_GPE_PROC_LEN];
} AcpiCpuHotplug;
-void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
+void AcpiCpuHotplug_handle(ACPIGPE *gpe, AcpiCpuHotplug *g,
+ CPUNotifier *notify);
void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
AcpiCpuHotplug *gpe_cpu, uint16_t base);
diff --git a/qom/cpu.c b/qom/cpu.c
index add92b1..f921282 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -25,6 +25,7 @@
#include "qemu/log.h"
#include "qemu/error-report.h"
#include "sysemu/sysemu.h"
+#include "hw/acpi/cpu_hotplug.h"
bool cpu_exists(int64_t id)
{
@@ -304,8 +305,12 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
cpu_vmstate_register(cpu);
if (dev->hotplugged) {
+ CPUNotifier notifier;
+
+ notifier.dev = dev;
+ notifier.type = PLUG;
cpu_synchronize_post_init(cpu);
- notifier_list_notify(&cpu_hotplug_notifiers, dev);
+ notifier_list_notify(&cpu_hotplug_notifiers, ¬ifier);
cpu_resume(cpu);
}
}
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [RFC V2 08/10] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (6 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 07/10] qom cpu: add UNPLUG cpu notify support Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 09/10] cpu hotplug: implement function cpu_status_write() for vcpu ejection Gu Zheng
` (3 subsequent siblings)
11 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
add interface cpu_common_unrealizefn() for emiting vcpu unplug
notifier to ACPI, then ACPI could send sci interrupt
to OS for hot-remove vcpu.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
qom/cpu.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/qom/cpu.c b/qom/cpu.c
index f921282..8bb2b4f 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -315,6 +315,17 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
}
}
+static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
+{
+ CPUNotifier notifier;
+
+ notifier.dev = dev;
+ notifier.type = UNPLUG;
+
+ notifier_list_notify(&cpu_hotplug_notifiers, ¬ifier);
+}
+
+
static void cpu_common_initfn(Object *obj)
{
CPUState *cpu = CPU(obj);
@@ -348,6 +359,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
k->gdb_write_register = cpu_common_gdb_write_register;
k->virtio_is_big_endian = cpu_common_virtio_is_big_endian;
dc->realize = cpu_common_realizefn;
+ dc->unrealize = cpu_common_unrealizefn;
/*
* Reason: CPUs still need special care by board code: wiring up
* IRQs, adding reset handlers, halting non-first CPUs, ...
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* [Qemu-devel] [RFC V2 09/10] cpu hotplug: implement function cpu_status_write() for vcpu ejection
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (7 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 08/10] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-09-09 14:28 ` Igor Mammedov
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects Gu Zheng
` (2 subsequent siblings)
11 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
When OS ejected a vcpu (like: echo 1 > /sys/bus/acpi/devices/LNXCPUXX/eject),
it would call acpi EJ0 method, the firmware need to write the new cpumap, QEMU
would know which vcpu need to be ejected.
TODO:
-confirm the hotplug result via OST if guest support it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
cpus.c | 7 ++++++
hw/acpi/cpu_hotplug.c | 40 ++++++++++++++++++++++++++++++++++++-
hw/i386/acpi-dsdt-cpu-hotplug.dsl | 6 ++++-
include/hw/acpi/cpu_hotplug.h | 1 +
include/qom/cpu.h | 9 ++++++++
5 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/cpus.c b/cpus.c
index cce2744..eee693b 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1182,6 +1182,13 @@ void resume_all_vcpus(void)
}
}
+void cpu_remove(CPUState *cpu)
+{
+ cpu->stop = true;
+ cpu->exit = true;
+ qemu_cpu_kick(cpu);
+}
+
/* For temporary buffers for forming a name */
#define VCPU_THREAD_NAME_SIZE 16
diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 56cb316..7cbce69 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -20,10 +20,46 @@ static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
return val;
}
+static void acpi_eject_vcpu(AcpiCpuHotplug *cpus_status, int64_t cpu_id)
+{
+ CPUState *cpu;
+
+ CPU_FOREACH(cpu) {
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ int64_t id = cc->get_arch_id(cpu);
+
+ if (cpu_id == id) {
+ cpus_status->old_sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
+ cpus_status->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
+ cpu_remove(cpu);
+ break;
+ }
+ }
+}
+
static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
unsigned int size)
{
- /* TODO: implement VCPU removal on guest signal that CPU can be removed */
+ AcpiCpuHotplug *cpus = opaque;
+ uint8_t val;
+ int i;
+ int64_t cpu_id = -1;
+
+ val = cpus->old_sts[addr] ^ data;
+
+ if (val == 0) {
+ return;
+ }
+
+ for (i = 0; i < 8; i++) {
+ if (val & 1 << i) {
+ cpu_id = 8 * addr + i;
+ }
+ }
+
+ if (cpu_id != -1) {
+ acpi_eject_vcpu(cpus, cpu_id);
+ }
}
static const MemoryRegionOps AcpiCpuHotplug_ops = {
@@ -49,6 +85,7 @@ void AcpiCpuHotplug_handle(ACPIGPE *gpe, AcpiCpuHotplug *g,
if (type == PLUG) {
g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
+ g->old_sts[cpu_id / 8] |= (1 << (cpu_id % 8));
} else {
g->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
}
@@ -65,6 +102,7 @@ void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
g_assert((id / 8) < ACPI_GPE_PROC_LEN);
gpe_cpu->sts[id / 8] |= (1 << (id % 8));
+ gpe_cpu->old_sts[id / 8] |= (1 << (id % 8));
}
memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
index 34aab5a..9485f12 100644
--- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
@@ -50,7 +50,11 @@ Scope(\_SB) {
}
Method(CPEJ, 2, NotSerialized) {
// _EJ0 method - eject callback
- Sleep(200)
+ Store(Zero, Index(CPON, ToInteger(Arg0)))
+ Store(One, Local0)
+ ShiftLeft(Local0, Arg0, Local0)
+ Not(Local0, Local0)
+ And(PRS, Local0, PRS)
}
#define CPU_STATUS_LEN ACPI_GPE_PROC_LEN
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 4fe0066..3bffc69 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -28,6 +28,7 @@ typedef struct CPUNotifier {
typedef struct AcpiCpuHotplug {
MemoryRegion io;
uint8_t sts[ACPI_GPE_PROC_LEN];
+ uint8_t old_sts[ACPI_GPE_PROC_LEN];
} AcpiCpuHotplug;
void AcpiCpuHotplug_handle(ACPIGPE *gpe, AcpiCpuHotplug *g,
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 2fc00ef..9108dc6 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,7 @@ struct CPUState {
bool created;
bool stop;
bool stopped;
+ bool exit;
volatile sig_atomic_t exit_request;
uint32_t interrupt_request;
int singlestep_enabled;
@@ -600,6 +601,14 @@ void cpu_exit(CPUState *cpu);
void cpu_resume(CPUState *cpu);
/**
+ * cpu_remove:
+ * @cpu: The vCPU to remove.
+ *
+ * Requests the CPU @cpu to be removed.
+ */
+void cpu_remove(CPUState *cpu);
+
+/**
* qemu_init_vcpu:
* @cpu: The vCPU to initialize.
*
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 09/10] cpu hotplug: implement function cpu_status_write() for vcpu ejection
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 09/10] cpu hotplug: implement function cpu_status_write() for vcpu ejection Gu Zheng
@ 2014-09-09 14:28 ` Igor Mammedov
0 siblings, 0 replies; 52+ messages in thread
From: Igor Mammedov @ 2014-09-09 14:28 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:41 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>
> When OS ejected a vcpu (like: echo 1 > /sys/bus/acpi/devices/LNXCPUXX/eject),
> it would call acpi EJ0 method, the firmware need to write the new cpumap, QEMU
> would know which vcpu need to be ejected.
>
> TODO:
> -confirm the hotplug result via OST if guest support it.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> cpus.c | 7 ++++++
> hw/acpi/cpu_hotplug.c | 40 ++++++++++++++++++++++++++++++++++++-
> hw/i386/acpi-dsdt-cpu-hotplug.dsl | 6 ++++-
> include/hw/acpi/cpu_hotplug.h | 1 +
> include/qom/cpu.h | 9 ++++++++
> 5 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index cce2744..eee693b 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1182,6 +1182,13 @@ void resume_all_vcpus(void)
> }
> }
>
> +void cpu_remove(CPUState *cpu)
> +{
> + cpu->stop = true;
> + cpu->exit = true;
> + qemu_cpu_kick(cpu);
> +}
> +
> /* For temporary buffers for forming a name */
> #define VCPU_THREAD_NAME_SIZE 16
>
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index 56cb316..7cbce69 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -20,10 +20,46 @@ static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
> return val;
> }
>
> +static void acpi_eject_vcpu(AcpiCpuHotplug *cpus_status, int64_t cpu_id)
> +{
> + CPUState *cpu;
> +
> + CPU_FOREACH(cpu) {
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> + int64_t id = cc->get_arch_id(cpu);
> +
> + if (cpu_id == id) {
> + cpus_status->old_sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
> + cpus_status->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
> + cpu_remove(cpu);
Could CPU be destoyed at this time, instead of marking it destruction?
> + break;
> + }
> + }
> +}
> +
> static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
> unsigned int size)
> {
> - /* TODO: implement VCPU removal on guest signal that CPU can be removed */
> + AcpiCpuHotplug *cpus = opaque;
> + uint8_t val;
> + int i;
> + int64_t cpu_id = -1;
> +
> + val = cpus->old_sts[addr] ^ data;
> +
> + if (val == 0) {
> + return;
> + }
> +
> + for (i = 0; i < 8; i++) {
> + if (val & 1 << i) {
> + cpu_id = 8 * addr + i;
> + }
> + }
> +
> + if (cpu_id != -1) {
> + acpi_eject_vcpu(cpus, cpu_id);
> + }
> }
>
> static const MemoryRegionOps AcpiCpuHotplug_ops = {
> @@ -49,6 +85,7 @@ void AcpiCpuHotplug_handle(ACPIGPE *gpe, AcpiCpuHotplug *g,
>
> if (type == PLUG) {
> g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
> + g->old_sts[cpu_id / 8] |= (1 << (cpu_id % 8));
> } else {
> g->sts[cpu_id / 8] &= ~(1 << (cpu_id % 8));
> }
> @@ -65,6 +102,7 @@ void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
>
> g_assert((id / 8) < ACPI_GPE_PROC_LEN);
> gpe_cpu->sts[id / 8] |= (1 << (id % 8));
> + gpe_cpu->old_sts[id / 8] |= (1 << (id % 8));
> }
> memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
> gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> index 34aab5a..9485f12 100644
> --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
> @@ -50,7 +50,11 @@ Scope(\_SB) {
> }
> Method(CPEJ, 2, NotSerialized) {
> // _EJ0 method - eject callback
> - Sleep(200)
> + Store(Zero, Index(CPON, ToInteger(Arg0)))
> + Store(One, Local0)
> + ShiftLeft(Local0, Arg0, Local0)
> + Not(Local0, Local0)
> + And(PRS, Local0, PRS)
> }
>
> #define CPU_STATUS_LEN ACPI_GPE_PROC_LEN
> diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
> index 4fe0066..3bffc69 100644
> --- a/include/hw/acpi/cpu_hotplug.h
> +++ b/include/hw/acpi/cpu_hotplug.h
> @@ -28,6 +28,7 @@ typedef struct CPUNotifier {
> typedef struct AcpiCpuHotplug {
> MemoryRegion io;
> uint8_t sts[ACPI_GPE_PROC_LEN];
> + uint8_t old_sts[ACPI_GPE_PROC_LEN];
> } AcpiCpuHotplug;
>
> void AcpiCpuHotplug_handle(ACPIGPE *gpe, AcpiCpuHotplug *g,
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 2fc00ef..9108dc6 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -236,6 +236,7 @@ struct CPUState {
> bool created;
> bool stop;
> bool stopped;
> + bool exit;
> volatile sig_atomic_t exit_request;
> uint32_t interrupt_request;
> int singlestep_enabled;
> @@ -600,6 +601,14 @@ void cpu_exit(CPUState *cpu);
> void cpu_resume(CPUState *cpu);
>
> /**
> + * cpu_remove:
> + * @cpu: The vCPU to remove.
> + *
> + * Requests the CPU @cpu to be removed.
> + */
> +void cpu_remove(CPUState *cpu);
> +
> +/**
> * qemu_init_vcpu:
> * @cpu: The vCPU to initialize.
> *
^ permalink raw reply [flat|nested] 52+ messages in thread
* [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (8 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 09/10] cpu hotplug: implement function cpu_status_write() for vcpu ejection Gu Zheng
@ 2014-08-28 3:36 ` Gu Zheng
2014-09-09 14:40 ` Igor Mammedov
` (3 more replies)
2014-11-12 1:46 ` [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
2014-11-12 1:46 ` Gu Zheng
11 siblings, 4 replies; 52+ messages in thread
From: Gu Zheng @ 2014-08-28 3:36 UTC (permalink / raw)
To: qemu-devel, imammedo, afaerber
Cc: chen.fan.fnst, anshul.makkar, isimatu.yasuaki, Gu Zheng, tangchen
After ACPI get a signal to eject a vCPU, the vCPU must be
removed from CPU list，before the vCPU really removed, then
release the all related vCPU objects.
But we do not close KVM vcpu fd, just record it into a list, in
order to reuse it.
Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
cpus.c | 37 ++++++++++++++++++++++++++++++++
include/sysemu/kvm.h | 1 +
kvm-all.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 94 insertions(+), 1 deletions(-)
diff --git a/cpus.c b/cpus.c
index eee693b..0608b41 100644
--- a/cpus.c
+++ b/cpus.c
@@ -851,6 +851,24 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
qemu_cpu_kick(cpu);
}
+static void qemu_kvm_destroy_vcpu(CPUState *cpu)
+{
+ CPU_REMOVE(cpu);
+
+ if (kvm_destroy_vcpu(cpu) < 0) {
+ fprintf(stderr, "kvm_destroy_vcpu failed.\n");
+ exit(1);
+ }
+
+ object_unparent(OBJECT(cpu));
+}
+
+static void qemu_tcg_destroy_vcpu(CPUState *cpu)
+{
+ CPU_REMOVE(cpu);
+ object_unparent(OBJECT(cpu));
+}
+
static void flush_queued_work(CPUState *cpu)
{
struct qemu_work_item *wi;
@@ -942,6 +960,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
}
}
qemu_kvm_wait_io_event(cpu);
+ if (cpu->exit && !cpu_can_run(cpu)) {
+ qemu_kvm_destroy_vcpu(cpu);
+ qemu_mutex_unlock(&qemu_global_mutex);
+ return NULL;
+ }
}
return NULL;
@@ -994,6 +1017,7 @@ static void tcg_exec_all(void);
static void *qemu_tcg_cpu_thread_fn(void *arg)
{
CPUState *cpu = arg;
+ CPUState *remove_cpu = NULL;
qemu_tcg_init_cpu_signals();
qemu_thread_get_self(cpu->thread);
@@ -1026,6 +1050,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
}
}
qemu_tcg_wait_io_event();
+ CPU_FOREACH(cpu) {
+ if (cpu->exit && !cpu_can_run(cpu)) {
+ remove_cpu = cpu;
+ break;
+ }
+ }
+ if (remove_cpu) {
+ qemu_tcg_destroy_vcpu(remove_cpu);
+ remove_cpu = NULL;
+ }
}
return NULL;
@@ -1383,6 +1417,9 @@ static void tcg_exec_all(void)
break;
}
} else if (cpu->stop || cpu->stopped) {
+ if (cpu->exit) {
+ next_cpu = CPU_NEXT(cpu);
+ }
break;
}
}
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 174ea36..88e2403 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -178,6 +178,7 @@ int kvm_has_intx_set_mask(void);
int kvm_init_vcpu(CPUState *cpu);
int kvm_cpu_exec(CPUState *cpu);
+int kvm_destroy_vcpu(CPUState *cpu);
#ifdef NEED_CPU_H
diff --git a/kvm-all.c b/kvm-all.c
index 1402f4f..d0caeff 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -74,6 +74,12 @@ typedef struct KVMSlot
typedef struct kvm_dirty_log KVMDirtyLog;
+struct KVMParkedVcpu {
+ unsigned long vcpu_id;
+ int kvm_fd;
+ QLIST_ENTRY(KVMParkedVcpu) node;
+};
+
struct KVMState
{
KVMSlot *slots;
@@ -108,6 +114,7 @@ struct KVMState
QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
bool direct_msi;
#endif
+ QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
};
KVMState *kvm_state;
@@ -226,6 +233,53 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
}
+int kvm_destroy_vcpu(CPUState *cpu)
+{
+ KVMState *s = kvm_state;
+ long mmap_size;
+ struct KVMParkedVcpu *vcpu = NULL;
+ int ret = 0;
+
+ DPRINTF("kvm_destroy_vcpu\n");
+
+ mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
+ if (mmap_size < 0) {
+ ret = mmap_size;
+ DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
+ goto err;
+ }
+
+ ret = munmap(cpu->kvm_run, mmap_size);
+ if (ret < 0) {
+ goto err;
+ }
+
+ vcpu = g_malloc0(sizeof(*vcpu));
+ vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
+ vcpu->kvm_fd = cpu->kvm_fd;
+ QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
+err:
+ return ret;
+}
+
+static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
+{
+ struct KVMParkedVcpu *cpu;
+
+ QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
+ if (cpu->vcpu_id == vcpu_id) {
+ int kvm_fd;
+
+ QLIST_REMOVE(cpu, node);
+ kvm_fd = cpu->kvm_fd;
+ g_free(cpu);
+ return kvm_fd;
+ }
+ }
+
+ return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
+}
+
int kvm_init_vcpu(CPUState *cpu)
{
KVMState *s = kvm_state;
@@ -234,7 +288,7 @@ int kvm_init_vcpu(CPUState *cpu)
DPRINTF("kvm_init_vcpu\n");
- ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
+ ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
if (ret < 0) {
DPRINTF("kvm_create_vcpu failed\n");
goto err;
@@ -1404,6 +1458,7 @@ int kvm_init(MachineClass *mc)
#ifdef KVM_CAP_SET_GUEST_DEBUG
QTAILQ_INIT(&s->kvm_sw_breakpoints);
#endif
+ QLIST_INIT(&s->kvm_parked_vcpus);
s->vmfd = -1;
s->fd = qemu_open("/dev/kvm", O_RDWR);
if (s->fd == -1) {
--
1.7.7
^ permalink raw reply related [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects Gu Zheng
@ 2014-09-09 14:40 ` Igor Mammedov
2014-09-10 3:54 ` Gu Zheng
2014-09-11 9:35 ` Bharata B Rao
` (2 subsequent siblings)
3 siblings, 1 reply; 52+ messages in thread
From: Igor Mammedov @ 2014-09-09 14:40 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:42 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> After ACPI get a signal to eject a vCPU, the vCPU must be
> removed from CPU list，before the vCPU really removed, then
> release the all related vCPU objects.
> But we do not close KVM vcpu fd, just record it into a list, in
> order to reuse it.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> cpus.c | 37 ++++++++++++++++++++++++++++++++
> include/sysemu/kvm.h | 1 +
> kvm-all.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 94 insertions(+), 1 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index eee693b..0608b41 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -851,6 +851,24 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> qemu_cpu_kick(cpu);
> }
>
> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
> +{
> + CPU_REMOVE(cpu);
> +
> + if (kvm_destroy_vcpu(cpu) < 0) {
> + fprintf(stderr, "kvm_destroy_vcpu failed.\n");
> + exit(1);
> + }
> +
> + object_unparent(OBJECT(cpu));
> +}
> +
> +static void qemu_tcg_destroy_vcpu(CPUState *cpu)
> +{
> + CPU_REMOVE(cpu);
> + object_unparent(OBJECT(cpu));
> +}
> +
> static void flush_queued_work(CPUState *cpu)
> {
> struct qemu_work_item *wi;
> @@ -942,6 +960,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
> }
> }
> qemu_kvm_wait_io_event(cpu);
> + if (cpu->exit && !cpu_can_run(cpu)) {
> + qemu_kvm_destroy_vcpu(cpu);
> + qemu_mutex_unlock(&qemu_global_mutex);
> + return NULL;
> + }
> }
>
> return NULL;
> @@ -994,6 +1017,7 @@ static void tcg_exec_all(void);
> static void *qemu_tcg_cpu_thread_fn(void *arg)
> {
> CPUState *cpu = arg;
> + CPUState *remove_cpu = NULL;
>
> qemu_tcg_init_cpu_signals();
> qemu_thread_get_self(cpu->thread);
> @@ -1026,6 +1050,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> }
> }
> qemu_tcg_wait_io_event();
> + CPU_FOREACH(cpu) {
> + if (cpu->exit && !cpu_can_run(cpu)) {
> + remove_cpu = cpu;
> + break;
> + }
> + }
> + if (remove_cpu) {
> + qemu_tcg_destroy_vcpu(remove_cpu);
> + remove_cpu = NULL;
> + }
> }
>
> return NULL;
> @@ -1383,6 +1417,9 @@ static void tcg_exec_all(void)
> break;
> }
> } else if (cpu->stop || cpu->stopped) {
> + if (cpu->exit) {
> + next_cpu = CPU_NEXT(cpu);
> + }
> break;
> }
> }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 174ea36..88e2403 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -178,6 +178,7 @@ int kvm_has_intx_set_mask(void);
>
> int kvm_init_vcpu(CPUState *cpu);
> int kvm_cpu_exec(CPUState *cpu);
> +int kvm_destroy_vcpu(CPUState *cpu);
>
> #ifdef NEED_CPU_H
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 1402f4f..d0caeff 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -74,6 +74,12 @@ typedef struct KVMSlot
>
> typedef struct kvm_dirty_log KVMDirtyLog;
>
> +struct KVMParkedVcpu {
> + unsigned long vcpu_id;
> + int kvm_fd;
> + QLIST_ENTRY(KVMParkedVcpu) node;
> +};
> +
> struct KVMState
> {
> KVMSlot *slots;
> @@ -108,6 +114,7 @@ struct KVMState
> QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
> bool direct_msi;
> #endif
> + QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
> };
>
> KVMState *kvm_state;
> @@ -226,6 +233,53 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
> return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
> }
>
> +int kvm_destroy_vcpu(CPUState *cpu)
> +{
> + KVMState *s = kvm_state;
> + long mmap_size;
> + struct KVMParkedVcpu *vcpu = NULL;
> + int ret = 0;
> +
> + DPRINTF("kvm_destroy_vcpu\n");
maybe it would be good to put cpu in RESET state before parking it?
Do we need a kernel patch to park VCPU so it wouldn't be possible
to wake it up by sending INIT/SIPI to it?
> +
> + mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
> + if (mmap_size < 0) {
> + ret = mmap_size;
> + DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
> + goto err;
> + }
> +
> + ret = munmap(cpu->kvm_run, mmap_size);
> + if (ret < 0) {
> + goto err;
> + }
> +
> + vcpu = g_malloc0(sizeof(*vcpu));
> + vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
> + vcpu->kvm_fd = cpu->kvm_fd;
> + QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
> +err:
> + return ret;
> +}
> +
> +static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
> +{
> + struct KVMParkedVcpu *cpu;
> +
> + QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
> + if (cpu->vcpu_id == vcpu_id) {
> + int kvm_fd;
> +
> + QLIST_REMOVE(cpu, node);
> + kvm_fd = cpu->kvm_fd;
> + g_free(cpu);
> + return kvm_fd;
> + }
> + }
> +
> + return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
> +}
> +
> int kvm_init_vcpu(CPUState *cpu)
> {
> KVMState *s = kvm_state;
> @@ -234,7 +288,7 @@ int kvm_init_vcpu(CPUState *cpu)
>
> DPRINTF("kvm_init_vcpu\n");
>
> - ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
> + ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
> if (ret < 0) {
> DPRINTF("kvm_create_vcpu failed\n");
> goto err;
> @@ -1404,6 +1458,7 @@ int kvm_init(MachineClass *mc)
> #ifdef KVM_CAP_SET_GUEST_DEBUG
> QTAILQ_INIT(&s->kvm_sw_breakpoints);
> #endif
> + QLIST_INIT(&s->kvm_parked_vcpus);
> s->vmfd = -1;
> s->fd = qemu_open("/dev/kvm", O_RDWR);
> if (s->fd == -1) {
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-09 14:40 ` Igor Mammedov
@ 2014-09-10 3:54 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-10 3:54 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
Hi Igor,
On 09/09/2014 10:40 PM, Igor Mammedov wrote:
> On Thu, 28 Aug 2014 11:36:42 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>
>> After ACPI get a signal to eject a vCPU, the vCPU must be
>> removed from CPU list，before the vCPU really removed, then
>> release the all related vCPU objects.
>> But we do not close KVM vcpu fd, just record it into a list, in
>> order to reuse it.
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>> cpus.c | 37 ++++++++++++++++++++++++++++++++
>> include/sysemu/kvm.h | 1 +
>> kvm-all.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 94 insertions(+), 1 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index eee693b..0608b41 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -851,6 +851,24 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
>> qemu_cpu_kick(cpu);
>> }
>>
>> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
>> +{
>> + CPU_REMOVE(cpu);
>> +
>> + if (kvm_destroy_vcpu(cpu) < 0) {
>> + fprintf(stderr, "kvm_destroy_vcpu failed.\n");
>> + exit(1);
>> + }
>> +
>> + object_unparent(OBJECT(cpu));
>> +}
>> +
>> +static void qemu_tcg_destroy_vcpu(CPUState *cpu)
>> +{
>> + CPU_REMOVE(cpu);
>> + object_unparent(OBJECT(cpu));
>> +}
>> +
>> static void flush_queued_work(CPUState *cpu)
>> {
>> struct qemu_work_item *wi;
>> @@ -942,6 +960,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>> }
>> }
>> qemu_kvm_wait_io_event(cpu);
>> + if (cpu->exit && !cpu_can_run(cpu)) {
>> + qemu_kvm_destroy_vcpu(cpu);
>> + qemu_mutex_unlock(&qemu_global_mutex);
>> + return NULL;
>> + }
>> }
>>
>> return NULL;
>> @@ -994,6 +1017,7 @@ static void tcg_exec_all(void);
>> static void *qemu_tcg_cpu_thread_fn(void *arg)
>> {
>> CPUState *cpu = arg;
>> + CPUState *remove_cpu = NULL;
>>
>> qemu_tcg_init_cpu_signals();
>> qemu_thread_get_self(cpu->thread);
>> @@ -1026,6 +1050,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>> }
>> }
>> qemu_tcg_wait_io_event();
>> + CPU_FOREACH(cpu) {
>> + if (cpu->exit && !cpu_can_run(cpu)) {
>> + remove_cpu = cpu;
>> + break;
>> + }
>> + }
>> + if (remove_cpu) {
>> + qemu_tcg_destroy_vcpu(remove_cpu);
>> + remove_cpu = NULL;
>> + }
>> }
>>
>> return NULL;
>> @@ -1383,6 +1417,9 @@ static void tcg_exec_all(void)
>> break;
>> }
>> } else if (cpu->stop || cpu->stopped) {
>> + if (cpu->exit) {
>> + next_cpu = CPU_NEXT(cpu);
>> + }
>> break;
>> }
>> }
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index 174ea36..88e2403 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -178,6 +178,7 @@ int kvm_has_intx_set_mask(void);
>>
>> int kvm_init_vcpu(CPUState *cpu);
>> int kvm_cpu_exec(CPUState *cpu);
>> +int kvm_destroy_vcpu(CPUState *cpu);
>>
>> #ifdef NEED_CPU_H
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 1402f4f..d0caeff 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -74,6 +74,12 @@ typedef struct KVMSlot
>>
>> typedef struct kvm_dirty_log KVMDirtyLog;
>>
>> +struct KVMParkedVcpu {
>> + unsigned long vcpu_id;
>> + int kvm_fd;
>> + QLIST_ENTRY(KVMParkedVcpu) node;
>> +};
>> +
>> struct KVMState
>> {
>> KVMSlot *slots;
>> @@ -108,6 +114,7 @@ struct KVMState
>> QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
>> bool direct_msi;
>> #endif
>> + QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
>> };
>>
>> KVMState *kvm_state;
>> @@ -226,6 +233,53 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>> return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
>> }
>>
>> +int kvm_destroy_vcpu(CPUState *cpu)
>> +{
>> + KVMState *s = kvm_state;
>> + long mmap_size;
>> + struct KVMParkedVcpu *vcpu = NULL;
>> + int ret = 0;
>> +
>> + DPRINTF("kvm_destroy_vcpu\n");
> maybe it would be good to put cpu in RESET state before parking it?
It would be better.
>
> Do we need a kernel patch to park VCPU so it wouldn't be possible
> to wake it up by sending INIT/SIPI to it?
We sent a kernel patch before, but things is harder than we think, and
it will lead to performance regression. So the suggestion is just parking
vcpu in the QEMU side.
Thanks,
Gu
>
>> +
>> + mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
>> + if (mmap_size < 0) {
>> + ret = mmap_size;
>> + DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
>> + goto err;
>> + }
>> +
>> + ret = munmap(cpu->kvm_run, mmap_size);
>> + if (ret < 0) {
>> + goto err;
>> + }
>> +
>> + vcpu = g_malloc0(sizeof(*vcpu));
>> + vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
>> + vcpu->kvm_fd = cpu->kvm_fd;
>> + QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
>> +err:
>> + return ret;
>> +}
>> +
>> +static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
>> +{
>> + struct KVMParkedVcpu *cpu;
>> +
>> + QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
>> + if (cpu->vcpu_id == vcpu_id) {
>> + int kvm_fd;
>> +
>> + QLIST_REMOVE(cpu, node);
>> + kvm_fd = cpu->kvm_fd;
>> + g_free(cpu);
>> + return kvm_fd;
>> + }
>> + }
>> +
>> + return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
>> +}
>> +
>> int kvm_init_vcpu(CPUState *cpu)
>> {
>> KVMState *s = kvm_state;
>> @@ -234,7 +288,7 @@ int kvm_init_vcpu(CPUState *cpu)
>>
>> DPRINTF("kvm_init_vcpu\n");
>>
>> - ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
>> + ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
>> if (ret < 0) {
>> DPRINTF("kvm_create_vcpu failed\n");
>> goto err;
>> @@ -1404,6 +1458,7 @@ int kvm_init(MachineClass *mc)
>> #ifdef KVM_CAP_SET_GUEST_DEBUG
>> QTAILQ_INIT(&s->kvm_sw_breakpoints);
>> #endif
>> + QLIST_INIT(&s->kvm_parked_vcpus);
>> s->vmfd = -1;
>> s->fd = qemu_open("/dev/kvm", O_RDWR);
>> if (s->fd == -1) {
>
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects Gu Zheng
2014-09-09 14:40 ` Igor Mammedov
@ 2014-09-11 9:35 ` Bharata B Rao
2014-09-11 9:49 ` Gu Zheng
` (2 more replies)
2014-09-12 14:15 ` Igor Mammedov
2014-12-08 9:16 ` Bharata B Rao
3 siblings, 3 replies; 52+ messages in thread
From: Bharata B Rao @ 2014-09-11 9:35 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, anshul.makkar,
chen.fan.fnst, imammedo, afaerber
On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> After ACPI get a signal to eject a vCPU, the vCPU must be
> removed from CPU list，before the vCPU really removed, then
> release the all related vCPU objects.
> But we do not close KVM vcpu fd, just record it into a list, in
> order to reuse it.
After I add and delete a CPU, "info cpus" from monitor still lists the
removed CPU. Is this expected ?
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-11 9:35 ` Bharata B Rao
@ 2014-09-11 9:49 ` Gu Zheng
2014-09-11 9:53 ` Gu Zheng
2014-09-11 10:03 ` Anshul Makkar
2 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-11 9:49 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, isimatu.yasuaki, anshul.makkar,
chen.fan.fnst, imammedo, afaerber
On 09/11/2014 05:35 PM, Bharata B Rao wrote:
> On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> After ACPI get a signal to eject a vCPU, the vCPU must be
>> removed from CPU list，before the vCPU really removed, then
>> release the all related vCPU objects.
>> But we do not close KVM vcpu fd, just record it into a list, in
>> order to reuse it.
>
> After I add and delete a CPU, "info cpus" from monitor still lists the
> removed CPU. Is this expected ?
No, neither from QEMU (e.g. monitor) nor guest side, you can not see the
removed cpu, but from the kernel side it is still there, just no one uses it
any more.
Thanks,
Gu
>
> Regards,
> Bharata.
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-11 9:35 ` Bharata B Rao
2014-09-11 9:49 ` Gu Zheng
@ 2014-09-11 9:53 ` Gu Zheng
2014-09-11 12:37 ` Bharata B Rao
2014-09-11 10:03 ` Anshul Makkar
2 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-09-11 9:53 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, isimatu.yasuaki, anshul.makkar,
chen.fan.fnst, imammedo, afaerber
On 09/11/2014 05:35 PM, Bharata B Rao wrote:
> On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> After ACPI get a signal to eject a vCPU, the vCPU must be
>> removed from CPU list，before the vCPU really removed, then
>> release the all related vCPU objects.
>> But we do not close KVM vcpu fd, just record it into a list, in
>> order to reuse it.
>
> After I add and delete a CPU, "info cpus" from monitor still lists the
> removed CPU. Is this expected ?
No, you can not see the removed cpu from QEMU (e.g. monitor) or guest side,
but from the kernel side it is still there, just no one uses it any more.
Thanks,
Gu
>
> Regards,
> Bharata.
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-11 9:53 ` Gu Zheng
@ 2014-09-11 12:37 ` Bharata B Rao
2014-09-12 1:24 ` Gu Zheng
0 siblings, 1 reply; 52+ messages in thread
From: Bharata B Rao @ 2014-09-11 12:37 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, anshul.makkar,
chen.fan.fnst, imammedo, afaerber
On Thu, Sep 11, 2014 at 3:23 PM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> On 09/11/2014 05:35 PM, Bharata B Rao wrote:
>
>> On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>>> After ACPI get a signal to eject a vCPU, the vCPU must be
>>> removed from CPU list，before the vCPU really removed, then
>>> release the all related vCPU objects.
>>> But we do not close KVM vcpu fd, just record it into a list, in
>>> order to reuse it.
>>
>> After I add and delete a CPU, "info cpus" from monitor still lists the
>> removed CPU. Is this expected ?
>
> No, you can not see the removed cpu from QEMU (e.g. monitor) or guest side,
> but from the kernel side it is still there, just no one uses it any more.
I am trying your patches, but still see the removed CPU after deletion.
(qemu) info cpus
* CPU #0: pc=0xffffffff8100b82c (halted) thread_id=7812
CPU #1: pc=0xffffffff8100b82c (halted) thread_id=7813
CPU #2: pc=0xffffffff8100b82c (halted) thread_id=7814
CPU #3: pc=0xffffffff8100b82c (halted) thread_id=7815
CPU #4: pc=0xffffffff8100b82c (halted) thread_id=7816
CPU #5: pc=0xffffffff8100b82c (halted) thread_id=7817
CPU #6: pc=0xffffffff8100b82c (halted) thread_id=7818
CPU #7: pc=0xffffffff8100b82c (halted) thread_id=7819
(qemu) device_add qemu64-x86_64-cpu,id=cpu8
(qemu) info cpus
* CPU #0: pc=0xffffffff8100b82c (halted) thread_id=7812
CPU #1: pc=0xffffffff8100b82c (halted) thread_id=7813
CPU #2: pc=0xffffffff8100b82c (halted) thread_id=7814
CPU #3: pc=0xffffffff8100b82c (halted) thread_id=7815
CPU #4: pc=0xffffffff8100b82c (halted) thread_id=7816
CPU #5: pc=0xffffffff8100b82c (halted) thread_id=7817
CPU #6: pc=0xffffffff8100b82c (halted) thread_id=7818
CPU #7: pc=0xffffffff8100b82c (halted) thread_id=7819
CPU #8: pc=0xffffffff8100b82c (halted) thread_id=8041
(qemu) device_del cpu8
(qemu) info cpus
* CPU #0: pc=0xffffffff8100b82c (halted) thread_id=7812
CPU #1: pc=0xffffffff8100b82c (halted) thread_id=7813
CPU #2: pc=0xffffffff8100b82c (halted) thread_id=7814
CPU #3: pc=0xffffffff8100b82c (halted) thread_id=7815
CPU #4: pc=0xffffffff8100b82c (halted) thread_id=7816
CPU #5: pc=0xffffffff8100b82c (halted) thread_id=7817
CPU #6: pc=0xffffffff8100b82c (halted) thread_id=7818
CPU #7: pc=0xffffffff8100b82c (halted) thread_id=7819
CPU #8: pc=0xffffffff81031722 (halted) thread_id=8041
I applied your patchset on commit 69f87f713069f1f70, may be I should
try with latest QEMU ?
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-11 12:37 ` Bharata B Rao
@ 2014-09-12 1:24 ` Gu Zheng
2014-09-12 8:09 ` Bharata B Rao
0 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-09-12 1:24 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, isimatu.yasuaki, anshul.makkar,
chen.fan.fnst, imammedo, afaerber
Hi Bharata,
On 09/11/2014 08:37 PM, Bharata B Rao wrote:
> On Thu, Sep 11, 2014 at 3:23 PM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> On 09/11/2014 05:35 PM, Bharata B Rao wrote:
>>
>>> On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>>>> After ACPI get a signal to eject a vCPU, the vCPU must be
>>>> removed from CPU list，before the vCPU really removed, then
>>>> release the all related vCPU objects.
>>>> But we do not close KVM vcpu fd, just record it into a list, in
>>>> order to reuse it.
>>>
>>> After I add and delete a CPU, "info cpus" from monitor still lists the
>>> removed CPU. Is this expected ?
>>
>> No, you can not see the removed cpu from QEMU (e.g. monitor) or guest side,
>> but from the kernel side it is still there, just no one uses it any more.
>
> I am trying your patches, but still see the removed CPU after deletion.
>
> (qemu) info cpus
> * CPU #0: pc=0xffffffff8100b82c (halted) thread_id=7812
> CPU #1: pc=0xffffffff8100b82c (halted) thread_id=7813
> CPU #2: pc=0xffffffff8100b82c (halted) thread_id=7814
> CPU #3: pc=0xffffffff8100b82c (halted) thread_id=7815
> CPU #4: pc=0xffffffff8100b82c (halted) thread_id=7816
> CPU #5: pc=0xffffffff8100b82c (halted) thread_id=7817
> CPU #6: pc=0xffffffff8100b82c (halted) thread_id=7818
> CPU #7: pc=0xffffffff8100b82c (halted) thread_id=7819
> (qemu) device_add qemu64-x86_64-cpu,id=cpu8
> (qemu) info cpus
> * CPU #0: pc=0xffffffff8100b82c (halted) thread_id=7812
> CPU #1: pc=0xffffffff8100b82c (halted) thread_id=7813
> CPU #2: pc=0xffffffff8100b82c (halted) thread_id=7814
> CPU #3: pc=0xffffffff8100b82c (halted) thread_id=7815
> CPU #4: pc=0xffffffff8100b82c (halted) thread_id=7816
> CPU #5: pc=0xffffffff8100b82c (halted) thread_id=7817
> CPU #6: pc=0xffffffff8100b82c (halted) thread_id=7818
> CPU #7: pc=0xffffffff8100b82c (halted) thread_id=7819
> CPU #8: pc=0xffffffff8100b82c (halted) thread_id=8041
> (qemu) device_del cpu8
> (qemu) info cpus
> * CPU #0: pc=0xffffffff8100b82c (halted) thread_id=7812
> CPU #1: pc=0xffffffff8100b82c (halted) thread_id=7813
> CPU #2: pc=0xffffffff8100b82c (halted) thread_id=7814
> CPU #3: pc=0xffffffff8100b82c (halted) thread_id=7815
> CPU #4: pc=0xffffffff8100b82c (halted) thread_id=7816
> CPU #5: pc=0xffffffff8100b82c (halted) thread_id=7817
> CPU #6: pc=0xffffffff8100b82c (halted) thread_id=7818
> CPU #7: pc=0xffffffff8100b82c (halted) thread_id=7819
> CPU #8: pc=0xffffffff81031722 (halted) thread_id=8041
>
> I applied your patchset on commit 69f87f713069f1f70, may be I should
> try with latest QEMU ?
Is guest os enabled acpi cpu hotplug? What's the guest's cpu info?
Please try latest QEMU, and any feedback is welcome.
Thanks,
Gu
>
> Regards,
> Bharata.
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 1:24 ` Gu Zheng
@ 2014-09-12 8:09 ` Bharata B Rao
2014-09-12 9:53 ` Gu Zheng
0 siblings, 1 reply; 52+ messages in thread
From: Bharata B Rao @ 2014-09-12 8:09 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, Anshul Makkar,
chen.fan.fnst, imammedo, afaerber
On Fri, Sep 12, 2014 at 6:54 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Is guest os enabled acpi cpu hotplug? What's the guest's cpu info?
> Please try latest QEMU, and any feedback is welcome.
>
Tried with latest QEMU git + your patchset and Fedora 20 guest, but
QEMU monitor still shows the removed CPU.
Guest kernel messages during hotplug:
[root@localhost cpu]# echo 1 > cpu8/online
[ 72.936069] smpboot: Booting Node 0 Processor 8 APIC 0x8
[ 0.003000] kvm-clock: cpu 8, msr 0:7ffc9201, secondary cpu clock
[ 72.950003] TSC synchronization [CPU#0 -> CPU#8]:
[ 72.950003] Measured 199886723309 cycles TSC warp between CPUs,
turning off TSC clock.
[ 72.950003] tsc: Marking TSC unstable due to check_tsc_sync_source failed
[ 72.972976] KVM setup async PF for cpu 8
[ 72.973648] kvm-stealtime: cpu 8, msr 7d30df00
[ 72.974415] Will online and init hotplugged CPU: 8
[ 72.975307] microcode: CPU8 sig=0x663, pf=0x1, revision=0x1
Guest kernel messages during hotunplug:
[root@localhost cpu]# [ 95.482172] Unregister pv shared memory for cpu 8
[ 95.487169] smpboot: CPU 8 is now offline
[ 95.488667] ACPI: Device does not support D3cold
Guest cpuinfo (showing for the last CPU only after adding and removing CPU 8)
processor : 7
vendor_id : GenuineIntel
cpu family : 6
model : 6
model name : QEMU Virtual CPU version 2.1.50
stepping : 3
microcode : 0x1
cpu MHz : 2899.998
cache size : 4096 KB
fpu : yes
fpu_exception : yes
cpuid level : 4
wp : yes
flags : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca
cmov pse36 clflush mmx fxsr sse sse2 syscall nx lm rep_good nopl pni
cx16 x2apic popcnt hypervisor lahf_lm
bogomips : 5799.99
clflush size : 64
cache_alignment : 64
address sizes : 40 bits physical, 48 bits virtual
power management:
[root@localhost boot]# grep -ir hot config-3.11.10-301.fc20.x86_64
CONFIG_TICK_ONESHOT=y
# CONFIG_MEMORY_HOTPLUG is not set
CONFIG_HOTPLUG_CPU=y
# CONFIG_BOOTPARAM_HOTPLUG_CPU0 is not set
# CONFIG_DEBUG_HOTPLUG_CPU0 is not set
CONFIG_ARCH_ENABLE_MEMORY_HOTPLUG=y
CONFIG_ACPI_HOTPLUG_CPU=y
CONFIG_HOTPLUG_PCI_PCIE=y
CONFIG_HOTPLUG_PCI=y
CONFIG_HOTPLUG_PCI_ACPI=y
CONFIG_HOTPLUG_PCI_ACPI_IBM=m
# CONFIG_HOTPLUG_PCI_CPCI is not set
CONFIG_HOTPLUG_PCI_SHPC=m
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 8:09 ` Bharata B Rao
@ 2014-09-12 9:53 ` Gu Zheng
2014-09-12 10:30 ` Bharata B Rao
2014-09-12 10:53 ` Anshul Makkar
0 siblings, 2 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-12 9:53 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, isimatu.yasuaki, Anshul Makkar,
chen.fan.fnst, imammedo, afaerber
Hi Bharata,
On 09/12/2014 04:09 PM, Bharata B Rao wrote:
> On Fri, Sep 12, 2014 at 6:54 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> Is guest os enabled acpi cpu hotplug? What's the guest's cpu info?
>> Please try latest QEMU, and any feedback is welcome.
>>
>
> Tried with latest QEMU git + your patchset and Fedora 20 guest, but
> QEMU monitor still shows the removed CPU.
>
> Guest kernel messages during hotplug:
>
> [root@localhost cpu]# echo 1 > cpu8/online
> [ 72.936069] smpboot: Booting Node 0 Processor 8 APIC 0x8
> [ 0.003000] kvm-clock: cpu 8, msr 0:7ffc9201, secondary cpu clock
> [ 72.950003] TSC synchronization [CPU#0 -> CPU#8]:
> [ 72.950003] Measured 199886723309 cycles TSC warp between CPUs,
> turning off TSC clock.
> [ 72.950003] tsc: Marking TSC unstable due to check_tsc_sync_source failed
> [ 72.972976] KVM setup async PF for cpu 8
> [ 72.973648] kvm-stealtime: cpu 8, msr 7d30df00
> [ 72.974415] Will online and init hotplugged CPU: 8
> [ 72.975307] microcode: CPU8 sig=0x663, pf=0x1, revision=0x1
>
> Guest kernel messages during hotunplug:
>
> [root@localhost cpu]# [ 95.482172] Unregister pv shared memory for cpu 8
> [ 95.487169] smpboot: CPU 8 is now offline
> [ 95.488667] ACPI: Device does not support D3cold
>
>
> Guest cpuinfo (showing for the last CPU only after adding and removing CPU 8)
>
> processor : 7
> vendor_id : GenuineIntel
> cpu family : 6
> model : 6
> model name : QEMU Virtual CPU version 2.1.50
> stepping : 3
> microcode : 0x1
> cpu MHz : 2899.998
> cache size : 4096 KB
> fpu : yes
> fpu_exception : yes
> cpuid level : 4
> wp : yes
> flags : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca
> cmov pse36 clflush mmx fxsr sse sse2 syscall nx lm rep_good nopl pni
> cx16 x2apic popcnt hypervisor lahf_lm
> bogomips : 5799.99
> clflush size : 64
> cache_alignment : 64
> address sizes : 40 bits physical, 48 bits virtual
> power management:
Guest ejected CPU 8 successfully.
I confirmed it with the same environment as yours, it works well.
Could you please offer your QEMU config and the guest start cmd?
It may help me to investigate the issue.
Thanks,
Gu
>
> [root@localhost boot]# grep -ir hot config-3.11.10-301.fc20.x86_64
> CONFIG_TICK_ONESHOT=y
> # CONFIG_MEMORY_HOTPLUG is not set
> CONFIG_HOTPLUG_CPU=y
> # CONFIG_BOOTPARAM_HOTPLUG_CPU0 is not set
> # CONFIG_DEBUG_HOTPLUG_CPU0 is not set
> CONFIG_ARCH_ENABLE_MEMORY_HOTPLUG=y
> CONFIG_ACPI_HOTPLUG_CPU=y
> CONFIG_HOTPLUG_PCI_PCIE=y
> CONFIG_HOTPLUG_PCI=y
> CONFIG_HOTPLUG_PCI_ACPI=y
> CONFIG_HOTPLUG_PCI_ACPI_IBM=m
> # CONFIG_HOTPLUG_PCI_CPCI is not set
> CONFIG_HOTPLUG_PCI_SHPC=m
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 9:53 ` Gu Zheng
@ 2014-09-12 10:30 ` Bharata B Rao
2014-09-12 10:53 ` Anshul Makkar
1 sibling, 0 replies; 52+ messages in thread
From: Bharata B Rao @ 2014-09-12 10:30 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, Anshul Makkar,
chen.fan.fnst, imammedo, afaerber
On Fri, Sep 12, 2014 at 3:23 PM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Hi Bharata,
> On 09/12/2014 04:09 PM, Bharata B Rao wrote:
>
>> On Fri, Sep 12, 2014 at 6:54 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>>> Is guest os enabled acpi cpu hotplug? What's the guest's cpu info?
>>> Please try latest QEMU, and any feedback is welcome.
>>>
>>
>> Tried with latest QEMU git + your patchset and Fedora 20 guest, but
>> QEMU monitor still shows the removed CPU.
>>
>> Guest kernel messages during hotplug:
>>
>> [root@localhost cpu]# echo 1 > cpu8/online
>> [ 72.936069] smpboot: Booting Node 0 Processor 8 APIC 0x8
>> [ 0.003000] kvm-clock: cpu 8, msr 0:7ffc9201, secondary cpu clock
>> [ 72.950003] TSC synchronization [CPU#0 -> CPU#8]:
>> [ 72.950003] Measured 199886723309 cycles TSC warp between CPUs,
>> turning off TSC clock.
>> [ 72.950003] tsc: Marking TSC unstable due to check_tsc_sync_source failed
>> [ 72.972976] KVM setup async PF for cpu 8
>> [ 72.973648] kvm-stealtime: cpu 8, msr 7d30df00
>> [ 72.974415] Will online and init hotplugged CPU: 8
>> [ 72.975307] microcode: CPU8 sig=0x663, pf=0x1, revision=0x1
>>
>> Guest kernel messages during hotunplug:
>>
>> [root@localhost cpu]# [ 95.482172] Unregister pv shared memory for cpu 8
>> [ 95.487169] smpboot: CPU 8 is now offline
>> [ 95.488667] ACPI: Device does not support D3cold
>>
>>
>> Guest cpuinfo (showing for the last CPU only after adding and removing CPU 8)
>>
>> processor : 7
>> vendor_id : GenuineIntel
>> cpu family : 6
>> model : 6
>> model name : QEMU Virtual CPU version 2.1.50
>> stepping : 3
>> microcode : 0x1
>> cpu MHz : 2899.998
>> cache size : 4096 KB
>> fpu : yes
>> fpu_exception : yes
>> cpuid level : 4
>> wp : yes
>> flags : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca
>> cmov pse36 clflush mmx fxsr sse sse2 syscall nx lm rep_good nopl pni
>> cx16 x2apic popcnt hypervisor lahf_lm
>> bogomips : 5799.99
>> clflush size : 64
>> cache_alignment : 64
>> address sizes : 40 bits physical, 48 bits virtual
>> power management:
>
> Guest ejected CPU 8 successfully.
> I confirmed it with the same environment as yours, it works well.
> Could you please offer your QEMU config and the guest start cmd?
> It may help me to investigate the issue.
Let me also debug this a bit in my environment, but here are the
details you asked for.
# ./x86_64-softmmu/qemu-system-x86_64 --nographic --enable-kvm -m 2G
-smp 8,maxcpus=32 -device virtio-blk-pci,drive=rootdisk -drive
file=/home/bharata/F20.img,if=none,cache=none,format=qcow2,id=rootdisk
-monitor telnet:localhost:1234,server,nowait
# cat config-host.h
/* Automatically generated by create_config - do not modify */
#define CONFIG_QEMU_CONFDIR "/usr/local/etc/qemu"
#define CONFIG_QEMU_DATADIR "/usr/local/share/qemu"
#define CONFIG_QEMU_DOCDIR "/usr/local/share/doc/qemu"
#define CONFIG_QEMU_MODDIR "/usr/local/lib/qemu"
#define CONFIG_QEMU_LOCALSTATEDIR "/usr/local/var"
#define CONFIG_QEMU_HELPERDIR "/usr/local/libexec"
#define CONFIG_QEMU_LOCALEDIR "/usr/local/share/locale"
#define HOST_X86_64 1
#define CONFIG_DEBUG_TCG 1
#define CONFIG_POSIX 1
#define CONFIG_LINUX 1
#define CONFIG_SLIRP 1
#define CONFIG_SMBD_COMMAND "/usr/sbin/smbd"
#define CONFIG_L2TPV3 1
#define CONFIG_AUDIO_DRIVERS \
&oss_audio_driver,\
#define CONFIG_OSS 1
#define CONFIG_BDRV_RW_WHITELIST\
NULL
#define CONFIG_BDRV_RO_WHITELIST\
NULL
#define CONFIG_VNC 1
#define CONFIG_FNMATCH 1
#define QEMU_VERSION "2.1.50"
#define QEMU_PKGVERSION ""
#define CONFIG_UTIMENSAT 1
#define CONFIG_PIPE2 1
#define CONFIG_ACCEPT4 1
#define CONFIG_SPLICE 1
#define CONFIG_EVENTFD 1
#define CONFIG_FALLOCATE 1
#define CONFIG_FALLOCATE_PUNCH_HOLE 1
#define CONFIG_SYNC_FILE_RANGE 1
#define CONFIG_FIEMAP 1
#define CONFIG_DUP3 1
#define CONFIG_PPOLL 1
#define CONFIG_PRCTL_PR_SET_TIMERSLACK 1
#define CONFIG_EPOLL 1
#define CONFIG_EPOLL_CREATE1 1
#define CONFIG_EPOLL_PWAIT 1
#define CONFIG_SENDFILE 1
#define CONFIG_TIMERFD 1
#define CONFIG_INOTIFY 1
#define CONFIG_INOTIFY1 1
#define CONFIG_BYTESWAP_H 1
#define CONFIG_ATTR 1
#define CONFIG_VHOST_SCSI 1
#define CONFIG_VHOST_NET_USED 1
#define CONFIG_IOVEC 1
#define CONFIG_PREADV 1
#define CONFIG_SIGNALFD 1
#define CONFIG_FDATASYNC 1
#define CONFIG_MADVISE 1
#define CONFIG_POSIX_MADVISE 1
#define CONFIG_SIGEV_THREAD_ID 1
#define CONFIG_QOM_CAST_DEBUG 1
#define CONFIG_COROUTINE_BACKEND ucontext
#define CONFIG_COROUTINE_POOL 1
#define CONFIG_LINUX_MAGIC_H 1
#define CONFIG_HAS_ENVIRON 1
#define CONFIG_CPUID_H 1
#define CONFIG_INT128 1
#define CONFIG_TPM $(CONFIG_SOFTMMU)
#define CONFIG_TPM_PASSTHROUGH 1
#define CONFIG_TRACE_NOP 1
#define CONFIG_TRACE_FILE trace
#define CONFIG_THREAD_SETNAME_BYTHREAD 1
#define CONFIG_PTHREAD_SETNAME_NP 1
#define HOST_DSOSUF ".so
And I am on commit ID 0dfa7e30126364c434a4
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 9:53 ` Gu Zheng
2014-09-12 10:30 ` Bharata B Rao
@ 2014-09-12 10:53 ` Anshul Makkar
2014-09-12 13:52 ` Bharata B Rao
1 sibling, 1 reply; 52+ messages in thread
From: Anshul Makkar @ 2014-09-12 10:53 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, Bharata B Rao, ChenFan,
Igor Mammedov, afaerber
[-- Attachment #1: Type: text/plain, Size: 3245 bytes --]
During plugging we can see this event: echo 1 > cpu8/online.
But during unplugging , we can't see the event echo 0 > cpu8/online.
Just for additional check, in my code I have added following udev rule
echo 0 > cpu[0-9]*/online. May be this is of any help.
Thanks
Anshul Makkar
On Fri, Sep 12, 2014 at 11:53 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Hi Bharata,
> On 09/12/2014 04:09 PM, Bharata B Rao wrote:
>
> > On Fri, Sep 12, 2014 at 6:54 AM, Gu Zheng <guz.fnst@cn.fujitsu.com>
> wrote:
> >> Is guest os enabled acpi cpu hotplug? What's the guest's cpu info?
> >> Please try latest QEMU, and any feedback is welcome.
> >>
> >
> > Tried with latest QEMU git + your patchset and Fedora 20 guest, but
> > QEMU monitor still shows the removed CPU.
> >
> > Guest kernel messages during hotplug:
> >
> > [root@localhost cpu]# echo 1 > cpu8/online
> > [ 72.936069] smpboot: Booting Node 0 Processor 8 APIC 0x8
> > [ 0.003000] kvm-clock: cpu 8, msr 0:7ffc9201, secondary cpu clock
> > [ 72.950003] TSC synchronization [CPU#0 -> CPU#8]:
> > [ 72.950003] Measured 199886723309 cycles TSC warp between CPUs,
> > turning off TSC clock.
> > [ 72.950003] tsc: Marking TSC unstable due to check_tsc_sync_source
> failed
> > [ 72.972976] KVM setup async PF for cpu 8
> > [ 72.973648] kvm-stealtime: cpu 8, msr 7d30df00
> > [ 72.974415] Will online and init hotplugged CPU: 8
> > [ 72.975307] microcode: CPU8 sig=0x663, pf=0x1, revision=0x1
> >
> > Guest kernel messages during hotunplug:
> >
> > [root@localhost cpu]# [ 95.482172] Unregister pv shared memory for
> cpu 8
> > [ 95.487169] smpboot: CPU 8 is now offline
> > [ 95.488667] ACPI: Device does not support D3cold
> >
> >
> > Guest cpuinfo (showing for the last CPU only after adding and removing
> CPU 8)
> >
> > processor : 7
> > vendor_id : GenuineIntel
> > cpu family : 6
> > model : 6
> > model name : QEMU Virtual CPU version 2.1.50
> > stepping : 3
> > microcode : 0x1
> > cpu MHz : 2899.998
> > cache size : 4096 KB
> > fpu : yes
> > fpu_exception : yes
> > cpuid level : 4
> > wp : yes
> > flags : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca
> > cmov pse36 clflush mmx fxsr sse sse2 syscall nx lm rep_good nopl pni
> > cx16 x2apic popcnt hypervisor lahf_lm
> > bogomips : 5799.99
> > clflush size : 64
> > cache_alignment : 64
> > address sizes : 40 bits physical, 48 bits virtual
> > power management:
>
> Guest ejected CPU 8 successfully.
> I confirmed it with the same environment as yours, it works well.
> Could you please offer your QEMU config and the guest start cmd?
> It may help me to investigate the issue.
>
> Thanks,
> Gu
>
> >
> > [root@localhost boot]# grep -ir hot config-3.11.10-301.fc20.x86_64
> > CONFIG_TICK_ONESHOT=y
> > # CONFIG_MEMORY_HOTPLUG is not set
> > CONFIG_HOTPLUG_CPU=y
> > # CONFIG_BOOTPARAM_HOTPLUG_CPU0 is not set
> > # CONFIG_DEBUG_HOTPLUG_CPU0 is not set
> > CONFIG_ARCH_ENABLE_MEMORY_HOTPLUG=y
> > CONFIG_ACPI_HOTPLUG_CPU=y
> > CONFIG_HOTPLUG_PCI_PCIE=y
> > CONFIG_HOTPLUG_PCI=y
> > CONFIG_HOTPLUG_PCI_ACPI=y
> > CONFIG_HOTPLUG_PCI_ACPI_IBM=m
> > # CONFIG_HOTPLUG_PCI_CPCI is not set
> > CONFIG_HOTPLUG_PCI_SHPC=m
> > .
> >
>
>
>
[-- Attachment #2: Type: text/html, Size: 4246 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 10:53 ` Anshul Makkar
@ 2014-09-12 13:52 ` Bharata B Rao
2014-09-12 15:34 ` Anshul Makkar
2014-09-15 6:39 ` Gu Zheng
0 siblings, 2 replies; 52+ messages in thread
From: Bharata B Rao @ 2014-09-12 13:52 UTC (permalink / raw)
To: Anshul Makkar
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Gu Zheng,
Igor Mammedov, afaerber
On Fri, Sep 12, 2014 at 4:23 PM, Anshul Makkar
<anshul.makkar@profitbricks.com> wrote:
> During plugging we can see this event: echo 1 > cpu8/online.
>
> But during unplugging , we can't see the event echo 0 > cpu8/online.
That's because I didn't do that explicitly, was always trying to
remove an online cpu from the monitor w/o explicitly offlining it from
inside the guest. Either ways I still see the removed CPU being listed
in QEMU monitor.
I don't ever hit any of the below code paths during CPU removal:
cpus.c: qemu_kvm_destroy_vcpu()
cpus.c: x86_cpu_finalizefn()
I see CPU_REMOVE() being called from above two routines.
And neither does hw/acpi/cpu_hotplug.c:cpu_status_write() gets called
here. Does the message "ACPI: Device does not support D3cold" guest
kernel throws during hot removal is causing this behaviour here ?
Guest kernel is 3.11.10, should I be on latest kernel ?
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 13:52 ` Bharata B Rao
@ 2014-09-12 15:34 ` Anshul Makkar
2014-09-15 6:39 ` Gu Zheng
1 sibling, 0 replies; 52+ messages in thread
From: Anshul Makkar @ 2014-09-12 15:34 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Gu Zheng,
Igor Mammedov, afaerber
[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]
I have tested with 3.11 kernel, Kernel should be fine.. But it wouldn't
harm testing with latest kernel, may be it can provide some extra hints..
Anshul Makkar
On Fri, Sep 12, 2014 at 3:52 PM, Bharata B Rao <bharata.rao@gmail.com>
wrote:
> On Fri, Sep 12, 2014 at 4:23 PM, Anshul Makkar
> <anshul.makkar@profitbricks.com> wrote:
> > During plugging we can see this event: echo 1 > cpu8/online.
> >
> > But during unplugging , we can't see the event echo 0 > cpu8/online.
>
> That's because I didn't do that explicitly, was always trying to
> remove an online cpu from the monitor w/o explicitly offlining it from
> inside the guest. Either ways I still see the removed CPU being listed
> in QEMU monitor.
>
> I don't ever hit any of the below code paths during CPU removal:
>
> cpus.c: qemu_kvm_destroy_vcpu()
> cpus.c: x86_cpu_finalizefn()
>
> I see CPU_REMOVE() being called from above two routines.
>
> And neither does hw/acpi/cpu_hotplug.c:cpu_status_write() gets called
> here. Does the message "ACPI: Device does not support D3cold" guest
> kernel throws during hot removal is causing this behaviour here ?
> Guest kernel is 3.11.10, should I be on latest kernel ?
>
> Regards,
> Bharata.
>
[-- Attachment #2: Type: text/html, Size: 1711 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 13:52 ` Bharata B Rao
2014-09-12 15:34 ` Anshul Makkar
@ 2014-09-15 6:39 ` Gu Zheng
2014-09-15 10:09 ` Bharata B Rao
1 sibling, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-09-15 6:39 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, isimatu.yasuaki, Igor Mammedov, ChenFan,
Anshul Makkar, afaerber
Hi Bharata,
On 09/12/2014 09:52 PM, Bharata B Rao wrote:
> On Fri, Sep 12, 2014 at 4:23 PM, Anshul Makkar
> <anshul.makkar@profitbricks.com> wrote:
>> During plugging we can see this event: echo 1 > cpu8/online.
>>
>> But during unplugging , we can't see the event echo 0 > cpu8/online.
>
> That's because I didn't do that explicitly, was always trying to
> remove an online cpu from the monitor w/o explicitly offlining it from
> inside the guest. Either ways I still see the removed CPU being listed
> in QEMU monitor.
>
> I don't ever hit any of the below code paths during CPU removal:
It seems that the guest OS did not call the "_EJ0" method.
Could you please dump the guest os' acpi dsdt, and check whether the "_EJ0"
method exists?
>
> cpus.c: qemu_kvm_destroy_vcpu()
> cpus.c: x86_cpu_finalizefn()
>
> I see CPU_REMOVE() being called from above two routines.
>
> And neither does hw/acpi/cpu_hotplug.c:cpu_status_write() gets called
> here. Does the message "ACPI: Device does not support D3cold" guest
> kernel throws during hot removal is causing this behaviour here ?
> Guest kernel is 3.11.10, should I be on latest kernel ?
I have tested the fedora 20 guest with the default kernel, it works well,
so the kernel should be fine.
One more question, is the guest kernel config the same as fedora 20's default one?
Thanks,
Gu
>
> Regards,
> Bharata.
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-15 6:39 ` Gu Zheng
@ 2014-09-15 10:09 ` Bharata B Rao
2014-09-15 10:33 ` Anshul Makkar
0 siblings, 1 reply; 52+ messages in thread
From: Bharata B Rao @ 2014-09-15 10:09 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, Igor Mammedov, ChenFan,
Anshul Makkar, afaerber
On Mon, Sep 15, 2014 at 12:09 PM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Hi Bharata,
> On 09/12/2014 09:52 PM, Bharata B Rao wrote:
>
>> On Fri, Sep 12, 2014 at 4:23 PM, Anshul Makkar
>> <anshul.makkar@profitbricks.com> wrote:
>>> During plugging we can see this event: echo 1 > cpu8/online.
>>>
>>> But during unplugging , we can't see the event echo 0 > cpu8/online.
>>
>> That's because I didn't do that explicitly, was always trying to
>> remove an online cpu from the monitor w/o explicitly offlining it from
>> inside the guest. Either ways I still see the removed CPU being listed
>> in QEMU monitor.
>>
>> I don't ever hit any of the below code paths during CPU removal:
>
> It seems that the guest OS did not call the "_EJ0" method.
> Could you please dump the guest os' acpi dsdt, and check whether the "_EJ0"
> method exists?
_EJ0 doesn't exist in my DSDT.
>
>>
>> cpus.c: qemu_kvm_destroy_vcpu()
>> cpus.c: x86_cpu_finalizefn()
>>
>> I see CPU_REMOVE() being called from above two routines.
>>
>> And neither does hw/acpi/cpu_hotplug.c:cpu_status_write() gets called
>> here. Does the message "ACPI: Device does not support D3cold" guest
>> kernel throws during hot removal is causing this behaviour here ?
>> Guest kernel is 3.11.10, should I be on latest kernel ?
>
> I have tested the fedora 20 guest with the default kernel, it works well,
> so the kernel should be fine.
> One more question, is the guest kernel config the same as fedora 20's default one?
Yes, I am running the distro provided kernel.
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-15 10:09 ` Bharata B Rao
@ 2014-09-15 10:33 ` Anshul Makkar
2014-09-15 13:53 ` Bharata B Rao
0 siblings, 1 reply; 52+ messages in thread
From: Anshul Makkar @ 2014-09-15 10:33 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Gu Zheng,
Igor Mammedov, afaerber
[-- Attachment #1: Type: text/plain, Size: 503 bytes --]
That explains the cause.
Please verify you have the iasl compiler installed and are not using the
hold .hex (compile .dsl ) files. (Faced this issue in our build setup using
sbuil.).
I hope you have verified that your .dsl file has the changes as mentioned
in the patch.
I have also verified with fedora 20 (unmodified kernel) and cpu-plug/unplug
is working fine.
Thanks
Anshul Makkar
On Mon, Sep 15, 2014 at 12:09 PM, Bharata B Rao <bharata.rao@gmail.com>
wrote:
> _EJ0 doesn't exist in my DSDT.
[-- Attachment #2: Type: text/html, Size: 991 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-15 10:33 ` Anshul Makkar
@ 2014-09-15 13:53 ` Bharata B Rao
2014-09-15 14:29 ` Anshul Makkar
0 siblings, 1 reply; 52+ messages in thread
From: Bharata B Rao @ 2014-09-15 13:53 UTC (permalink / raw)
To: Anshul Makkar
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Gu Zheng,
Igor Mammedov, afaerber
On Mon, Sep 15, 2014 at 4:03 PM, Anshul Makkar
<anshul.makkar@profitbricks.com> wrote:
> That explains the cause.
>
> Please verify you have the iasl compiler installed and are not using the
> hold .hex (compile .dsl ) files. (Faced this issue in our build setup using
> sbuil.).
>
> I hope you have verified that your .dsl file has the changes as mentioned in
> the patch.
Oh, it was not obvious to me that I have to install iasl, use
--iasl=XXX during configure stage to get the new _EJ0 method to be
included in the ACPI DSDT!
So finally I now see the CPU getting removed from QEMU. Thanks for all
the inputs.
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-15 13:53 ` Bharata B Rao
@ 2014-09-15 14:29 ` Anshul Makkar
0 siblings, 0 replies; 52+ messages in thread
From: Anshul Makkar @ 2014-09-15 14:29 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Gu Zheng,
Igor Mammedov, afaerber
[-- Attachment #1: Type: text/plain, Size: 781 bytes --]
Great !!
Anshul Makkar
On Mon, Sep 15, 2014 at 3:53 PM, Bharata B Rao <bharata.rao@gmail.com>
wrote:
> On Mon, Sep 15, 2014 at 4:03 PM, Anshul Makkar
> <anshul.makkar@profitbricks.com> wrote:
> > That explains the cause.
> >
> > Please verify you have the iasl compiler installed and are not using the
> > hold .hex (compile .dsl ) files. (Faced this issue in our build setup
> using
> > sbuil.).
> >
> > I hope you have verified that your .dsl file has the changes as
> mentioned in
> > the patch.
>
> Oh, it was not obvious to me that I have to install iasl, use
> --iasl=XXX during configure stage to get the new _EJ0 method to be
> included in the ACPI DSDT!
>
> So finally I now see the CPU getting removed from QEMU. Thanks for all
> the inputs.
>
> Regards,
> Bharata.
>
[-- Attachment #2: Type: text/html, Size: 1259 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-11 9:35 ` Bharata B Rao
2014-09-11 9:49 ` Gu Zheng
2014-09-11 9:53 ` Gu Zheng
@ 2014-09-11 10:03 ` Anshul Makkar
2 siblings, 0 replies; 52+ messages in thread
From: Anshul Makkar @ 2014-09-11 10:03 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Gu Zheng,
Igor Mammedov, afaerber
[-- Attachment #1: Type: text/plain, Size: 202 bytes --]
Bharata, this not expected. info cpus should indicate report proper number
of cpus after deletion.
Anshul Makkar
On Thu, Sep 11, 2014 at 11:35 AM, Bharata B Rao <bharata.rao@gmail.com>
wrote:
> from
[-- Attachment #2: Type: text/html, Size: 542 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects Gu Zheng
2014-09-09 14:40 ` Igor Mammedov
2014-09-11 9:35 ` Bharata B Rao
@ 2014-09-12 14:15 ` Igor Mammedov
2014-09-15 5:03 ` Gu Zheng
2014-12-08 9:16 ` Bharata B Rao
3 siblings, 1 reply; 52+ messages in thread
From: Igor Mammedov @ 2014-09-12 14:15 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Thu, 28 Aug 2014 11:36:42 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> After ACPI get a signal to eject a vCPU, the vCPU must be
> removed from CPU list，before the vCPU really removed, then
> release the all related vCPU objects.
> But we do not close KVM vcpu fd, just record it into a list, in
> order to reuse it.
An additional question,
Have you checked if migration works after CPU unplug?
>
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> cpus.c | 37 ++++++++++++++++++++++++++++++++
> include/sysemu/kvm.h | 1 +
> kvm-all.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 94 insertions(+), 1 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index eee693b..0608b41 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -851,6 +851,24 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> qemu_cpu_kick(cpu);
> }
>
> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
> +{
> + CPU_REMOVE(cpu);
> +
> + if (kvm_destroy_vcpu(cpu) < 0) {
> + fprintf(stderr, "kvm_destroy_vcpu failed.\n");
> + exit(1);
> + }
> +
> + object_unparent(OBJECT(cpu));
> +}
> +
> +static void qemu_tcg_destroy_vcpu(CPUState *cpu)
> +{
> + CPU_REMOVE(cpu);
> + object_unparent(OBJECT(cpu));
> +}
> +
> static void flush_queued_work(CPUState *cpu)
> {
> struct qemu_work_item *wi;
> @@ -942,6 +960,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
> }
> }
> qemu_kvm_wait_io_event(cpu);
> + if (cpu->exit && !cpu_can_run(cpu)) {
> + qemu_kvm_destroy_vcpu(cpu);
> + qemu_mutex_unlock(&qemu_global_mutex);
> + return NULL;
> + }
> }
>
> return NULL;
> @@ -994,6 +1017,7 @@ static void tcg_exec_all(void);
> static void *qemu_tcg_cpu_thread_fn(void *arg)
> {
> CPUState *cpu = arg;
> + CPUState *remove_cpu = NULL;
>
> qemu_tcg_init_cpu_signals();
> qemu_thread_get_self(cpu->thread);
> @@ -1026,6 +1050,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> }
> }
> qemu_tcg_wait_io_event();
> + CPU_FOREACH(cpu) {
> + if (cpu->exit && !cpu_can_run(cpu)) {
> + remove_cpu = cpu;
> + break;
> + }
> + }
> + if (remove_cpu) {
> + qemu_tcg_destroy_vcpu(remove_cpu);
> + remove_cpu = NULL;
> + }
> }
>
> return NULL;
> @@ -1383,6 +1417,9 @@ static void tcg_exec_all(void)
> break;
> }
> } else if (cpu->stop || cpu->stopped) {
> + if (cpu->exit) {
> + next_cpu = CPU_NEXT(cpu);
> + }
> break;
> }
> }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 174ea36..88e2403 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -178,6 +178,7 @@ int kvm_has_intx_set_mask(void);
>
> int kvm_init_vcpu(CPUState *cpu);
> int kvm_cpu_exec(CPUState *cpu);
> +int kvm_destroy_vcpu(CPUState *cpu);
>
> #ifdef NEED_CPU_H
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 1402f4f..d0caeff 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -74,6 +74,12 @@ typedef struct KVMSlot
>
> typedef struct kvm_dirty_log KVMDirtyLog;
>
> +struct KVMParkedVcpu {
> + unsigned long vcpu_id;
> + int kvm_fd;
> + QLIST_ENTRY(KVMParkedVcpu) node;
> +};
> +
> struct KVMState
> {
> KVMSlot *slots;
> @@ -108,6 +114,7 @@ struct KVMState
> QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
> bool direct_msi;
> #endif
> + QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
> };
>
> KVMState *kvm_state;
> @@ -226,6 +233,53 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
> return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
> }
>
> +int kvm_destroy_vcpu(CPUState *cpu)
> +{
> + KVMState *s = kvm_state;
> + long mmap_size;
> + struct KVMParkedVcpu *vcpu = NULL;
> + int ret = 0;
> +
> + DPRINTF("kvm_destroy_vcpu\n");
> +
> + mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
> + if (mmap_size < 0) {
> + ret = mmap_size;
> + DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
> + goto err;
> + }
> +
> + ret = munmap(cpu->kvm_run, mmap_size);
> + if (ret < 0) {
> + goto err;
> + }
> +
> + vcpu = g_malloc0(sizeof(*vcpu));
> + vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
> + vcpu->kvm_fd = cpu->kvm_fd;
> + QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
> +err:
> + return ret;
> +}
> +
> +static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
> +{
> + struct KVMParkedVcpu *cpu;
> +
> + QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
> + if (cpu->vcpu_id == vcpu_id) {
> + int kvm_fd;
> +
> + QLIST_REMOVE(cpu, node);
> + kvm_fd = cpu->kvm_fd;
> + g_free(cpu);
> + return kvm_fd;
> + }
> + }
> +
> + return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
> +}
> +
> int kvm_init_vcpu(CPUState *cpu)
> {
> KVMState *s = kvm_state;
> @@ -234,7 +288,7 @@ int kvm_init_vcpu(CPUState *cpu)
>
> DPRINTF("kvm_init_vcpu\n");
>
> - ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
> + ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
> if (ret < 0) {
> DPRINTF("kvm_create_vcpu failed\n");
> goto err;
> @@ -1404,6 +1458,7 @@ int kvm_init(MachineClass *mc)
> #ifdef KVM_CAP_SET_GUEST_DEBUG
> QTAILQ_INIT(&s->kvm_sw_breakpoints);
> #endif
> + QLIST_INIT(&s->kvm_parked_vcpus);
> s->vmfd = -1;
> s->fd = qemu_open("/dev/kvm", O_RDWR);
> if (s->fd == -1) {
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-09-12 14:15 ` Igor Mammedov
@ 2014-09-15 5:03 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-09-15 5:03 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
Hi Igor,
On 09/12/2014 10:15 PM, Igor Mammedov wrote:
> On Thu, 28 Aug 2014 11:36:42 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>
>> After ACPI get a signal to eject a vCPU, the vCPU must be
>> removed from CPU list，before the vCPU really removed, then
>> release the all related vCPU objects.
>> But we do not close KVM vcpu fd, just record it into a list, in
>> order to reuse it.
> An additional question,
> Have you checked if migration works after CPU unplug?
Yes, migration works fine after CPU unplug.
Thanks,
Gu
>
>>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>> cpus.c | 37 ++++++++++++++++++++++++++++++++
>> include/sysemu/kvm.h | 1 +
>> kvm-all.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 94 insertions(+), 1 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index eee693b..0608b41 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -851,6 +851,24 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
>> qemu_cpu_kick(cpu);
>> }
>>
>> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
>> +{
>> + CPU_REMOVE(cpu);
>> +
>> + if (kvm_destroy_vcpu(cpu) < 0) {
>> + fprintf(stderr, "kvm_destroy_vcpu failed.\n");
>> + exit(1);
>> + }
>> +
>> + object_unparent(OBJECT(cpu));
>> +}
>> +
>> +static void qemu_tcg_destroy_vcpu(CPUState *cpu)
>> +{
>> + CPU_REMOVE(cpu);
>> + object_unparent(OBJECT(cpu));
>> +}
>> +
>> static void flush_queued_work(CPUState *cpu)
>> {
>> struct qemu_work_item *wi;
>> @@ -942,6 +960,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>> }
>> }
>> qemu_kvm_wait_io_event(cpu);
>> + if (cpu->exit && !cpu_can_run(cpu)) {
>> + qemu_kvm_destroy_vcpu(cpu);
>> + qemu_mutex_unlock(&qemu_global_mutex);
>> + return NULL;
>> + }
>> }
>>
>> return NULL;
>> @@ -994,6 +1017,7 @@ static void tcg_exec_all(void);
>> static void *qemu_tcg_cpu_thread_fn(void *arg)
>> {
>> CPUState *cpu = arg;
>> + CPUState *remove_cpu = NULL;
>>
>> qemu_tcg_init_cpu_signals();
>> qemu_thread_get_self(cpu->thread);
>> @@ -1026,6 +1050,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>> }
>> }
>> qemu_tcg_wait_io_event();
>> + CPU_FOREACH(cpu) {
>> + if (cpu->exit && !cpu_can_run(cpu)) {
>> + remove_cpu = cpu;
>> + break;
>> + }
>> + }
>> + if (remove_cpu) {
>> + qemu_tcg_destroy_vcpu(remove_cpu);
>> + remove_cpu = NULL;
>> + }
>> }
>>
>> return NULL;
>> @@ -1383,6 +1417,9 @@ static void tcg_exec_all(void)
>> break;
>> }
>> } else if (cpu->stop || cpu->stopped) {
>> + if (cpu->exit) {
>> + next_cpu = CPU_NEXT(cpu);
>> + }
>> break;
>> }
>> }
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index 174ea36..88e2403 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -178,6 +178,7 @@ int kvm_has_intx_set_mask(void);
>>
>> int kvm_init_vcpu(CPUState *cpu);
>> int kvm_cpu_exec(CPUState *cpu);
>> +int kvm_destroy_vcpu(CPUState *cpu);
>>
>> #ifdef NEED_CPU_H
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 1402f4f..d0caeff 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -74,6 +74,12 @@ typedef struct KVMSlot
>>
>> typedef struct kvm_dirty_log KVMDirtyLog;
>>
>> +struct KVMParkedVcpu {
>> + unsigned long vcpu_id;
>> + int kvm_fd;
>> + QLIST_ENTRY(KVMParkedVcpu) node;
>> +};
>> +
>> struct KVMState
>> {
>> KVMSlot *slots;
>> @@ -108,6 +114,7 @@ struct KVMState
>> QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
>> bool direct_msi;
>> #endif
>> + QLIST_HEAD(, KVMParkedVcpu) kvm_parked_vcpus;
>> };
>>
>> KVMState *kvm_state;
>> @@ -226,6 +233,53 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>> return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
>> }
>>
>> +int kvm_destroy_vcpu(CPUState *cpu)
>> +{
>> + KVMState *s = kvm_state;
>> + long mmap_size;
>> + struct KVMParkedVcpu *vcpu = NULL;
>> + int ret = 0;
>> +
>> + DPRINTF("kvm_destroy_vcpu\n");
>> +
>> + mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
>> + if (mmap_size < 0) {
>> + ret = mmap_size;
>> + DPRINTF("KVM_GET_VCPU_MMAP_SIZE failed\n");
>> + goto err;
>> + }
>> +
>> + ret = munmap(cpu->kvm_run, mmap_size);
>> + if (ret < 0) {
>> + goto err;
>> + }
>> +
>> + vcpu = g_malloc0(sizeof(*vcpu));
>> + vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
>> + vcpu->kvm_fd = cpu->kvm_fd;
>> + QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
>> +err:
>> + return ret;
>> +}
>> +
>> +static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
>> +{
>> + struct KVMParkedVcpu *cpu;
>> +
>> + QLIST_FOREACH(cpu, &s->kvm_parked_vcpus, node) {
>> + if (cpu->vcpu_id == vcpu_id) {
>> + int kvm_fd;
>> +
>> + QLIST_REMOVE(cpu, node);
>> + kvm_fd = cpu->kvm_fd;
>> + g_free(cpu);
>> + return kvm_fd;
>> + }
>> + }
>> +
>> + return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
>> +}
>> +
>> int kvm_init_vcpu(CPUState *cpu)
>> {
>> KVMState *s = kvm_state;
>> @@ -234,7 +288,7 @@ int kvm_init_vcpu(CPUState *cpu)
>>
>> DPRINTF("kvm_init_vcpu\n");
>>
>> - ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)kvm_arch_vcpu_id(cpu));
>> + ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
>> if (ret < 0) {
>> DPRINTF("kvm_create_vcpu failed\n");
>> goto err;
>> @@ -1404,6 +1458,7 @@ int kvm_init(MachineClass *mc)
>> #ifdef KVM_CAP_SET_GUEST_DEBUG
>> QTAILQ_INIT(&s->kvm_sw_breakpoints);
>> #endif
>> + QLIST_INIT(&s->kvm_parked_vcpus);
>> s->vmfd = -1;
>> s->fd = qemu_open("/dev/kvm", O_RDWR);
>> if (s->fd == -1) {
>
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects Gu Zheng
` (2 preceding siblings ...)
2014-09-12 14:15 ` Igor Mammedov
@ 2014-12-08 9:16 ` Bharata B Rao
2014-12-08 9:26 ` Peter Maydell
2014-12-08 10:12 ` Gu Zheng
3 siblings, 2 replies; 52+ messages in thread
From: Bharata B Rao @ 2014-12-08 9:16 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, Anshul Makkar, ChenFan,
Igor Mammedov, afaerber
On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> After ACPI get a signal to eject a vCPU, the vCPU must be
> removed from CPU list，before the vCPU really removed, then
> release the all related vCPU objects.
> But we do not close KVM vcpu fd, just record it into a list, in
> order to reuse it.
Hi Gu Zheng,
So this approach of parking the vcpu fd in QEMU and reusing it next
time during CPU hotplug will only work if all architectures supported
by KVM driver are ok with reuse of vcpu object in the kernel. I am
using relevant bits of this patch of yours with PowerPC architecture
to support cpu unplug and I realize that there is at least one case
where reuse of vcpu object in the kernel causes problems. Before I
attempt to fix that in ppc KVM, I wanted to check if there is
consensus on this approach (parking and reusing kvm vcpu fd) in QEMU.
Also is there a latest version of this patchset that I can try ?
Regards,
Bharata.
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 9:16 ` Bharata B Rao
@ 2014-12-08 9:26 ` Peter Maydell
2014-12-08 10:28 ` Gu Zheng
2014-12-08 10:12 ` Gu Zheng
1 sibling, 1 reply; 52+ messages in thread
From: Peter Maydell @ 2014-12-08 9:26 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Igor Mammedov,
Gu Zheng, Anshul Makkar, afaerber
On 8 December 2014 at 09:16, Bharata B Rao <bharata.rao@gmail.com> wrote:
> So this approach of parking the vcpu fd in QEMU and reusing it next
> time during CPU hotplug will only work if all architectures supported
> by KVM driver are ok with reuse of vcpu object in the kernel. I am
> using relevant bits of this patch of yours with PowerPC architecture
> to support cpu unplug and I realize that there is at least one case
> where reuse of vcpu object in the kernel causes problems. Before I
> attempt to fix that in ppc KVM, I wanted to check if there is
> consensus on this approach (parking and reusing kvm vcpu fd) in QEMU.
For ARM we've just clarified the kernel-to-userspace ABI, and
among other things it says you can't reuse the VCPU unless it's
for exactly the same CPU type and feature configuration. So
I'm not sure this park-and-reuse approach will work...
-- PMM
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 9:26 ` Peter Maydell
@ 2014-12-08 10:28 ` Gu Zheng
2014-12-08 10:50 ` Peter Maydell
0 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-12-08 10:28 UTC (permalink / raw)
To: Peter Maydell
Cc: Igor Mammedov, qemu-devel, tangchen, isimatu.yasuaki,
Bharata B Rao, ChenFan, Anshul Makkar, afaerber
Hi Peter,
On 12/08/2014 05:26 PM, Peter Maydell wrote:
> On 8 December 2014 at 09:16, Bharata B Rao <bharata.rao@gmail.com> wrote:
>> So this approach of parking the vcpu fd in QEMU and reusing it next
>> time during CPU hotplug will only work if all architectures supported
>> by KVM driver are ok with reuse of vcpu object in the kernel. I am
>> using relevant bits of this patch of yours with PowerPC architecture
>> to support cpu unplug and I realize that there is at least one case
>> where reuse of vcpu object in the kernel causes problems. Before I
>> attempt to fix that in ppc KVM, I wanted to check if there is
>> consensus on this approach (parking and reusing kvm vcpu fd) in QEMU.
>
> For ARM we've just clarified the kernel-to-userspace ABI, and
> among other things it says you can't reuse the VCPU unless it's
> for exactly the same CPU type and feature configuration. So
Yes, it's a limitation now, but it is not the final implementation.
As to the QEMU side, the vcpu object in kernel is stateless, and a
reset operation will be done when we hot add a vcpu, so I think we
can kill the limitation, just choose a parked vcpu and reconfigure
it before we use it.
Thanks,
Gu
> I'm not sure this park-and-reuse approach will work...
>
> -- PMM
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 10:28 ` Gu Zheng
@ 2014-12-08 10:50 ` Peter Maydell
2014-12-08 15:38 ` Igor Mammedov
0 siblings, 1 reply; 52+ messages in thread
From: Peter Maydell @ 2014-12-08 10:50 UTC (permalink / raw)
To: Gu Zheng
Cc: Igor Mammedov, qemu-devel, tangchen, isimatu.yasuaki,
Bharata B Rao, ChenFan, Anshul Makkar, afaerber
On 8 December 2014 at 10:28, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Yes, it's a limitation now, but it is not the final implementation.
> As to the QEMU side, the vcpu object in kernel is stateless, and a
> reset operation will be done when we hot add a vcpu, so I think we
> can kill the limitation, just choose a parked vcpu and reconfigure
> it before we use it.
Why can't the kernel handle our just destroying the vcpu and
later recreating it if necessary? That seems the more logical
approach than trying to keep fds hanging around in userspace
for reuse.
-- PMM
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 10:50 ` Peter Maydell
@ 2014-12-08 15:38 ` Igor Mammedov
2014-12-08 16:38 ` Peter Maydell
0 siblings, 1 reply; 52+ messages in thread
From: Igor Mammedov @ 2014-12-08 15:38 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Bharata B Rao,
Gu Zheng, Anshul Makkar, afaerber
On Mon, 8 Dec 2014 10:50:21 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:
> On 8 December 2014 at 10:28, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> > Yes, it's a limitation now, but it is not the final implementation.
> > As to the QEMU side, the vcpu object in kernel is stateless, and a
> > reset operation will be done when we hot add a vcpu, so I think we
> > can kill the limitation, just choose a parked vcpu and reconfigure
> > it before we use it.
>
> Why can't the kernel handle our just destroying the vcpu and
> later recreating it if necessary? That seems the more logical
> approach than trying to keep fds hanging around in userspace
> for reuse.
It's somewhat complex approach and it was suggested on KVM list to go
parking route. for more details see thread
https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html
>
> -- PMM
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 15:38 ` Igor Mammedov
@ 2014-12-08 16:38 ` Peter Maydell
2014-12-09 0:58 ` Gu Zheng
0 siblings, 1 reply; 52+ messages in thread
From: Peter Maydell @ 2014-12-08 16:38 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, tangchen, ChenFan, isimatu.yasuaki, Bharata B Rao,
Gu Zheng, Anshul Makkar, afaerber
On 8 December 2014 at 15:38, Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 8 Dec 2014 10:50:21 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> Why can't the kernel handle our just destroying the vcpu and
>> later recreating it if necessary? That seems the more logical
>> approach than trying to keep fds hanging around in userspace
>> for reuse.
>
> It's somewhat complex approach and it was suggested on KVM list to go
> parking route. for more details see thread
> https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html
If the kernel can't cope with userspace creating and destroying
vCPUs dynamically then that seems like a kernel bug to me.
It seems better to me to fix that directly rather than make
non-x86 architectures change things around to help with working
around that bug...
thanks
-- PMM
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 16:38 ` Peter Maydell
@ 2014-12-09 0:58 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-12-09 0:58 UTC (permalink / raw)
To: Peter Maydell
Cc: Igor Mammedov, qemu-devel, tangchen, isimatu.yasuaki,
Bharata B Rao, ChenFan, Anshul Makkar, afaerber, Gleb Natapov,
kvm
+cc Gleb, KVM guys,
On 12/09/2014 12:38 AM, Peter Maydell wrote:
> On 8 December 2014 at 15:38, Igor Mammedov <imammedo@redhat.com> wrote:
>> On Mon, 8 Dec 2014 10:50:21 +0000
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> Why can't the kernel handle our just destroying the vcpu and
>>> later recreating it if necessary? That seems the more logical
>>> approach than trying to keep fds hanging around in userspace
>>> for reuse.
>>
>> It's somewhat complex approach and it was suggested on KVM list to go
>> parking route. for more details see thread
>> https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html
>
> If the kernel can't cope with userspace creating and destroying
> vCPUs dynamically then that seems like a kernel bug to me.
Yes, it's a flaw.
> It seems better to me to fix that directly rather than make
> non-x86 architectures change things around to help with working
> around that bug...
Agree.
But as we discussed before:
"CPU array is accessed locklessly in a lot of places, so it will have to be
RCUified. There was attempt to do so 2 year or so ago, but it didn't go anyware.
Adding locks is to big a price to pay for ability to free a little bit
of memory by destroying vcpu."
We worry about the regression if we add lock in a lot of places.
I'm not very familiar with non-x86 architectures. So I'm not sure how long we
need to go to help "vcpu hot-unplug" working with parking route.
Gleb,
Is any guys still working on the RCUing CPUarray access?
Is there any plan for this issue, or just leave it as it is?
Thanks,
Gu
>
> thanks
> -- PMM
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
@ 2014-12-09 0:58 ` Gu Zheng
0 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-12-09 0:58 UTC (permalink / raw)
To: Peter Maydell
Cc: kvm, Gleb Natapov, qemu-devel, tangchen, isimatu.yasuaki,
Bharata B Rao, ChenFan, Igor Mammedov, Anshul Makkar, afaerber
+cc Gleb, KVM guys,
On 12/09/2014 12:38 AM, Peter Maydell wrote:
> On 8 December 2014 at 15:38, Igor Mammedov <imammedo@redhat.com> wrote:
>> On Mon, 8 Dec 2014 10:50:21 +0000
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> Why can't the kernel handle our just destroying the vcpu and
>>> later recreating it if necessary? That seems the more logical
>>> approach than trying to keep fds hanging around in userspace
>>> for reuse.
>>
>> It's somewhat complex approach and it was suggested on KVM list to go
>> parking route. for more details see thread
>> https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html
>
> If the kernel can't cope with userspace creating and destroying
> vCPUs dynamically then that seems like a kernel bug to me.
Yes, it's a flaw.
> It seems better to me to fix that directly rather than make
> non-x86 architectures change things around to help with working
> around that bug...
Agree.
But as we discussed before:
"CPU array is accessed locklessly in a lot of places, so it will have to be
RCUified. There was attempt to do so 2 year or so ago, but it didn't go anyware.
Adding locks is to big a price to pay for ability to free a little bit
of memory by destroying vcpu."
We worry about the regression if we add lock in a lot of places.
I'm not very familiar with non-x86 architectures. So I'm not sure how long we
need to go to help "vcpu hot-unplug" working with parking route.
Gleb,
Is any guys still working on the RCUing CPUarray access?
Is there any plan for this issue, or just leave it as it is?
Thanks,
Gu
>
> thanks
> -- PMM
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects
2014-12-08 9:16 ` Bharata B Rao
2014-12-08 9:26 ` Peter Maydell
@ 2014-12-08 10:12 ` Gu Zheng
1 sibling, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-12-08 10:12 UTC (permalink / raw)
To: Bharata B Rao
Cc: qemu-devel, tangchen, isimatu.yasuaki, Anshul Makkar, ChenFan,
Igor Mammedov, afaerber
Hi Bharata,
On 12/08/2014 05:16 PM, Bharata B Rao wrote:
> On Thu, Aug 28, 2014 at 9:06 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> After ACPI get a signal to eject a vCPU, the vCPU must be
>> removed from CPU list，before the vCPU really removed, then
>> release the all related vCPU objects.
>> But we do not close KVM vcpu fd, just record it into a list, in
>> order to reuse it.
>
> Hi Gu Zheng,
>
> So this approach of parking the vcpu fd in QEMU and reusing it next
> time during CPU hotplug will only work if all architectures supported
> by KVM driver are ok with reuse of vcpu object in the kernel. I am
> using relevant bits of this patch of yours with PowerPC architecture
> to support cpu unplug and I realize that there is at least one case
> where reuse of vcpu object in the kernel causes problems. Before I
> attempt to fix that in ppc KVM, I wanted to check if there is
> consensus on this approach (parking and reusing kvm vcpu fd) in QEMU.
Not yet, also no objection.
But it's the best way that I can find to handle the vcpu object without
touching the CPU array in kernel.
>
> Also is there a latest version of this patchset that I can try ?
A new version is on the respinning, and will be sent out once the 2.2 released.
Thanks,
Gu
>
> Regards,
> Bharata.
> .
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (9 preceding siblings ...)
2014-08-28 3:36 ` [Qemu-devel] [RFC V2 10/10] cpus: reclaim allocated vCPU objects Gu Zheng
@ 2014-11-12 1:46 ` Gu Zheng
2014-11-12 1:46 ` Gu Zheng
11 siblings, 0 replies; 52+ messages in thread
From: Gu Zheng @ 2014-11-12 1:46 UTC (permalink / raw)
To: imammedo
Cc: qemu-devel, tangchen, chen.fan.fnst, isimatu.yasuaki, Gu Zheng,
anshul.makkar, afaerber
Hi Igor,
As your hot-plug rework has been merged in upstream, and we need to respin
the i386 cpu hot remove a lot based on the latest tree.
So I want to split the series into two parts:
Part 1/2: add device_add foo-x86_64-cpu support.
Part 2/2: i386 cpu hot remove support.
, which will be easy to review.
The Part 1/2 will come soon, and Part 2/2 will be sent out once the rework
is done.
What's your opinion?
Thanks,
Gu
On 08/28/2014 11:36 AM, Gu Zheng wrote:
> This series is based on the previous patchset from Chen Fan:
> https://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg02360.html
> https://lists.nongnu.org/archive/html/qemu-devel/2013-12/msg04266.html
>
> Patch 1~3: add device_add foo-x86_64-cpu support
> These three patches try to make cpu hotplug with device_add, and make
> "-device foo-x86_64-cpu" available，also we can set apic-id
> property with command line, if without setting apic-id property,
> we offer the first unoccupied apic id as the default new apic id.
> When hotplug cpu with device_add, additional check of APIC ID will be
> done after cpu object initialization which was different from
> 'cpu_add' command that check 'ids' at the beginning.
>
> Patch 4~10: add i386 cpu hot remove support
> Via implementing ACPI standard methods _EJ0 in ACPI table, after Guest
> OS remove one vCPU online, the fireware will store removed bitmap to
> QEMU, then QEMU could know to notify the assigned vCPU of exiting.
> Meanwhile, intruduce the QOM command 'device_del' to remove vCPU from
> QEMU itself.
>
> v2:
> -fix the cpu index duplicated issue in the QMP/HMP command output.
>
> Chen Fan (6):
> cpu: introduce CpuTopoInfo structure for argument simplification
> cpu: add device_add foo-x86_64-cpu support
> x86: add x86_cpu_unrealizefn() for cpu apic remove
> qom cpu: rename variable 'cpu_added_notifier' to
> 'cpu_hotplug_notifier'
> i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c
> cpu hotplug: implement function cpu_status_write() for vcpu ejection
>
> Gu Zheng (4):
> qom/cpu: move register_vmstate to common CPUClass.realizefn
> i386: add cpu device_del support
> qom cpu: add UNPLUG cpu notify support
> cpus: reclaim allocated vCPU objects
>
> cpus.c | 48 +++++++++++++-
> exec.c | 32 +++++----
> hw/acpi/cpu_hotplug.c | 55 +++++++++++++--
> hw/acpi/ich9.c | 13 ++--
> hw/acpi/piix4.c | 21 +++---
> hw/i386/acpi-dsdt-cpu-hotplug.dsl | 6 ++-
> hw/i386/kvm/apic.c | 8 ++
> hw/i386/pc.c | 2 +-
> hw/intc/apic.c | 10 +++
> hw/intc/apic_common.c | 26 ++++++-
> include/hw/acpi/cpu_hotplug.h | 14 ++++-
> include/hw/acpi/ich9.h | 2 +-
> include/hw/cpu/icc_bus.h | 1 +
> include/hw/i386/apic_internal.h | 3 +
> include/qom/cpu.h | 12 +++
> include/sysemu/kvm.h | 1 +
> include/sysemu/sysemu.h | 2 +-
> kvm-all.c | 57 +++++++++++++++-
> monitor.c | 4 +-
> qdev-monitor.c | 1 +
> qom/cpu.c | 29 +++++++--
> target-i386/cpu-qom.h | 1 +
> target-i386/cpu.c | 138 ++++++++++++++++++++++++++++++++++++-
> target-i386/topology.h | 51 +++++++++----
> 24 files changed, 470 insertions(+), 67 deletions(-)
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support
2014-08-28 3:36 [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
` (10 preceding siblings ...)
2014-11-12 1:46 ` [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support Gu Zheng
@ 2014-11-12 1:46 ` Gu Zheng
2014-11-12 7:57 ` Igor Mammedov
11 siblings, 1 reply; 52+ messages in thread
From: Gu Zheng @ 2014-11-12 1:46 UTC (permalink / raw)
To: imammedo
Cc: qemu-devel, tangchen, chen.fan.fnst, isimatu.yasuaki, Gu Zheng,
anshul.makkar, afaerber
Hi Igor,
As your hot-plug rework has been merged in upstream, and we need to respin
the i386 cpu hot remove a lot based on the latest tree.
So I want to split the series into two parts:
Part 1/2: add device_add foo-x86_64-cpu support.
Part 2/2: i386 cpu hot remove support.
, which will be easy to review.
The Part 1/2 will come soon, and Part 2/2 will be sent out once the rework
is done.
What's your opinion?
Thanks,
Gu
On 08/28/2014 11:36 AM, Gu Zheng wrote:
> This series is based on the previous patchset from Chen Fan:
> https://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg02360.html
> https://lists.nongnu.org/archive/html/qemu-devel/2013-12/msg04266.html
>
> Patch 1~3: add device_add foo-x86_64-cpu support
> These three patches try to make cpu hotplug with device_add, and make
> "-device foo-x86_64-cpu" available，also we can set apic-id
> property with command line, if without setting apic-id property,
> we offer the first unoccupied apic id as the default new apic id.
> When hotplug cpu with device_add, additional check of APIC ID will be
> done after cpu object initialization which was different from
> 'cpu_add' command that check 'ids' at the beginning.
>
> Patch 4~10: add i386 cpu hot remove support
> Via implementing ACPI standard methods _EJ0 in ACPI table, after Guest
> OS remove one vCPU online, the fireware will store removed bitmap to
> QEMU, then QEMU could know to notify the assigned vCPU of exiting.
> Meanwhile, intruduce the QOM command 'device_del' to remove vCPU from
> QEMU itself.
>
> v2:
> -fix the cpu index duplicated issue in the QMP/HMP command output.
>
> Chen Fan (6):
> cpu: introduce CpuTopoInfo structure for argument simplification
> cpu: add device_add foo-x86_64-cpu support
> x86: add x86_cpu_unrealizefn() for cpu apic remove
> qom cpu: rename variable 'cpu_added_notifier' to
> 'cpu_hotplug_notifier'
> i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c
> cpu hotplug: implement function cpu_status_write() for vcpu ejection
>
> Gu Zheng (4):
> qom/cpu: move register_vmstate to common CPUClass.realizefn
> i386: add cpu device_del support
> qom cpu: add UNPLUG cpu notify support
> cpus: reclaim allocated vCPU objects
>
> cpus.c | 48 +++++++++++++-
> exec.c | 32 +++++----
> hw/acpi/cpu_hotplug.c | 55 +++++++++++++--
> hw/acpi/ich9.c | 13 ++--
> hw/acpi/piix4.c | 21 +++---
> hw/i386/acpi-dsdt-cpu-hotplug.dsl | 6 ++-
> hw/i386/kvm/apic.c | 8 ++
> hw/i386/pc.c | 2 +-
> hw/intc/apic.c | 10 +++
> hw/intc/apic_common.c | 26 ++++++-
> include/hw/acpi/cpu_hotplug.h | 14 ++++-
> include/hw/acpi/ich9.h | 2 +-
> include/hw/cpu/icc_bus.h | 1 +
> include/hw/i386/apic_internal.h | 3 +
> include/qom/cpu.h | 12 +++
> include/sysemu/kvm.h | 1 +
> include/sysemu/sysemu.h | 2 +-
> kvm-all.c | 57 +++++++++++++++-
> monitor.c | 4 +-
> qdev-monitor.c | 1 +
> qom/cpu.c | 29 +++++++--
> target-i386/cpu-qom.h | 1 +
> target-i386/cpu.c | 138 ++++++++++++++++++++++++++++++++++++-
> target-i386/topology.h | 51 +++++++++----
> 24 files changed, 470 insertions(+), 67 deletions(-)
>
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: [Qemu-devel] [RFC V2 00/10] cpu: add device_add foo-x86_64-cpu and i386 cpu hot remove support
2014-11-12 1:46 ` Gu Zheng
@ 2014-11-12 7:57 ` Igor Mammedov
0 siblings, 0 replies; 52+ messages in thread
From: Igor Mammedov @ 2014-11-12 7:57 UTC (permalink / raw)
To: Gu Zheng
Cc: qemu-devel, tangchen, isimatu.yasuaki, chen.fan.fnst,
anshul.makkar, afaerber
On Wed, 12 Nov 2014 09:46:30 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> Hi Igor,
> As your hot-plug rework has been merged in upstream, and we need to respin
> the i386 cpu hot remove a lot based on the latest tree.
> So I want to split the series into two parts:
> Part 1/2: add device_add foo-x86_64-cpu support.
> Part 2/2: i386 cpu hot remove support.
> , which will be easy to review.
> The Part 1/2 will come soon, and Part 2/2 will be sent out once the rework
> is done.
> What's your opinion?
It makes sence to me, plug it will be easier to review/ammend smaller series.
>
> Thanks,
> Gu
>
> On 08/28/2014 11:36 AM, Gu Zheng wrote:
>
> > This series is based on the previous patchset from Chen Fan:
> > https://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg02360.html
> > https://lists.nongnu.org/archive/html/qemu-devel/2013-12/msg04266.html
> >
> > Patch 1~3: add device_add foo-x86_64-cpu support
> > These three patches try to make cpu hotplug with device_add, and make
> > "-device foo-x86_64-cpu" available，also we can set apic-id
> > property with command line, if without setting apic-id property,
> > we offer the first unoccupied apic id as the default new apic id.
> > When hotplug cpu with device_add, additional check of APIC ID will be
> > done after cpu object initialization which was different from
> > 'cpu_add' command that check 'ids' at the beginning.
> >
> > Patch 4~10: add i386 cpu hot remove support
> > Via implementing ACPI standard methods _EJ0 in ACPI table, after Guest
> > OS remove one vCPU online, the fireware will store removed bitmap to
> > QEMU, then QEMU could know to notify the assigned vCPU of exiting.
> > Meanwhile, intruduce the QOM command 'device_del' to remove vCPU from
> > QEMU itself.
> >
> > v2:
> > -fix the cpu index duplicated issue in the QMP/HMP command output.
> >
> > Chen Fan (6):
> > cpu: introduce CpuTopoInfo structure for argument simplification
> > cpu: add device_add foo-x86_64-cpu support
> > x86: add x86_cpu_unrealizefn() for cpu apic remove
> > qom cpu: rename variable 'cpu_added_notifier' to
> > 'cpu_hotplug_notifier'
> > i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c
> > cpu hotplug: implement function cpu_status_write() for vcpu ejection
> >
> > Gu Zheng (4):
> > qom/cpu: move register_vmstate to common CPUClass.realizefn
> > i386: add cpu device_del support
> > qom cpu: add UNPLUG cpu notify support
> > cpus: reclaim allocated vCPU objects
> >
> > cpus.c | 48 +++++++++++++-
> > exec.c | 32 +++++----
> > hw/acpi/cpu_hotplug.c | 55 +++++++++++++--
> > hw/acpi/ich9.c | 13 ++--
> > hw/acpi/piix4.c | 21 +++---
> > hw/i386/acpi-dsdt-cpu-hotplug.dsl | 6 ++-
> > hw/i386/kvm/apic.c | 8 ++
> > hw/i386/pc.c | 2 +-
> > hw/intc/apic.c | 10 +++
> > hw/intc/apic_common.c | 26 ++++++-
> > include/hw/acpi/cpu_hotplug.h | 14 ++++-
> > include/hw/acpi/ich9.h | 2 +-
> > include/hw/cpu/icc_bus.h | 1 +
> > include/hw/i386/apic_internal.h | 3 +
> > include/qom/cpu.h | 12 +++
> > include/sysemu/kvm.h | 1 +
> > include/sysemu/sysemu.h | 2 +-
> > kvm-all.c | 57 +++++++++++++++-
> > monitor.c | 4 +-
> > qdev-monitor.c | 1 +
> > qom/cpu.c | 29 +++++++--
> > target-i386/cpu-qom.h | 1 +
> > target-i386/cpu.c | 138 ++++++++++++++++++++++++++++++++++++-
> > target-i386/topology.h | 51 +++++++++----
> > 24 files changed, 470 insertions(+), 67 deletions(-)
> >
>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread