linux-kernel - Re: [PATCH v3 03/27] KVM: VMX: Add support for the secondary VM exit controls

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <5582cf56-0b22-4603-b8e2-6b652c09b4fa@zytor.com>
Date: Tue, 22 Oct 2024 09:30:47 -0700
From: Xin Li <xin@...or.com>
To: Chao Gao <chao.gao@...el.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, seanjc@...gle.com, pbonzini@...hat.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
Subject: Re: [PATCH v3 03/27] KVM: VMX: Add support for the secondary VM exit
 controls

>>>> 		_vmentry_control &= ~n_ctrl;
>>>> 		_vmexit_control &= ~x_ctrl;
>>>
>>> w/ patch 4, VM_EXIT_ACTIVATE_SECONDARY_CONTROLS is cleared if FRED fails in the
>>> consistent check. this means, all features in the secondary vm-exit controls
>>> are removed. it is overkill.
>>
>> Good catch!
>>
>>>
>>> I prefer to maintain a separate table for the secondary VM-exit controls:
>>>
>>>    	struct {
>>>    		u32 entry_control;
>>>    		u64 exit2_control;
>>> 	} const vmcs_entry_exit2_pairs[] = {
>>> 		{ VM_ENTRY_LOAD_IA32_FRED, SECONDARY_VM_EXIT_SAVE_IA32_FRED |
>>> 					   SECONDARY_VM_EXIT_LOAD_IA32_FRED},
>>> 	};
>>>
>>> 	for (i = 0; i < ARRAY_SIZE(vmcs_entry_exit2_pairs); i++) {
>>> 	...
>>> 	}
>>
>> Hmm, I prefer one table, as it's more straight forward.
> 
> One table is fine if we can fix the issue and improve readability. The three
> nested if() statements hurts readability.

You're right!  Let's try to make it clearer.

> I just thought using two tables would eliminate the need for any if() statements.
>

One more thing, IIUC, Sean prefers to keep
VM_EXIT_ACTIVATE_SECONDARY_CONTROLS set if it's allowed to be set and
even bits in the 2nd VM exit controls are all 0.  I may be able to make
it simpler.