Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP

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

2016-07-12 21:20+0200, Paolo Bonzini:
> UMIP (User-Mode Instruction Prevention) is a feature of future
> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
> and SMSW from user-mode processes.
> 
> On Intel systems it's *almost* possible to emulate it; it slows
> down the instructions when they're executed in ring 0, but they
> are really never executed in practice.  The catch is that SMSW
> doesn't cause a vmexit, and hence SMSW will not fault.
> 
> When UMIP is enabled but not supported by the host, descriptor table
> exits are enabled, and the emulator takes care of injecting a #GP when
> any of SLDT, SGDT, STR, SIDT are encountered.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  		(to_vmx(vcpu)->rmode.vm86_active ?
>  		 KVM_RMODE_VM_CR4_ALWAYS_ON : 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);

If UMIP support is not exposed in CPUID, we ought to #GP(0), because it
is a write to reserved bits.  It could also mean that the vm control is
not supported.

> +		hw_cr4 &= ~X86_CR4_UMIP;
> +	} else
> +		vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
> +				SECONDARY_EXEC_DESC);

I think we don't have to do anything when the CPU supports UMIP,

 if (!boot_cpu_has(X86_FEATURE_UMIP) {
   if ((cr4 & X86_CR4_UMIP)) { ... } else ...
 }

And we could then return true in vmx_umip_emulated() when
boot_cpu_has(X86_FEATURE_UMIP).
(Just for self-documentation, because occurrence of X86_FEATURE_UMIP is
 most likely a subset of SECONDARY_EXEC_DESC.)

> @@ -8597,7 +8627,8 @@ static bool vmx_xsaves_supported(void)
>  
>  static bool vmx_umip_emulated(void)
>  {
> -	return false;
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_DESC;
>  }