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

[Qemu-devel] [PATCH V4 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.

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 handle to hotplug_handler API
  acpi:piix4: convert cpu hotplug handle 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                  |   63 ++++++++++++++++++++++++++--------------
 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, 82 insertions(+), 79 deletions(-)

-- 
1.7.7

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

[Gu Zheng]

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.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 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

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

[Igor Mammedov]

On Mon, 29 Sep 2014 18:52:30 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> 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.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
Changelog is usually put below this line, so it wouldn't stay in commit message
+ into a cover letter of series.

The same applies to the rest of patches in series.


>  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,

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

[Gu Zheng]

Convert notifier based hotplug handle to hotplug_handler API.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 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

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

[Igor Mammedov]

On Mon, 29 Sep 2014 18:52:31 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Convert notifier based hotplug handle to hotplug_handler API.

drop 'handle' above and in subj.

the same applies to the next patch

> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
>  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;

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

[Gu Zheng]

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

v2:
-remove the unused AcpiCpuHotplug_add().

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 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 V4 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.

v3:
 -deal with start up cpus in a more neat way as Igor suggested.
v2:
 -just rebase.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 82a7daa..dcb9332 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1616,11 +1616,34 @@ 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 (!pcms->acpi_dev) {
+        if (dev->hotplugged) {
+            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);
     }
 }
 
@@ -1629,7 +1652,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

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

[Igor Mammedov]

On Mon, 29 Sep 2014 18:52:33 +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.
> 
> v3:
>  -deal with start up cpus in a more neat way as Igor suggested.
> v2:
>  -just rebase.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
>  hw/i386/pc.c |   26 +++++++++++++++++++++++++-
>  1 files changed, 25 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 82a7daa..dcb9332 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1616,11 +1616,34 @@ 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 (!pcms->acpi_dev) {
> +        if (dev->hotplugged) {
it could be better to move this check out of acpi_dev block
so it wouldn't rely on acpi_dev being uninitialized for
initial CPUs.
 
> +            error_setg(&local_err,
> +                       "cpu hotplug is not enabled: missing acpi device");
> +        }
> +        goto out;
> +    }

something like this:

    if (!pcms->acpi_dev) {
        error_setg(&local_err, "missing acpi device");
        goto out;
    }

    if (!dev->hotplugged) {
        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);
>      }
>  }
>  
> @@ -1629,7 +1652,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);
>      }
>  

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

[Gu Zheng]

Update rtc_cmos in pc_cpu_plug directly instead of the notifier.

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

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 hw/i386/pc.c         |   37 ++++++++++++++++---------------------
 hw/i386/pc_piix.c    |    2 +-
 hw/i386/pc_q35.c     |    2 +-
 include/hw/i386/pc.h |    3 ++-
 qom/cpu.c            |    1 -
 5 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index dcb9332..301e704 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);
@@ -1633,6 +1622,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 103d756..2c8d4dc 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 d4a907c..94ba98d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -266,7 +266,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);
diff --git a/qom/cpu.c b/qom/cpu.c
index ba8b402..09c7e41 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -307,7 +307,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

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

[Igor Mammedov]

On Mon, 29 Sep 2014 18:52:34 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Update rtc_cmos in pc_cpu_plug directly instead of the notifier.
> 
> v4:
>  -Make link<rtc> property in PCMachine rather than the global
>   variables.
>  -Split out the removal of unused notifier into separate patch.
> 
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
>  hw/i386/pc.c         |   37 ++++++++++++++++---------------------
>  hw/i386/pc_piix.c    |    2 +-
>  hw/i386/pc_q35.c     |    2 +-
>  include/hw/i386/pc.h |    3 ++-
>  qom/cpu.c            |    1 -
>  5 files changed, 20 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index dcb9332..301e704 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);
> @@ -1633,6 +1622,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 103d756..2c8d4dc 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 d4a907c..94ba98d 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -266,7 +266,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);
> diff --git a/qom/cpu.c b/qom/cpu.c
> index ba8b402..09c7e41 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -307,7 +307,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);
this hunk belongs to the next patch

>          cpu_resume(cpu);
>      }
>  }

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

[Gu Zheng]

Hi Igor,

On 10/07/2014 09:01 PM, Igor Mammedov wrote:

> On Mon, 29 Sep 2014 18:52:34 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> 
>> Update rtc_cmos in pc_cpu_plug directly instead of the notifier.
>>
>> v4:
>>  -Make link<rtc> property in PCMachine rather than the global
>>   variables.
>>  -Split out the removal of unused notifier into separate patch.
>>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>>  hw/i386/pc.c         |   37 ++++++++++++++++---------------------
>>  hw/i386/pc_piix.c    |    2 +-
>>  hw/i386/pc_q35.c     |    2 +-
>>  include/hw/i386/pc.h |    3 ++-
>>  qom/cpu.c            |    1 -
>>  5 files changed, 20 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index dcb9332..301e704 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);
>> @@ -1633,6 +1622,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 103d756..2c8d4dc 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 d4a907c..94ba98d 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -266,7 +266,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);
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index ba8b402..09c7e41 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -307,7 +307,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);
> this hunk belongs to the next patch

This notifier was only used to update rtc state, and after we dropped
rtc state update into pc_cpu_plug, this line will be dead code, so I
think deleting it here is more reasonable.
And in the next patch, just removing the unused CPU hot-plug notifier.

Thanks,
Gu

> 
>>          cpu_resume(cpu);
>>      }
>>  }
> 
> .
> 

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

[Igor Mammedov]

On Wed, 8 Oct 2014 09:12:11 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Hi Igor,
> 
> On 10/07/2014 09:01 PM, Igor Mammedov wrote:
> 
> > On Mon, 29 Sep 2014 18:52:34 +0800
> > Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> > 
> >> Update rtc_cmos in pc_cpu_plug directly instead of the notifier.
> >>
> >> v4:
> >>  -Make link<rtc> property in PCMachine rather than the global
> >>   variables.
> >>  -Split out the removal of unused notifier into separate patch.
> >>
> >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> >> ---
> >>  hw/i386/pc.c         |   37 ++++++++++++++++---------------------
> >>  hw/i386/pc_piix.c    |    2 +-
> >>  hw/i386/pc_q35.c     |    2 +-
> >>  include/hw/i386/pc.h |    3 ++-
> >>  qom/cpu.c            |    1 -
> >>  5 files changed, 20 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index dcb9332..301e704 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);
> >> @@ -1633,6 +1622,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 103d756..2c8d4dc 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 d4a907c..94ba98d 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -266,7 +266,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);
> >> diff --git a/qom/cpu.c b/qom/cpu.c
> >> index ba8b402..09c7e41 100644
> >> --- a/qom/cpu.c
> >> +++ b/qom/cpu.c
> >> @@ -307,7 +307,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);
> > this hunk belongs to the next patch
> 
> This notifier was only used to update rtc state, and after we dropped
> rtc state update into pc_cpu_plug, this line will be dead code, so I
> think deleting it here is more reasonable.
Well the same reasoning would apply to CPU hot-plug notifier as well.
But above hunk is not related to RTC but it's a part of notifier itself.
RTC related part is refactoring RTC callback but not mechanism that
called it (i.e. CPU hot-plug notifier).

> And in the next patch, just removing the unused CPU hot-plug notifier.

> 
> Thanks,
> Gu
> 
> > 
> >>          cpu_resume(cpu);
> >>      }
> >>  }
> > 
> > .
> > 
> 
> 
> 

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

[Gu Zheng]

On 10/08/2014 08:25 PM, Igor Mammedov wrote:

> On Wed, 8 Oct 2014 09:12:11 +0800
> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> 
>> Hi Igor,
>>
>> On 10/07/2014 09:01 PM, Igor Mammedov wrote:
>>
>>> On Mon, 29 Sep 2014 18:52:34 +0800
>>> Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>>>
>>>> Update rtc_cmos in pc_cpu_plug directly instead of the notifier.
>>>>
>>>> v4:
>>>>  -Make link<rtc> property in PCMachine rather than the global
>>>>   variables.
>>>>  -Split out the removal of unused notifier into separate patch.
>>>>
>>>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>>>> ---
>>>>  hw/i386/pc.c         |   37 ++++++++++++++++---------------------
>>>>  hw/i386/pc_piix.c    |    2 +-
>>>>  hw/i386/pc_q35.c     |    2 +-
>>>>  include/hw/i386/pc.h |    3 ++-
>>>>  qom/cpu.c            |    1 -
>>>>  5 files changed, 20 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>>> index dcb9332..301e704 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);
>>>> @@ -1633,6 +1622,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 103d756..2c8d4dc 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 d4a907c..94ba98d 100644
>>>> --- a/hw/i386/pc_q35.c
>>>> +++ b/hw/i386/pc_q35.c
>>>> @@ -266,7 +266,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);
>>>> diff --git a/qom/cpu.c b/qom/cpu.c
>>>> index ba8b402..09c7e41 100644
>>>> --- a/qom/cpu.c
>>>> +++ b/qom/cpu.c
>>>> @@ -307,7 +307,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);
>>> this hunk belongs to the next patch
>>
>> This notifier was only used to update rtc state, and after we dropped
>> rtc state update into pc_cpu_plug, this line will be dead code, so I
>> think deleting it here is more reasonable.
> Well the same reasoning would apply to CPU hot-plug notifier as well.
> But above hunk is not related to RTC but it's a part of notifier itself.
> RTC related part is refactoring RTC callback but not mechanism that
> called it (i.e. CPU hot-plug notifier).

Got it, thanks.

Best regards,
Gu

> 
>> And in the next patch, just removing the unused CPU hot-plug notifier.
> 
>>
>> Thanks,
>> Gu
>>
>>>
>>>>          cpu_resume(cpu);
>>>>      }
>>>>  }
>>>
>>> .
>>>
>>
>>
>>
> 
> .
> 

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

[Gu Zheng]

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 include/sysemu/sysemu.h |    3 ---
 qom/cpu.c               |    9 ---------
 2 files changed, 0 insertions(+), 12 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 09c7e41..83c3559 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;
-- 
1.7.7

[Qemu-devel] [PATCH V4 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 V4 8/8] acpi/cpu-hotplug: introduce help function to keep bit setting in one place

[Gu Zheng]

Introduce help function acpi_set_local_sts() 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>
---
 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..8ff8c4d 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_local_sts(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_local_sts(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_local_sts(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 V4 8/8] acpi/cpu-hotplug: introduce help function to keep bit setting in one place

[Igor Mammedov]

On Mon, 29 Sep 2014 18:52:37 +0800
Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:

> Introduce help function acpi_set_local_sts() to simplify acpi_cpu_plug_cb
s/help/helper/

> 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>
> ---
>  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..8ff8c4d 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_local_sts(AcpiCpuHotplug *g, CPUState *cpu,
> +                               Error **errp)
Maybe better would be: s/acpi_set_local_sts/acpi_set_cpu_present_bit/

>  {
> -    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_local_sts(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_local_sts(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);