diff mbox

[06/16] target-i386: pc: update rtc_cmos on CPU hot-plug

Message ID 1366063976-4909-7-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov April 15, 2013, 10:12 p.m. UTC 
... so that on reboot BIOS could read current available CPU count

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c                   | 20 ++++++++++++++++++++
 hw/timer/mc146818rtc.c         |  7 +++++++
 include/hw/timer/mc146818rtc.h |  1 +
 3 files changed, 28 insertions(+)

Comments

Eduardo Habkost April 18, 2013, 5:09 p.m. UTC | #1 
On Tue, Apr 16, 2013 at 12:12:46AM +0200, Igor Mammedov wrote:
> ... so that on reboot BIOS could read current available CPU count
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hw/i386/pc.c                   | 20 ++++++++++++++++++++
>  hw/timer/mc146818rtc.c         |  7 +++++++
>  include/hw/timer/mc146818rtc.h |  1 +
>  3 files changed, 28 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8d75b34..dc1a78b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -337,6 +337,21 @@ static void pc_cmos_init_late(void *opaque)
>      qemu_unregister_reset(pc_cmos_init_late, opaque);
>  }
>  
> +typedef struct rtc_cpu_hotplug_arg {
> +    ISADevice *rtc_state;
> +    Notifier cpu_added_notifier;
> +} rtc_cpu_hotplug_arg;
> +
> +static void rtc_notify_cpu_added(Notifier *notifier, void *data)
> +{
> +    rtc_cpu_hotplug_arg *arg = container_of(notifier, rtc_cpu_hotplug_arg,
> +                                           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,
>                    ISADevice *floppy, BusState *idebus0, BusState *idebus1,
> @@ -345,6 +360,7 @@ 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 rtc_cpu_hotplug_arg cpu_hotplug_cb;
>  
>      /* various important CMOS locations needed by PC/Bochs bios */
>  
> @@ -383,6 +399,10 @@ 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);
>  
>      /* set boot devices, and disable floppy signature check if requested */
>      if (set_boot_dev(s, boot_device, fd_bootchk)) {
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 69e6844..e639942 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -677,6 +677,13 @@ void rtc_set_memory(ISADevice *dev, int addr, int val)
>          s->cmos_data[addr] = val;
>  }
>  
> +int rtc_get_memory(ISADevice *dev, int addr)
> +{
> +    RTCState *s = DO_UPCAST(RTCState, dev, dev);
> +    assert(addr >= 0 && addr <= 127);
> +    return s->cmos_data[addr];
> +}
> +
>  static void rtc_set_date_from_host(ISADevice *dev)
>  {
>      RTCState *s = DO_UPCAST(RTCState, dev, dev);
> diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h
> index 854ea3f..09f37b7 100644
> --- a/include/hw/timer/mc146818rtc.h
> +++ b/include/hw/timer/mc146818rtc.h
> @@ -6,6 +6,7 @@
>  
>  ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
>  void rtc_set_memory(ISADevice *dev, int addr, int val);
> +int rtc_get_memory(ISADevice *dev, int addr);
>  void rtc_set_date(ISADevice *dev, const struct tm *tm);
>  
>  #endif /* !MC146818RTC_H */
> -- 
> 1.8.2
> 
>
Andreas Färber April 22, 2013, 2:56 p.m. UTC | #2 
Am 16.04.2013 00:12, schrieb Igor Mammedov:
> ... so that on reboot BIOS could read current available CPU count
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/i386/pc.c                   | 20 ++++++++++++++++++++
>  hw/timer/mc146818rtc.c         |  7 +++++++
>  include/hw/timer/mc146818rtc.h |  1 +
>  3 files changed, 28 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8d75b34..dc1a78b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -337,6 +337,21 @@ static void pc_cmos_init_late(void *opaque)
>      qemu_unregister_reset(pc_cmos_init_late, opaque);
>  }
>  
> +typedef struct rtc_cpu_hotplug_arg {
> +    ISADevice *rtc_state;
> +    Notifier cpu_added_notifier;
> +} rtc_cpu_hotplug_arg;

Are the arguments (rtc_state) intentionally placed before the common part?

> +
> +static void rtc_notify_cpu_added(Notifier *notifier, void *data)
> +{
> +    rtc_cpu_hotplug_arg *arg = container_of(notifier, rtc_cpu_hotplug_arg,
> +                                           cpu_added_notifier);
> +    ISADevice *s = arg->rtc_state;
> +
> +    /* increment the number of CPUs */
> +    rtc_set_memory(s, 0x5f, rtc_get_memory(s, 0x5f) + 1);
> +}

I think this proves Gleb's point of reordering the notifier and CPU
resumption since the first instruction executed might theoretically be
reading the RTC memory.

Otherwise looks good with one minor nit...

> +
>  void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
>                    const char *boot_device,
>                    ISADevice *floppy, BusState *idebus0, BusState *idebus1,
> @@ -345,6 +360,7 @@ 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 rtc_cpu_hotplug_arg cpu_hotplug_cb;
>  
>      /* various important CMOS locations needed by PC/Bochs bios */
>  
> @@ -383,6 +399,10 @@ 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);
>  
>      /* set boot devices, and disable floppy signature check if requested */
>      if (set_boot_dev(s, boot_device, fd_bootchk)) {
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 69e6844..e639942 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -677,6 +677,13 @@ void rtc_set_memory(ISADevice *dev, int addr, int val)
>          s->cmos_data[addr] = val;
>  }
>  
> +int rtc_get_memory(ISADevice *dev, int addr)
> +{
> +    RTCState *s = DO_UPCAST(RTCState, dev, dev);

If we apply my mc146818rtc.c QOM'ification patch to qom-cpu first, this
can become MC146818_RTC(dev). Anthony just ack'ed that, but qemu.git is
currently broken for testing, so I'm still holding off changes.

Andreas

> +    assert(addr >= 0 && addr <= 127);
> +    return s->cmos_data[addr];
> +}
> +
>  static void rtc_set_date_from_host(ISADevice *dev)
>  {
>      RTCState *s = DO_UPCAST(RTCState, dev, dev);
> diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h
> index 854ea3f..09f37b7 100644
> --- a/include/hw/timer/mc146818rtc.h
> +++ b/include/hw/timer/mc146818rtc.h
> @@ -6,6 +6,7 @@
>  
>  ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
>  void rtc_set_memory(ISADevice *dev, int addr, int val);
> +int rtc_get_memory(ISADevice *dev, int addr);
>  void rtc_set_date(ISADevice *dev, const struct tm *tm);
>  
>  #endif /* !MC146818RTC_H */
Igor Mammedov April 22, 2013, 3:18 p.m. UTC | #3 
On Mon, 22 Apr 2013 16:56:59 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 16.04.2013 00:12, schrieb Igor Mammedov:
> > ... so that on reboot BIOS could read current available CPU count
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/i386/pc.c                   | 20 ++++++++++++++++++++
> >  hw/timer/mc146818rtc.c         |  7 +++++++
> >  include/hw/timer/mc146818rtc.h |  1 +
> >  3 files changed, 28 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 8d75b34..dc1a78b 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -337,6 +337,21 @@ static void pc_cmos_init_late(void *opaque)
> >      qemu_unregister_reset(pc_cmos_init_late, opaque);
> >  }
> >  
> > +typedef struct rtc_cpu_hotplug_arg {
> > +    ISADevice *rtc_state;
> > +    Notifier cpu_added_notifier;
> > +} rtc_cpu_hotplug_arg;
> 
> Are the arguments (rtc_state) intentionally placed before the common part?
no, but it's not QOM type so it doesn't matter.
I'll reorder it on next respin.

> 
> > +
> > +static void rtc_notify_cpu_added(Notifier *notifier, void *data)
> > +{
> > +    rtc_cpu_hotplug_arg *arg = container_of(notifier,
> > rtc_cpu_hotplug_arg,
> > +                                           cpu_added_notifier);
> > +    ISADevice *s = arg->rtc_state;
> > +
> > +    /* increment the number of CPUs */
> > +    rtc_set_memory(s, 0x5f, rtc_get_memory(s, 0x5f) + 1);
> > +}
> 
> I think this proves Gleb's point of reordering the notifier and CPU
> resumption since the first instruction executed might theoretically be
> reading the RTC memory.
I've already changed notifier clall place according to Gleb's suggestions.

> 
> Otherwise looks good with one minor nit...
> 
> > +
> >  void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
> >                    const char *boot_device,
> >                    ISADevice *floppy, BusState *idebus0, BusState
> > *idebus1, @@ -345,6 +360,7 @@ 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 rtc_cpu_hotplug_arg cpu_hotplug_cb;
> >  
> >      /* various important CMOS locations needed by PC/Bochs bios */
> >  
> > @@ -383,6 +399,10 @@ 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);
> >  
> >      /* set boot devices, and disable floppy signature check if requested
> > */ if (set_boot_dev(s, boot_device, fd_bootchk)) {
> > diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> > index 69e6844..e639942 100644
> > --- a/hw/timer/mc146818rtc.c
> > +++ b/hw/timer/mc146818rtc.c
> > @@ -677,6 +677,13 @@ void rtc_set_memory(ISADevice *dev, int addr, int
> > val) s->cmos_data[addr] = val;
> >  }
> >  
> > +int rtc_get_memory(ISADevice *dev, int addr)
> > +{
> > +    RTCState *s = DO_UPCAST(RTCState, dev, dev);
> 
> If we apply my mc146818rtc.c QOM'ification patch to qom-cpu first, this
> can become MC146818_RTC(dev). Anthony just ack'ed that, but qemu.git is
> currently broken for testing, so I'm still holding off changes.
if you push it into your qom-cpu tree now, I'll rebase on top of it.

> 
> Andreas
> 
> > +    assert(addr >= 0 && addr <= 127);
> > +    return s->cmos_data[addr];
> > +}
> > +
> >  static void rtc_set_date_from_host(ISADevice *dev)
> >  {
> >      RTCState *s = DO_UPCAST(RTCState, dev, dev);
> > diff --git a/include/hw/timer/mc146818rtc.h
> > b/include/hw/timer/mc146818rtc.h index 854ea3f..09f37b7 100644
> > --- a/include/hw/timer/mc146818rtc.h
> > +++ b/include/hw/timer/mc146818rtc.h
> > @@ -6,6 +6,7 @@
> >  
> >  ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
> >  void rtc_set_memory(ISADevice *dev, int addr, int val);
> > +int rtc_get_memory(ISADevice *dev, int addr);
> >  void rtc_set_date(ISADevice *dev, const struct tm *tm);
> >  
> >  #endif /* !MC146818RTC_H */
>
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8d75b34..dc1a78b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -337,6 +337,21 @@  static void pc_cmos_init_late(void *opaque)
     qemu_unregister_reset(pc_cmos_init_late, opaque);
 }
 
+typedef struct rtc_cpu_hotplug_arg {
+    ISADevice *rtc_state;
+    Notifier cpu_added_notifier;
+} rtc_cpu_hotplug_arg;
+
+static void rtc_notify_cpu_added(Notifier *notifier, void *data)
+{
+    rtc_cpu_hotplug_arg *arg = container_of(notifier, rtc_cpu_hotplug_arg,
+                                           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,
                   ISADevice *floppy, BusState *idebus0, BusState *idebus1,
@@ -345,6 +360,7 @@  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 rtc_cpu_hotplug_arg cpu_hotplug_cb;
 
     /* various important CMOS locations needed by PC/Bochs bios */
 
@@ -383,6 +399,10 @@  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);
 
     /* set boot devices, and disable floppy signature check if requested */
     if (set_boot_dev(s, boot_device, fd_bootchk)) {
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 69e6844..e639942 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -677,6 +677,13 @@  void rtc_set_memory(ISADevice *dev, int addr, int val)
         s->cmos_data[addr] = val;
 }
 
+int rtc_get_memory(ISADevice *dev, int addr)
+{
+    RTCState *s = DO_UPCAST(RTCState, dev, dev);
+    assert(addr >= 0 && addr <= 127);
+    return s->cmos_data[addr];
+}
+
 static void rtc_set_date_from_host(ISADevice *dev)
 {
     RTCState *s = DO_UPCAST(RTCState, dev, dev);
diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h
index 854ea3f..09f37b7 100644
--- a/include/hw/timer/mc146818rtc.h
+++ b/include/hw/timer/mc146818rtc.h
@@ -6,6 +6,7 @@ 
 
 ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq);
 void rtc_set_memory(ISADevice *dev, int addr, int val);
+int rtc_get_memory(ISADevice *dev, int addr);
 void rtc_set_date(ISADevice *dev, const struct tm *tm);
 
 #endif /* !MC146818RTC_H */