Re: [Xen-devel] [RESEND PATCH v4 09/10] vmx: Add VMX RDTSC(P) scaling support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.01.16 at 03:55, <haozhong.zhang@xxxxxxxxx> wrote:
> @@ -2107,6 +2115,14 @@ const struct hvm_function_table * __init 
> start_vmx(void)
>           && cpu_has_vmx_secondary_exec_control )
>          vmx_function_table.pvh_supported = 1;
>  
> +    if ( cpu_has_vmx_tsc_scaling )
> +    {
> +        vmx_function_table.default_tsc_scaling_ratio = 
> VMX_TSC_MULTIPLIER_DEFAULT;
> +        vmx_function_table.max_tsc_scaling_ratio = VMX_TSC_MULTIPLIER_MAX;
> +        vmx_function_table.tsc_scaling_ratio_frac_bits = 48;
> +        vmx_function_table.setup_tsc_scaling = vmx_setup_tsc_scaling;
> +    }

Same comments here as on the earlier patch - it indeed looks as if
tsc_scaling_ratio_frac_bits would be the ideal field to dynamically
initialize, as it being zero will not yield any bad behavior afaict.

Also please consider making all fields together a sub-structure
of struct hvm_function_table, such that the above would become

        vmx_function_table.tsc_scaling.default_ratio = 
VMX_TSC_MULTIPLIER_DEFAULT;
        vmx_function_table.tsc_scaling.max_ratio = VMX_TSC_MULTIPLIER_MAX;
        vmx_function_table.tsc_scaling.ratio_frac_bits = 48;
        vmx_function_table.tsc_scaling.setup = vmx_setup_tsc_scaling;

keeping everything nicely together.

> @@ -258,6 +259,9 @@ extern u64 vmx_ept_vpid_cap;
>  #define VMX_MISC_CR3_TARGET                     0x01ff0000
>  #define VMX_MISC_VMWRITE_ALL                    0x20000000
>  
> +#define VMX_TSC_MULTIPLIER_DEFAULT              0x0001000000000000ULL

Considering this and the respective SVM value - do we really
need the separate field in struct hvm_function_table? Both are
1ULL << tsc_scaling.ratio_frac_bits after all.

Jan


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