Re: [PATCH v10 02/28] KVM: VMX: Initialize VM entry/exit FRED controls in vmcs_config

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

On 10/22/2026 2:48 AM, Binbin Wu wrote:
> On 9/12/2026 5:36 AM, Sohil Mehta wrote:
> [...]
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index b4aec4218b7d..062631e2dd37 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -2778,6 +2778,8 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>>  		u32 entry_control;
>>  		u64 exit_control;
>>  	} const vmcs_entry_exit2_pairs[] = {
>> +		{ VM_ENTRY_LOAD_IA32_FRED,
>> +			SECONDARY_VM_EXIT_SAVE_IA32_FRED | SECONDARY_VM_EXIT_LOAD_IA32_FRED },
> 
> I think it can be break into two lines for much better readability:
> 
> 	} const vmcs_entry_exit2_pairs[] = {
> 		{ VM_ENTRY_LOAD_IA32_FRED,	SECONDARY_VM_EXIT_SAVE_IA32_FRED },
> 		{ VM_ENTRY_LOAD_IA32_FRED,	SECONDARY_VM_EXIT_LOAD_IA32_FRED },
> 	};
> 
> Oh, it also fixes the issue reported by the sashiko.

Agreed, splitting it improves readability. However, I am not sure if it
reliably fixes the issue. Splitting the entry makes the partial case
detectable, but with error_on_inconsistent_vmcs_config=0 could it still
leave vmcs_config inconsistent?

For example, if the first row passes but the second row fails, it clears
VM_ENTRY_LOAD_IA32_FRED but leaves SECONDARY_VM_EXIT_SAVE_IA32_FRED set.
Only leaving EXIT_SAVE set is probably harmless but if the order were
reversed:

{ VM_ENTRY_LOAD_IA32_FRED,	SECONDARY_VM_EXIT_LOAD_IA32_FRED },
{ VM_ENTRY_LOAD_IA32_FRED,	SECONDARY_VM_EXIT_SAVE_IA32_FRED },

then, leaving EXIT_LOAD uncleared would cause issues, right?

It probably doesn't matter much because
error_on_inconsistent_vmcs_config is set by default. But, I feel relying
on the ordering makes it a little fragile.

The other option is to leave this patch as-is and add a check to the
existing one in vmx_check_entry_exit_pairs() which covers both exit
controls for FRED.

if (!(entry_controls & n_ctrl) == !(exit_controls & x_ctrl) &&
    !(entry_controls & n_ctrl) == ((exit_controls & x_ctrl) != x_ctrl))
		continue;

I couldn't find any precedent for this. I don't have a strong preference
here. Is it worth making future-proof?