linux-kernel - Re: [PATCH v5A 20/23] KVM: nVMX: Add FRED VMCS fields to nested VMX context handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5ca5d98e-6a3c-48fc-8aa2-7db0980543e6@zytor.com>
Date: Sun, 3 Aug 2025 23:03:16 -0700
From: Xin Li <xin@...or.com>
To: linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-doc@...r.kernel.org
Cc: pbonzini@...hat.com, seanjc@...gle.com, corbet@....net, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        x86@...nel.org, hpa@...or.com, luto@...nel.org, peterz@...radead.org,
        andrew.cooper3@...rix.com, chao.gao@...el.com, hch@...radead.org
Subject: Re: [PATCH v5A 20/23] KVM: nVMX: Add FRED VMCS fields to nested VMX
 context handling

On 8/2/2025 10:33 AM, Xin Li wrote:
>> @@ -4531,6 +4593,27 @@ static void sync_vmcs02_to_vmcs12_rare(struct 
>> kvm_vcpu *vcpu,
>>       vmcs12->guest_tr_base = vmcs_readl(GUEST_TR_BASE);
>>       vmcs12->guest_gdtr_base = vmcs_readl(GUEST_GDTR_BASE);
>>       vmcs12->guest_idtr_base = vmcs_readl(GUEST_IDTR_BASE);
>> +
>> +    vmx->nested.pre_vmexit_fred_config = 
>> vmcs_read64(GUEST_IA32_FRED_CONFIG);
>> +    vmx->nested.pre_vmexit_fred_rsp1 = 
>> vmcs_read64(GUEST_IA32_FRED_RSP1);
>> +    vmx->nested.pre_vmexit_fred_rsp2 = 
>> vmcs_read64(GUEST_IA32_FRED_RSP2);
>> +    vmx->nested.pre_vmexit_fred_rsp3 = 
>> vmcs_read64(GUEST_IA32_FRED_RSP3);
>> +    vmx->nested.pre_vmexit_fred_stklvls = 
>> vmcs_read64(GUEST_IA32_FRED_STKLVLS);
>> +    vmx->nested.pre_vmexit_fred_ssp1 = 
>> vmcs_read64(GUEST_IA32_FRED_SSP1);
>> +    vmx->nested.pre_vmexit_fred_ssp2 = 
>> vmcs_read64(GUEST_IA32_FRED_SSP2);
>> +    vmx->nested.pre_vmexit_fred_ssp3 = 
>> vmcs_read64(GUEST_IA32_FRED_SSP3);
> 
> This ...
> 
>> +
>> +    if (nested_cpu_save_guest_fred_state(vmcs12)) {
>> +        vmcs12->guest_ia32_fred_config = vmx- 
>> >nested.pre_vmexit_fred_config;
>> +        vmcs12->guest_ia32_fred_rsp1 = vmx->nested.pre_vmexit_fred_rsp1;
>> +        vmcs12->guest_ia32_fred_rsp2 = vmx->nested.pre_vmexit_fred_rsp2;
>> +        vmcs12->guest_ia32_fred_rsp3 = vmx->nested.pre_vmexit_fred_rsp3;
>> +        vmcs12->guest_ia32_fred_stklvls = vmx- 
>> >nested.pre_vmexit_fred_stklvls;
>> +        vmcs12->guest_ia32_fred_ssp1 = vmx->nested.pre_vmexit_fred_ssp1;
>> +        vmcs12->guest_ia32_fred_ssp2 = vmx->nested.pre_vmexit_fred_ssp2;
>> +        vmcs12->guest_ia32_fred_ssp3 = vmx->nested.pre_vmexit_fred_ssp3;
>> +    }
>> +
>>       vmcs12->guest_pending_dbg_exceptions =
>>           vmcs_readl(GUEST_PENDING_DBG_EXCEPTIONS);
>> @@ -4761,6 +4860,26 @@ static void load_vmcs12_host_state(struct 
>> kvm_vcpu *vcpu,
>>       vmcs_write32(GUEST_IDTR_LIMIT, 0xFFFF);
>>       vmcs_write32(GUEST_GDTR_LIMIT, 0xFFFF);
>> +    if (nested_cpu_load_host_fred_state(vmcs12)) {
>> +        vmcs_write64(GUEST_IA32_FRED_CONFIG, vmcs12- 
>> >host_ia32_fred_config);
>> +        vmcs_write64(GUEST_IA32_FRED_RSP1, vmcs12->host_ia32_fred_rsp1);
>> +        vmcs_write64(GUEST_IA32_FRED_RSP2, vmcs12->host_ia32_fred_rsp2);
>> +        vmcs_write64(GUEST_IA32_FRED_RSP3, vmcs12->host_ia32_fred_rsp3);
>> +        vmcs_write64(GUEST_IA32_FRED_STKLVLS, vmcs12- 
>> >host_ia32_fred_stklvls);
>> +        vmcs_write64(GUEST_IA32_FRED_SSP1, vmcs12->host_ia32_fred_ssp1);
>> +        vmcs_write64(GUEST_IA32_FRED_SSP2, vmcs12->host_ia32_fred_ssp2);
>> +        vmcs_write64(GUEST_IA32_FRED_SSP3, vmcs12->host_ia32_fred_ssp3);
>> +    } else {
>> +        vmcs_write64(GUEST_IA32_FRED_CONFIG, vmx- 
>> >nested.pre_vmexit_fred_config);
>> +        vmcs_write64(GUEST_IA32_FRED_RSP1, vmx- 
>> >nested.pre_vmexit_fred_rsp1);
>> +        vmcs_write64(GUEST_IA32_FRED_RSP2, vmx- 
>> >nested.pre_vmexit_fred_rsp2);
>> +        vmcs_write64(GUEST_IA32_FRED_RSP3, vmx- 
>> >nested.pre_vmexit_fred_rsp3);
>> +        vmcs_write64(GUEST_IA32_FRED_STKLVLS, vmx- 
>> >nested.pre_vmexit_fred_stklvls);
>> +        vmcs_write64(GUEST_IA32_FRED_SSP1, vmx- 
>> >nested.pre_vmexit_fred_ssp1);
>> +        vmcs_write64(GUEST_IA32_FRED_SSP2, vmx- 
>> >nested.pre_vmexit_fred_ssp2);
>> +        vmcs_write64(GUEST_IA32_FRED_SSP3, vmx- 
>> >nested.pre_vmexit_fred_ssp3);
> 
> And this are actually nops. IOW, if I don't add this snippet of code,
> the CPU still retains the guest FRED MSRs, i.e., using guest FRED state 
> from vmcs02 as that of vmcs01.

I confused myself.  They are NOT nops, because __nested_vmx_vmexit()
switches from vmcs02 to vmcs01.  The code should be (as the patch does):

__nested_vmx_vmexit()
{
	...

	/*
	 * Save guest FRED state of vmcs02 to nested.pre_vmexit_fred
	 * no matter if SECONDARY_VM_EXIT_SAVE_IA32_FRED is set.
	 */
	sync_vmcs02_to_vmcs12();

	...
	vmx_switch_vmcs();
	...

	/*
	 * Load nested.pre_vmexit_fred to guest FRED state of vmcs01
	 * if SECONDARY_VM_EXIT_LOAD_IA32_FRED is NOT set.
	 */
	load_vmcs12_host_state();

	...
   }


As not setting any of the two FRED VM-Exit controls are rare cases, we
need to add KVM tests with L1 that:
1) doesn't set SECONDARY_VM_EXIT_SAVE_IA32_FRED in VM-Exit controls.
2) doesn't set SECONDARY_VM_EXIT_LOAD_IA32_FRED in VM-Exit controls.
3) doesn't set both of the FRED VM-Exit controls.

Looks we need a framework for all VM-Exit controls which control whether
to save/load specific MSRs related to CPU features during VM-Exit?