On 30/04/2018 19:01, Sean Christopherson wrote:
> Update SECONDARY_EXEC_DESC for UMIP emulation if and only UMIP
> is actually being emulated. Skipping the VMCS update eliminates
> unnecessary VMREAD/VMWRITE when UMIP is supported in hardware,
> and on platforms that don't have SECONDARY_VM_EXEC_CONTROL. The
> latter case resolves a bug where KVM would fill the kernel log
> with warnings due to failed VMWRITEs on older platforms.
>
> Fixes: 0367f205a3b7 ("KVM: vmx: add support for emulating UMIP")
> Cc: stable@xxxxxxxxxxxxxxx #4.16
> Reported-by: Paolo Zeppegno <pzeppegno@xxxxxxxxx>
> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Suggested-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> v2: Fix the bug simply by skipping VMCS updates when UMIP is not
> being emulated, as suggested by Paolo and again by Radim.
> The approach of updating the VMCS only when CR4.UMIP changed
> was not guaranteed to work in all cases. Optimizing away
> VMREAD/VMWRITE will be tackled in a separate series.
>
> arch/x86/kvm/vmx.c | 28 +++++++++++++++-------------
> 1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index aafcc9881e88..53d85439e5e5 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1494,6 +1494,12 @@ static inline bool cpu_has_vmx_vmfunc(void)
> SECONDARY_EXEC_ENABLE_VMFUNC;
> }
>
> +static bool vmx_umip_emulated(void)
> +{
> + return vmcs_config.cpu_based_2nd_exec_ctrl &
> + SECONDARY_EXEC_DESC;
> +}
> +
> static inline bool report_flexpriority(void)
> {
> return flexpriority_enabled;
> @@ -4776,14 +4782,16 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> else
> hw_cr4 |= KVM_PMODE_VM_CR4_ALWAYS_ON;
>
> - if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
> - vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
> - SECONDARY_EXEC_DESC);
> - hw_cr4 &= ~X86_CR4_UMIP;
> - } else if (!is_guest_mode(vcpu) ||
> - !nested_cpu_has2(get_vmcs12(vcpu), SECONDARY_EXEC_DESC))
> - vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
> + if (!boot_cpu_has(X86_FEATURE_UMIP) && vmx_umip_emulated()) {
> + if (cr4 & X86_CR4_UMIP) {
> + vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
> SECONDARY_EXEC_DESC);
> + hw_cr4 &= ~X86_CR4_UMIP;
> + } else if (!is_guest_mode(vcpu) ||
> + !nested_cpu_has2(get_vmcs12(vcpu), SECONDARY_EXEC_DESC))
> + vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
> + SECONDARY_EXEC_DESC);
> + }
>
> if (cr4 & X86_CR4_VMXE) {
> /*
> @@ -9512,12 +9520,6 @@ static bool vmx_xsaves_supported(void)
> SECONDARY_EXEC_XSAVES;
> }
>
> -static bool vmx_umip_emulated(void)
> -{
> - return vmcs_config.cpu_based_2nd_exec_ctrl &
> - SECONDARY_EXEC_DESC;
> -}
> -
> static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
> {
> u32 exit_intr_info;
>
Queued, thanks.
Paolo
