> On 23 Nov 2019, at 1:43, Jim Mattson <jmattson@xxxxxxxxxx> wrote:
>
> Commit 37e4c997dadf ("KVM: VMX: validate individual bits of guest
> MSR_IA32_FEATURE_CONTROL") broke the KVM_SET_MSRS ABI by instituting
> new constraints on the data values that kvm would accept for the guest
> MSR, IA32_FEATURE_CONTROL. Perhaps these constraints should have been
> opt-in via a new KVM capability, but they were applied
> indiscriminately, breaking at least one existing hypervisor.
>
> Relax the constraints to allow either or both of
> FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX and
> FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX to be set when nVMX is
> enabled. This change is sufficient to fix the aforementioned breakage.
>
> Fixes: 37e4c997dadf ("KVM: VMX: validate individual bits of guest MSR_IA32_FEATURE_CONTROL")
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
I suggest to also add a comment in code to clarify why we allow setting
FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX even though we expose a vCPU that doesn’t support Intel TXT.
(I think the compatibility to existing workloads that sets this blindly on boot is a legit reason. Just recommend documenting it.)
In addition, if the nested hypervisor which relies on this is public, please also mention it in commit message for reference.
Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx/vmx.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 04a8212704c17..9f46023451810 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7097,10 +7097,12 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>
> if (nested_vmx_allowed(vcpu))
> to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
> + FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX |
> FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
> else
> to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &=
> - ~FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
> + ~(FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX |
> + FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX);
>
> if (nested_vmx_allowed(vcpu)) {
> nested_vmx_cr_fixed1_bits_update(vcpu);
> --
> 2.24.0.432.g9d3f5f5b63-goog
>
