qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support
@ 2013-12-23  9:04 Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU Chen Fan
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

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
'cpu-del' to remove vCPU from QEMU itself. currently, this patches only support
the cpu deleted sequentially from the last one, and command 'cpu-del' would always
delete the last cpu every time. in libvirt, command 'virsh set-vcpus' always add/delete
vcpu sequentially, so I think this patches will be OK.

this work is based on Andreas Färber's qom-next branch tree.
    git://github.com/afaerber/qemu-next.git

this series patches must be used with KVM patch together.

for KVM patches:
    http://comments.gmane.org/gmane.comp.emulators.kvm.devel/114347


v4-v5: delete command 'cpu-del' argument to remove cpu from the last one
rather than specify vcpuid and fix migration bug.

Chen Fan (8):
  x86: move apic_state field from CPUX86State to X86CPU
  x86: add x86_cpu_unrealizefn() for cpu apic remove
  qmp: add 'cpu-del' command support
  qom cpu: rename variable 'cpu_added_notifier' to
    'cpu_hotplug_notifier'
  qom cpu: add UNPLUG cpu notifier support
  i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c
  piix4: implement function cpu_status_write() for vcpu ejection
  cpus: reclaim allocated vCPU objects

 cpu-exec.c                        |  2 +-
 cpus.c                            | 51 ++++++++++++++++++++++++++++--
 hw/acpi/piix4.c                   | 66 ++++++++++++++++++++++++++++++++-------
 hw/i386/acpi-dsdt-cpu-hotplug.dsl |  6 +++-
 hw/i386/kvm/apic.c                |  8 +++++
 hw/i386/kvmvapic.c                |  8 ++---
 hw/i386/pc.c                      | 38 ++++++++++++++++------
 hw/i386/pc_piix.c                 |  3 +-
 hw/intc/apic.c                    | 10 ++++++
 hw/intc/apic_common.c             | 23 +++++++++++++-
 hw/xen/xen_apic.c                 |  8 +++++
 include/hw/boards.h               |  2 ++
 include/hw/cpu/icc_bus.h          |  1 +
 include/hw/i386/apic_internal.h   |  1 +
 include/hw/i386/pc.h              |  1 +
 include/qom/cpu.h                 | 21 +++++++++++++
 include/sysemu/kvm.h              |  1 +
 include/sysemu/sysemu.h           |  2 +-
 kvm-all.c                         | 25 +++++++++++++++
 qapi-schema.json                  | 10 ++++++
 qmp-commands.hx                   | 21 +++++++++++++
 qmp.c                             |  9 ++++++
 qom/cpu.c                         | 26 ++++++++++++---
 target-i386/cpu-qom.h             |  5 +++
 target-i386/cpu.c                 | 66 ++++++++++++++++++++++++++++++++-------
 target-i386/cpu.h                 |  4 ---
 target-i386/helper.c              |  9 +++---
 target-i386/kvm.c                 | 23 ++++++--------
 target-i386/misc_helper.c         |  8 ++---
 29 files changed, 380 insertions(+), 78 deletions(-)

-- 
1.8.1.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23 15:36   ` Andreas Färber
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove Chen Fan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

This motion is preparing for refactoring vCPU apic subsequently.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 cpu-exec.c                |  2 +-
 cpus.c                    |  5 ++---
 hw/i386/kvmvapic.c        |  8 +++-----
 hw/i386/pc.c              | 17 ++++++++---------
 target-i386/cpu-qom.h     |  4 ++++
 target-i386/cpu.c         | 22 ++++++++++------------
 target-i386/cpu.h         |  4 ----
 target-i386/helper.c      |  9 ++++-----
 target-i386/kvm.c         | 23 ++++++++++-------------
 target-i386/misc_helper.c |  8 ++++----
 10 files changed, 46 insertions(+), 56 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 30cfa2a..2711c58 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -320,7 +320,7 @@ int cpu_exec(CPUArchState *env)
 #if !defined(CONFIG_USER_ONLY)
                     if (interrupt_request & CPU_INTERRUPT_POLL) {
                         cpu->interrupt_request &= ~CPU_INTERRUPT_POLL;
-                        apic_poll_irq(env->apic_state);
+                        apic_poll_irq(x86_env_get_cpu(env)->apic_state);
                     }
 #endif
                     if (interrupt_request & CPU_INTERRUPT_INIT) {
diff --git a/cpus.c b/cpus.c
index 01d128d..ca4c59f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1458,12 +1458,11 @@ void qmp_inject_nmi(Error **errp)
 
     CPU_FOREACH(cs) {
         X86CPU *cpu = X86_CPU(cs);
-        CPUX86State *env = &cpu->env;
 
-        if (!env->apic_state) {
+        if (!cpu->apic_state) {
             cpu_interrupt(cs, CPU_INTERRUPT_NMI);
         } else {
-            apic_deliver_nmi(env->apic_state);
+            apic_deliver_nmi(cpu->apic_state);
         }
     }
 #elif defined(TARGET_S390X)
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 44ee62a..72025d0 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -366,7 +366,7 @@ static int vapic_enable(VAPICROMState *s, X86CPU *cpu)
         (((hwaddr)cpu_number) << VAPIC_CPU_SHIFT);
     cpu_physical_memory_rw(vapic_paddr + offsetof(VAPICState, enabled),
                            (void *)&enabled, sizeof(enabled), 1);
-    apic_enable_vapic(cpu->env.apic_state, vapic_paddr);
+    apic_enable_vapic(cpu->apic_state, vapic_paddr);
 
     s->state = VAPIC_ACTIVE;
 
@@ -496,12 +496,10 @@ static void vapic_enable_tpr_reporting(bool enable)
     };
     CPUState *cs;
     X86CPU *cpu;
-    CPUX86State *env;
 
     CPU_FOREACH(cs) {
         cpu = X86_CPU(cs);
-        env = &cpu->env;
-        info.apic = env->apic_state;
+        info.apic = cpu->apic_state;
         run_on_cpu(cs, vapic_do_enable_tpr_reporting, &info);
     }
 }
@@ -700,7 +698,7 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
     default:
     case 4:
         if (!kvm_irqchip_in_kernel()) {
-            apic_poll_irq(env->apic_state);
+            apic_poll_irq(cpu->apic_state);
         }
         break;
     }
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e9831ca..d000995 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -172,13 +172,14 @@ void cpu_smm_update(CPUX86State *env)
 int cpu_get_pic_interrupt(CPUX86State *env)
 {
     int intno;
+    X86CPU *cpu = x86_env_get_cpu(env);
 
-    intno = apic_get_interrupt(env->apic_state);
+    intno = apic_get_interrupt(cpu->apic_state);
     if (intno >= 0) {
         return intno;
     }
     /* read the irq from the PIC */
-    if (!apic_accept_pic_intr(env->apic_state)) {
+    if (!apic_accept_pic_intr(cpu->apic_state)) {
         return -1;
     }
 
@@ -190,15 +191,13 @@ static void pic_irq_request(void *opaque, int irq, int level)
 {
     CPUState *cs = first_cpu;
     X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
 
     DPRINTF("pic_irqs: %s irq %d\n", level? "raise" : "lower", irq);
-    if (env->apic_state) {
+    if (cpu->apic_state) {
         CPU_FOREACH(cs) {
             cpu = X86_CPU(cs);
-            env = &cpu->env;
-            if (apic_accept_pic_intr(env->apic_state)) {
-                apic_deliver_pic_intr(env->apic_state, level);
+            if (apic_accept_pic_intr(cpu->apic_state)) {
+                apic_deliver_pic_intr(cpu->apic_state, level);
             }
         }
     } else {
@@ -913,7 +912,7 @@ DeviceState *cpu_get_current_apic(void)
 {
     if (current_cpu) {
         X86CPU *cpu = X86_CPU(current_cpu);
-        return cpu->env.apic_state;
+        return cpu->apic_state;
     } else {
         return NULL;
     }
@@ -1007,7 +1006,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
     }
 
     /* map APIC MMIO area if CPU has APIC */
-    if (cpu && cpu->env.apic_state) {
+    if (cpu && cpu->apic_state) {
         /* XXX: what if the base changes? */
         sysbus_mmio_map_overlap(SYS_BUS_DEVICE(icc_bridge), 0,
                                 APIC_DEFAULT_ADDRESS, 0x1000);
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index f4fab15..775c82d 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -66,6 +66,10 @@ typedef struct X86CPU {
 
     CPUX86State env;
 
+    /* in order to simplify APIC support, we leave this pointer to the
+       user */
+    struct DeviceState *apic_state;
+
     bool hyperv_vapic;
     bool hyperv_relaxed_timing;
     int hyperv_spinlock_attempts;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bb98f6d..e20b0c8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2449,7 +2449,7 @@ static void x86_cpu_reset(CPUState *s)
 #if !defined(CONFIG_USER_ONLY)
     /* We hard-wire the BSP to the first CPU. */
     if (s->cpu_index == 0) {
-        apic_designate_bsp(env->apic_state);
+        apic_designate_bsp(cpu->apic_state);
     }
 
     s->halted = !cpu_is_bsp(cpu);
@@ -2459,7 +2459,7 @@ static void x86_cpu_reset(CPUState *s)
 #ifndef CONFIG_USER_ONLY
 bool cpu_is_bsp(X86CPU *cpu)
 {
-    return cpu_get_apic_base(cpu->env.apic_state) & MSR_IA32_APICBASE_BSP;
+    return cpu_get_apic_base(cpu->apic_state) & MSR_IA32_APICBASE_BSP;
 }
 
 /* TODO: remove me, when reset over QOM tree is implemented */
@@ -2500,31 +2500,29 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error **errp)
         apic_type = "xen-apic";
     }
 
-    env->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
-    if (env->apic_state == NULL) {
+    cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type);
+    if (cpu->apic_state == NULL) {
         error_setg(errp, "APIC device '%s' could not be created", apic_type);
         return;
     }
 
     object_property_add_child(OBJECT(cpu), "apic",
-                              OBJECT(env->apic_state), NULL);
-    qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
+                              OBJECT(cpu->apic_state), NULL);
+    qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id);
     /* TODO: convert to link<> */
-    apic = APIC_COMMON(env->apic_state);
+    apic = APIC_COMMON(cpu->apic_state);
     apic->cpu = cpu;
 }
 
 static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
 {
-    CPUX86State *env = &cpu->env;
-
-    if (env->apic_state == NULL) {
+    if (cpu->apic_state == NULL) {
         return;
     }
 
-    if (qdev_init(env->apic_state)) {
+    if (qdev_init(cpu->apic_state)) {
         error_setg(errp, "APIC device '%s' could not be initialized",
-                   object_get_typename(OBJECT(env->apic_state)));
+                   object_get_typename(OBJECT(cpu->apic_state)));
         return;
     }
 }
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ea373e8..1d94a9d 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -895,10 +895,6 @@ typedef struct CPUX86State {
     int tsc_khz;
     void *kvm_xsave_buf;
 
-    /* in order to simplify APIC support, we leave this pointer to the
-       user */
-    struct DeviceState *apic_state;
-
     uint64_t mcg_cap;
     uint64_t mcg_ctl;
     uint64_t mce_banks[MCE_BANKS_DEF*4];
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 7c196ff..f2e76ad 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1248,7 +1248,8 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access)
     } else {
         cpu_restore_state(env, env->mem_io_pc);
 
-        apic_handle_tpr_access_report(env->apic_state, env->eip, access);
+        apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state,
+                                      env->eip, access);
     }
 }
 #endif /* !CONFIG_USER_ONLY */
@@ -1295,14 +1296,12 @@ void do_cpu_init(X86CPU *cpu)
     cpu_reset(cs);
     cs->interrupt_request = sipi;
     env->pat = pat;
-    apic_init_reset(env->apic_state);
+    apic_init_reset(cpu->apic_state);
 }
 
 void do_cpu_sipi(X86CPU *cpu)
 {
-    CPUX86State *env = &cpu->env;
-
-    apic_sipi(env->apic_state);
+    apic_sipi(cpu->apic_state);
 }
 #else
 void do_cpu_init(X86CPU *cpu)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1188482..7522e98 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1069,8 +1069,8 @@ static int kvm_put_sregs(X86CPU *cpu)
     sregs.cr3 = env->cr[3];
     sregs.cr4 = env->cr[4];
 
-    sregs.cr8 = cpu_get_apic_tpr(env->apic_state);
-    sregs.apic_base = cpu_get_apic_base(env->apic_state);
+    sregs.cr8 = cpu_get_apic_tpr(cpu->apic_state);
+    sregs.apic_base = cpu_get_apic_base(cpu->apic_state);
 
     sregs.efer = env->efer;
 
@@ -1619,8 +1619,7 @@ static int kvm_get_mp_state(X86CPU *cpu)
 
 static int kvm_get_apic(X86CPU *cpu)
 {
-    CPUX86State *env = &cpu->env;
-    DeviceState *apic = env->apic_state;
+    DeviceState *apic = cpu->apic_state;
     struct kvm_lapic_state kapic;
     int ret;
 
@@ -1637,8 +1636,7 @@ static int kvm_get_apic(X86CPU *cpu)
 
 static int kvm_put_apic(X86CPU *cpu)
 {
-    CPUX86State *env = &cpu->env;
-    DeviceState *apic = env->apic_state;
+    DeviceState *apic = cpu->apic_state;
     struct kvm_lapic_state kapic;
 
     if (apic && kvm_irqchip_in_kernel()) {
@@ -1962,7 +1960,7 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
         }
 
         DPRINTF("setting tpr\n");
-        run->cr8 = cpu_get_apic_tpr(env->apic_state);
+        run->cr8 = cpu_get_apic_tpr(x86_cpu->apic_state);
     }
 }
 
@@ -1976,8 +1974,8 @@ void kvm_arch_post_run(CPUState *cpu, struct kvm_run *run)
     } else {
         env->eflags &= ~IF_MASK;
     }
-    cpu_set_apic_tpr(env->apic_state, run->cr8);
-    cpu_set_apic_base(env->apic_state, run->apic_base);
+    cpu_set_apic_tpr(x86_cpu->apic_state, run->cr8);
+    cpu_set_apic_base(x86_cpu->apic_state, run->apic_base);
 }
 
 int kvm_arch_process_async_events(CPUState *cs)
@@ -2014,7 +2012,7 @@ int kvm_arch_process_async_events(CPUState *cs)
 
     if (cs->interrupt_request & CPU_INTERRUPT_POLL) {
         cs->interrupt_request &= ~CPU_INTERRUPT_POLL;
-        apic_poll_irq(env->apic_state);
+        apic_poll_irq(cpu->apic_state);
     }
     if (((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
          (env->eflags & IF_MASK)) ||
@@ -2032,7 +2030,7 @@ int kvm_arch_process_async_events(CPUState *cs)
     if (cs->interrupt_request & CPU_INTERRUPT_TPR) {
         cs->interrupt_request &= ~CPU_INTERRUPT_TPR;
         kvm_cpu_synchronize_state(cs);
-        apic_handle_tpr_access_report(env->apic_state, env->eip,
+        apic_handle_tpr_access_report(cpu->apic_state, env->eip,
                                       env->tpr_access_type);
     }
 
@@ -2056,11 +2054,10 @@ static int kvm_handle_halt(X86CPU *cpu)
 
 static int kvm_handle_tpr_access(X86CPU *cpu)
 {
-    CPUX86State *env = &cpu->env;
     CPUState *cs = CPU(cpu);
     struct kvm_run *run = cs->kvm_run;
 
-    apic_handle_tpr_access_report(env->apic_state, run->tpr_access.rip,
+    apic_handle_tpr_access_report(cpu->apic_state, run->tpr_access.rip,
                                   run->tpr_access.is_write ? TPR_ACCESS_WRITE
                                                            : TPR_ACCESS_READ);
     return 1;
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index b6307ca..47f6a2f 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -155,7 +155,7 @@ target_ulong helper_read_crN(CPUX86State *env, int reg)
         break;
     case 8:
         if (!(env->hflags2 & HF2_VINTR_MASK)) {
-            val = cpu_get_apic_tpr(env->apic_state);
+            val = cpu_get_apic_tpr(x86_env_get_cpu(env)->apic_state);
         } else {
             val = env->v_tpr;
         }
@@ -179,7 +179,7 @@ void helper_write_crN(CPUX86State *env, int reg, target_ulong t0)
         break;
     case 8:
         if (!(env->hflags2 & HF2_VINTR_MASK)) {
-            cpu_set_apic_tpr(env->apic_state, t0);
+            cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
         }
         env->v_tpr = t0 & 0x0f;
         break;
@@ -286,7 +286,7 @@ void helper_wrmsr(CPUX86State *env)
         env->sysenter_eip = val;
         break;
     case MSR_IA32_APICBASE:
-        cpu_set_apic_base(env->apic_state, val);
+        cpu_set_apic_base(x86_env_get_cpu(env)->apic_state, val);
         break;
     case MSR_EFER:
         {
@@ -437,7 +437,7 @@ void helper_rdmsr(CPUX86State *env)
         val = env->sysenter_eip;
         break;
     case MSR_IA32_APICBASE:
-        val = cpu_get_apic_base(env->apic_state);
+        val = cpu_get_apic_base(x86_env_get_cpu(env)->apic_state);
         break;
     case MSR_EFER:
         val = env->efer;
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 3/8] qmp: add 'cpu-del' command support Chen Fan
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

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>
---
 hw/i386/kvm/apic.c              |  8 ++++++++
 hw/intc/apic.c                  | 10 ++++++++++
 hw/intc/apic_common.c           | 23 ++++++++++++++++++++-
 hw/xen/xen_apic.c               |  8 ++++++++
 include/hw/cpu/icc_bus.h        |  1 +
 include/hw/i386/apic_internal.h |  1 +
 target-i386/cpu-qom.h           |  1 +
 target-i386/cpu.c               | 44 +++++++++++++++++++++++++++++++++++++++++
 8 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index e873b50..593ca19 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);
+
+    memory_region_destroy(&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 3d3deb6..d852624 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -884,11 +884,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);
+
+    memory_region_destroy(&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 c623fcc..88d596d 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,12 +284,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) {
@@ -319,6 +320,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);
@@ -389,6 +409,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/hw/xen/xen_apic.c b/hw/xen/xen_apic.c
index 63bb7f7..4008e07 100644
--- a/hw/xen/xen_apic.c
+++ b/hw/xen/xen_apic.c
@@ -49,6 +49,13 @@ static void xen_apic_realize(DeviceState *dev, Error **errp)
 #endif
 }
 
+static void xen_apic_unrealize(DeviceState *dev, Error **errp)
+{
+    APICCommonState *s = APIC_COMMON(dev);
+
+    memory_region_destroy(&s->io_memory);
+}
+
 static void xen_apic_set_base(APICCommonState *s, uint64_t val)
 {
 }
@@ -75,6 +82,7 @@ static void xen_apic_class_init(ObjectClass *klass, void *data)
     APICCommonClass *k = APIC_COMMON_CLASS(klass);
 
     k->realize = xen_apic_realize;
+    k->unrealize = xen_apic_unrealize;
     k->set_base = xen_apic_set_base;
     k->set_tpr = xen_apic_set_tpr;
     k->get_tpr = xen_apic_get_tpr;
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 70542a6..a0a44c6 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -81,6 +81,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 775c82d..f36e4ff 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -50,6 +50,7 @@ typedef struct X86CPUClass {
     /*< public >*/
 
     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 e20b0c8..9238554 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2526,10 +2526,31 @@ 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;
+    }
+
+    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)
@@ -2605,6 +2626,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;
 
@@ -2736,7 +2778,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.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 3/8] qmp: add 'cpu-del' command support
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 4/8] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

add cpu hot-remove interface pc_hot_del_cpu() for unrealizing vcpu device.
when using 'cpu-del' command, not need to specify vcpuid, the last one cpu
will be removed.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c         | 19 +++++++++++++++++++
 hw/i386/pc_piix.c    |  3 ++-
 include/hw/boards.h  |  2 ++
 include/hw/i386/pc.h |  1 +
 qapi-schema.json     | 10 ++++++++++
 qmp-commands.hx      | 21 +++++++++++++++++++++
 qmp.c                |  9 +++++++++
 7 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d000995..485e2ce 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -979,6 +979,25 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
     pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
 }
 
+void pc_hot_del_cpu(Error **errp)
+{
+    CPUState *cpu = first_cpu;
+    X86CPUClass *xcc;
+
+    while (CPU_NEXT(cpu)) {
+        cpu = CPU_NEXT(cpu);
+    }
+
+    if (cpu == first_cpu) {
+        error_setg(errp, "Unable to delete the last "
+                   "one cpu.");
+        return;
+    }
+
+    xcc = X86_CPU_GET_CLASS(DEVICE(cpu));
+    xcc->parent_unrealize(DEVICE(cpu), errp);
+}
+
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
 {
     int i;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4e0dae7..50c860b 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -362,7 +362,8 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 
 #define PC_I440FX_2_0_MACHINE_OPTIONS                           \
     PC_I440FX_MACHINE_OPTIONS,                                  \
-    .default_machine_opts = "firmware=bios-256k.bin"
+    .default_machine_opts = "firmware=bios-256k.bin",           \
+    .hot_del_cpu = pc_hot_del_cpu
 
 static QEMUMachine pc_i440fx_machine_v2_0 = {
     PC_I440FX_2_0_MACHINE_OPTIONS,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2151460..74334cb 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -23,6 +23,7 @@ typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
 typedef void QEMUMachineResetFunc(void);
 
 typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
+typedef void QEMUMachineHotDelCPUFunc(Error **errp);
 
 struct QEMUMachine {
     const char *name;
@@ -31,6 +32,7 @@ struct QEMUMachine {
     QEMUMachineInitFunc *init;
     QEMUMachineResetFunc *reset;
     QEMUMachineHotAddCPUFunc *hot_add_cpu;
+    QEMUMachineHotDelCPUFunc *hot_del_cpu;
     BlockInterfaceType block_default_type;
     int max_cpus;
     unsigned int no_serial:1,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 24eb3de..08bccfb 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -118,6 +118,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
 void pc_hot_add_cpu(const int64_t id, Error **errp);
+void pc_hot_del_cpu(Error **errp);
 void pc_acpi_init(const char *default_dsdt);
 
 PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
diff --git a/qapi-schema.json b/qapi-schema.json
index c3c939c..42600b7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1553,6 +1553,16 @@
 ##
 { 'command': 'cpu-add', 'data': {'id': 'int'} }
 
+# @cpu-del
+
+# Deletes CPU from the last ID
+#
+# Returns: Nothing on success
+#
+# Since 2.0
+##
+{ 'command': 'cpu-del' }
+
 ##
 # @memsave:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fba15cd..865eb91 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -411,6 +411,27 @@ Example:
 EQMP
 
     {
+        .name       = "cpu-del",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_cpu_del,
+    },
+
+SQMP
+cpu-del
+-------
+
+Deletes virtual cpu
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "cpu-del" }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "memsave",
         .args_type  = "val:l,size:i,filename:s,cpu:i?",
         .mhandler.cmd_new = qmp_marshal_input_memsave,
diff --git a/qmp.c b/qmp.c
index 1d7a04d..935310a 100644
--- a/qmp.c
+++ b/qmp.c
@@ -118,6 +118,15 @@ void qmp_cpu_add(int64_t id, Error **errp)
     }
 }
 
+void qmp_cpu_del(Error **errp)
+{
+    if (current_machine->hot_del_cpu) {
+        current_machine->hot_del_cpu(errp);
+    } else {
+        error_setg(errp, "Not supported");
+    }
+}
+
 #ifndef CONFIG_VNC
 /* If VNC support is enabled, the "true" query-vnc command is
    defined in the VNC subsystem */
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 4/8] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier'
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
                   ` (2 preceding siblings ...)
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 3/8] qmp: add 'cpu-del' command support Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 5/8] qom cpu: add UNPLUG cpu notifier support Chen Fan
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

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>
---
 hw/acpi/piix4.c         | 10 +++++-----
 hw/i386/pc.c            |  2 +-
 include/sysemu/sysemu.h |  2 +-
 qom/cpu.c               | 10 +++++-----
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 9e8a89c..ecb6104 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -98,7 +98,7 @@ typedef struct PIIX4PMState {
     uint8_t s4_val;
 
     CPUStatus gpe_cpu;
-    Notifier cpu_added_notifier;
+    Notifier cpu_hotplug_notifier;
 } PIIX4PMState;
 
 #define TYPE_PIIX4_PM "PIIX4_PM"
@@ -701,9 +701,9 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
     pm_update_sci(s);
 }
 
-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);
 
     piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
 }
@@ -736,8 +736,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
     memory_region_init_io(&s->io_cpu, OBJECT(s), &cpu_hotplug_ops, s,
                           "acpi-cpu-hotplug", PIIX4_PROC_LEN);
     memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu);
-    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);
 }
 
 static void enable_device(PIIX4PMState *s, int slot)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 485e2ce..3330f55 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -409,7 +409,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/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 495dae8..779b782 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -158,7 +158,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 9d62479..83006e2 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,12 +67,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)
@@ -219,7 +219,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.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 5/8] qom cpu: add UNPLUG cpu notifier support
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
                   ` (3 preceding siblings ...)
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 4/8] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 6/8] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c Chen Fan
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

Move struct HotplugEventType from file piix4.c to file qom/cpu.c,
and add struct CPUNotifier for supporting UNPLUG cpu notifier.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/acpi/piix4.c   |  8 ++------
 include/qom/cpu.h | 10 ++++++++++
 qom/cpu.c         |  6 +++++-
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index ecb6104..d2cd4cd 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -676,11 +676,6 @@ static const MemoryRegionOps cpu_hotplug_ops = {
     },
 };
 
-typedef enum {
-    PLUG,
-    UNPLUG,
-} HotplugEventType;
-
 static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
                                   HotplugEventType action)
 {
@@ -704,8 +699,9 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
 static void piix4_cpu_hotplug(Notifier *n, void *opaque)
 {
     PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_hotplug_notifier);
+    CPUNotifier *notifier = opaque;
 
-    piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
+    piix4_cpu_hotplug_req(s, CPU(notifier->dev), notifier->type);
 }
 
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..0238532 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -507,6 +507,16 @@ void qemu_init_vcpu(CPUState *cpu);
  */
 void cpu_single_step(CPUState *cpu, int enabled);
 
+typedef enum {
+    PLUG,
+    UNPLUG,
+} HotplugEventType;
+
+typedef struct CPUNotifier {
+    DeviceState *dev;
+    HotplugEventType type;
+} CPUNotifier;
+
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;
 #else
diff --git a/qom/cpu.c b/qom/cpu.c
index 83006e2..728b83b 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -216,10 +216,14 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model)
 static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cpu = CPU(dev);
+    CPUNotifier notifier;
+
+    notifier.dev = dev;
+    notifier.type = PLUG;
 
     if (dev->hotplugged) {
         cpu_synchronize_post_init(cpu);
-        notifier_list_notify(&cpu_hotplug_notifiers, dev);
+        notifier_list_notify(&cpu_hotplug_notifiers, &notifier);
         cpu_resume(cpu);
     }
 }
-- 
1.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 6/8] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
                   ` (4 preceding siblings ...)
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 5/8] qom cpu: add UNPLUG cpu notifier support Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 7/8] piix4: implement function cpu_status_write() for vcpu ejection Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 8/8] cpus: reclaim allocated vCPU objects Chen Fan
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

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>
---
 qom/cpu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/qom/cpu.c b/qom/cpu.c
index 728b83b..78038ab 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -228,6 +228,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, &notifier);
+}
+
+
 static void cpu_common_initfn(Object *obj)
 {
     CPUState *cpu = CPU(obj);
@@ -258,6 +269,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
     k->gdb_read_register = cpu_common_gdb_read_register;
     k->gdb_write_register = cpu_common_gdb_write_register;
     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.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 7/8] piix4: implement function cpu_status_write() for vcpu ejection
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
                   ` (5 preceding siblings ...)
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 6/8] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 8/8] cpus: reclaim allocated vCPU objects Chen Fan
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

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: for now QEMU only supported that cpu was deleted sequentially from the last
one in OS, in the further OS should reject vcpu arbitrarily.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 cpus.c                            |  7 ++++++
 hw/acpi/piix4.c                   | 48 ++++++++++++++++++++++++++++++++++++++-
 hw/i386/acpi-dsdt-cpu-hotplug.dsl |  6 ++++-
 include/qom/cpu.h                 | 10 ++++++++
 4 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index ca4c59f..5829d24 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1117,6 +1117,13 @@ void resume_all_vcpus(void)
     }
 }
 
+void cpu_remove(CPUState *cpu)
+{
+    cpu->stop = true;
+    cpu->exit = true;
+    qemu_cpu_kick(cpu);
+}
+
 static void qemu_tcg_init_vcpu(CPUState *cpu)
 {
     /* share a single thread for all cpus with TCG */
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index d2cd4cd..6407f8d 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -62,6 +62,7 @@ struct pci_status {
 
 typedef struct CPUStatus {
     uint8_t sts[PIIX4_PROC_LEN];
+    uint8_t old_sts[PIIX4_PROC_LEN];
 } CPUStatus;
 
 typedef struct PIIX4PMState {
@@ -651,6 +652,23 @@ static const MemoryRegionOps piix4_pci_ops = {
     },
 };
 
+static void acpi_piix_eject_vcpu(PIIX4PMState *s, int64_t cpuid)
+{
+    CPUStatus *g = &s->gpe_cpu;
+    CPUState *cpu;
+
+    CPU_FOREACH(cpu) {
+        CPUClass *cc = CPU_GET_CLASS(cpu);
+        int64_t id = cc->get_arch_id(cpu);
+
+        if (cpuid == id) {
+            g->old_sts[cpuid / 8] &= ~(1 << (cpuid % 8));
+            cpu_remove(cpu);
+            break;
+        }
+    }
+}
+
 static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
 {
     PIIX4PMState *s = opaque;
@@ -663,7 +681,27 @@ static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned int size)
 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 */
+    PIIX4PMState *s = opaque;
+    CPUStatus *cpus = &s->gpe_cpu;
+    uint8_t val;
+    int i;
+    int64_t cpuid = -1;
+
+    val = cpus->old_sts[addr] ^ data;
+
+    if (val == 0) {
+        return;
+    }
+
+    for (i = 0; i < 8; i++) {
+        if (val & 1 << i) {
+            cpuid = 8 * addr + i;
+        }
+    }
+
+    if (cpuid != -1) {
+        acpi_piix_eject_vcpu(s, cpuid);
+    }
 }
 
 static const MemoryRegionOps cpu_hotplug_ops = {
@@ -683,13 +721,20 @@ static void piix4_cpu_hotplug_req(PIIX4PMState *s, CPUState *cpu,
     ACPIGPE *gpe = &s->ar.gpe;
     CPUClass *k = CPU_GET_CLASS(cpu);
     int64_t cpu_id;
+    int i;
 
     assert(s != NULL);
 
     *gpe->sts = *gpe->sts | PIIX4_CPU_HOTPLUG_STATUS;
     cpu_id = k->get_arch_id(CPU(cpu));
+
+    for (i = 0; i < PIIX4_PROC_LEN; i++) {
+        g->old_sts[i] = g->sts[i];
+    }
+
     if (action == 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));
     }
@@ -728,6 +773,7 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
 
         g_assert((id / 8) < PIIX4_PROC_LEN);
         s->gpe_cpu.sts[id / 8] |= (1 << (id % 8));
+        s->gpe_cpu.old_sts[id / 8] |= (1 << (id % 8));
     }
     memory_region_init_io(&s->io_cpu, OBJECT(s), &cpu_hotplug_ops, s,
                           "acpi-cpu-hotplug", PIIX4_PROC_LEN);
diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
index c96ac42..8327b0d 100644
--- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
@@ -49,7 +49,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)
     }
 
     /* CPU hotplug notify method */
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 0238532..4412fb0 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -146,6 +146,7 @@ struct kvm_run;
  * @halted: Nonzero if the CPU is in suspended state.
  * @stop: Indicates a pending stop request.
  * @stopped: Indicates the CPU has been artificially stopped.
+ * @exit: Indicates the CPU is in impending exit state.
  * @tcg_exit_req: Set to force TCG to stop executing linked TBs for this
  *           CPU and return to its top level loop.
  * @singlestep_enabled: Flags for single-stepping.
@@ -181,6 +182,7 @@ struct CPUState {
     bool created;
     bool stop;
     bool stopped;
+    bool exit;
     volatile sig_atomic_t exit_request;
     volatile sig_atomic_t tcg_exit_req;
     uint32_t interrupt_request;
@@ -487,6 +489,14 @@ void cpu_exit(CPUState *cpu);
 void cpu_resume(CPUState *cpu);
 
 /**
+ * qemu_remove_vcpu:
+ * @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.8.1.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC qom-next v5 8/8] cpus: reclaim allocated vCPU objects
  2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
                   ` (6 preceding siblings ...)
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 7/8] piix4: implement function cpu_status_write() for vcpu ejection Chen Fan
@ 2013-12-23  9:04 ` Chen Fan
  7 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-23  9:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Igor Mammedov, Andreas Färber

After ACPI get a signal to eject a vCPU, then it will notify
the vCPU thread to exit in KVM, and the vCPU must be removed from CPU list,
before the vCPU really removed, there will release the all related vCPU objects.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 cpus.c               | 39 +++++++++++++++++++++++++++++++++++++++
 include/qom/cpu.h    |  1 +
 include/sysemu/kvm.h |  1 +
 kvm-all.c            | 25 +++++++++++++++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/cpus.c b/cpus.c
index 5829d24..c16476d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -786,6 +786,26 @@ 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_property_set_bool(OBJECT(cpu), false, "realized", NULL);
+    object_unparent(OBJECT(cpu));
+}
+
+static void qemu_tcg_destroy_vcpu(CPUState *cpu)
+{
+    CPU_REMOVE(cpu);
+    object_property_set_bool(OBJECT(cpu), false, "realized", NULL);
+    object_unparent(OBJECT(cpu));
+}
+
 static void flush_queued_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
@@ -877,6 +897,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;
@@ -929,6 +954,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);
@@ -961,6 +987,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;
@@ -1298,6 +1334,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/qom/cpu.h b/include/qom/cpu.h
index 4412fb0..f1a440e 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -208,6 +208,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/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 3b25f27..f3f1279 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -167,6 +167,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 3937754..5a42bf1 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -227,6 +227,31 @@ static void kvm_reset_vcpu(void *opaque)
     kvm_arch_reset_vcpu(cpu);
 }
 
+int kvm_destroy_vcpu(CPUState *cpu)
+{
+    KVMState *s = kvm_state;
+    long mmap_size;
+    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;
+    }
+
+    close(cpu->kvm_fd);
+err:
+    return ret;
+}
+
 int kvm_init_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU
  2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU Chen Fan
@ 2013-12-23 15:36   ` Andreas Färber
  2013-12-24  2:08     ` Chen Fan
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Färber @ 2013-12-23 15:36 UTC (permalink / raw)
  To: Chen Fan, qemu-devel; +Cc: Igor Mammedov

Am 23.12.2013 10:04, schrieb Chen Fan:
> This motion is preparing for refactoring vCPU apic subsequently.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  cpu-exec.c                |  2 +-
>  cpus.c                    |  5 ++---
>  hw/i386/kvmvapic.c        |  8 +++-----
>  hw/i386/pc.c              | 17 ++++++++---------
>  target-i386/cpu-qom.h     |  4 ++++
>  target-i386/cpu.c         | 22 ++++++++++------------
>  target-i386/cpu.h         |  4 ----
>  target-i386/helper.c      |  9 ++++-----
>  target-i386/kvm.c         | 23 ++++++++++-------------
>  target-i386/misc_helper.c |  8 ++++----
>  10 files changed, 46 insertions(+), 56 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 30cfa2a..2711c58 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -320,7 +320,7 @@ int cpu_exec(CPUArchState *env)
>  #if !defined(CONFIG_USER_ONLY)
>                      if (interrupt_request & CPU_INTERRUPT_POLL) {
>                          cpu->interrupt_request &= ~CPU_INTERRUPT_POLL;
> -                        apic_poll_irq(env->apic_state);
> +                        apic_poll_irq(x86_env_get_cpu(env)->apic_state);

These are starting to become too many inline usages inside that double
loop, I'll look into providing a follow-up patch to clean this up.

>                      }
>  #endif
>                      if (interrupt_request & CPU_INTERRUPT_INIT) {
[...]
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index e9831ca..d000995 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -172,13 +172,14 @@ void cpu_smm_update(CPUX86State *env)
>  int cpu_get_pic_interrupt(CPUX86State *env)
>  {
>      int intno;
> +    X86CPU *cpu = x86_env_get_cpu(env);

I've swapped these two lines to keep cpu and env close together, with a
view to a function argument type change.

>  
> -    intno = apic_get_interrupt(env->apic_state);
> +    intno = apic_get_interrupt(cpu->apic_state);
>      if (intno >= 0) {
>          return intno;
>      }
>      /* read the irq from the PIC */
> -    if (!apic_accept_pic_intr(env->apic_state)) {
> +    if (!apic_accept_pic_intr(cpu->apic_state)) {
>          return -1;
>      }
>  
[...]
> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> index f4fab15..775c82d 100644
> --- a/target-i386/cpu-qom.h
> +++ b/target-i386/cpu-qom.h
> @@ -66,6 +66,10 @@ typedef struct X86CPU {
>  
>      CPUX86State env;
>  
> +    /* in order to simplify APIC support, we leave this pointer to the
> +       user */
> +    struct DeviceState *apic_state;

Moving this further down since used as a child<> property, with a view
to refactoring this further into a non-pointer field.

> +
>      bool hyperv_vapic;
>      bool hyperv_relaxed_timing;
>      int hyperv_spinlock_attempts;
[...]
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 7c196ff..f2e76ad 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1248,7 +1248,8 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access)
>      } else {
>          cpu_restore_state(env, env->mem_io_pc);
>  
> -        apic_handle_tpr_access_report(env->apic_state, env->eip, access);
> +        apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state,
> +                                      env->eip, access);
>      }
>  }
>  #endif /* !CONFIG_USER_ONLY */
[snip]

Since we would now be using x86_env_get_cpu() in both arms of 'if' (and
tpr_access_type being another candidate for a field movement), I'm
changing this as follows:

diff --git a/target-i386/helper.c b/target-i386/helper.c
index f2e76ad..8132ca8 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1241,15 +1241,16 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU
*cpu, int bank,

 void cpu_report_tpr_access(CPUX86State *env, TPRAccess access)
 {
+    X86CPU *cpu = x86_env_get_cpu(env);
+
     if (kvm_enabled()) {
         env->tpr_access_type = access;

-        cpu_interrupt(CPU(x86_env_get_cpu(env)), CPU_INTERRUPT_TPR);
+        cpu_interrupt(CPU(cpu), CPU_INTERRUPT_TPR);
     } else {
         cpu_restore_state(env, env->mem_io_pc);

-        apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state,
-                                      env->eip, access);
+        apic_handle_tpr_access_report(cpu->apic_state, env->eip, access);
     }
 }
 #endif /* !CONFIG_USER_ONLY */


Despite this still being an RFC, this patch is a really nice cleanup
contribution, so I'm applying this to qom-cpu already with the
above-mentioned modifications:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU
  2013-12-23 15:36   ` Andreas Färber
@ 2013-12-24  2:08     ` Chen Fan
  0 siblings, 0 replies; 11+ messages in thread
From: Chen Fan @ 2013-12-24  2:08 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel

On Mon, 2013-12-23 at 16:36 +0100, Andreas Färber wrote:
> Am 23.12.2013 10:04, schrieb Chen Fan:
> > This motion is preparing for refactoring vCPU apic subsequently.
> > 
> > Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> > ---
> >  cpu-exec.c                |  2 +-
> >  cpus.c                    |  5 ++---
> >  hw/i386/kvmvapic.c        |  8 +++-----
> >  hw/i386/pc.c              | 17 ++++++++---------
> >  target-i386/cpu-qom.h     |  4 ++++
> >  target-i386/cpu.c         | 22 ++++++++++------------
> >  target-i386/cpu.h         |  4 ----
> >  target-i386/helper.c      |  9 ++++-----
> >  target-i386/kvm.c         | 23 ++++++++++-------------
> >  target-i386/misc_helper.c |  8 ++++----
> >  10 files changed, 46 insertions(+), 56 deletions(-)
> > 
> > diff --git a/cpu-exec.c b/cpu-exec.c
> > index 30cfa2a..2711c58 100644
> > --- a/cpu-exec.c
> > +++ b/cpu-exec.c
> > @@ -320,7 +320,7 @@ int cpu_exec(CPUArchState *env)
> >  #if !defined(CONFIG_USER_ONLY)
> >                      if (interrupt_request & CPU_INTERRUPT_POLL) {
> >                          cpu->interrupt_request &= ~CPU_INTERRUPT_POLL;
> > -                        apic_poll_irq(env->apic_state);
> > +                        apic_poll_irq(x86_env_get_cpu(env)->apic_state);
> 
> These are starting to become too many inline usages inside that double
> loop, I'll look into providing a follow-up patch to clean this up.
> 
> >                      }
> >  #endif
> >                      if (interrupt_request & CPU_INTERRUPT_INIT) {
> [...]
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index e9831ca..d000995 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -172,13 +172,14 @@ void cpu_smm_update(CPUX86State *env)
> >  int cpu_get_pic_interrupt(CPUX86State *env)
> >  {
> >      int intno;
> > +    X86CPU *cpu = x86_env_get_cpu(env);
> 
> I've swapped these two lines to keep cpu and env close together, with a
> view to a function argument type change.
> 
> >  
> > -    intno = apic_get_interrupt(env->apic_state);
> > +    intno = apic_get_interrupt(cpu->apic_state);
> >      if (intno >= 0) {
> >          return intno;
> >      }
> >      /* read the irq from the PIC */
> > -    if (!apic_accept_pic_intr(env->apic_state)) {
> > +    if (!apic_accept_pic_intr(cpu->apic_state)) {
> >          return -1;
> >      }
> >  
> [...]
> > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> > index f4fab15..775c82d 100644
> > --- a/target-i386/cpu-qom.h
> > +++ b/target-i386/cpu-qom.h
> > @@ -66,6 +66,10 @@ typedef struct X86CPU {
> >  
> >      CPUX86State env;
> >  
> > +    /* in order to simplify APIC support, we leave this pointer to the
> > +       user */
> > +    struct DeviceState *apic_state;
> 
> Moving this further down since used as a child<> property, with a view
> to refactoring this further into a non-pointer field.
> 
> > +
> >      bool hyperv_vapic;
> >      bool hyperv_relaxed_timing;
> >      int hyperv_spinlock_attempts;
> [...]
> > diff --git a/target-i386/helper.c b/target-i386/helper.c
> > index 7c196ff..f2e76ad 100644
> > --- a/target-i386/helper.c
> > +++ b/target-i386/helper.c
> > @@ -1248,7 +1248,8 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess access)
> >      } else {
> >          cpu_restore_state(env, env->mem_io_pc);
> >  
> > -        apic_handle_tpr_access_report(env->apic_state, env->eip, access);
> > +        apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state,
> > +                                      env->eip, access);
> >      }
> >  }
> >  #endif /* !CONFIG_USER_ONLY */
> [snip]
> 
> Since we would now be using x86_env_get_cpu() in both arms of 'if' (and
> tpr_access_type being another candidate for a field movement), I'm
> changing this as follows:
> 
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index f2e76ad..8132ca8 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1241,15 +1241,16 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU
> *cpu, int bank,
> 
>  void cpu_report_tpr_access(CPUX86State *env, TPRAccess access)
>  {
> +    X86CPU *cpu = x86_env_get_cpu(env);
> +
>      if (kvm_enabled()) {
>          env->tpr_access_type = access;
> 
> -        cpu_interrupt(CPU(x86_env_get_cpu(env)), CPU_INTERRUPT_TPR);
> +        cpu_interrupt(CPU(cpu), CPU_INTERRUPT_TPR);
>      } else {
>          cpu_restore_state(env, env->mem_io_pc);
> 
> -        apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state,
> -                                      env->eip, access);
> +        apic_handle_tpr_access_report(cpu->apic_state, env->eip, access);
>      }
>  }
>  #endif /* !CONFIG_USER_ONLY */
> 
> 
> Despite this still being an RFC, this patch is a really nice cleanup
> contribution, so I'm applying this to qom-cpu already with the
> above-mentioned modifications:
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu

looks nice, Thanks.
Chen

> Thanks,
> Andreas
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-12-24  2:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-23  9:04 [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU Chen Fan
2013-12-23 15:36   ` Andreas Färber
2013-12-24  2:08     ` Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 3/8] qmp: add 'cpu-del' command support Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 4/8] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier' Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 5/8] qom cpu: add UNPLUG cpu notifier support Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 6/8] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 7/8] piix4: implement function cpu_status_write() for vcpu ejection Chen Fan
2013-12-23  9:04 ` [Qemu-devel] [RFC qom-next v5 8/8] cpus: reclaim allocated vCPU objects Chen Fan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).