Patchwork [08/12] introduce CPU hot-plug notifier

login
register
mail settings
Submitter Igor Mammedov
Date March 21, 2013, 2:28 p.m.
Message ID <1363876125-8264-9-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/229727/
State New
Headers show

Comments

Igor Mammedov - March 21, 2013, 2:28 p.m.
hot-added CPU id (APIC ID) will be distributed to acpi_piix4 and rtc_cmos

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 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
Paolo Bonzini - March 27, 2013, 11:06 a.m.
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 <imammedo@redhat.com>
> ---
>  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.

For example, this notifier can go in qom/cpu.c.

(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).

Paolo

> diff --git a/stubs/qemu_system_cpu_hotplug_request.c b/stubs/qemu_system_cpu_hotplug_request.c
> new file mode 100644
> index 0000000..ad4f394
> --- /dev/null
> +++ b/stubs/qemu_system_cpu_hotplug_request.c
> @@ -0,0 +1,5 @@
> +#include <sysemu/sysemu.h>
> +
> +void qemu_system_cpu_hotplug_request(uint32_t id)
> +{
> +}
> diff --git a/vl.c b/vl.c
> index aeed7f4..fd95e43 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1723,6 +1723,20 @@ void vm_start(void)
>      }
>  }
>  
> +/* CPU hot-plug notifiers */
> +static NotifierList cpu_add_notifiers =
> +    NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
> +
> +void qemu_register_cpu_add_notifier(Notifier *notifier)
> +{
> +    notifier_list_add(&cpu_add_notifiers, notifier);
> +}
> +
> +void qemu_system_cpu_hotplug_request(uint32_t id)
> +{
> +    notifier_list_notify(&cpu_add_notifiers, &id);
> +}
> +
>  /* reset/shutdown handler */
>  
>  typedef struct QEMUResetEntry {
>
Igor Mammedov - March 27, 2013, 3:24 p.m.
On Wed, 27 Mar 2013 12:06:10 +0100
Paolo Bonzini <pbonzini@redhat.com> 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 <imammedo@redhat.com>
> > ---
> >  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
Paolo Bonzini - March 27, 2013, 3:36 p.m.
Il 27/03/2013 16:24, Igor Mammedov ha scritto:
> I've put notifier in vl.c since most of them are there

They are there, because the code that invokes them is also there.  In
fact, most calls of notifier_list_notify in vl.c are from static functions.

> 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)?

I think adding dead code to *-user is fine.

>> > (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?

Nothing. :)

Seriously, you really should use it only when an entire subsystem does
not exist in tools or user-level emulation (monitor, vm_clock,
migration, slirp).  Everything else will probably be served better by
methods, in all likelihood.

Paolo

Patch

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
diff --git a/stubs/qemu_system_cpu_hotplug_request.c b/stubs/qemu_system_cpu_hotplug_request.c
new file mode 100644
index 0000000..ad4f394
--- /dev/null
+++ b/stubs/qemu_system_cpu_hotplug_request.c
@@ -0,0 +1,5 @@ 
+#include <sysemu/sysemu.h>
+
+void qemu_system_cpu_hotplug_request(uint32_t id)
+{
+}
diff --git a/vl.c b/vl.c
index aeed7f4..fd95e43 100644
--- a/vl.c
+++ b/vl.c
@@ -1723,6 +1723,20 @@  void vm_start(void)
     }
 }
 
+/* CPU hot-plug notifiers */
+static NotifierList cpu_add_notifiers =
+    NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
+
+void qemu_register_cpu_add_notifier(Notifier *notifier)
+{
+    notifier_list_add(&cpu_add_notifiers, notifier);
+}
+
+void qemu_system_cpu_hotplug_request(uint32_t id)
+{
+    notifier_list_notify(&cpu_add_notifiers, &id);
+}
+
 /* reset/shutdown handler */
 
 typedef struct QEMUResetEntry {