Re: PATCH: setup_vmcs_config: disable TSC scaling on unlike processors

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-12-09 10:12-0500, Don Bowman:
> OK, based on previous feedback, this patch version simply ignores any
> inconsistency if a knowing and trusting user wishes.
> 
> In my case, two identical processors in stepping and version and all
> others have 1 flag missing (retail vs tray version of chip), but the
> next person might have another flag.
> 
> Comments?
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5382b82..264870e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -103,6 +103,9 @@ module_param_named(enable_shadow_vmcs,
> enable_shadow_vmcs, bool, S_IRUGO);
>  static bool __read_mostly nested = 0;
>  module_param(nested, bool, S_IRUGO);
> 
> +static bool __read_mostly ignore_inconsistency = false;
> +module_param(ignore_inconsistency, bool, S_IRUGO);
> +
>  static u64 __read_mostly host_xss;
> 
>  static bool __read_mostly enable_pml = 1;
> @@ -3449,6 +3452,7 @@ static __init int setup_vmcs_config(struct
> vmcs_config *vmcs_conf)
>         vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
>         vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;
>         vmcs_conf->cpu_based_2nd_exec_ctrl = _cpu_based_2nd_exec_control;
> +       vmcs_conf->cpu_based_2nd_exec_ctrl &= ~SECONDARY_EXEC_TSC_SCALING;

This would disable TSC_SCALING for everyone.  You need a second patch
that makes tsc_scaling optional.

>         vmcs_conf->vmexit_ctrl         = _vmexit_control;
>         vmcs_conf->vmentry_ctrl        = _vmentry_control;
> 
> @@ -9202,9 +9206,11 @@ static void __init vmx_check_processor_compat(void *rtn)
>         if (setup_vmcs_config(&vmcs_conf) < 0)
>                 *(int *)rtn = -EIO;
>         if (memcmp(&vmcs_config, &vmcs_conf, sizeof(struct vmcs_config)) != 0) {
> -               printk(KERN_ERR "kvm: CPU %d feature inconsistency!\n",
> -                               smp_processor_id());
> -               *(int *)rtn = -EIO;
> +               printk(KERN_ERR "kvm: CPU %d feature inconsistency%s!\n",
> +                               smp_processor_id(),
> +                               ignore_inconsistency ? " -- ignored" : "");
> +               if (!ignore_inconsistency)
> +                       *(int *)rtn = -EIO;

Please add a note in the spirit of: "KVM is going to fail unless you
explicitly disable features that are not present on all CPUs."

If this becomes a normal feature, we'd remove the toggle, check that all
enabled features are supported, and pass if KVM will work with selected
features ... the check seems like a waste of code for this rarity of
machines.

Paolo, are you ok with the toggle?
(We can just make it the default without any significant issues.)

Thanks.