Re: [PATCH v3 08/13] x86/hvm: Setup TSC scaling ratio

[Haozhong Zhang <haozhong.zhang@intel.com>]

On 01/08/16 02:44, Jan Beulich wrote:
> >>> On 31.12.15 at 04:03, <haozhong.zhang@intel.com> wrote:
> > @@ -301,6 +302,42 @@ int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat)
> >      return 1;
> >  }
> >  
> > +bool_t hvm_validate_tsc_scaling_ratio(uint32_t gtsc_khz)
> > +{
> > +    u64 ratio;
> > +
> > +    if ( !hvm_funcs.tsc_scaling_supported )
> > +        return FALSE;
> 
> We use 0 and 1, not FALSE and TRUE (except in ACPI and EFI code).
>

I'll change in the next version.

> > +    ratio = mul_u64_u32_div(1ULL << hvm_funcs.tsc_scaling_ratio_frac_bits,
> > +                            gtsc_khz, cpu_khz);
> > +
> > +    return (!ratio || ratio > hvm_funcs.max_tsc_scaling_ratio) ? FALSE : TRUE;
>
> There no point in using a conditional expression here.

Ah, right, I'll  change it to
         return !!(ratio && ratio <= hvm_funcs.max_tsc_scaling_ratio);
	 
> 
> > --- a/xen/include/asm-x86/hvm/hvm.h
> > +++ b/xen/include/asm-x86/hvm/hvm.h
> > @@ -226,6 +226,9 @@ struct hvm_function_table {
> >      int (*altp2m_vcpu_emulate_vmfunc)(struct cpu_user_regs *regs);
> >  
> >      uint64_t (*scale_tsc)(struct vcpu *v, uint64_t tsc);
> > +
> > +    /* Architecture function to setup TSC scaling ratio */
> > +    void (*setup_tsc_scaling)(struct vcpu *v);
> 
> SVM doesn't use this hook, and VMX gets to use it only later. I.e.
> everything related to it is dead code right now, and would therefore
> better go into the VMX (I suppose) patch actually making use of it.
> 

Yes, it's used in later VMX patch. I'll move it that patch.

Haozhong