On Mon, Oct 21, 2024 at 10:03:45AM -0700, Xin Li wrote:
>On 10/21/2024 1:28 AM, Chao Gao wrote:
>> > + for (i = 0; i < ARRAY_SIZE(vmcs_entry_exit_triplets); i++) {
>> > + u32 n_ctrl = vmcs_entry_exit_triplets[i].entry_control;
>> > + u32 x_ctrl = vmcs_entry_exit_triplets[i].exit_control;
>> > + u64 x_ctrl_2 = vmcs_entry_exit_triplets[i].exit_2nd_control;
>> > + bool has_n = n_ctrl && ((_vmentry_control & n_ctrl) == n_ctrl);
>> > + bool has_x = x_ctrl && ((_vmexit_control & x_ctrl) == x_ctrl);
>> > + bool has_x_2 = x_ctrl_2 && ((_secondary_vmexit_control & x_ctrl_2) == x_ctrl_2);
>> > +
>> > + if (x_ctrl_2) {
>> > + /* Only activate secondary VM exit control bit should be set */
>> > + if ((_vmexit_control & x_ctrl) == VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) {
>> > + if (has_n == has_x_2)
>> > + continue;
>> > + } else {
>> > + /* The feature should not be supported in any control */
>> > + if (!has_n && !has_x && !has_x_2)
>> > + continue;
>> > + }
>> > + } else if (has_n == has_x) {
>> > continue;
>> > + }
>> >
>> > - pr_warn_once("Inconsistent VM-Entry/VM-Exit pair, entry = %x, exit = %x\n",
>> > - _vmentry_control & n_ctrl, _vmexit_control & x_ctrl);
>> > + pr_warn_once("Inconsistent VM-Entry/VM-Exit triplet, entry = %x, exit = %x, secondary_exit = %llx\n",
>> > + _vmentry_control & n_ctrl, _vmexit_control & x_ctrl,
>> > + _secondary_vmexit_control & x_ctrl_2);
>> >
>> > if (error_on_inconsistent_vmcs_config)
>> > return -EIO;
>> >
>> > _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.
I just thought using two tables would eliminate the need for any if() statements.
