| Message ID | 20190628221447.23498-2-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nVMX: Check Host Segment Registers and Descriptor Tables on vmentry of nested guests | expand |
On 29/06/19 00:14, Krish Sadhukhan wrote: > According to section "Checks on Host Segment and Descriptor-Table > Registers" in Intel SDM vol 3C, the following checks are performed on > vmentry of nested guests: > > - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the > RPL (bits 1:0) and the TI flag (bit 2) must be 0. > - The selector fields for CS and TR cannot be 0000H. > - The selector field for SS cannot be 0000H if the "host address-space > size" VM-exit control is 0. > - On processors that support Intel 64 architecture, the base-address > fields for FS, GS and TR must contain canonical addresses. > > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com> All these tests are getting expensive. Can you look into skipping them whenever dirty_vmcs12 is clear? Thanks, Paolo > --- > arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index f1a69117ac0f..856a83aa42f5 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -2609,6 +2609,30 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, > !kvm_pat_valid(vmcs12->host_ia32_pat)) > return -EINVAL; > > + ia32e = (vmcs12->vm_exit_controls & > + VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0; > + > + if (vmcs12->host_cs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_ss_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_ds_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_es_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_fs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_gs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_tr_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || > + vmcs12->host_cs_selector == 0 || > + vmcs12->host_tr_selector == 0 || > + (vmcs12->host_ss_selector == 0 && !ia32e)) > + return -EINVAL; > + > +#ifdef CONFIG_X86_64 > + if (is_noncanonical_address(vmcs12->host_fs_base, vcpu) || > + is_noncanonical_address(vmcs12->host_gs_base, vcpu) || > + is_noncanonical_address(vmcs12->host_gdtr_base, vcpu) || > + is_noncanonical_address(vmcs12->host_idtr_base, vcpu) || > + is_noncanonical_address(vmcs12->host_tr_base, vcpu)) > + return -EINVAL; > +#endif > + > /* > * If the load IA32_EFER VM-exit control is 1, bits reserved in the > * IA32_EFER MSR must be 0 in the field for that register. In addition, > @@ -2616,8 +2640,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, > * the host address-space size VM-exit control. > */ > if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) { > - ia32e = (vmcs12->vm_exit_controls & > - VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0; > if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) || > ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) || > ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) >
On 07/02/2019 09:25 AM, Paolo Bonzini wrote: > On 29/06/19 00:14, Krish Sadhukhan wrote: >> According to section "Checks on Host Segment and Descriptor-Table >> Registers" in Intel SDM vol 3C, the following checks are performed on >> vmentry of nested guests: >> >> - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the >> RPL (bits 1:0) and the TI flag (bit 2) must be 0. >> - The selector fields for CS and TR cannot be 0000H. >> - The selector field for SS cannot be 0000H if the "host address-space >> size" VM-exit control is 0. >> - On processors that support Intel 64 architecture, the base-address >> fields for FS, GS and TR must contain canonical addresses. >> >> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> >> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com> > All these tests are getting expensive. Can you look into skipping them > whenever dirty_vmcs12 is clear? OK. I am working on it and will send a separate patch-set for that. In the meantime, I will send v2 of this patchset containing a fix for a compilation error. Thanks. > Thanks, > > Paolo > >> --- >> arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++-- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >> index f1a69117ac0f..856a83aa42f5 100644 >> --- a/arch/x86/kvm/vmx/nested.c >> +++ b/arch/x86/kvm/vmx/nested.c >> @@ -2609,6 +2609,30 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, >> !kvm_pat_valid(vmcs12->host_ia32_pat)) >> return -EINVAL; >> >> + ia32e = (vmcs12->vm_exit_controls & >> + VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0; >> + >> + if (vmcs12->host_cs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_ss_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_ds_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_es_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_fs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_gs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_tr_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || >> + vmcs12->host_cs_selector == 0 || >> + vmcs12->host_tr_selector == 0 || >> + (vmcs12->host_ss_selector == 0 && !ia32e)) >> + return -EINVAL; >> + >> +#ifdef CONFIG_X86_64 >> + if (is_noncanonical_address(vmcs12->host_fs_base, vcpu) || >> + is_noncanonical_address(vmcs12->host_gs_base, vcpu) || >> + is_noncanonical_address(vmcs12->host_gdtr_base, vcpu) || >> + is_noncanonical_address(vmcs12->host_idtr_base, vcpu) || >> + is_noncanonical_address(vmcs12->host_tr_base, vcpu)) >> + return -EINVAL; >> +#endif >> + >> /* >> * If the load IA32_EFER VM-exit control is 1, bits reserved in the >> * IA32_EFER MSR must be 0 in the field for that register. In addition, >> @@ -2616,8 +2640,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, >> * the host address-space size VM-exit control. >> */ >> if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) { >> - ia32e = (vmcs12->vm_exit_controls & >> - VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0; >> if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) || >> ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) || >> ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) >>
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index f1a69117ac0f..856a83aa42f5 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -2609,6 +2609,30 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, !kvm_pat_valid(vmcs12->host_ia32_pat)) return -EINVAL; + ia32e = (vmcs12->vm_exit_controls & + VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0; + + if (vmcs12->host_cs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_ss_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_ds_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_es_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_fs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_gs_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_tr_selector & (SEGMENT_RPL_MASK | SEGMENT_TI_MASK) || + vmcs12->host_cs_selector == 0 || + vmcs12->host_tr_selector == 0 || + (vmcs12->host_ss_selector == 0 && !ia32e)) + return -EINVAL; + +#ifdef CONFIG_X86_64 + if (is_noncanonical_address(vmcs12->host_fs_base, vcpu) || + is_noncanonical_address(vmcs12->host_gs_base, vcpu) || + is_noncanonical_address(vmcs12->host_gdtr_base, vcpu) || + is_noncanonical_address(vmcs12->host_idtr_base, vcpu) || + is_noncanonical_address(vmcs12->host_tr_base, vcpu)) + return -EINVAL; +#endif + /* * If the load IA32_EFER VM-exit control is 1, bits reserved in the * IA32_EFER MSR must be 0 in the field for that register. In addition, @@ -2616,8 +2640,6 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, * the host address-space size VM-exit control. */ if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) { - ia32e = (vmcs12->vm_exit_controls & - VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0; if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) || ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) || ia32e != !!(vmcs12->host_ia32_efer & EFER_LME))
According to section "Checks on Host Segment and Descriptor-Table Registers" in Intel SDM vol 3C, the following checks are performed on vmentry of nested guests: - In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the RPL (bits 1:0) and the TI flag (bit 2) must be 0. - The selector fields for CS and TR cannot be 0000H. - The selector field for SS cannot be 0000H if the "host address-space size" VM-exit control is 0. - On processors that support Intel 64 architecture, the base-address fields for FS, GS and TR must contain canonical addresses. Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com> --- arch/x86/kvm/vmx/nested.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)