Re: [Qemu-devel] [RFC PATCH] spapr: support time base offset migration

[Alexander Graf]

On 05.09.2013, at 11:48, Alexey Kardashevskiy wrote:

> On 09/05/2013 07:16 PM, Alexander Graf wrote:
>> 
>> On 05.09.2013, at 06:54, Alexey Kardashevskiy wrote:
>> 
>>> On 09/05/2013 02:30 PM, David Gibson wrote:

[...]

>>> 
>>>>> #endif /* TARGET_PPC64 */
>>>>>    }
>>>>> 
>>>>> @@ -1082,6 +1102,9 @@ int kvm_arch_get_registers(CPUState *cs)
>>>>>                DPRINTF("Warning: Unable to get VPA information from 
>>>>> KVM\n");
>>>>>            }
>>>>>        }
>>>>> +
>>>>> +        kvm_access_one_reg(cs, 0, KVM_REG_PPC_TB_OFFSET,
>>>>> +                           &env->tb_env->tb_offset);
>>>>> #endif
>>>>>    }
>>>>> 
>>>>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
>>>>> index 12e1512..d1ffc7f 100644
>>>>> --- a/target-ppc/machine.c
>>>>> +++ b/target-ppc/machine.c
>>>>> @@ -1,5 +1,6 @@
>>>>> #include "hw/hw.h"
>>>>> #include "hw/boards.h"
>>>>> +#include "hw/ppc/ppc.h"
>>>>> #include "sysemu/kvm.h"
>>>>> #include "helper_regs.h"
>>>>> 
>>>>> @@ -459,6 +460,45 @@ static const VMStateDescription vmstate_tlbmas = {
>>>>>    }
>>>>> };
>>>>> 
>>>>> +static void timebase_pre_save(void *opaque)
>>>>> +{
>>>>> +    ppc_tb_t *tb_env = opaque;
>>>>> +    struct timeval tv;
>>>>> +
>>>>> +    gettimeofday(&tv, NULL);
>>>>> +    tb_env->time_of_the_day = tv.tv_sec * 1000000 + tv.tv_usec;
>>>>> +    tb_env->timebase = cpu_get_real_ticks();
>>>>> +}
>>>>> +
>>>>> +static int timebase_post_load(void *opaque, int version_id)
>>>>> +{
>>>>> +    ppc_tb_t *tb_env = opaque;
>>>>> +
>>>>> +    if (!tb_env) {
>>>>> +        printf("NO TB!\n");
>>>>> +        return -1;
>>>>> +    }
>>>>> +    cpu_ppc_adjust_tb_offset(tb_env);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static const VMStateDescription vmstate_timebase = {
>>>>> +    .name = "cpu/timebase",
>>>>> +    .version_id = 1,
>>>>> +    .minimum_version_id = 1,
>>>>> +    .minimum_version_id_old = 1,
>>>>> +    .pre_save = timebase_pre_save,
>>>>> +    .post_load = timebase_post_load,
>>>>> +    .fields      = (VMStateField []) {
>>>>> +        VMSTATE_UINT64(timebase, ppc_tb_t),
>>>>> +        VMSTATE_INT64(tb_offset, ppc_tb_t),
>>>> 
>>>> tb_offset is inherently a local concept, since it depends on the host
>>>> timebase.  So how can it belong in the migration stream?
>>> 
>>> 
>>> I do not have pure guest timebase in QEMU and I need it on the destination.
>>> But I have host timebase + offset to calculate it. And tb_offset is already
>>> in ppc_tb_t. It looked logical to me to send the existing field and add
>>> only the missing part.
>> 
>> I still don't understand. You want the guest visible timebase in the 
>> migration stream, no?
> 
> 
> Yes. I do not really understand the problem here (and I am not playing
> dump). Do you suggest sending just the guest timebase and do not send the
> host timebase and the offset (one number instead of two)? I can do that,
> makes sense, no problem, thanks for the idea.

Yup, pretty much :). The receiving end should have no business in knowing how 
far off the guest and the host timebase were skewed on the sending end :).


Alex