Re: [Xen-devel] [PATCH 2/2] x86/hvm: fix corrupt ACPI PM-Timer during live migration

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.03.13 at 02:12, Kouya Shimura <kouya@xxxxxxxxxxxxxx> wrote:
> On 03/21/2013 07:01 PM, Jan Beulich wrote:
>>>>> On 21.03.13 at 08:32, Kouya Shimura <kouya@xxxxxxxxxxxxxx> wrote:
>>> @@ -244,19 +245,11 @@ static int handle_pmt_io(
>>>    static int pmtimer_save(struct domain *d, hvm_domain_context_t *h)
>>>    {
>>>        PMTState *s = &d->arch.hvm_domain.pl_time.vpmt;
>>> -    uint32_t x, msb = s->pm.tmr_val & TMR_VAL_MSB;
>>>        int rc;
>>>
>>>        spin_lock(&s->lock);
>>>
>>> -    /* Update the counter to the guest's current time.  We always save
>>> -     * with the domain paused, so the saved time should be after the
>>> -     * last_gtime, but just in case, make sure we only go forwards */
>>
>> So on the previous patch version (which you said this one is
>> identical to) I stated that you lose this property of guaranteeing
>> no backward move. Am I right in assuming that patch 1 is now
>> supposed to take care of this?
> 
> Yes.  Patch 1 guarantees  that the timer counter only goes forwards.
> 
>> In any case I'll have to defer to Keir or Tim for that first one.
>>
>>> -    x = ((s->vcpu->arch.hvm_vcpu.guest_time - s->last_gtime) * s->scale) 
>>> >> 32;
>>> -    if ( x < 1UL<<31 )
>>> -        s->pm.tmr_val += x;
>>> -    if ( (s->pm.tmr_val & TMR_VAL_MSB) != msb )
>>> -        s->pm.pm1a_sts |= TMR_STS;
>>> +    pmt_update_time(s, 0);
>>
>> Here I can only quote part of my previous reply, which I don't
>> think you responded to:
>>
>> "Also, in delay_for_missed_ticks mode you now use a slightly
>>   different time for updating s->pm - did you double check that
>>   this is not going to be a problem? Or else, the flag above could
>>   similarly be used to circumvent this, or hvm_get_guest_time()
>>   could be made return the frozen time (I suppose, but didn't
>>   verify - as it appears to be an assumption already before your
>>   patch -, that pt_freeze_time() runs before pmtimer_save())."
>>
>> You said you'd think about it, but I don't recall seeing any other
>> outcome from that than the two patches, and I can't relate the
>> first patch to this aspect.
> 
> In patch 1, pmt_update_time() calls the new function hvm_get_base_time()
> which returns the frozen time when the vcpu is de-scheduled.
> And I confirmed pmtimer_save() is surely called after pt_freeze_time()
> from my test and review.
> So, if both patches are applied, there is no difference in
> delay_for_missed_ticks mode.

Okay, thanks for confirming!

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel