diff mbox

[3/3] ppc: include timebase in migration stream for g3beige/mac99 machines

Message ID 1454267976-27242-4-git-send-email-mark.cave-ayland@ilande.co.uk
State New
Headers show

Commit Message

Mark Cave-Ayland Jan. 31, 2016, 7:19 p.m. UTC 
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/ppc/mac_newworld.c |    4 ++++
 hw/ppc/mac_oldworld.c |    4 ++++
 2 files changed, 8 insertions(+)

Comments

Peter Maydell Jan. 31, 2016, 7:58 p.m. UTC | #1 
On 31 January 2016 at 19:19, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/ppc/mac_newworld.c |    4 ++++
>  hw/ppc/mac_oldworld.c |    4 ++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index f95086b..3283f1d 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -179,6 +179,7 @@ static void ppc_core99_init(MachineState *machine)
>      int *token = g_new(int, 1);
>      hwaddr nvram_addr = 0xFFF04000;
>      uint64_t tbfreq;
> +    PPCTimebase *tb;
>
>      linux_boot = (kernel_filename != NULL);
>
> @@ -201,6 +202,9 @@ static void ppc_core99_init(MachineState *machine)
>          /* Set time-base frequency to 100 Mhz */
>          cpu_ppc_tb_init(env, TBFREQ);
>          qemu_register_reset(ppc_core99_reset, cpu);
> +
> +        tb = g_malloc0(sizeof(PPCTimebase));
> +        vmstate_register(NULL, -1, &vmstate_ppc_timebase, tb);

Is there no way to avoid the vmstate_register here (ie to
tie the migration data to an actual device or CPU object) ?

thanks
-- PMM
Mark Cave-Ayland Jan. 31, 2016, 8:10 p.m. UTC | #2 
On 31/01/16 19:58, Peter Maydell wrote:

> On 31 January 2016 at 19:19, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/ppc/mac_newworld.c |    4 ++++
>>  hw/ppc/mac_oldworld.c |    4 ++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
>> index f95086b..3283f1d 100644
>> --- a/hw/ppc/mac_newworld.c
>> +++ b/hw/ppc/mac_newworld.c
>> @@ -179,6 +179,7 @@ static void ppc_core99_init(MachineState *machine)
>>      int *token = g_new(int, 1);
>>      hwaddr nvram_addr = 0xFFF04000;
>>      uint64_t tbfreq;
>> +    PPCTimebase *tb;
>>
>>      linux_boot = (kernel_filename != NULL);
>>
>> @@ -201,6 +202,9 @@ static void ppc_core99_init(MachineState *machine)
>>          /* Set time-base frequency to 100 Mhz */
>>          cpu_ppc_tb_init(env, TBFREQ);
>>          qemu_register_reset(ppc_core99_reset, cpu);
>> +
>> +        tb = g_malloc0(sizeof(PPCTimebase));
>> +        vmstate_register(NULL, -1, &vmstate_ppc_timebase, tb);
> 
> Is there no way to avoid the vmstate_register here (ie to
> tie the migration data to an actual device or CPU object) ?

Not exactly that I know of - although I shamelessly borrowed this part
from similar code in spapr which has this comment:

    /* FIXME: Should register things through the MachineState's qdev
     * interface, this is a legacy from the sPAPREnvironment structure
     * which predated MachineState but had a similar function */

Is this something that is now possible?


ATB,

Mark.
David Gibson Feb. 1, 2016, 1:36 a.m. UTC | #3 
On Sun, Jan 31, 2016 at 08:10:08PM +0000, Mark Cave-Ayland wrote:
> On 31/01/16 19:58, Peter Maydell wrote:
> 
> > On 31 January 2016 at 19:19, Mark Cave-Ayland
> > <mark.cave-ayland@ilande.co.uk> wrote:
> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> >> ---
> >>  hw/ppc/mac_newworld.c |    4 ++++
> >>  hw/ppc/mac_oldworld.c |    4 ++++
> >>  2 files changed, 8 insertions(+)
> >>
> >> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> >> index f95086b..3283f1d 100644
> >> --- a/hw/ppc/mac_newworld.c
> >> +++ b/hw/ppc/mac_newworld.c
> >> @@ -179,6 +179,7 @@ static void ppc_core99_init(MachineState *machine)
> >>      int *token = g_new(int, 1);
> >>      hwaddr nvram_addr = 0xFFF04000;
> >>      uint64_t tbfreq;
> >> +    PPCTimebase *tb;
> >>
> >>      linux_boot = (kernel_filename != NULL);
> >>
> >> @@ -201,6 +202,9 @@ static void ppc_core99_init(MachineState *machine)
> >>          /* Set time-base frequency to 100 Mhz */
> >>          cpu_ppc_tb_init(env, TBFREQ);
> >>          qemu_register_reset(ppc_core99_reset, cpu);
> >> +
> >> +        tb = g_malloc0(sizeof(PPCTimebase));
> >> +        vmstate_register(NULL, -1, &vmstate_ppc_timebase, tb);
> > 
> > Is there no way to avoid the vmstate_register here (ie to
> > tie the migration data to an actual device or CPU object) ?
> 
> Not exactly that I know of - although I shamelessly borrowed this part
> from similar code in spapr which has this comment:
> 
>     /* FIXME: Should register things through the MachineState's qdev
>      * interface, this is a legacy from the sPAPREnvironment structure
>      * which predated MachineState but had a similar function */
> 
> Is this something that is now possible?

Well, it's certainly possible to do better than this.  You want to
make a vmstate_g3beige and vmstate_mac99 which contain all the machine
level things to migrate for these machines, similar to vmstate_spapr.
They will be attached to the MachineState object.

That will at least mean that if more things need to get added to
migration for these machines, then additional vmstate_register() calls
won't be needed.

I'm not sure if there's a better way to register a vmstate for a
machine type.  I thought there was, but I couldn't spot it in a quick
lock.


Peter,

I believe this does need to be attached to the machine, not to the
cpu, even though the cpu would seem to make more sense on a first
look.  The reason is that attaching it to the cpu means it will be
transferred separately for each cpu, and unless we're super-careful
about timing the destination cpus could end up with slightly different
values.  That would be bad, because ppc has a pretty strong
requirement that the timebases be synchronized across all cpus in an
smp system.  The means of initially accomplishing that vary by
platform - usually there's some board level register to freeze /
resume all the timebases - but however it's been done, we don't want
to mess it up on migration.
diff mbox

Patch

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index f95086b..3283f1d 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -179,6 +179,7 @@  static void ppc_core99_init(MachineState *machine)
     int *token = g_new(int, 1);
     hwaddr nvram_addr = 0xFFF04000;
     uint64_t tbfreq;
+    PPCTimebase *tb;
 
     linux_boot = (kernel_filename != NULL);
 
@@ -201,6 +202,9 @@  static void ppc_core99_init(MachineState *machine)
         /* Set time-base frequency to 100 Mhz */
         cpu_ppc_tb_init(env, TBFREQ);
         qemu_register_reset(ppc_core99_reset, cpu);
+
+        tb = g_malloc0(sizeof(PPCTimebase));
+        vmstate_register(NULL, -1, &vmstate_ppc_timebase, tb);
     }
 
     /* allocate RAM */
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 8984398..45e410b 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -104,6 +104,7 @@  static void ppc_heathrow_init(MachineState *machine)
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
     uint64_t tbfreq;
+    PPCTimebase *tb;
 
     linux_boot = (kernel_filename != NULL);
 
@@ -121,6 +122,9 @@  static void ppc_heathrow_init(MachineState *machine)
         /* Set time-base frequency to 16.6 Mhz */
         cpu_ppc_tb_init(env,  TBFREQ);
         qemu_register_reset(ppc_heathrow_reset, cpu);
+
+        tb = g_malloc0(sizeof(PPCTimebase));
+        vmstate_register(NULL, -1, &vmstate_ppc_timebase, tb);
     }
 
     /* allocate RAM */