PATCH: setup_vmcs_config: disable TSC scaling on unlike processors

Commit Message

Don Bowman Dec. 2, 2016, 8:58 p.m. UTC

On 2 December 2016 at 14:10, Don Bowman <[e-mail]> wrote:
> On 2 December 2016 at 10:07, Radim Krčmář <[e-mail]> wrote:
>> Subjects that touch only arch/x86/kvm/vmx.c are usually prefixed with
>> "[PATCH] KVM: VMX:".
>>
>> 2016-12-01 21:32-0500, Don Bowman:
>>> Add an enable_tsc_scaling parameter to kvm
>>>
>>> Signed-off-by: Don Bowman <[e-mail]>
>>> ---
>>>
>>> My system has what i thought were two identical processors (same
>>> stepping ID etc).
>>> However, bafflingly, one of them has the ability to do TSC scaling,
>>> and one does not (as reported in the vmcs).
>>
>> Wow, the chip doesn't sound trustworthy.
>

OK, how about this? The check has to be in setup_vmcs_config() not
elsewhere I think. This is where the rdmsr occurs, and immediately
following that is the compare against the other processor(s). Unless
I'm missing something I don't see how vmx_secondary_exec_control()
could work. For the enable I was following the 'enable_pml' which is
already there, but have changed it below. vmx_check_processor_compat()
calls setup_vmcs_config() and then the memcmp() immediately
afterwards.

 static bool __read_mostly enable_pml = 1;
@@ -3449,6 +3452,8 @@ 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;
+        if (!tsc_scaling)
+            vmcs_conf->cpu_based_2nd_exec_ctrl &= ~SECONDARY_EXEC_TSC_SCALING;
        vmcs_conf->vmexit_ctrl         = _vmexit_control;
        vmcs_conf->vmentry_ctrl        = _vmentry_control;
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Radim Krčmář Dec. 5, 2016, 4:37 p.m. UTC | #1

2016-12-02 15:58-0500, Don Bowman:
> On 2 December 2016 at 14:10, Don Bowman <[e-mail]> wrote:
>> On 2 December 2016 at 10:07, Radim Krčmář <[e-mail]> wrote:
>>> 2016-12-01 21:32-0500, Don Bowman:
>>>> My system has what i thought were two identical processors (same
>>>> stepping ID etc).
>>>> However, bafflingly, one of them has the ability to do TSC scaling,
>>>> and one does not (as reported in the vmcs).
>>>
> OK, how about this? The check has to be in setup_vmcs_config() not
> elsewhere I think. This is where the rdmsr occurs, and immediately
> following that is the compare against the other processor(s). Unless
> I'm missing something I don't see how vmx_secondary_exec_control()
> could work. For the enable I was following the 'enable_pml' which is
> already there, but have changed it below. vmx_check_processor_compat()
> calls setup_vmcs_config() and then the memcmp() immediately
> afterwards.

Right, KVM checks this early ... I don't like that the patch treats
tsc_scaling specially while the same could happen with some other
feature.  What about warning in vmx_check_processor_compat() if features
that won't be used don't match, but letting the check pass?

(I'd even prefer an unsafe option to disable the check than to treat
 tsc_scaling differently.)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 5382b82..503ee1e 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 tsc_scaling = true;
+module_param(tsc_scaling, bool, S_IRUGO);
+
 static u64 __read_mostly host_xss;