Re: [Qemu-devel] [PATCH 2/3] ppc: add support for timebase migration on non-PPC hosts

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3094 bytes --]

On Sun, Jan 31, 2016 at 07:19:35PM +0000, Mark Cave-Ayland wrote:
> This patch provides support for migration of the PPC guest timebase on non-PPC
> host architectures (i.e those using QEMU's virtual emulated timebase).
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

We shouldn't need an explicit test for a ppc host.  Instead we should
never be touching any host-dependent ticks values, only using host
side interfaces which work in realtime units like ns.

Worse, the ppc host variants here will still be wrong if the host has
a different timebase frequency to the guest, which will always be true
for a g3beige (16MHz) on a modern ppc host (512 MHz).


> ---
>  hw/ppc/ppc.c |   33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 19f4570..9b80c1d 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -832,6 +832,15 @@ static void cpu_ppc_set_tb_clk (void *opaque, uint32_t freq)
>      cpu_ppc_store_purr(cpu, 0x0000000000000000ULL);
>  }
>  
> +static int host_cpu_is_ppc(void)
> +{
> +#if defined(_ARCH_PPC)
> +    return -1;
> +#else
> +    return 0;
> +#endif
> +}
> +
>  static void timebase_pre_save(void *opaque)
>  {
>      PPCTimebase *tb = opaque;
> @@ -844,11 +853,16 @@ static void timebase_pre_save(void *opaque)
>      }
>  
>      tb->time_of_the_day_ns = qemu_clock_get_ns(QEMU_CLOCK_HOST);
> -    /*
> -     * tb_offset is only expected to be changed by migration so
> -     * there is no need to update it from KVM here
> -     */
> -    tb->guest_timebase = ticks + first_ppc_cpu->env.tb_env->tb_offset;
> +
> +    if (host_cpu_is_ppc()) {
> +        /*
> +         * tb_offset is only expected to be changed by migration so
> +         * there is no need to update it from KVM here
> +         */
> +        tb->guest_timebase = ticks + first_ppc_cpu->env.tb_env->tb_offset;
> +    } else {
> +        tb->guest_timebase = cpu_ppc_load_tbl(&first_ppc_cpu->env);
> +    }
>  }
>  
>  static int timebase_post_load(void *opaque, int version_id)
> @@ -879,7 +893,14 @@ static int timebase_post_load(void *opaque, int version_id)
>                                       NANOSECONDS_PER_SECOND);
>      guest_tb = tb_remote->guest_timebase + migration_duration_tb;
>  
> -    tb_off_adj = guest_tb - cpu_get_host_ticks();
> +    if (host_cpu_is_ppc()) {
> +        /* Hardware timebase */
> +        tb_off_adj = guest_tb - cpu_get_host_ticks();
> +    } else {
> +        /* Software timebase */
> +        tb_off_adj = guest_tb - muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> +                                         freq, get_ticks_per_sec());
> +    }
>  
>      tb_off = first_ppc_cpu->env.tb_env->tb_offset;
>      trace_ppc_tb_adjust(tb_off, tb_off_adj, tb_off_adj - tb_off,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]