| Message ID | 1411987957-31190-6-git-send-email-guz.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
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); > } > }
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); >> } >> } > > . >
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); > >> } > >> } > > > > . > > > > >
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); >>>> } >>>> } >>> >>> . >>> >> >> >> > > . >
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); } }
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(-)