Patchwork [05/16] introduce CPU hot-plug notifier

login
register
mail settings
Submitter Igor Mammedov
Date April 15, 2013, 10:12 p.m.
Message ID <1366063976-4909-6-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/236741/
State New
Headers show

Comments

Igor Mammedov - April 15, 2013, 10:12 p.m.
hot-added CPU will be distributed to acpi_piix4 and rtc_cmos

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
v2:
  * move notifier to qom/cpu.c and call it from CPUClass.realize() on hotplug
  * remove get_firmware_id() since it belong to other patch
---
 include/sysemu/sysemu.h |  3 +++
 qom/cpu.c               | 12 ++++++++++++
 2 files changed, 15 insertions(+)
Gleb Natapov - April 22, 2013, 11 a.m.
On Tue, Apr 16, 2013 at 12:12:45AM +0200, Igor Mammedov wrote:
> hot-added CPU will be distributed to acpi_piix4 and rtc_cmos
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> v2:
>   * move notifier to qom/cpu.c and call it from CPUClass.realize() on hotplug
>   * remove get_firmware_id() since it belong to other patch
> ---
>  include/sysemu/sysemu.h |  3 +++
>  qom/cpu.c               | 12 ++++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 6578782..a8c3de1 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -152,6 +152,9 @@ 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 40a4259..819986e 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -21,6 +21,17 @@
>  #include "qom/cpu.h"
>  #include "qemu-common.h"
>  #include "sysemu/kvm.h"
> +#include "qemu/notify.h"
> +#include "sysemu/sysemu.h"
> +
> +/* 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)
>  {
> @@ -61,6 +72,7 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
>      if (dev->hotplugged) {
>          cpu_synchronize_post_init(CPU(dev));
>          resume_vcpu(CPU(dev));
> +        notifier_list_notify(&cpu_added_notifiers, dev);
Why do it after vcpu is running? May be notifier want to do something
that should be done before vcpu is running.

>      }
>  }
>  
> -- 
> 1.8.2
> 
> 

--
			Gleb.
Igor Mammedov - April 22, 2013, 11:09 a.m.
On Mon, 22 Apr 2013 14:00:01 +0300
Gleb Natapov <gleb@redhat.com> wrote:

> On Tue, Apr 16, 2013 at 12:12:45AM +0200, Igor Mammedov wrote:
> > hot-added CPU will be distributed to acpi_piix4 and rtc_cmos
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > v2:
> >   * move notifier to qom/cpu.c and call it from CPUClass.realize() on
> > hotplug
> >   * remove get_firmware_id() since it belong to other patch
> > ---
> >  include/sysemu/sysemu.h |  3 +++
> >  qom/cpu.c               | 12 ++++++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 6578782..a8c3de1 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -152,6 +152,9 @@ 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 40a4259..819986e 100644
> > --- a/qom/cpu.c
> > +++ b/qom/cpu.c
> > @@ -21,6 +21,17 @@
> >  #include "qom/cpu.h"
> >  #include "qemu-common.h"
> >  #include "sysemu/kvm.h"
> > +#include "qemu/notify.h"
> > +#include "sysemu/sysemu.h"
> > +
> > +/* 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)
> >  {
> > @@ -61,6 +72,7 @@ static void cpu_common_realizefn(DeviceState *dev,
> > Error **errp) if (dev->hotplugged) {
> >          cpu_synchronize_post_init(CPU(dev));
> >          resume_vcpu(CPU(dev));
> > +        notifier_list_notify(&cpu_added_notifiers, dev);
> Why do it after vcpu is running? May be notifier want to do something
> that should be done before vcpu is running.
currently there is not need for adding_cpu notifier.
This one signals that there is a new fully functional CPU available.

> >      }
> >  }
> >  
> > -- 
> > 1.8.2
> > 
> > 
> 
> --
> 			Gleb.
Gleb Natapov - April 22, 2013, 11:24 a.m.
On Mon, Apr 22, 2013 at 01:09:19PM +0200, Igor Mammedov wrote:
> On Mon, 22 Apr 2013 14:00:01 +0300
> Gleb Natapov <gleb@redhat.com> wrote:
> 
> > On Tue, Apr 16, 2013 at 12:12:45AM +0200, Igor Mammedov wrote:
> > > hot-added CPU will be distributed to acpi_piix4 and rtc_cmos
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > v2:
> > >   * move notifier to qom/cpu.c and call it from CPUClass.realize() on
> > > hotplug
> > >   * remove get_firmware_id() since it belong to other patch
> > > ---
> > >  include/sysemu/sysemu.h |  3 +++
> > >  qom/cpu.c               | 12 ++++++++++++
> > >  2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > index 6578782..a8c3de1 100644
> > > --- a/include/sysemu/sysemu.h
> > > +++ b/include/sysemu/sysemu.h
> > > @@ -152,6 +152,9 @@ 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 40a4259..819986e 100644
> > > --- a/qom/cpu.c
> > > +++ b/qom/cpu.c
> > > @@ -21,6 +21,17 @@
> > >  #include "qom/cpu.h"
> > >  #include "qemu-common.h"
> > >  #include "sysemu/kvm.h"
> > > +#include "qemu/notify.h"
> > > +#include "sysemu/sysemu.h"
> > > +
> > > +/* 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)
> > >  {
> > > @@ -61,6 +72,7 @@ static void cpu_common_realizefn(DeviceState *dev,
> > > Error **errp) if (dev->hotplugged) {
> > >          cpu_synchronize_post_init(CPU(dev));
> > >          resume_vcpu(CPU(dev));
> > > +        notifier_list_notify(&cpu_added_notifiers, dev);
> > Why do it after vcpu is running? May be notifier want to do something
> > that should be done before vcpu is running.
> currently there is not need for adding_cpu notifier.
> This one signals that there is a new fully functional CPU available.
> 
You are using it to update cmos information for BIOS. It would be strange
for newly added vcpu to see incorrect number of cpus in cmos if, for
some reason, it will check it. The "new cpu is added and ready to tun,
do your last things before it is launched" callback seams more reasonable
then "new cpu is running wild" one.

--
			Gleb.
Igor Mammedov - April 22, 2013, 8:01 p.m.
On Mon, 22 Apr 2013 14:24:00 +0300
Gleb Natapov <gleb@redhat.com> wrote:

> On Mon, Apr 22, 2013 at 01:09:19PM +0200, Igor Mammedov wrote:
> > On Mon, 22 Apr 2013 14:00:01 +0300
> > Gleb Natapov <gleb@redhat.com> wrote:
> > 
> > > On Tue, Apr 16, 2013 at 12:12:45AM +0200, Igor Mammedov wrote:
> > > > hot-added CPU will be distributed to acpi_piix4 and rtc_cmos
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > ---
> > > > v2:
> > > >   * move notifier to qom/cpu.c and call it from CPUClass.realize() on
> > > > hotplug
> > > >   * remove get_firmware_id() since it belong to other patch
> > > > ---
> > > >  include/sysemu/sysemu.h |  3 +++
> > > >  qom/cpu.c               | 12 ++++++++++++
> > > >  2 files changed, 15 insertions(+)
> > > > 
> > > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > > > index 6578782..a8c3de1 100644
> > > > --- a/include/sysemu/sysemu.h
> > > > +++ b/include/sysemu/sysemu.h
> > > > @@ -152,6 +152,9 @@ 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 40a4259..819986e 100644
> > > > --- a/qom/cpu.c
> > > > +++ b/qom/cpu.c
> > > > @@ -21,6 +21,17 @@
> > > >  #include "qom/cpu.h"
> > > >  #include "qemu-common.h"
> > > >  #include "sysemu/kvm.h"
> > > > +#include "qemu/notify.h"
> > > > +#include "sysemu/sysemu.h"
> > > > +
> > > > +/* 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)
> > > >  {
> > > > @@ -61,6 +72,7 @@ static void cpu_common_realizefn(DeviceState *dev,
> > > > Error **errp) if (dev->hotplugged) {
> > > >          cpu_synchronize_post_init(CPU(dev));
> > > >          resume_vcpu(CPU(dev));
> > > > +        notifier_list_notify(&cpu_added_notifiers, dev);
> > > Why do it after vcpu is running? May be notifier want to do something
> > > that should be done before vcpu is running.
> > currently there is not need for adding_cpu notifier.
> > This one signals that there is a new fully functional CPU available.
> > 
> You are using it to update cmos information for BIOS. It would be strange
> for newly added vcpu to see incorrect number of cpus in cmos if, for
> some reason, it will check it. The "new cpu is added and ready to tun,
> do your last things before it is launched" callback seams more reasonable
> then "new cpu is running wild" one.
IRC,
I'm moving it between cpu_synchronize_post_init() and resume_vcpu(), since
cpu_synchronize_post_init() pushes apic_state into kvm.

> --
> 			Gleb.

Patch

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6578782..a8c3de1 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -152,6 +152,9 @@  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 40a4259..819986e 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -21,6 +21,17 @@ 
 #include "qom/cpu.h"
 #include "qemu-common.h"
 #include "sysemu/kvm.h"
+#include "qemu/notify.h"
+#include "sysemu/sysemu.h"
+
+/* 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)
 {
@@ -61,6 +72,7 @@  static void cpu_common_realizefn(DeviceState *dev, Error **errp)
     if (dev->hotplugged) {
         cpu_synchronize_post_init(CPU(dev));
         resume_vcpu(CPU(dev));
+        notifier_list_notify(&cpu_added_notifiers, dev);
     }
 }