[Qemu-devel] [PATCH V5 0/8] cpu/acpi: convert cpu hot plug to hotplug_handler API

[Qemu-devel] [PATCH V5 0/8] cpu/acpi: convert cpu hot plug to hotplug_handler API

[Gu Zheng]

Previously we use cpu_added_notifiers to register cpu hotplug notifier callback
which is not able to pass/handle errors, so we switch it to unified hotplug
handler API which allows to pass errors and would allow to cancel device_add
in case of error.
Thanks very much for Igor's review and suggestion.

---
v5:
 -rebase on the latest upstream and fix some comments.
 Patch 4/8:
 -split the check out of acpi_dev block.
 Patch 5/8:
 -move CPU hot-plug notifier cleanup hunk into Patch 6/8.
 Patch 6/8:
 -delete the caller of notifier_list_notify() in this patch.
 Patch 8/8:
 -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.

v4:
 -split removal of CPU hotplug notifier into separate patch (Patch 6/8).
 Patch 1/7:
 -convert CPUState *cpu to DeviceState *dev like it's done for other handlers
  and do cast to CPU inside.
 Patch 5/7:
 -Make rtc_state as a link property in PCMachine rather than the global
  variables.
 -Split out the removal of unused notifier into separate patch.
 -Check the result of plug callback before update rtc_state.

v3:
 -deal with start-up cpus in pc_cpu_plug as Igor suggested.

v2:
 -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5.
  1/5-->1/7
  2/5-->2/7
  3/5-->3/7
  4/5-->4/7
 Patch 1/7:
 -add errp argument to catch error.
 -return error instead of aborting if cpu id is invalid.
 -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.
 Patch 3/7:
 -remove unused AcpiCpuHotplug_add directly.
 Patch 5/7:
 -switch the last user of cpu hotplug notifier to hotplug handler API, and
  remove the unused cpu hotplug notify.
 Patch 6/7:
 -split the function rename (just cleanup) into single patch.
 Patch 7/7:
 -introduce help function acpi_set_local_sts to keep the bit setting in
  one place.
---

Gu Zheng (8):
  acpi/cpu: add cpu hotplug callback function to match hotplug_handler
    API
  acpi:ich9: convert cpu hotplug to hotplug_handler API
  acpi:piix4: convert cpu hotplug to hotplug_handler API
  pc: add cpu hotplug handler to PC_MACHINE
  pc: Update rtc_cmos in pc_cpu_plug
  qom/cpu: remove the unused CPU hot-plug notifier
  cpu-hotplug: rename function for better readability
  acpi/cpu-hotplug: introduce help function to keep bit setting in one
    place

 hw/acpi/cpu_hotplug.c         |   35 ++++++++++++++++------
 hw/acpi/ich9.c                |   17 ++--------
 hw/acpi/piix4.c               |   18 ++---------
 hw/i386/pc.c                  |   65 +++++++++++++++++++++++++++--------------
 hw/i386/pc_piix.c             |    2 +-
 hw/i386/pc_q35.c              |    2 +-
 include/hw/acpi/cpu_hotplug.h |    7 ++--
 include/hw/acpi/ich9.h        |    1 -
 include/hw/i386/pc.h          |    3 +-
 include/sysemu/sysemu.h       |    3 --
 qom/cpu.c                     |   10 ------
 11 files changed, 84 insertions(+), 79 deletions(-)

-- 
1.7.7

[Qemu-devel] [PATCH V5 1/8] acpi/cpu: add cpu hotplug callback function to match hotplug_handler API

[Gu Zheng]

Add cpu hotplug callback function (acpi_cpu_plug_cb) to match hotplug_handler API.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v4:
 -convert CPUState *cpu to DeviceState *dev like it's done for other handlers
  and do cast to CPU inside.
v2:
 -add errp argument to catch error.
 -return error instead of aborting if cpu id is invalid.
 -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.

 hw/acpi/cpu_hotplug.c         |   18 ++++++++++++++++++
 include/hw/acpi/cpu_hotplug.h |    3 +++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 2ad83a0..06e9c61 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -36,6 +36,24 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
     },
 };
 
+void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
+{
+    CPUState *cpu = CPU(dev);
+    CPUClass *k = CPU_GET_CLASS(cpu);
+    int64_t cpu_id;
+
+    cpu_id = k->get_arch_id(cpu);
+    if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
+        error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
+        return;
+    }
+
+    AcpiCpuHotplug_add(&ar->gpe, g, cpu);
+
+    acpi_update_sci(ar, irq);
+}
+
 void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
 {
     CPUClass *k = CPU_GET_CLASS(cpu);
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 9e5d30c..5dca8d7 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -20,6 +20,9 @@ typedef struct AcpiCpuHotplug {
     uint8_t sts[ACPI_GPE_PROC_LEN];
 } AcpiCpuHotplug;
 
+void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
+
 void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
 
 void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
-- 
1.7.7

[Qemu-devel] [PATCH V5 2/8] acpi:ich9: convert cpu hotplug to hotplug_handler API

[Gu Zheng]

Convert notifier based hotplug to hotplug_handler API.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v5:
 -drop 'handle' from the comment and subject.

 hw/acpi/ich9.c         |   13 ++-----------
 include/hw/acpi/ich9.h |    1 -
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 7b14bbb..7585364 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -209,15 +209,6 @@ 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)
-{
-    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
-
-    assert(pm != NULL);
-    AcpiCpuHotplug_add(&pm->acpi_regs.gpe, &pm->gpe_cpu, CPU(opaque));
-    acpi_update_sci(&pm->acpi_regs, pm->irq);
-}
-
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
                   qemu_irq sci_irq)
 {
@@ -246,8 +237,6 @@ 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);
 
     if (pm->acpi_memory_hotplug.is_enabled) {
         acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
@@ -304,6 +293,8 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
         object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug,
                             dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq, &pm->gpe_cpu, dev, errp);
     } else {
         error_setg(errp, "acpi: device plug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 7e42448..fe975e6 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -47,7 +47,6 @@ typedef struct ICH9LPCPMRegs {
     Notifier powerdown_notifier;
 
     AcpiCpuHotplug gpe_cpu;
-    Notifier cpu_added_notifier;
 
     MemHotplugState acpi_memory_hotplug;
 } ICH9LPCPMRegs;
-- 
1.7.7

[Qemu-devel] [PATCH V5 3/8] acpi:piix4: convert cpu hotplug to hotplug_handler API

[Gu Zheng]

Convert notifier based hotplug to hotplug_handler API,
and remove the unused AcpiCpuHotplug_add().

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v5:
 -drop 'handle' from the comment and subject.
v2:
 -remove the unused AcpiCpuHotplug_add().

 hw/acpi/cpu_hotplug.c         |   14 ++------------
 hw/acpi/piix4.c               |   14 ++------------
 include/hw/acpi/cpu_hotplug.h |    2 --
 3 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 06e9c61..b69b16c 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -49,22 +49,12 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
         return;
     }
 
-    AcpiCpuHotplug_add(&ar->gpe, g, cpu);
+    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
+    g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
 
     acpi_update_sci(ar, irq);
 }
 
-void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
-{
-    CPUClass *k = CPU_GET_CLASS(cpu);
-    int64_t cpu_id;
-
-    *gpe->sts = *gpe->sts | ACPI_CPU_HOTPLUG_STATUS;
-    cpu_id = k->get_arch_id(CPU(cpu));
-    g_assert((cpu_id / 8) < ACPI_GPE_PROC_LEN);
-    g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
-}
-
 void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
                          AcpiCpuHotplug *gpe_cpu, uint16_t base)
 {
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..8320b18 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -83,7 +83,6 @@ typedef struct PIIX4PMState {
     uint8_t s4_val;
 
     AcpiCpuHotplug gpe_cpu;
-    Notifier cpu_added_notifier;
 
     MemHotplugState acpi_memory_hotplug;
 } PIIX4PMState;
@@ -348,6 +347,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
         acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
                                   errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, dev, errp);
     } else {
         error_setg(errp, "acpi: device plug request for not supported device"
                    " type: %s", object_get_typename(OBJECT(dev)));
@@ -544,15 +545,6 @@ static const MemoryRegionOps piix4_gpe_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static void piix4_cpu_added_req(Notifier *n, void *opaque)
-{
-    PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
-
-    assert(s != NULL);
-    AcpiCpuHotplug_add(&s->ar.gpe, &s->gpe_cpu, CPU(opaque));
-    acpi_update_sci(&s->ar, s->irq);
-}
-
 static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
                                            PCIBus *bus, PIIX4PMState *s)
 {
@@ -565,8 +557,6 @@ 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);
 
     if (s->acpi_memory_hotplug.is_enabled) {
         acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 5dca8d7..4657e71 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -23,8 +23,6 @@ typedef struct AcpiCpuHotplug {
 void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
                       AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
 
-void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
-
 void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
                          AcpiCpuHotplug *gpe_cpu, uint16_t base);
 #endif
-- 
1.7.7

[Qemu-devel] [PATCH V5 4/8] pc: add cpu hotplug handler to PC_MACHINE

[Gu Zheng]

Add cpu hotplug handler to PC_MACHINE, which will perform the acpi
cpu hotplug callback via hotplug_handler API.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v5:
 -split the check out of acpi_dev block.
v3:
 -deal with start up cpus in a more neat way as Igor suggested.
v2:
 -just rebase.

 hw/i386/pc.c |   28 +++++++++++++++++++++++++++-
 1 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d045e8b..eff19ef 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1617,11 +1617,36 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void pc_cpu_plug(HotplugHandler *hotplug_dev,
+                        DeviceState *dev, Error **errp)
+{
+    HotplugHandlerClass *hhc;
+    Error *local_err = NULL;
+    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+
+    if (!dev->hotplugged) {
+        goto out;
+    }
+
+    if (!pcms->acpi_dev) {
+        error_setg(&local_err,
+                   "cpu hotplug is not enabled: missing acpi device");
+        goto out;
+    }
+
+    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
+    hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+out:
+    error_propagate(errp, local_err);
+}
+
 static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
                                       DeviceState *dev, Error **errp)
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         pc_dimm_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+        pc_cpu_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -1630,7 +1655,8 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
 {
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
 
-    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
         return HOTPLUG_HANDLER(machine);
     }
 
-- 
1.7.7

[Qemu-devel] [PATCH V5 5/8] pc: Update rtc_cmos in pc_cpu_plug

[Gu Zheng]

Update rtc_cmos in pc_cpu_plug() directly, instead of the notifier.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v5:
 -move CPU hot-plug notifier cleanup hunk into Patch 6/8.

v4:
 -Make link<rtc> property in PCMachine rather than the global
  variables.
 -Split out the removal of unused notifier into separate patch.

 hw/i386/pc.c         |   37 ++++++++++++++++---------------------
 hw/i386/pc_piix.c    |    2 +-
 hw/i386/pc_q35.c     |    2 +-
 include/hw/i386/pc.h |    3 ++-
 4 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index eff19ef..0f10ec8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -355,30 +355,15 @@ static void pc_cmos_init_late(void *opaque)
     qemu_unregister_reset(pc_cmos_init_late, opaque);
 }
 
-typedef struct RTCCPUHotplugArg {
-    Notifier cpu_added_notifier;
-    ISADevice *rtc_state;
-} RTCCPUHotplugArg;
-
-static void rtc_notify_cpu_added(Notifier *notifier, void *data)
-{
-    RTCCPUHotplugArg *arg = container_of(notifier, RTCCPUHotplugArg,
-                                         cpu_added_notifier);
-    ISADevice *s = arg->rtc_state;
-
-    /* increment the number of CPUs */
-    rtc_set_memory(s, 0x5f, rtc_get_memory(s, 0x5f) + 1);
-}
-
 void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
-                  const char *boot_device,
+                  const char *boot_device, MachineState *machine,
                   ISADevice *floppy, BusState *idebus0, BusState *idebus1,
                   ISADevice *s)
 {
     int val, nb, i;
     FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
     static pc_cmos_init_late_arg arg;
-    static RTCCPUHotplugArg cpu_hotplug_cb;
+    PCMachineState *pc_machine = PC_MACHINE(machine);
 
     /* various important CMOS locations needed by PC/Bochs bios */
 
@@ -417,10 +402,14 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
 
     /* set the number of CPU */
     rtc_set_memory(s, 0x5f, smp_cpus - 1);
-    /* 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);
+
+    object_property_add_link(OBJECT(machine), "rtc_state",
+                             TYPE_ISA_DEVICE,
+                             (Object **)&pc_machine->rtc,
+                             object_property_allow_set_link,
+                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
+    object_property_set_link(OBJECT(machine), OBJECT(s),
+                             "rtc_state", &error_abort);
 
     if (set_boot_dev(s, boot_device)) {
         exit(1);
@@ -1636,6 +1625,12 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
 
     hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
     hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    /* increment the number of CPUs */
+    rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
 out:
     error_propagate(errp, local_err);
 }
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4384633..2ff7e84 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -266,7 +266,7 @@ static void pc_init1(MachineState *machine,
     }
 
     pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order,
-                 floppy, idebus[0], idebus[1], rtc_state);
+                 machine, floppy, idebus[0], idebus[1], rtc_state);
 
     if (pci_enabled && usb_enabled(false)) {
         pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index bb0dc8e..7cab8e4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -270,7 +270,7 @@ static void pc_q35_init(MachineState *machine)
                       8, NULL, 0);
 
     pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order,
-                 floppy, idebus[0], idebus[1], rtc_state);
+                 machine, floppy, idebus[0], idebus[1], rtc_state);
 
     /* the rest devices to which pci devfn is automatically assigned */
     pc_vga_init(isa_bus, host_bus);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 77316d5..7a4bff4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -33,6 +33,7 @@ struct PCMachineState {
     MemoryRegion hotplug_memory;
 
     HotplugHandler *acpi_dev;
+    ISADevice *rtc;
 
     uint64_t max_ram_below_4g;
 };
@@ -210,7 +211,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           uint32 hpet_irqs);
 void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
 void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
-                  const char *boot_device,
+                  const char *boot_device, MachineState *machine,
                   ISADevice *floppy, BusState *ide0, BusState *ide1,
                   ISADevice *s);
 void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus);
-- 
1.7.7

[Qemu-devel] [PATCH V5 6/8] qom/cpu: remove the unused CPU hot-plug notifier

[Gu Zheng]

Remove the unused CPU hot-plug notifier.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v5:
 -delete the caller of notifier_list_notify() in this patch.

 include/sysemu/sysemu.h |    3 ---
 qom/cpu.c               |   10 ----------
 2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..acfe494 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -183,9 +183,6 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
 /* generic hotplug */
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 
-/* CPU hotplug */
-void qemu_register_cpu_added_notifier(Notifier *notifier);
-
 /* pcie aer error injection */
 void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
 int do_pcie_aer_inject_error(Monitor *mon,
diff --git a/qom/cpu.c b/qom/cpu.c
index 0ec3337..79d2228 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -107,15 +107,6 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
     error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
 }
 
-/* CPU hot-plug notifiers */
-static NotifierList cpu_added_notifiers =
-    NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
-
-void qemu_register_cpu_added_notifier(Notifier *notifier)
-{
-    notifier_list_add(&cpu_added_notifiers, notifier);
-}
-
 void cpu_reset_interrupt(CPUState *cpu, int mask)
 {
     cpu->interrupt_request &= ~mask;
@@ -312,7 +303,6 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
 
     if (dev->hotplugged) {
         cpu_synchronize_post_init(cpu);
-        notifier_list_notify(&cpu_added_notifiers, dev);
         cpu_resume(cpu);
     }
 }
-- 
1.7.7

[Qemu-devel] [PATCH V5 7/8] cpu-hotplug: rename function for better readability

[Gu Zheng]

Rename:
AcpiCpuHotplug_init --> acpi_cpu_hotplug_init
AcpiCpuHotplug_ops --> acpi_cpu_hotplug_ops
for better readability, just cleanup.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 hw/acpi/cpu_hotplug.c         |    4 ++--
 hw/acpi/ich9.c                |    4 ++--
 hw/acpi/piix4.c               |    4 ++--
 include/hw/acpi/cpu_hotplug.h |    4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index b69b16c..ae48b63 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -55,8 +55,8 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
     acpi_update_sci(ar, irq);
 }
 
-void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
-                         AcpiCpuHotplug *gpe_cpu, uint16_t base)
+void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
+                           AcpiCpuHotplug *gpe_cpu, uint16_t base)
 {
     CPUState *cpu;
 
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 7585364..ea991a3 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -235,8 +235,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
     pm->powerdown_notifier.notify = pm_powerdown_req;
     qemu_register_powerdown_notifier(&pm->powerdown_notifier);
 
-    AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
-                        &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
+    acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
+                          &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
 
     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 8320b18..8fde808 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -555,8 +555,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
     acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent,
                     s->use_acpi_pci_hotplug);
 
-    AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu,
-                        PIIX4_CPU_HOTPLUG_IO_BASE);
+    acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu,
+                          PIIX4_CPU_HOTPLUG_IO_BASE);
 
     if (s->acpi_memory_hotplug.is_enabled) {
         acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 4657e71..f6d358d 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -23,6 +23,6 @@ typedef struct AcpiCpuHotplug {
 void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
                       AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
 
-void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
-                         AcpiCpuHotplug *gpe_cpu, uint16_t base);
+void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
+                           AcpiCpuHotplug *gpe_cpu, uint16_t base);
 #endif
-- 
1.7.7

[Qemu-devel] [PATCH V5 8/8] acpi/cpu-hotplug: introduce helper function to keep bit setting in one place

[Gu Zheng]

Introduce helper function acpi_set_cpu_present_bit() to simplify acpi_cpu_plug_cb
and acpi_cpu_hotplug_init, so that we can keep bit setting in one place.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
v5:
 -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.

 hw/acpi/cpu_hotplug.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index ae48b63..8b8d15a 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -36,10 +36,9 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
     },
 };
 
-void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
-                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
+static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
+                                     Error **errp)
 {
-    CPUState *cpu = CPU(dev);
     CPUClass *k = CPU_GET_CLASS(cpu);
     int64_t cpu_id;
 
@@ -49,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
         return;
     }
 
-    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
     g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
+}
 
+void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
+                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
+{
+    acpi_set_cpu_present_bit(g, CPU(dev), errp);
+    if (*errp != NULL) {
+        return;
+    }
+
+    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
     acpi_update_sci(ar, irq);
 }
 
@@ -61,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
     CPUState *cpu;
 
     CPU_FOREACH(cpu) {
-        CPUClass *cc = CPU_GET_CLASS(cpu);
-        int64_t id = cc->get_arch_id(cpu);
+        Error *local_err = NULL;
 
-        g_assert((id / 8) < ACPI_GPE_PROC_LEN);
-        gpe_cpu->sts[id / 8] |= (1 << (id % 8));
+        acpi_set_cpu_present_bit(gpe_cpu, cpu, &local_err);
+        g_assert(local_err == NULL);
     }
     memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
                           gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
-- 
1.7.7

Re: [Qemu-devel] [PATCH V5 0/8] cpu/acpi: convert cpu hot plug to hotplug_handler API

[Gu Zheng]

ping...

On 10/10/2014 10:15 AM, Gu Zheng wrote:

> Previously we use cpu_added_notifiers to register cpu hotplug notifier callback
> which is not able to pass/handle errors, so we switch it to unified hotplug
> handler API which allows to pass errors and would allow to cancel device_add
> in case of error.
> Thanks very much for Igor's review and suggestion.
> 
> ---
> v5:
>  -rebase on the latest upstream and fix some comments.
>  Patch 4/8:
>  -split the check out of acpi_dev block.
>  Patch 5/8:
>  -move CPU hot-plug notifier cleanup hunk into Patch 6/8.
>  Patch 6/8:
>  -delete the caller of notifier_list_notify() in this patch.
>  Patch 8/8:
>  -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.
> 
> v4:
>  -split removal of CPU hotplug notifier into separate patch (Patch 6/8).
>  Patch 1/7:
>  -convert CPUState *cpu to DeviceState *dev like it's done for other handlers
>   and do cast to CPU inside.
>  Patch 5/7:
>  -Make rtc_state as a link property in PCMachine rather than the global
>   variables.
>  -Split out the removal of unused notifier into separate patch.
>  -Check the result of plug callback before update rtc_state.
> 
> v3:
>  -deal with start-up cpus in pc_cpu_plug as Igor suggested.
> 
> v2:
>  -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5.
>   1/5-->1/7
>   2/5-->2/7
>   3/5-->3/7
>   4/5-->4/7
>  Patch 1/7:
>  -add errp argument to catch error.
>  -return error instead of aborting if cpu id is invalid.
>  -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.
>  Patch 3/7:
>  -remove unused AcpiCpuHotplug_add directly.
>  Patch 5/7:
>  -switch the last user of cpu hotplug notifier to hotplug handler API, and
>   remove the unused cpu hotplug notify.
>  Patch 6/7:
>  -split the function rename (just cleanup) into single patch.
>  Patch 7/7:
>  -introduce help function acpi_set_local_sts to keep the bit setting in
>   one place.
> ---
> 
> Gu Zheng (8):
>   acpi/cpu: add cpu hotplug callback function to match hotplug_handler
>     API
>   acpi:ich9: convert cpu hotplug to hotplug_handler API
>   acpi:piix4: convert cpu hotplug to hotplug_handler API
>   pc: add cpu hotplug handler to PC_MACHINE
>   pc: Update rtc_cmos in pc_cpu_plug
>   qom/cpu: remove the unused CPU hot-plug notifier
>   cpu-hotplug: rename function for better readability
>   acpi/cpu-hotplug: introduce help function to keep bit setting in one
>     place
> 
>  hw/acpi/cpu_hotplug.c         |   35 ++++++++++++++++------
>  hw/acpi/ich9.c                |   17 ++--------
>  hw/acpi/piix4.c               |   18 ++---------
>  hw/i386/pc.c                  |   65 +++++++++++++++++++++++++++--------------
>  hw/i386/pc_piix.c             |    2 +-
>  hw/i386/pc_q35.c              |    2 +-
>  include/hw/acpi/cpu_hotplug.h |    7 ++--
>  include/hw/acpi/ich9.h        |    1 -
>  include/hw/i386/pc.h          |    3 +-
>  include/sysemu/sysemu.h       |    3 --
>  qom/cpu.c                     |   10 ------
>  11 files changed, 84 insertions(+), 79 deletions(-)
> 

Re: [Qemu-devel] [PATCH V5 0/8] cpu/acpi: convert cpu hot plug to hotplug_handler API

[Gu Zheng]

Hi Igor,
How about this version?

Regards,
Gu
On 10/10/2014 10:15 AM, Gu Zheng wrote:

> Previously we use cpu_added_notifiers to register cpu hotplug notifier callback
> which is not able to pass/handle errors, so we switch it to unified hotplug
> handler API which allows to pass errors and would allow to cancel device_add
> in case of error.
> Thanks very much for Igor's review and suggestion.
> 
> ---
> v5:
>  -rebase on the latest upstream and fix some comments.
>  Patch 4/8:
>  -split the check out of acpi_dev block.
>  Patch 5/8:
>  -move CPU hot-plug notifier cleanup hunk into Patch 6/8.
>  Patch 6/8:
>  -delete the caller of notifier_list_notify() in this patch.
>  Patch 8/8:
>  -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.
> 
> v4:
>  -split removal of CPU hotplug notifier into separate patch (Patch 6/8).
>  Patch 1/7:
>  -convert CPUState *cpu to DeviceState *dev like it's done for other handlers
>   and do cast to CPU inside.
>  Patch 5/7:
>  -Make rtc_state as a link property in PCMachine rather than the global
>   variables.
>  -Split out the removal of unused notifier into separate patch.
>  -Check the result of plug callback before update rtc_state.
> 
> v3:
>  -deal with start-up cpus in pc_cpu_plug as Igor suggested.
> 
> v2:
>  -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5.
>   1/5-->1/7
>   2/5-->2/7
>   3/5-->3/7
>   4/5-->4/7
>  Patch 1/7:
>  -add errp argument to catch error.
>  -return error instead of aborting if cpu id is invalid.
>  -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.
>  Patch 3/7:
>  -remove unused AcpiCpuHotplug_add directly.
>  Patch 5/7:
>  -switch the last user of cpu hotplug notifier to hotplug handler API, and
>   remove the unused cpu hotplug notify.
>  Patch 6/7:
>  -split the function rename (just cleanup) into single patch.
>  Patch 7/7:
>  -introduce help function acpi_set_local_sts to keep the bit setting in
>   one place.
> ---
> 
> Gu Zheng (8):
>   acpi/cpu: add cpu hotplug callback function to match hotplug_handler
>     API
>   acpi:ich9: convert cpu hotplug to hotplug_handler API
>   acpi:piix4: convert cpu hotplug to hotplug_handler API
>   pc: add cpu hotplug handler to PC_MACHINE
>   pc: Update rtc_cmos in pc_cpu_plug
>   qom/cpu: remove the unused CPU hot-plug notifier
>   cpu-hotplug: rename function for better readability
>   acpi/cpu-hotplug: introduce help function to keep bit setting in one
>     place
> 
>  hw/acpi/cpu_hotplug.c         |   35 ++++++++++++++++------
>  hw/acpi/ich9.c                |   17 ++--------
>  hw/acpi/piix4.c               |   18 ++---------
>  hw/i386/pc.c                  |   65 +++++++++++++++++++++++++++--------------
>  hw/i386/pc_piix.c             |    2 +-
>  hw/i386/pc_q35.c              |    2 +-
>  include/hw/acpi/cpu_hotplug.h |    7 ++--
>  include/hw/acpi/ich9.h        |    1 -
>  include/hw/i386/pc.h          |    3 +-
>  include/sysemu/sysemu.h       |    3 --
>  qom/cpu.c                     |   10 ------
>  11 files changed, 84 insertions(+), 79 deletions(-)
> 

Re: [Qemu-devel] [PATCH V5 0/8] cpu/acpi: convert cpu hot plug to hotplug_handler API

[Igor Mammedov]

On Mon, 20 Oct 2014 17:42:40 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Hi Igor,
> How about this version?
I'm reviewing it now.

> 
> Regards,
> Gu
> On 10/10/2014 10:15 AM, Gu Zheng wrote:
> 
> > Previously we use cpu_added_notifiers to register cpu hotplug notifier callback
> > which is not able to pass/handle errors, so we switch it to unified hotplug
> > handler API which allows to pass errors and would allow to cancel device_add
> > in case of error.
> > Thanks very much for Igor's review and suggestion.
> > 
> > ---
> > v5:
> >  -rebase on the latest upstream and fix some comments.
> >  Patch 4/8:
> >  -split the check out of acpi_dev block.
> >  Patch 5/8:
> >  -move CPU hot-plug notifier cleanup hunk into Patch 6/8.
> >  Patch 6/8:
> >  -delete the caller of notifier_list_notify() in this patch.
> >  Patch 8/8:
> >  -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.
> > 
> > v4:
> >  -split removal of CPU hotplug notifier into separate patch (Patch 6/8).
> >  Patch 1/7:
> >  -convert CPUState *cpu to DeviceState *dev like it's done for other handlers
> >   and do cast to CPU inside.
> >  Patch 5/7:
> >  -Make rtc_state as a link property in PCMachine rather than the global
> >   variables.
> >  -Split out the removal of unused notifier into separate patch.
> >  -Check the result of plug callback before update rtc_state.
> > 
> > v3:
> >  -deal with start-up cpus in pc_cpu_plug as Igor suggested.
> > 
> > v2:
> >  -Add 3 new patches(5/7,6/7,7/7), delete original patch 5/5.
> >   1/5-->1/7
> >   2/5-->2/7
> >   3/5-->3/7
> >   4/5-->4/7
> >  Patch 1/7:
> >  -add errp argument to catch error.
> >  -return error instead of aborting if cpu id is invalid.
> >  -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.
> >  Patch 3/7:
> >  -remove unused AcpiCpuHotplug_add directly.
> >  Patch 5/7:
> >  -switch the last user of cpu hotplug notifier to hotplug handler API, and
> >   remove the unused cpu hotplug notify.
> >  Patch 6/7:
> >  -split the function rename (just cleanup) into single patch.
> >  Patch 7/7:
> >  -introduce help function acpi_set_local_sts to keep the bit setting in
> >   one place.
> > ---
> > 
> > Gu Zheng (8):
> >   acpi/cpu: add cpu hotplug callback function to match hotplug_handler
> >     API
> >   acpi:ich9: convert cpu hotplug to hotplug_handler API
> >   acpi:piix4: convert cpu hotplug to hotplug_handler API
> >   pc: add cpu hotplug handler to PC_MACHINE
> >   pc: Update rtc_cmos in pc_cpu_plug
> >   qom/cpu: remove the unused CPU hot-plug notifier
> >   cpu-hotplug: rename function for better readability
> >   acpi/cpu-hotplug: introduce help function to keep bit setting in one
> >     place
> > 
> >  hw/acpi/cpu_hotplug.c         |   35 ++++++++++++++++------
> >  hw/acpi/ich9.c                |   17 ++--------
> >  hw/acpi/piix4.c               |   18 ++---------
> >  hw/i386/pc.c                  |   65 +++++++++++++++++++++++++++--------------
> >  hw/i386/pc_piix.c             |    2 +-
> >  hw/i386/pc_q35.c              |    2 +-
> >  include/hw/acpi/cpu_hotplug.h |    7 ++--
> >  include/hw/acpi/ich9.h        |    1 -
> >  include/hw/i386/pc.h          |    3 +-
> >  include/sysemu/sysemu.h       |    3 --
> >  qom/cpu.c                     |   10 ------
> >  11 files changed, 84 insertions(+), 79 deletions(-)
> > 
> 
> 
> 

Re: [Qemu-devel] [PATCH V5 1/8] acpi/cpu: add cpu hotplug callback function to match hotplug_handler API

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:00 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Add cpu hotplug callback function (acpi_cpu_plug_cb) to match hotplug_handler API.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> v4:
>  -convert CPUState *cpu to DeviceState *dev like it's done for other handlers
>   and do cast to CPU inside.
> v2:
>  -add errp argument to catch error.
>  -return error instead of aborting if cpu id is invalid.
>  -make acpi_cpu_plug_cb as a wrapper around AcpiCpuHotplug_add.
> 
>  hw/acpi/cpu_hotplug.c         |   18 ++++++++++++++++++
>  include/hw/acpi/cpu_hotplug.h |    3 +++
>  2 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index 2ad83a0..06e9c61 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -36,6 +36,24 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
>      },
>  };
>  
> +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
> +                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
> +{
> +    CPUState *cpu = CPU(dev);
> +    CPUClass *k = CPU_GET_CLASS(cpu);
> +    int64_t cpu_id;
> +
> +    cpu_id = k->get_arch_id(cpu);
> +    if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> +        error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> +        return;
> +    }
> +
> +    AcpiCpuHotplug_add(&ar->gpe, g, cpu);
> +
> +    acpi_update_sci(ar, irq);
> +}
> +
>  void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
>  {
>      CPUClass *k = CPU_GET_CLASS(cpu);
> diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
> index 9e5d30c..5dca8d7 100644
> --- a/include/hw/acpi/cpu_hotplug.h
> +++ b/include/hw/acpi/cpu_hotplug.h
> @@ -20,6 +20,9 @@ typedef struct AcpiCpuHotplug {
>      uint8_t sts[ACPI_GPE_PROC_LEN];
>  } AcpiCpuHotplug;
>  
> +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
> +                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
> +
>  void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
>  
>  void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,

Re: [Qemu-devel] [PATCH V5 2/8] acpi:ich9: convert cpu hotplug to hotplug_handler API

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:01 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Convert notifier based hotplug to hotplug_handler API.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> v5:
>  -drop 'handle' from the comment and subject.
> 
>  hw/acpi/ich9.c         |   13 ++-----------
>  include/hw/acpi/ich9.h |    1 -
>  2 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 7b14bbb..7585364 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -209,15 +209,6 @@ 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)
> -{
> -    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, cpu_added_notifier);
> -
> -    assert(pm != NULL);
> -    AcpiCpuHotplug_add(&pm->acpi_regs.gpe, &pm->gpe_cpu, CPU(opaque));
> -    acpi_update_sci(&pm->acpi_regs, pm->irq);
> -}
> -
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>                    qemu_irq sci_irq)
>  {
> @@ -246,8 +237,6 @@ 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);
>  
>      if (pm->acpi_memory_hotplug.is_enabled) {
>          acpi_memory_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
> @@ -304,6 +293,8 @@ void ich9_pm_device_plug_cb(ICH9LPCPMRegs *pm, DeviceState *dev, Error **errp)
>          object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          acpi_memory_plug_cb(&pm->acpi_regs, pm->irq, &pm->acpi_memory_hotplug,
>                              dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> +        acpi_cpu_plug_cb(&pm->acpi_regs, pm->irq, &pm->gpe_cpu, dev, errp);
>      } else {
>          error_setg(errp, "acpi: device plug request for not supported device"
>                     " type: %s", object_get_typename(OBJECT(dev)));
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index 7e42448..fe975e6 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -47,7 +47,6 @@ typedef struct ICH9LPCPMRegs {
>      Notifier powerdown_notifier;
>  
>      AcpiCpuHotplug gpe_cpu;
> -    Notifier cpu_added_notifier;
>  
>      MemHotplugState acpi_memory_hotplug;
>  } ICH9LPCPMRegs;

Re: [Qemu-devel] [PATCH V5 3/8] acpi:piix4: convert cpu hotplug to hotplug_handler API

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:02 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Convert notifier based hotplug to hotplug_handler API,
> and remove the unused AcpiCpuHotplug_add().
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> v5:
>  -drop 'handle' from the comment and subject.
> v2:
>  -remove the unused AcpiCpuHotplug_add().
> 
>  hw/acpi/cpu_hotplug.c         |   14 ++------------
>  hw/acpi/piix4.c               |   14 ++------------
>  include/hw/acpi/cpu_hotplug.h |    2 --
>  3 files changed, 4 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index 06e9c61..b69b16c 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -49,22 +49,12 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>          return;
>      }
>  
> -    AcpiCpuHotplug_add(&ar->gpe, g, cpu);
> +    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
> +    g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
>  
>      acpi_update_sci(ar, irq);
>  }
>  
> -void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu)
> -{
> -    CPUClass *k = CPU_GET_CLASS(cpu);
> -    int64_t cpu_id;
> -
> -    *gpe->sts = *gpe->sts | ACPI_CPU_HOTPLUG_STATUS;
> -    cpu_id = k->get_arch_id(CPU(cpu));
> -    g_assert((cpu_id / 8) < ACPI_GPE_PROC_LEN);
> -    g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
> -}
> -
>  void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
>                           AcpiCpuHotplug *gpe_cpu, uint16_t base)
>  {
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index b72b34e..8320b18 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -83,7 +83,6 @@ typedef struct PIIX4PMState {
>      uint8_t s4_val;
>  
>      AcpiCpuHotplug gpe_cpu;
> -    Notifier cpu_added_notifier;
>  
>      MemHotplugState acpi_memory_hotplug;
>  } PIIX4PMState;
> @@ -348,6 +347,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>          acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
>                                    errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> +        acpi_cpu_plug_cb(&s->ar, s->irq, &s->gpe_cpu, dev, errp);
>      } else {
>          error_setg(errp, "acpi: device plug request for not supported device"
>                     " type: %s", object_get_typename(OBJECT(dev)));
> @@ -544,15 +545,6 @@ static const MemoryRegionOps piix4_gpe_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> -static void piix4_cpu_added_req(Notifier *n, void *opaque)
> -{
> -    PIIX4PMState *s = container_of(n, PIIX4PMState, cpu_added_notifier);
> -
> -    assert(s != NULL);
> -    AcpiCpuHotplug_add(&s->ar.gpe, &s->gpe_cpu, CPU(opaque));
> -    acpi_update_sci(&s->ar, s->irq);
> -}
> -
>  static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
>                                             PCIBus *bus, PIIX4PMState *s)
>  {
> @@ -565,8 +557,6 @@ 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);
>  
>      if (s->acpi_memory_hotplug.is_enabled) {
>          acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
> diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
> index 5dca8d7..4657e71 100644
> --- a/include/hw/acpi/cpu_hotplug.h
> +++ b/include/hw/acpi/cpu_hotplug.h
> @@ -23,8 +23,6 @@ typedef struct AcpiCpuHotplug {
>  void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>                        AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
>  
> -void AcpiCpuHotplug_add(ACPIGPE *gpe, AcpiCpuHotplug *g, CPUState *cpu);
> -
>  void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
>                           AcpiCpuHotplug *gpe_cpu, uint16_t base);
>  #endif

Re: [Qemu-devel] [PATCH V5 4/8] pc: add cpu hotplug handler to PC_MACHINE

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:03 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Add cpu hotplug handler to PC_MACHINE, which will perform the acpi
> cpu hotplug callback via hotplug_handler API.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> v5:
>  -split the check out of acpi_dev block.
> v3:
>  -deal with start up cpus in a more neat way as Igor suggested.
> v2:
>  -just rebase.
> 
>  hw/i386/pc.c |   28 +++++++++++++++++++++++++++-
>  1 files changed, 27 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d045e8b..eff19ef 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1617,11 +1617,36 @@ out:
>      error_propagate(errp, local_err);
>  }
>  
> +static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> +                        DeviceState *dev, Error **errp)
> +{
> +    HotplugHandlerClass *hhc;
> +    Error *local_err = NULL;
> +    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +
> +    if (!dev->hotplugged) {
> +        goto out;
> +    }
> +
> +    if (!pcms->acpi_dev) {
> +        error_setg(&local_err,
> +                   "cpu hotplug is not enabled: missing acpi device");
> +        goto out;
> +    }
> +
> +    hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
> +    hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>                                        DeviceState *dev, Error **errp)
>  {
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          pc_dimm_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> +        pc_cpu_plug(hotplug_dev, dev, errp);
>      }
>  }
>  
> @@ -1630,7 +1655,8 @@ static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
>  {
>      PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
>  
> -    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
> +        object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>          return HOTPLUG_HANDLER(machine);
>      }
>  

Re: [Qemu-devel] [PATCH V5 5/8] pc: Update rtc_cmos in pc_cpu_plug

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:04 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Update rtc_cmos in pc_cpu_plug() directly, instead of the notifier.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> v5:
>  -move CPU hot-plug notifier cleanup hunk into Patch 6/8.
> 
> v4:
>  -Make link<rtc> property in PCMachine rather than the global
>   variables.
>  -Split out the removal of unused notifier into separate patch.
> 
>  hw/i386/pc.c         |   37 ++++++++++++++++---------------------
>  hw/i386/pc_piix.c    |    2 +-
>  hw/i386/pc_q35.c     |    2 +-
>  include/hw/i386/pc.h |    3 ++-
>  4 files changed, 20 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index eff19ef..0f10ec8 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -355,30 +355,15 @@ static void pc_cmos_init_late(void *opaque)
>      qemu_unregister_reset(pc_cmos_init_late, opaque);
>  }
>  
> -typedef struct RTCCPUHotplugArg {
> -    Notifier cpu_added_notifier;
> -    ISADevice *rtc_state;
> -} RTCCPUHotplugArg;
> -
> -static void rtc_notify_cpu_added(Notifier *notifier, void *data)
> -{
> -    RTCCPUHotplugArg *arg = container_of(notifier, RTCCPUHotplugArg,
> -                                         cpu_added_notifier);
> -    ISADevice *s = arg->rtc_state;
> -
> -    /* increment the number of CPUs */
> -    rtc_set_memory(s, 0x5f, rtc_get_memory(s, 0x5f) + 1);
> -}
> -
>  void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
> -                  const char *boot_device,
> +                  const char *boot_device, MachineState *machine,
>                    ISADevice *floppy, BusState *idebus0, BusState *idebus1,
>                    ISADevice *s)
>  {
>      int val, nb, i;
>      FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE };
>      static pc_cmos_init_late_arg arg;
> -    static RTCCPUHotplugArg cpu_hotplug_cb;
> +    PCMachineState *pc_machine = PC_MACHINE(machine);
>  
>      /* various important CMOS locations needed by PC/Bochs bios */
>  
> @@ -417,10 +402,14 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
>  
>      /* set the number of CPU */
>      rtc_set_memory(s, 0x5f, smp_cpus - 1);
> -    /* 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);
> +
> +    object_property_add_link(OBJECT(machine), "rtc_state",
> +                             TYPE_ISA_DEVICE,
> +                             (Object **)&pc_machine->rtc,
> +                             object_property_allow_set_link,
> +                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);
> +    object_property_set_link(OBJECT(machine), OBJECT(s),
> +                             "rtc_state", &error_abort);
>  
>      if (set_boot_dev(s, boot_device)) {
>          exit(1);
> @@ -1636,6 +1625,12 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>  
>      hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
>      hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    /* increment the number of CPUs */
> +    rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
>  out:
>      error_propagate(errp, local_err);
>  }
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 4384633..2ff7e84 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -266,7 +266,7 @@ static void pc_init1(MachineState *machine,
>      }
>  
>      pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order,
> -                 floppy, idebus[0], idebus[1], rtc_state);
> +                 machine, floppy, idebus[0], idebus[1], rtc_state);
>  
>      if (pci_enabled && usb_enabled(false)) {
>          pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index bb0dc8e..7cab8e4 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -270,7 +270,7 @@ static void pc_q35_init(MachineState *machine)
>                        8, NULL, 0);
>  
>      pc_cmos_init(below_4g_mem_size, above_4g_mem_size, machine->boot_order,
> -                 floppy, idebus[0], idebus[1], rtc_state);
> +                 machine, floppy, idebus[0], idebus[1], rtc_state);
>  
>      /* the rest devices to which pci devfn is automatically assigned */
>      pc_vga_init(isa_bus, host_bus);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 77316d5..7a4bff4 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -33,6 +33,7 @@ struct PCMachineState {
>      MemoryRegion hotplug_memory;
>  
>      HotplugHandler *acpi_dev;
> +    ISADevice *rtc;
>  
>      uint64_t max_ram_below_4g;
>  };
> @@ -210,7 +211,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
>                            uint32 hpet_irqs);
>  void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd);
>  void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
> -                  const char *boot_device,
> +                  const char *boot_device, MachineState *machine,
>                    ISADevice *floppy, BusState *ide0, BusState *ide1,
>                    ISADevice *s);
>  void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus);

Re: [Qemu-devel] [PATCH V5 6/8] qom/cpu: remove the unused CPU hot-plug notifier

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:05 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Remove the unused CPU hot-plug notifier.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> v5:
>  -delete the caller of notifier_list_notify() in this patch.
> 
>  include/sysemu/sysemu.h |    3 ---
>  qom/cpu.c               |   10 ----------
>  2 files changed, 0 insertions(+), 13 deletions(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index d8539fd..acfe494 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -183,9 +183,6 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
>  /* generic hotplug */
>  void drive_hot_add(Monitor *mon, const QDict *qdict);
>  
> -/* CPU hotplug */
> -void qemu_register_cpu_added_notifier(Notifier *notifier);
> -
>  /* pcie aer error injection */
>  void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
>  int do_pcie_aer_inject_error(Monitor *mon,
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 0ec3337..79d2228 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -107,15 +107,6 @@ static void cpu_common_get_memory_mapping(CPUState *cpu,
>      error_setg(errp, "Obtaining memory mappings is unsupported on this CPU.");
>  }
>  
> -/* CPU hot-plug notifiers */
> -static NotifierList cpu_added_notifiers =
> -    NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> -
> -void qemu_register_cpu_added_notifier(Notifier *notifier)
> -{
> -    notifier_list_add(&cpu_added_notifiers, notifier);
> -}
> -
>  void cpu_reset_interrupt(CPUState *cpu, int mask)
>  {
>      cpu->interrupt_request &= ~mask;
> @@ -312,7 +303,6 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>  
>      if (dev->hotplugged) {
>          cpu_synchronize_post_init(cpu);
> -        notifier_list_notify(&cpu_added_notifiers, dev);
>          cpu_resume(cpu);
>      }
>  }

Re: [Qemu-devel] [PATCH V5 7/8] cpu-hotplug: rename function for better readability

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:06 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Rename:
> AcpiCpuHotplug_init --> acpi_cpu_hotplug_init
> AcpiCpuHotplug_ops --> acpi_cpu_hotplug_ops
> for better readability, just cleanup.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/cpu_hotplug.c         |    4 ++--
>  hw/acpi/ich9.c                |    4 ++--
>  hw/acpi/piix4.c               |    4 ++--
>  include/hw/acpi/cpu_hotplug.h |    4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index b69b16c..ae48b63 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -55,8 +55,8 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>      acpi_update_sci(ar, irq);
>  }
>  
> -void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
> -                         AcpiCpuHotplug *gpe_cpu, uint16_t base)
> +void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> +                           AcpiCpuHotplug *gpe_cpu, uint16_t base)
>  {
>      CPUState *cpu;
>  
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 7585364..ea991a3 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -235,8 +235,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>      pm->powerdown_notifier.notify = pm_powerdown_req;
>      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
>  
> -    AcpiCpuHotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
> -                        &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
> +    acpi_cpu_hotplug_init(pci_address_space_io(lpc_pci), OBJECT(lpc_pci),
> +                          &pm->gpe_cpu, ICH9_CPU_HOTPLUG_IO_BASE);
>  
>      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 8320b18..8fde808 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -555,8 +555,8 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
>      acpi_pcihp_init(&s->acpi_pci_hotplug, bus, parent,
>                      s->use_acpi_pci_hotplug);
>  
> -    AcpiCpuHotplug_init(parent, OBJECT(s), &s->gpe_cpu,
> -                        PIIX4_CPU_HOTPLUG_IO_BASE);
> +    acpi_cpu_hotplug_init(parent, OBJECT(s), &s->gpe_cpu,
> +                          PIIX4_CPU_HOTPLUG_IO_BASE);
>  
>      if (s->acpi_memory_hotplug.is_enabled) {
>          acpi_memory_hotplug_init(parent, OBJECT(s), &s->acpi_memory_hotplug);
> diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
> index 4657e71..f6d358d 100644
> --- a/include/hw/acpi/cpu_hotplug.h
> +++ b/include/hw/acpi/cpu_hotplug.h
> @@ -23,6 +23,6 @@ typedef struct AcpiCpuHotplug {
>  void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>                        AcpiCpuHotplug *g, DeviceState *dev, Error **errp);
>  
> -void AcpiCpuHotplug_init(MemoryRegion *parent, Object *owner,
> -                         AcpiCpuHotplug *gpe_cpu, uint16_t base);
> +void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
> +                           AcpiCpuHotplug *gpe_cpu, uint16_t base);
>  #endif

Re: [Qemu-devel] [PATCH V5 8/8] acpi/cpu-hotplug: introduce helper function to keep bit setting in one place

[Igor Mammedov]

On Fri, 10 Oct 2014 10:16:07 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Introduce helper function acpi_set_cpu_present_bit() to simplify acpi_cpu_plug_cb
> and acpi_cpu_hotplug_init, so that we can keep bit setting in one place.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
> v5:
>  -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.
> 
>  hw/acpi/cpu_hotplug.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index ae48b63..8b8d15a 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -36,10 +36,9 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
>      },
>  };
>  
> -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
> -                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
> +static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> +                                     Error **errp)
>  {
> -    CPUState *cpu = CPU(dev);
>      CPUClass *k = CPU_GET_CLASS(cpu);
>      int64_t cpu_id;
>  
> @@ -49,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>          return;
>      }
>  
> -    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
>      g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
> +}
>  
> +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
> +                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
> +{
> +    acpi_set_cpu_present_bit(g, CPU(dev), errp);
> +    if (*errp != NULL) {
> +        return;
> +    }
> +
> +    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
>      acpi_update_sci(ar, irq);
>  }
>  
> @@ -61,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
>      CPUState *cpu;
>  
>      CPU_FOREACH(cpu) {
> -        CPUClass *cc = CPU_GET_CLASS(cpu);
> -        int64_t id = cc->get_arch_id(cpu);
> +        Error *local_err = NULL;
>  
> -        g_assert((id / 8) < ACPI_GPE_PROC_LEN);
> -        gpe_cpu->sts[id / 8] |= (1 << (id % 8));
> +        acpi_set_cpu_present_bit(gpe_cpu, cpu, &local_err);
> +        g_assert(local_err == NULL);
it would be nice to report error before dying,
using error_abort instead of local_err will do that for you.

>      }
>      memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
>                            gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);

Re: [Qemu-devel] [PATCH V5 8/8] acpi/cpu-hotplug: introduce helper function to keep bit setting in one place

[Gu Zheng]

Hi Igor,
On 10/20/2014 11:09 PM, Igor Mammedov wrote:

> On Fri, 10 Oct 2014 10:16:07 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> 
>> Introduce helper function acpi_set_cpu_present_bit() to simplify acpi_cpu_plug_cb
>> and acpi_cpu_hotplug_init, so that we can keep bit setting in one place.
>>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>> v5:
>>  -rename acpi_set_local_sts to acpi_set_cpu_present_bit for better readability.
>>
>>  hw/acpi/cpu_hotplug.c |   23 +++++++++++++++--------
>>  1 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
>> index ae48b63..8b8d15a 100644
>> --- a/hw/acpi/cpu_hotplug.c
>> +++ b/hw/acpi/cpu_hotplug.c
>> @@ -36,10 +36,9 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
>>      },
>>  };
>>  
>> -void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>> -                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
>> +static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
>> +                                     Error **errp)
>>  {
>> -    CPUState *cpu = CPU(dev);
>>      CPUClass *k = CPU_GET_CLASS(cpu);
>>      int64_t cpu_id;
>>  
>> @@ -49,9 +48,18 @@ void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>>          return;
>>      }
>>  
>> -    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
>>      g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
>> +}
>>  
>> +void acpi_cpu_plug_cb(ACPIREGS *ar, qemu_irq irq,
>> +                      AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
>> +{
>> +    acpi_set_cpu_present_bit(g, CPU(dev), errp);
>> +    if (*errp != NULL) {
>> +        return;
>> +    }
>> +
>> +    ar->gpe.sts[0] |= ACPI_CPU_HOTPLUG_STATUS;
>>      acpi_update_sci(ar, irq);
>>  }
>>  
>> @@ -61,11 +69,10 @@ void acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
>>      CPUState *cpu;
>>  
>>      CPU_FOREACH(cpu) {
>> -        CPUClass *cc = CPU_GET_CLASS(cpu);
>> -        int64_t id = cc->get_arch_id(cpu);
>> +        Error *local_err = NULL;
>>  
>> -        g_assert((id / 8) < ACPI_GPE_PROC_LEN);
>> -        gpe_cpu->sts[id / 8] |= (1 << (id % 8));
>> +        acpi_set_cpu_present_bit(gpe_cpu, cpu, &local_err);
>> +        g_assert(local_err == NULL);
> it would be nice to report error before dying,
> using error_abort instead of local_err will do that for you.

It would be nice. I'll fix it.
Thanks very for your review.

Best regards,
Gu

> 
>>      }
>>      memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
>>                            gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> 
> .
>