Re: [Qemu-devel] [PATCH 08/12] introduce CPU hot-plug notifier

[Igor Mammedov]

On Wed, 27 Mar 2013 12:06:10 +0100
Paolo Bonzini <address@hidden> wrote:

> Il 21/03/2013 15:28, Igor Mammedov ha scritto:
> > hot-added CPU id (APIC ID) will be distributed to acpi_piix4 and rtc_cmos
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  include/sysemu/sysemu.h                 |    4 ++++
> >  stubs/Makefile.objs                     |    1 +
> >  stubs/qemu_system_cpu_hotplug_request.c |    5 +++++
> >  vl.c                                    |   14 ++++++++++++++
> >  4 files changed, 24 insertions(+), 0 deletions(-)
> >  create mode 100644 stubs/qemu_system_cpu_hotplug_request.c
> > 
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 6578782..4b8f721 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -152,6 +152,10 @@ 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_add_notifier(Notifier *notifier);
> > +void qemu_system_cpu_hotplug_request(uint32_t id);
> > +
> >  /* 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/stubs/Makefile.objs b/stubs/Makefile.objs
> > index 9741e16..6a492f5 100644
> > --- a/stubs/Makefile.objs
> > +++ b/stubs/Makefile.objs
> > @@ -25,3 +25,4 @@ stub-obj-y += vmstate.o
> >  stub-obj-$(CONFIG_WIN32) += fd-register.o
> >  stub-obj-y += resume_vcpu.o
> >  stub-obj-y += get_icc_bus.o
> > +stub-obj-y += qemu_system_cpu_hotplug_request.o
> 
> You're adding one stub per patch.  I think this is a sign that something
> can be abstracted at a higher level (e.g. put something in cpus.c if it
> is softmmu-specific), or that it is added at the wrong place.
I've put notifier in vl.c since most of them are there and it doesn't make
much difference if it is in cpus.c, but stub is to reduce ifdeffenery in
headers, although target-i386/cpu.c is build per target so I could use
ifdef in include/sysemu/cpus.h
===
#ifndef CONFIG_USER_ONLY
void qemu_system_cpu_hotplug_request(uint32_t id);
#esle
static inline void qemu_system_cpu_hotplug_request(uint32_t id)
{
}
#endif
===

> 
> For example, this notifier can go in qom/cpu.c.
Yep there wouldn't be need for stub if notifier is in qom/cpu.c,
but I've figured out that people would object to put it there
since it's build only once for softmmu and *-user targets and
*-user target doesn't need it at all.

Andreas,
 would it be acceptable if notifier goes in qom/cpu.c, (it would
 add nop code to *-user target)?

 
> (Besides, I noticed now the get_icc_bus stub.  I didn't understand why
> it's used, but anyway adding CPU-specific stuff to libqemustub is
> absolutely a no-no).

True, If icc_bus was created at board level then there wouldn't be any need
for get_icc_bus(), it could be just looked up in qom tree. I'll try to do it.

BTW: is there any guidelines what might be added to libqemustub?
> 
> Paolo