Message ID | 20190225132716.6982-6-weijiang.yang@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | This patch-set is to enable Guest CET support | expand |
On Mon, Mar 04, 2019 at 10:53:27AM -0800, Sean Christopherson wrote: > On Mon, Feb 25, 2019 at 09:27:13PM +0800, Yang Weijiang wrote: > > The CET runtime settings, i.e., CET state control bits(IA32_U_CET/ > > IA32_S_CET), CET SSP(IA32_PL3_SSP/IA32_PL0_SSP) and SSP table address > > (IA32_INTERRUPT_SSP_TABLE_ADDR) are task/thread specific, therefore, > > OS needs to save/restore the states properly during context switch, > > e.g., task/thread switching, interrupt/exception handling, it uses > > xsaves/xrstors to achieve that. > > > > The difference between VMCS CET area fields and xsave CET area, is that > > the former is for state retention during Guest/Host context > > switch while the latter is for state retention during OS execution. > > > > Linux currently doesn't support CPL1 and CPL2, so SSPs for these level > > are skipped here. > > But don't we want to allow a guest to access the MSRs regardless of > the host kernel's behavior? > Do you see any necessity of exposing the access to guest? > > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> > > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > > --- > > arch/x86/kvm/vmx.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > index 7bbb8b26e901..89ee086e1729 100644 > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -11769,6 +11769,7 @@ static void nested_vmx_entry_exit_ctls_update(struct kvm_vcpu *vcpu) > > static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > > { > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > + unsigned long *msr_bitmap; > > > > if (cpu_has_secondary_exec_ctrls()) { > > vmx_compute_secondary_exec_control(vmx); > > @@ -11786,6 +11787,18 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > > nested_vmx_cr_fixed1_bits_update(vcpu); > > nested_vmx_entry_exit_ctls_update(vcpu); > > } > > + > > + msr_bitmap = vmx->vmcs01.msr_bitmap; > > + > > + if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) | > > This should be a logical OR, not a bitwise OR. > Good capture, thanks! > > + guest_cpuid_has(vcpu, X86_FEATURE_IBT)) { > > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW); > > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW); > > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW); > > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW); > > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW); > > + } > > + > > } > > > > static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry) > > -- > > 2.17.1 > >
On Mon, Feb 25, 2019 at 09:27:13PM +0800, Yang Weijiang wrote: > The CET runtime settings, i.e., CET state control bits(IA32_U_CET/ > IA32_S_CET), CET SSP(IA32_PL3_SSP/IA32_PL0_SSP) and SSP table address > (IA32_INTERRUPT_SSP_TABLE_ADDR) are task/thread specific, therefore, > OS needs to save/restore the states properly during context switch, > e.g., task/thread switching, interrupt/exception handling, it uses > xsaves/xrstors to achieve that. > > The difference between VMCS CET area fields and xsave CET area, is that > the former is for state retention during Guest/Host context > switch while the latter is for state retention during OS execution. > > Linux currently doesn't support CPL1 and CPL2, so SSPs for these level > are skipped here. But don't we want to allow a guest to access the MSRs regardless of the host kernel's behavior? > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > --- > arch/x86/kvm/vmx.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 7bbb8b26e901..89ee086e1729 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -11769,6 +11769,7 @@ static void nested_vmx_entry_exit_ctls_update(struct kvm_vcpu *vcpu) > static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > { > struct vcpu_vmx *vmx = to_vmx(vcpu); > + unsigned long *msr_bitmap; > > if (cpu_has_secondary_exec_ctrls()) { > vmx_compute_secondary_exec_control(vmx); > @@ -11786,6 +11787,18 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > nested_vmx_cr_fixed1_bits_update(vcpu); > nested_vmx_entry_exit_ctls_update(vcpu); > } > + > + msr_bitmap = vmx->vmcs01.msr_bitmap; > + > + if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) | This should be a logical OR, not a bitwise OR. > + guest_cpuid_has(vcpu, X86_FEATURE_IBT)) { > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW); > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW); > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW); > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW); > + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW); > + } > + > } > > static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry) > -- > 2.17.1 >
On Mon, Mar 04, 2019 at 06:07:14PM +0800, Yang Weijiang wrote: > On Mon, Mar 04, 2019 at 10:53:27AM -0800, Sean Christopherson wrote: > > On Mon, Feb 25, 2019 at 09:27:13PM +0800, Yang Weijiang wrote: > > > The CET runtime settings, i.e., CET state control bits(IA32_U_CET/ > > > IA32_S_CET), CET SSP(IA32_PL3_SSP/IA32_PL0_SSP) and SSP table address > > > (IA32_INTERRUPT_SSP_TABLE_ADDR) are task/thread specific, therefore, > > > OS needs to save/restore the states properly during context switch, > > > e.g., task/thread switching, interrupt/exception handling, it uses > > > xsaves/xrstors to achieve that. > > > > > > The difference between VMCS CET area fields and xsave CET area, is that > > > the former is for state retention during Guest/Host context > > > switch while the latter is for state retention during OS execution. > > > > > > Linux currently doesn't support CPL1 and CPL2, so SSPs for these level > > > are skipped here. > > > > But don't we want to allow a guest to access the MSRs regardless of > > the host kernel's behavior? > > > Do you see any necessity of exposing the access to guest? No, but isn't exposing them to the guest effectively free since XSAVES and XRSTORS will always save/restore them along with SSP0 and SSP3?
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 7bbb8b26e901..89ee086e1729 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -11769,6 +11769,7 @@ static void nested_vmx_entry_exit_ctls_update(struct kvm_vcpu *vcpu) static void vmx_cpuid_update(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); + unsigned long *msr_bitmap; if (cpu_has_secondary_exec_ctrls()) { vmx_compute_secondary_exec_control(vmx); @@ -11786,6 +11787,18 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) nested_vmx_cr_fixed1_bits_update(vcpu); nested_vmx_entry_exit_ctls_update(vcpu); } + + msr_bitmap = vmx->vmcs01.msr_bitmap; + + if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) | + guest_cpuid_has(vcpu, X86_FEATURE_IBT)) { + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW); + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW); + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW); + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW); + vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW); + } + } static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)