| Message ID | 20230721030352.72414-16-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable CET Virtualization | expand |
On Thu, Jul 20, 2023 at 11:03:47PM -0400, Yang Weijiang wrote: >Save host MSR_IA32_S_CET to VMCS field as host constant state. >Kernel IBT is supported now and the setting in MSR_IA32_S_CET >is static after post-boot except in BIOS call case, but vCPU >won't execute such BIOS call path currently, so it's safe to >make the MSR as host constant. > >Suggested-by: Sean Christopherson <seanjc@google.com> >Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> >--- > arch/x86/kvm/vmx/capabilities.h | 4 ++++ > arch/x86/kvm/vmx/vmx.c | 8 ++++++++ > 2 files changed, 12 insertions(+) > >diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h >index d0abee35d7ba..b1883f6c08eb 100644 >--- a/arch/x86/kvm/vmx/capabilities.h >+++ b/arch/x86/kvm/vmx/capabilities.h >@@ -106,6 +106,10 @@ static inline bool cpu_has_load_perf_global_ctrl(void) > return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL; > } > >+static inline bool cpu_has_load_cet_ctrl(void) >+{ >+ return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_CET_STATE); VM_ENTRY_LOAD_CET_STATE is to load guest state. Strictly speaking, you should check VM_EXIT_LOAD_HOST_CET_STATE though I believe CPUs will support both or none. >+} > static inline bool cpu_has_vmx_mpx(void) > { > return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS; >diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >index 85cb7e748a89..cba24acf1a7a 100644 >--- a/arch/x86/kvm/vmx/vmx.c >+++ b/arch/x86/kvm/vmx/vmx.c >@@ -109,6 +109,8 @@ module_param(enable_apicv, bool, S_IRUGO); > bool __read_mostly enable_ipiv = true; > module_param(enable_ipiv, bool, 0444); > >+static u64 __read_mostly host_s_cet; caching host's value is to save an MSR read on vCPU creation? Otherwise I don't see why a local variable cannot work. >+ > /* > * If nested=1, nested virtualization is supported, i.e., guests may use > * VMX and be a hypervisor for its own guests. If nested=0, guests may not >@@ -4355,6 +4357,9 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx) > > if (cpu_has_load_ia32_efer()) > vmcs_write64(HOST_IA32_EFER, host_efer); >+ >+ if (cpu_has_load_cet_ctrl()) >+ vmcs_writel(HOST_S_CET, host_s_cet); > } > > void set_cr4_guest_host_mask(struct vcpu_vmx *vmx) >@@ -8633,6 +8638,9 @@ static __init int hardware_setup(void) > return r; > } > >+ if (cpu_has_load_cet_ctrl()) >+ rdmsrl_safe(MSR_IA32_S_CET, &host_s_cet); >+ > vmx_set_cpu_caps(); > > r = alloc_kvm_area(); >-- >2.27.0 >
On Wed, Jul 26, 2023, Chao Gao wrote: > On Thu, Jul 20, 2023 at 11:03:47PM -0400, Yang Weijiang wrote: > >Save host MSR_IA32_S_CET to VMCS field as host constant state. > >Kernel IBT is supported now and the setting in MSR_IA32_S_CET > >is static after post-boot except in BIOS call case, but vCPU > >won't execute such BIOS call path currently, so it's safe to > >make the MSR as host constant. > > > >Suggested-by: Sean Christopherson <seanjc@google.com> > >Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> > >--- > > arch/x86/kvm/vmx/capabilities.h | 4 ++++ > > arch/x86/kvm/vmx/vmx.c | 8 ++++++++ > > 2 files changed, 12 insertions(+) > > > >diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h > >index d0abee35d7ba..b1883f6c08eb 100644 > >--- a/arch/x86/kvm/vmx/capabilities.h > >+++ b/arch/x86/kvm/vmx/capabilities.h > >@@ -106,6 +106,10 @@ static inline bool cpu_has_load_perf_global_ctrl(void) > > return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL; > > } > > > >+static inline bool cpu_has_load_cet_ctrl(void) > >+{ > >+ return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_CET_STATE); > > VM_ENTRY_LOAD_CET_STATE is to load guest state. Strictly speaking, you > should check VM_EXIT_LOAD_HOST_CET_STATE though I believe CPUs will > support both or none. No need, pairs are now handled by setup_vmcs_config(). See commit f5a81d0eb01e ("KVM: VMX: Sanitize VM-Entry/VM-Exit control pairs at kvm_intel load time"), and then patch 17 does: diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 3eb4fe9c9ab6..3f2f966e327d 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -2641,6 +2641,7 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf, { VM_ENTRY_LOAD_IA32_EFER, VM_EXIT_LOAD_IA32_EFER }, { VM_ENTRY_LOAD_BNDCFGS, VM_EXIT_CLEAR_BNDCFGS }, { VM_ENTRY_LOAD_IA32_RTIT_CTL, VM_EXIT_CLEAR_IA32_RTIT_CTL }, + { VM_ENTRY_LOAD_CET_STATE, VM_EXIT_LOAD_CET_STATE }, }; > > >+} > > static inline bool cpu_has_vmx_mpx(void) > > { > > return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS; > >diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > >index 85cb7e748a89..cba24acf1a7a 100644 > >--- a/arch/x86/kvm/vmx/vmx.c > >+++ b/arch/x86/kvm/vmx/vmx.c > >@@ -109,6 +109,8 @@ module_param(enable_apicv, bool, S_IRUGO); > > bool __read_mostly enable_ipiv = true; > > module_param(enable_ipiv, bool, 0444); > > > >+static u64 __read_mostly host_s_cet; > > caching host's value is to save an MSR read on vCPU creation? Yep. And probably more importantly, to document that the host value is static, i.e. that KVM doesn't need to refresh S_CET before every VM-Enter/VM-Exit sequence.
On 7/26/2023 10:05 PM, Sean Christopherson wrote: > On Wed, Jul 26, 2023, Chao Gao wrote: >> On Thu, Jul 20, 2023 at 11:03:47PM -0400, Yang Weijiang wrote: >>> Save host MSR_IA32_S_CET to VMCS field as host constant state. >>> Kernel IBT is supported now and the setting in MSR_IA32_S_CET >>> is static after post-boot except in BIOS call case, but vCPU >>> won't execute such BIOS call path currently, so it's safe to >>> make the MSR as host constant. >>> >>> Suggested-by: Sean Christopherson <seanjc@google.com> >>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> >>> --- >>> arch/x86/kvm/vmx/capabilities.h | 4 ++++ >>> arch/x86/kvm/vmx/vmx.c | 8 ++++++++ >>> 2 files changed, 12 insertions(+) >>> >>> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h >>> index d0abee35d7ba..b1883f6c08eb 100644 >>> --- a/arch/x86/kvm/vmx/capabilities.h >>> +++ b/arch/x86/kvm/vmx/capabilities.h >>> @@ -106,6 +106,10 @@ static inline bool cpu_has_load_perf_global_ctrl(void) >>> return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL; >>> } >>> >>> +static inline bool cpu_has_load_cet_ctrl(void) >>> +{ >>> + return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_CET_STATE); >> VM_ENTRY_LOAD_CET_STATE is to load guest state. Strictly speaking, you >> should check VM_EXIT_LOAD_HOST_CET_STATE though I believe CPUs will >> support both or none. > No need, pairs are now handled by setup_vmcs_config(). See commit f5a81d0eb01e > ("KVM: VMX: Sanitize VM-Entry/VM-Exit control pairs at kvm_intel load time"), and > then patch 17 does: > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 3eb4fe9c9ab6..3f2f966e327d 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -2641,6 +2641,7 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf, > { VM_ENTRY_LOAD_IA32_EFER, VM_EXIT_LOAD_IA32_EFER }, > { VM_ENTRY_LOAD_BNDCFGS, VM_EXIT_CLEAR_BNDCFGS }, > { VM_ENTRY_LOAD_IA32_RTIT_CTL, VM_EXIT_CLEAR_IA32_RTIT_CTL }, > + { VM_ENTRY_LOAD_CET_STATE, VM_EXIT_LOAD_CET_STATE }, > }; > >>> +} >>> static inline bool cpu_has_vmx_mpx(void) >>> { >>> return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS; >>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >>> index 85cb7e748a89..cba24acf1a7a 100644 >>> --- a/arch/x86/kvm/vmx/vmx.c >>> +++ b/arch/x86/kvm/vmx/vmx.c >>> @@ -109,6 +109,8 @@ module_param(enable_apicv, bool, S_IRUGO); >>> bool __read_mostly enable_ipiv = true; >>> module_param(enable_ipiv, bool, 0444); >>> >>> +static u64 __read_mostly host_s_cet; >> caching host's value is to save an MSR read on vCPU creation? > Yep. And probably more importantly, to document that the host value is static, > i.e. that KVM doesn't need to refresh S_CET before every VM-Enter/VM-Exit sequence. OK, will add it to change log, thanks!
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h index d0abee35d7ba..b1883f6c08eb 100644 --- a/arch/x86/kvm/vmx/capabilities.h +++ b/arch/x86/kvm/vmx/capabilities.h @@ -106,6 +106,10 @@ static inline bool cpu_has_load_perf_global_ctrl(void) return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL; } +static inline bool cpu_has_load_cet_ctrl(void) +{ + return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_CET_STATE); +} static inline bool cpu_has_vmx_mpx(void) { return vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS; diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 85cb7e748a89..cba24acf1a7a 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -109,6 +109,8 @@ module_param(enable_apicv, bool, S_IRUGO); bool __read_mostly enable_ipiv = true; module_param(enable_ipiv, bool, 0444); +static u64 __read_mostly host_s_cet; + /* * If nested=1, nested virtualization is supported, i.e., guests may use * VMX and be a hypervisor for its own guests. If nested=0, guests may not @@ -4355,6 +4357,9 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx) if (cpu_has_load_ia32_efer()) vmcs_write64(HOST_IA32_EFER, host_efer); + + if (cpu_has_load_cet_ctrl()) + vmcs_writel(HOST_S_CET, host_s_cet); } void set_cr4_guest_host_mask(struct vcpu_vmx *vmx) @@ -8633,6 +8638,9 @@ static __init int hardware_setup(void) return r; } + if (cpu_has_load_cet_ctrl()) + rdmsrl_safe(MSR_IA32_S_CET, &host_s_cet); + vmx_set_cpu_caps(); r = alloc_kvm_area();
Save host MSR_IA32_S_CET to VMCS field as host constant state. Kernel IBT is supported now and the setting in MSR_IA32_S_CET is static after post-boot except in BIOS call case, but vCPU won't execute such BIOS call path currently, so it's safe to make the MSR as host constant. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- arch/x86/kvm/vmx/capabilities.h | 4 ++++ arch/x86/kvm/vmx/vmx.c | 8 ++++++++ 2 files changed, 12 insertions(+)