| Message ID | BLU436-SMTP172DC45E064A89EC074C06C804E0@phx.gbl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/09/2015 04:55, Wanpeng Li wrote: > Expose VPID capability to L1. > > Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> > --- > v1 -> v2: > * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT Thanks. I've checked more thoroughly your implementation against the SDM now, and there are a few missing things between this patch and the one that emulates INVVPID: - you're not setting bit 32 of the VMX_EPT_VPID_CAP MSR - you were not checking against the supported types in the implementation of the INVVPID instruction - the memory operand must always be read even if it isn't needed (e.g., for type==global), similar to INVEPT - for single-context invalidation you're not checking that VPID != 0, though in practice that doesn't matter because we don't want to support single-context invalidation - you're always setting the MSR's bits to 1 even if !enable_vpid At this point it's better if you resend the whole nested VPID implementation, i.e. the following five patches: KVM: VMX: adjust interface to allocate/free_vpid KVM: VMX: introduce __vmx_flush_tlb to handle specific vpid KVM: nVMX: emulate the INVVPID instruction KVM: nVMX: nested VPID emulation KVM: nVMX: expose VPID capability to L1 with the above issues fixed. Please also send kvm-unit-tests patches that tests for the error cases. Thanks, Paolo > arch/x86/kvm/vmx.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 75f3ee0..c4ea890 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -442,7 +442,7 @@ struct nested_vmx { > u32 nested_vmx_true_entry_ctls_low; > u32 nested_vmx_misc_low; > u32 nested_vmx_misc_high; > - u32 nested_vmx_ept_caps; > + u64 nested_vmx_ept_vpid_caps; > }; > > #define POSTED_INTR_ON 0 > @@ -2485,22 +2485,23 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) > SECONDARY_EXEC_WBINVD_EXITING | > SECONDARY_EXEC_XSAVES; > > - if (enable_ept) { > + if (enable_ept | enable_vpid) { > /* nested EPT: emulate EPT also to L1 */ > vmx->nested.nested_vmx_secondary_ctls_high |= > SECONDARY_EXEC_ENABLE_EPT; > - vmx->nested.nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT | > + vmx->nested.nested_vmx_ept_vpid_caps = VMX_EPT_PAGE_WALK_4_BIT | > VMX_EPTP_WB_BIT | VMX_EPT_2MB_PAGE_BIT | > VMX_EPT_INVEPT_BIT; > - vmx->nested.nested_vmx_ept_caps &= vmx_capability.ept; > + vmx->nested.nested_vmx_ept_vpid_caps &= vmx_capability.ept; > /* > * For nested guests, we don't do anything specific > * for single context invalidation. Hence, only advertise > * support for global context invalidation. > */ > - vmx->nested.nested_vmx_ept_caps |= VMX_EPT_EXTENT_GLOBAL_BIT; > + vmx->nested.nested_vmx_ept_vpid_caps |= VMX_EPT_EXTENT_GLOBAL_BIT; > + vmx->nested.nested_vmx_ept_vpid_caps |= VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT << 32; > } else > - vmx->nested.nested_vmx_ept_caps = 0; > + vmx->nested.nested_vmx_ept_vpid_caps = 0; > > if (enable_unrestricted_guest) > vmx->nested.nested_vmx_secondary_ctls_high |= > @@ -2616,8 +2617,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) > vmx->nested.nested_vmx_secondary_ctls_high); > break; > case MSR_IA32_VMX_EPT_VPID_CAP: > - /* Currently, no nested vpid support */ > - *pdata = vmx->nested.nested_vmx_ept_caps; > + *pdata = vmx->nested.nested_vmx_ept_vpid_caps; > break; > default: > return 1; > @@ -7142,7 +7142,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) > > if (!(vmx->nested.nested_vmx_secondary_ctls_high & > SECONDARY_EXEC_ENABLE_EPT) || > - !(vmx->nested.nested_vmx_ept_caps & VMX_EPT_INVEPT_BIT)) { > + !(vmx->nested.nested_vmx_ept_vpid_caps & VMX_EPT_INVEPT_BIT)) { > kvm_queue_exception(vcpu, UD_VECTOR); > return 1; > } > @@ -7158,7 +7158,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) > vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); > type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf); > > - types = (vmx->nested.nested_vmx_ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6; > + types = (vmx->nested.nested_vmx_ept_vpid_caps >> VMX_EPT_EXTENT_SHIFT) & 6; > > if (!(types & (1UL << type))) { > nested_vmx_failValid(vcpu, > @@ -8763,7 +8763,7 @@ static void nested_ept_init_mmu_context(struct kvm_vcpu *vcpu) > { > WARN_ON(mmu_is_nested(vcpu)); > kvm_init_shadow_ept_mmu(vcpu, > - to_vmx(vcpu)->nested.nested_vmx_ept_caps & > + to_vmx(vcpu)->nested.nested_vmx_ept_vpid_caps & > VMX_EPT_EXECUTE_ONLY_BIT); > vcpu->arch.mmu.set_cr3 = vmx_set_cr3; > vcpu->arch.mmu.get_cr3 = nested_ept_get_cr3; > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9/29/15 6:39 PM, Paolo Bonzini wrote: > On 29/09/2015 04:55, Wanpeng Li wrote: >> Expose VPID capability to L1. >> >> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> >> --- >> v1 -> v2: >> * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT > Thanks. I've checked more thoroughly your implementation against the > SDM now, and there are a few missing things between this patch and the > one that emulates INVVPID: > > - you're not setting bit 32 of the VMX_EPT_VPID_CAP MSR > > - you were not checking against the supported types in the > implementation of the INVVPID instruction > > - the memory operand must always be read even if it isn't needed (e.g., > for type==global), similar to INVEPT > > - for single-context invalidation you're not checking that VPID != 0, > though in practice that doesn't matter because we don't want to support > single-context invalidation > > - you're always setting the MSR's bits to 1 even if !enable_vpid > > At this point it's better if you resend the whole nested VPID > implementation, i.e. the following five patches: > > KVM: VMX: adjust interface to allocate/free_vpid > KVM: VMX: introduce __vmx_flush_tlb to handle specific vpid > KVM: nVMX: emulate the INVVPID instruction > KVM: nVMX: nested VPID emulation > KVM: nVMX: expose VPID capability to L1 > > with the above issues fixed. Please also send kvm-unit-tests patches > that tests for the error cases. Ok, I will do this after the vacation(until 10.7) in my country. :-) Regards, Wanpeng Li -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9/29/15 6:39 PM, Paolo Bonzini wrote: > On 29/09/2015 04:55, Wanpeng Li wrote: >> Expose VPID capability to L1. >> >> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> >> --- >> v1 -> v2: >> * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT > Thanks. I've checked more thoroughly your implementation against the > SDM now, and there are a few missing things between this patch and the > one that emulates INVVPID: > > - you're not setting bit 32 of the VMX_EPT_VPID_CAP MSR > > - you were not checking against the supported types in the > implementation of the INVVPID instruction > > - the memory operand must always be read even if it isn't needed (e.g., > for type==global), similar to INVEPT > > - for single-context invalidation you're not checking that VPID != 0, > though in practice that doesn't matter because we don't want to support > single-context invalidation > > - you're always setting the MSR's bits to 1 even if !enable_vpid I believe you mean always setting the MSR's bits to 1 when !enable_ept and enable_vpid. > > At this point it's better if you resend the whole nested VPID > implementation, i.e. the following five patches: > > KVM: VMX: adjust interface to allocate/free_vpid > KVM: VMX: introduce __vmx_flush_tlb to handle specific vpid > KVM: nVMX: emulate the INVVPID instruction > KVM: nVMX: nested VPID emulation > KVM: nVMX: expose VPID capability to L1 > > with the above issues fixed. I just send out v2. > Please also send kvm-unit-tests patches > that tests for the error cases. I will do it later. Regards, Wanpeng Li -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 75f3ee0..c4ea890 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -442,7 +442,7 @@ struct nested_vmx { u32 nested_vmx_true_entry_ctls_low; u32 nested_vmx_misc_low; u32 nested_vmx_misc_high; - u32 nested_vmx_ept_caps; + u64 nested_vmx_ept_vpid_caps; }; #define POSTED_INTR_ON 0 @@ -2485,22 +2485,23 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) SECONDARY_EXEC_WBINVD_EXITING | SECONDARY_EXEC_XSAVES; - if (enable_ept) { + if (enable_ept | enable_vpid) { /* nested EPT: emulate EPT also to L1 */ vmx->nested.nested_vmx_secondary_ctls_high |= SECONDARY_EXEC_ENABLE_EPT; - vmx->nested.nested_vmx_ept_caps = VMX_EPT_PAGE_WALK_4_BIT | + vmx->nested.nested_vmx_ept_vpid_caps = VMX_EPT_PAGE_WALK_4_BIT | VMX_EPTP_WB_BIT | VMX_EPT_2MB_PAGE_BIT | VMX_EPT_INVEPT_BIT; - vmx->nested.nested_vmx_ept_caps &= vmx_capability.ept; + vmx->nested.nested_vmx_ept_vpid_caps &= vmx_capability.ept; /* * For nested guests, we don't do anything specific * for single context invalidation. Hence, only advertise * support for global context invalidation. */ - vmx->nested.nested_vmx_ept_caps |= VMX_EPT_EXTENT_GLOBAL_BIT; + vmx->nested.nested_vmx_ept_vpid_caps |= VMX_EPT_EXTENT_GLOBAL_BIT; + vmx->nested.nested_vmx_ept_vpid_caps |= VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT << 32; } else - vmx->nested.nested_vmx_ept_caps = 0; + vmx->nested.nested_vmx_ept_vpid_caps = 0; if (enable_unrestricted_guest) vmx->nested.nested_vmx_secondary_ctls_high |= @@ -2616,8 +2617,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) vmx->nested.nested_vmx_secondary_ctls_high); break; case MSR_IA32_VMX_EPT_VPID_CAP: - /* Currently, no nested vpid support */ - *pdata = vmx->nested.nested_vmx_ept_caps; + *pdata = vmx->nested.nested_vmx_ept_vpid_caps; break; default: return 1; @@ -7142,7 +7142,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) if (!(vmx->nested.nested_vmx_secondary_ctls_high & SECONDARY_EXEC_ENABLE_EPT) || - !(vmx->nested.nested_vmx_ept_caps & VMX_EPT_INVEPT_BIT)) { + !(vmx->nested.nested_vmx_ept_vpid_caps & VMX_EPT_INVEPT_BIT)) { kvm_queue_exception(vcpu, UD_VECTOR); return 1; } @@ -7158,7 +7158,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO); type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf); - types = (vmx->nested.nested_vmx_ept_caps >> VMX_EPT_EXTENT_SHIFT) & 6; + types = (vmx->nested.nested_vmx_ept_vpid_caps >> VMX_EPT_EXTENT_SHIFT) & 6; if (!(types & (1UL << type))) { nested_vmx_failValid(vcpu, @@ -8763,7 +8763,7 @@ static void nested_ept_init_mmu_context(struct kvm_vcpu *vcpu) { WARN_ON(mmu_is_nested(vcpu)); kvm_init_shadow_ept_mmu(vcpu, - to_vmx(vcpu)->nested.nested_vmx_ept_caps & + to_vmx(vcpu)->nested.nested_vmx_ept_vpid_caps & VMX_EPT_EXECUTE_ONLY_BIT); vcpu->arch.mmu.set_cr3 = vmx_set_cr3; vcpu->arch.mmu.get_cr3 = nested_ept_get_cr3;
Expose VPID capability to L1. Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com> --- v1 -> v2: * set only VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT arch/x86/kvm/vmx.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)