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

[Kouya Shimura <kouya@xxxxxxxxxxxxxx>]

Hi Jan,

Thanks for reviewing the code.

On 02/12/2013 09:26 PM, Jan Beulich wrote:
> > > > On 05.02.13 at 07:12, Kouya Shimura <kouya@xxxxxxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/hvm/pmtimer.c        Tue Jan 22 09:33:10 2013 +0100
> > +++ b/xen/arch/x86/hvm/pmtimer.c        Tue Feb 05 10:26:13 2013 +0900
> > @@ -244,21 +247,34 @@ 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;
> > +    uint64_t guest_time;
> >      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 */
> > -    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;
> > -    /* No point in setting the SCI here because we'll already have saved the
> > -     * IRQ and *PIC state; we'll fix it up when we restore the domain */
> > +    guest_time = s->vcpu->arch.hvm_vcpu.guest_time;
> > +    /* Update the counter to the guest's current time only if the
> > +     * timer mode is delay_for_missed_ticks */
> > +    if ( guest_time != 0 )
> How is guest_time being (non-)zero an indication of mode being
> delay_for_missed_ticks? I think you should check for the mode
> explicitly, as the value here being zero can just be an effect of
> a large enough negative v->arch.hvm_vcpu.stime_offset.
>
> And besides that you don't explain why the update being done
> here is unnecessary in other modes - all you explain is that the
> way it's being done in those modes is wrong.
After due consideration, the update of PM-timer is necessary
for other modes. I misunderstood it's just an adjustment for
the delay_for_missed_ticks mode.
The cause of this bug is to refer the vcpu->arch.hvm_vcpu.guest_time.

Attached is the revised patch.

Aside from this patch, I've found another problem about
delay_for_missed_ticks.  PM-timer might be broken after
pmt_timer_callback is invoked.
I'll think it over.

Thanks,
Kouya

Attachment: fix_corrupt_saved_pmtimer_v2.patch
Description: Text Data

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