Hi Igor, On 09/26/2014 09:23 PM, Igor Mammedov wrote: > On Wed, 17 Sep 2014 09:24:01 +0800 > Gu Zheng <guz.f...@cn.fujitsu.com> wrote: > >> Update rtc_cmos in pc_cpu_plug directly instead of the notifier, with >> this change, there will no user of CPU hot-plug notifier any more, so >> remove it. >> >> Signed-off-by: Gu Zheng <guz.f...@cn.fujitsu.com> >> --- >> hw/i386/pc.c | 25 ++++++------------------- >> include/sysemu/sysemu.h | 3 --- >> qom/cpu.c | 10 ---------- >> 3 files changed, 6 insertions(+), 32 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index e25e71b..8b887c7 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -353,20 +353,7 @@ 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); >> -} >> +static ISADevice *rtc_state; > It's not recommended to use global variables. > > Instead of make link<rtc> property in PCMachine and use it pretty much like > acpi_dev > is used in 4/7.
Right, this way is more reasonable and neat. > >> >> void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, >> const char *boot_device, >> @@ -376,7 +363,6 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t >> above_4g_mem_size, >> 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; >> >> /* various important CMOS locations needed by PC/Bochs bios */ >> >> @@ -415,10 +401,8 @@ 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); >> + >> + rtc_state = s; >> >> if (set_boot_dev(s, boot_device)) { >> exit(1); >> @@ -1630,6 +1614,9 @@ 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); >> + >> + /* increment the number of CPUs */ >> + rtc_set_memory(rtc_state, 0x5f, rtc_get_memory(rtc_state, 0x5f) + 1); > looks wrong, what if error happens in plug() handler??? Ah, yes, we need to confirm the plug cb result before update rtc_state. Thanks, Gu > >> out: >> error_propagate(errp, local_err); >> } >> 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 b32dd0a..19c5de5 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; >> @@ -303,7 +294,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); >> } >> } > > . >