On Wed, Feb 07, 2024, Xin Li wrote: > @@ -6856,13 +6865,17 @@ static void nested_vmx_setup_exit_ctls(struct vmcs_config *vmcs_conf, > VM_EXIT_HOST_ADDR_SPACE_SIZE | > #endif > VM_EXIT_LOAD_IA32_PAT | VM_EXIT_SAVE_IA32_PAT | > - VM_EXIT_CLEAR_BNDCFGS; > + VM_EXIT_CLEAR_BNDCFGS | VM_EXIT_ACTIVATE_SECONDARY_CONTROLS; > msrs->exit_ctls_high |= > VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR | > VM_EXIT_LOAD_IA32_EFER | VM_EXIT_SAVE_IA32_EFER | > VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT | > VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL; > > + /* secondary exit controls */ Drop the comment, it's pretty obvious what field is being setup. > + if (msrs->exit_ctls_high & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) > + rdmsrl(MSR_IA32_VMX_EXIT_CTLS2, msrs->secondary_exit_ctls); This is wrong, the resulting msrs->secondary_exit_ctls needs to be sanitized based on what KVM supports for nVMX. On a very related topic, this should not do a raw RDMSR. One of the reasons why KVM uses vmcs_config as the base is to avoid advertising features to L1 that KVM itself doesn't support, e.g. because the expected entry+exit pairs aren't supported. And by pulling state from vmcs_conf->secondary_exit_ctls there's no need to check the activation bit. I.e. literaly just this: msrs->secondary_exit_ctls = vmcs_conf->secondary_exit_ctls; msrs->secondary_exit_ctls &= 0; and then when nVMX FRED support is ready, it becomes: msrs->secondary_exit_ctls = vmcs_conf->secondary_exit_ctls; msrs->secondary_exit_ctls &= SECONDARY_VM_EXIT_SAVE_IA32_FRED | SECONDARY_VM_EXIT_LOAD_IA32_FRED;
