| Message ID | 20190707071147.11651-3-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: Skip vmentry checks that are necessary only if VMCS12 is dirty | expand |
On 07/07/19 09:11, Krish Sadhukhan wrote: > > if (!vmx_control_verify(vmcs12->vm_exit_controls, > vmx->nested.msrs.exit_ctls_low, > - vmx->nested.msrs.exit_ctls_high) || > - nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12)) > + vmx->nested.msrs.exit_ctls_high)) > + return -EINVAL; > + Exit controls are not shadowed, are they? Paolo
On 7/10/19 7:28 AM, Paolo Bonzini wrote: > On 07/07/19 09:11, Krish Sadhukhan wrote: >> >> if (!vmx_control_verify(vmcs12->vm_exit_controls, >> vmx->nested.msrs.exit_ctls_low, >> - vmx->nested.msrs.exit_ctls_high) || >> - nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12)) >> + vmx->nested.msrs.exit_ctls_high)) >> + return -EINVAL; >> + > Exit controls are not shadowed, are they? No, they aren't. However, I see that prepare_vmcs02_constant_state() which is called in the path of prepare_vmcs02_early_full() writes those Exit Control fields: vmcs_write32(VM_EXIT_MSR_STORE_COUNT, 0); vmcs_write64(VM_EXIT_MSR_LOAD_ADDR, __pa(vmx->msr_autoload.host.val)); vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR, __pa(vmx->msr_autoload.guest.val)); Should we add these fields to the shadow list then ? > > Paolo
On Wed, Jul 10, 2019 at 12:18:55PM -0700, Krish Sadhukhan wrote: > > On 7/10/19 7:28 AM, Paolo Bonzini wrote: > >On 07/07/19 09:11, Krish Sadhukhan wrote: > >> if (!vmx_control_verify(vmcs12->vm_exit_controls, > >> vmx->nested.msrs.exit_ctls_low, > >>- vmx->nested.msrs.exit_ctls_high) || > >>- nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12)) > >>+ vmx->nested.msrs.exit_ctls_high)) > >>+ return -EINVAL; > >>+ > >Exit controls are not shadowed, are they? > > No, they aren't. However, I see that prepare_vmcs02_constant_state() which > is called in the path of prepare_vmcs02_early_full() writes those Exit > Control fields: > > vmcs_write32(VM_EXIT_MSR_STORE_COUNT, 0); > vmcs_write64(VM_EXIT_MSR_LOAD_ADDR, > __pa(vmx->msr_autoload.host.val)); > vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR, > __pa(vmx->msr_autoload.guest.val)); That's writing L0's values into vmcs02 when vmcs02 is first used. L1's MSR load lists are processed purely in software, e.g. nested_vmx_load_msr(). The vmcs12 entries are consumed by KVM on every nested transition to emulate the load/store functionality, but the validity of the *controls* only needs to be checked when vmcs12 is dirty. > > > Should we add these fields to the shadow list then ? > > > > >Paolo
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index b0b59c78b3e8..056eba497730 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -2505,6 +2505,15 @@ static int nested_check_vm_execution_controls(struct kvm_vcpu *vcpu, return 0; } +static int nested_check_vm_exit_controls_full(struct kvm_vcpu *vcpu, + struct vmcs12 *vmcs12) +{ + if (nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12)) + return -EINVAL; + + return 0; +} + /* * Checks related to VM-Exit Control Fields */ @@ -2515,8 +2524,11 @@ static int nested_check_vm_exit_controls(struct kvm_vcpu *vcpu, if (!vmx_control_verify(vmcs12->vm_exit_controls, vmx->nested.msrs.exit_ctls_low, - vmx->nested.msrs.exit_ctls_high) || - nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12)) + vmx->nested.msrs.exit_ctls_high)) + return -EINVAL; + + if ((vmx->nested.dirty_vmcs12) && + nested_check_vm_exit_controls_full(vcpu, vmcs12)) return -EINVAL; return 0;
..so that every nested vmentry is not slowed down by those checks. Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> --- arch/x86/kvm/vmx/nested.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)