Re: [PATCH v10 16/28] KVM: x86: Mark CR4.FRED as not reserved

[Sohil Mehta <sohil.mehta@intel.com>]

>> diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
>> index 8f66438989e4..ba6c8856b4ff 100644
>> --- a/arch/x86/kvm/regs.c
>> +++ b/arch/x86/kvm/regs.c
>> @@ -429,6 +429,13 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>>  			return 1;
>>  	}
>>  
>> +	/*
>> +	 * FRED can not be enabled when EFER.LMA=0.  Note, MOV to CR4 outside
>> +	 * 64-bit mode clears CR4[63:32] so only emulation can get here.
>> +	 */
> 
> Could you elaborate what "emulation" means here?
> 

I think the only emulation component that can currently get here is RSM.
As we discussed in patch 15, an emulated MOV to CR4 is Op3264 which is
automatically zero extended.

FRED spec Chapter 12 describes the RSM implication for FRED (see next
patch). In a nutshell, with KVM's RSM emulation it may be possible to
call kvm_set_cr4() with CR4.FRED=1 but EFER.LMA=0.

This check mainly handles that situation and makes RSM shut down.
Though, I am not sure how important it is to correctly emulate RSM. The
checks were simple enough so I added them.


>> +	if ((cr4 & X86_CR4_FRED) && !is_long_mode(vcpu))
>> +		return 1;
>> +
>>  	if ((cr4 & X86_CR4_CET) && !kvm_is_cr0_bit_set(vcpu, X86_CR0_WP))
>>  		return 1;
>>  
>> diff --git a/arch/x86/kvm/regs.h b/arch/x86/kvm/regs.h
>> index ef46918daf43..5db01693ad07 100644
>> --- a/arch/x86/kvm/regs.h
>> +++ b/arch/x86/kvm/regs.h
>> @@ -28,7 +28,7 @@ static_assert(!(KVM_POSSIBLE_CR0_GUEST_BITS & X86_CR0_PDPTR_BITS));
>>  			  | X86_CR4_OSXSAVE | X86_CR4_SMEP | X86_CR4_FSGSBASE \
>>  			  | X86_CR4_OSXMMEXCPT | X86_CR4_LA57 | X86_CR4_VMXE \
>>  			  | X86_CR4_SMAP | X86_CR4_PKE | X86_CR4_UMIP \
>> -			  | X86_CR4_LAM_SUP | X86_CR4_CET))
>> +			  | X86_CR4_LAM_SUP | X86_CR4_CET | X86_CR4_FRED))
>>  
>>  #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)
>>  
>> @@ -423,6 +423,8 @@ static inline bool __kvm_is_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>>  	if (!__cpu_has(__c, X86_FEATURE_SHSTK) &&       \
>>  	    !__cpu_has(__c, X86_FEATURE_IBT))           \
>>  		__reserved_bits |= X86_CR4_CET;         \
>> +	if (!__cpu_has(__c, X86_FEATURE_FRED))          \
>> +		__reserved_bits |= X86_CR4_FRED;        \
>>  	__reserved_bits;                                \
>>  })
>>  
>