Re: [Xen-devel] [PATCH 04/13] x86/hvm: Setup TSC scaling ratio

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.10.15 at 10:40, <haozhong.zhang@xxxxxxxxx> wrote:
> On Fri, Oct 23, 2015 at 02:31:11AM -0600, Jan Beulich wrote:
>> >>> On 23.10.15 at 10:18, <haozhong.zhang@xxxxxxxxx> wrote:
>> > On Fri, Oct 23, 2015 at 01:59:46AM -0600, Jan Beulich wrote:
>> >> >>> On 23.10.15 at 09:44, <haozhong.zhang@xxxxxxxxx> wrote:
>> >> > On Thu, Oct 22, 2015 at 07:13:07AM -0600, Jan Beulich wrote:
>> >> >> >>> On 28.09.15 at 09:13, <haozhong.zhang@xxxxxxxxx> wrote:
>> >> 
>> >> Please remember to trim your replies.
>> >> 
>> >> >> > @@ -2023,6 +2051,9 @@ static int hvm_load_cpu_ctxt(struct domain *d, 
>> > hvm_domain_context_t *h)
>> >> >> >      if ( hvm_funcs.load_cpu_ctxt(v, &ctxt) < 0 )
>> >> >> >          return -EINVAL;
>> >> >> >  
>> >> >> > +    if ( !v->domain->arch.vtsc && hvm_funcs.tsc_scaling_supported )
>> >> >> > +        hvm_setup_tsc_scaling(v);
>> >> >> 
>> >> >> What's the rationale for putting it in this function?
>> >> > 
>> >> > hvm_load_cpu_ctxt() is called in the migration to restore vcpu's state
>> >> > including TSC related things, so hvm_setup_tsc_scaling() is called
>> >> > here.
>> >> > 
>> >> > hvm_vcpu_reset_state() is not called in the migration, so we cannot
>> >> > rely on the call to hvm_setup_tsc_scaling() there.
>> >> 
>> >> All that is understood, but doesn't explain why the scaling setup gets
>> >> done here instead of somewhere after _all_ state got loaded.
>> > 
>> > Because vcpu is waken up at the end of hvm_vcpu_reset_state(), the
>> > setup of TSC scaling should be done before that.
>> 
>> But we're talking about hvm_load_cpu_ctxt() here.
> 
> Sorry for the typo. s/hvm_vcpu_reset_state/hvm_load_cpu_ctxt

Hmm, interesting. I don't really understand why we do so, and I
don't see how this can be correct unless we rely on either something
else to keep the vCPU from running or this always being the last
restored item. Plus the commit that introduced this (89fdf2860a) only
talks about waking APs, yet I don't see any distinction between BP
and APs here.

Andrew - you probably know the restore logic best: Any thoughts?

Jan


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