RE: [Xen-devel] [PATCH] TSC scaling for live migration betweenplatforms with different TSC frequecies

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

> I am still confused by opt_softtsc check here. If want to use 
> platform timer to emulate guest's tsc, hvm_set_guest_tsc 
> should also need  perform this check to get correct 
> cache_tsc_offset, but I didn't see it.   A bug ? 
> If use host's tsc to emulate guest's tsc, the check is 
> useless, so I removed it in my patch.  Maybe we need Dan's 
> explanation about the check here to determin whether keep it or not.

Please do keep the opt_softtsc check.  I agree that there
is a bug, that hvm_set_guest_tsc should check as well.
IIRC, my guest never set the TSC.

The softtsc option is for handling skew problems
not scaling/migration problems but should probably
be updated to handle your TSC scaling as well.

http://lists.xensource.com/archives/html/xen-devel/2008-07/msg00495.html

Thanks,
Dan

> -----Original Message-----
> From: Zhang, Xiantao [mailto:xiantao.zhang@xxxxxxxxx]
> Sent: Thursday, June 18, 2009 2:52 AM
> To: Jan Beulich; Dan Magenheimer
> Cc: Keir Fraser; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: RE: [Xen-devel] [PATCH] TSC scaling for live migration
> betweenplatforms with different TSC frequecies
> 
> 
> Jan Beulich wrote:
> >>>> "Zhang, Xiantao" <xiantao.zhang@xxxxxxxxx> 18.06.09 04:56 >>>
> >> PATCH 0003-- Scaling host TSC freqeuncy patch.
> > 
> >> +int hvm_gtsc_need_scale(struct domain *d)
> >> +{
> >> +    uint32_t gtsc_khz;
> >> +
> >> +    gtsc_khz = d->arch.hvm_domain.gtsc_khz / 1000;
> > 
> > Can the variable please be renamed to what it contains (i.e.
> > gtsc_mhz)? 
> > 
> >> u64 hvm_get_guest_tsc(struct vcpu *v)
> >> {
> >> -    u64 host_tsc;
> >> -
> >> -    if ( opt_softtsc )
> >> -        host_tsc = hvm_get_guest_time(v);
> >> -    else
> >> -        rdtscll(host_tsc);
> >> -
> >> -    return host_tsc + v->arch.hvm_vcpu.cache_tsc_offset;
> >> +    u64 host_tsc, scaled_htsc;
> >> +
> >> +    rdtscll(host_tsc);
> >> +    scaled_htsc = hvm_h2g_scale_tsc(v, host_tsc);
> >> +
> >> +    return scaled_htsc + v->arch.hvm_vcpu.cache_tsc_offset; }
> >> 
> >> void hvm_migrate_timers(struct vcpu *v)
> > 
> > I'm getting the impression that the opt_softtsc functionality got
> > lost here.
> 
> I am still confused by opt_softtsc check here. If want to use 
> platform timer to emulate guest's tsc, hvm_set_guest_tsc 
> should also need  perform this check to get correct 
> cache_tsc_offset, but I didn't see it.   A bug ? 
> If use host's tsc to emulate guest's tsc, the check is 
> useless, so I removed it in my patch.  Maybe we need Dan's 
> explanation about the check here to determin whether keep it or not.
> 
> > 
> >> +        printk("Migrate to a platform with different 
> freq:%ldMhz, "
> >> +            "expected freq:%dMhz, enable rdtsc exiting!\n",
> >> +                    cpu_khz / 1000, hdr->gtsc_khz / 1000);
> > 
> > gdprintk()? At least, I think, any guest related printk-s should
> > identify which guest they're about. 
> 
> Added in the attached patch. Thanks!
> Xiantao
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel