Re: [PATCH] kvm: x86: vmx: reorder some msr writing

[Paolo Bonzini <pbonzini@redhat.com>]

On 23/12/2014 09:21, Tiejun Chen wrote:
> The commit 34a1cd60d17f, "x86: vmx: move some vmx setting from
> vmx_init() to hardware_setup()", tried to refactor some codes
> specific to vmx hardware setting into hardware_setup(), but some
> msr writing should depend on our previous setting condition like
> enable_apicv, enable_ept and so on, then there's such an error
> in some cases,
> 
> KVM: entry failed, hardware error 0x80000021
> 
> If you're running a guest on an Intel machine without unrestricted mode
> support, the failure can be most likely due to the guest entering an invalid
> state for Intel VT. For example, the guest maybe running in big real mode
> which is not supported on less recent Intel processors.
> 
> EAX=00000000 EBX=00000000 ECX=00000000 EDX=00000663
> ESI=00000000 EDI=00000000 EBP=00000000 ESP=00000000
> EIP=0000e05b EFL=00010002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0000 00000000 0000ffff 00009300
> CS =f000 000f0000 0000ffff 00009b00
> SS =0000 00000000 0000ffff 00009300
> DS =0000 00000000 0000ffff 00009300
> FS =0000 00000000 0000ffff 00009300
> GS =0000 00000000 0000ffff 00009300
> LDT=0000 00000000 0000ffff 00008200
> TR =0000 00000000 0000ffff 00008b00
> GDT=     00000000 0000ffff
> IDT=     00000000 0000ffff
> CR0=60000010 CR2=00000000 CR3=00000000 CR4=00000000
> DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000 DR3=0000000000000000
> DR6=00000000ffff0ff0 DR7=0000000000000400
> EFER=0000000000000000
> Code=85 00 87 00 89 00 8b 00 00 00 86 00 88 00 8a 00 8c 00 00 90 <2e> 66 83 3e 30 6c 00 0f 85 e7 f2 31 c0 8e d0 66 bc 00 70 00 00 66 ba 31 2e 0f 00 e9 45 f1
> 
> so reorder them to work properly under those previous setting condition.
> 
> Reported-by: Jamie Heilman <jamie@audible.transient.net>
> Tested-by: Jamie Heilman <jamie@audible.transient.net>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 86 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 6b6bfce..e378dff 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5840,49 +5840,6 @@ static __init int hardware_setup(void)
>  	memset(vmx_msr_bitmap_legacy, 0xff, PAGE_SIZE);
>  	memset(vmx_msr_bitmap_longmode, 0xff, PAGE_SIZE);
>  
> -	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> -	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> -	vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
> -	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
> -	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
> -	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
> -	vmx_disable_intercept_for_msr(MSR_IA32_BNDCFGS, true);
> -
> -	memcpy(vmx_msr_bitmap_legacy_x2apic,
> -			vmx_msr_bitmap_legacy, PAGE_SIZE);
> -	memcpy(vmx_msr_bitmap_longmode_x2apic,
> -			vmx_msr_bitmap_longmode, PAGE_SIZE);
> -
> -	if (enable_apicv) {
> -		for (msr = 0x800; msr <= 0x8ff; msr++)
> -			vmx_disable_intercept_msr_read_x2apic(msr);
> -
> -		/* According SDM, in x2apic mode, the whole id reg is used.
> -		 * But in KVM, it only use the highest eight bits. Need to
> -		 * intercept it */
> -		vmx_enable_intercept_msr_read_x2apic(0x802);
> -		/* TMCCT */
> -		vmx_enable_intercept_msr_read_x2apic(0x839);
> -		/* TPR */
> -		vmx_disable_intercept_msr_write_x2apic(0x808);
> -		/* EOI */
> -		vmx_disable_intercept_msr_write_x2apic(0x80b);
> -		/* SELF-IPI */
> -		vmx_disable_intercept_msr_write_x2apic(0x83f);
> -	}
> -
> -	if (enable_ept) {
> -		kvm_mmu_set_mask_ptes(0ull,
> -			(enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull,
> -			(enable_ept_ad_bits) ? VMX_EPT_DIRTY_BIT : 0ull,
> -			0ull, VMX_EPT_EXECUTABLE_MASK);
> -		ept_set_mmio_spte_mask();
> -		kvm_enable_tdp();
> -	} else
> -		kvm_disable_tdp();
> -
> -	update_ple_window_actual_max();
> -
>  	if (setup_vmcs_config(&vmcs_config) < 0) {
>  		r = -EIO;
>  		goto out7;
> @@ -5946,6 +5903,49 @@ static __init int hardware_setup(void)
>  	if (nested)
>  		nested_vmx_setup_ctls_msrs();
>  
> +	vmx_disable_intercept_for_msr(MSR_FS_BASE, false);
> +	vmx_disable_intercept_for_msr(MSR_GS_BASE, false);
> +	vmx_disable_intercept_for_msr(MSR_KERNEL_GS_BASE, true);
> +	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_CS, false);
> +	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_ESP, false);
> +	vmx_disable_intercept_for_msr(MSR_IA32_SYSENTER_EIP, false);
> +	vmx_disable_intercept_for_msr(MSR_IA32_BNDCFGS, true);
> +
> +	memcpy(vmx_msr_bitmap_legacy_x2apic,
> +			vmx_msr_bitmap_legacy, PAGE_SIZE);
> +	memcpy(vmx_msr_bitmap_longmode_x2apic,
> +			vmx_msr_bitmap_longmode, PAGE_SIZE);
> +
> +	if (enable_apicv) {
> +		for (msr = 0x800; msr <= 0x8ff; msr++)
> +			vmx_disable_intercept_msr_read_x2apic(msr);
> +
> +		/* According SDM, in x2apic mode, the whole id reg is used.
> +		 * But in KVM, it only use the highest eight bits. Need to
> +		 * intercept it */
> +		vmx_enable_intercept_msr_read_x2apic(0x802);
> +		/* TMCCT */
> +		vmx_enable_intercept_msr_read_x2apic(0x839);
> +		/* TPR */
> +		vmx_disable_intercept_msr_write_x2apic(0x808);
> +		/* EOI */
> +		vmx_disable_intercept_msr_write_x2apic(0x80b);
> +		/* SELF-IPI */
> +		vmx_disable_intercept_msr_write_x2apic(0x83f);
> +	}
> +
> +	if (enable_ept) {
> +		kvm_mmu_set_mask_ptes(0ull,
> +			(enable_ept_ad_bits) ? VMX_EPT_ACCESS_BIT : 0ull,
> +			(enable_ept_ad_bits) ? VMX_EPT_DIRTY_BIT : 0ull,
> +			0ull, VMX_EPT_EXECUTABLE_MASK);
> +		ept_set_mmio_spte_mask();
> +		kvm_enable_tdp();
> +	} else
> +		kvm_disable_tdp();
> +
> +	update_ple_window_actual_max();
> +
>  	return alloc_kvm_area();
>  
>  out7:
> 

Thanks, I'll test this and send it for -rc2.

Paolo