linux-kernel - Re: [PATCH v4 15/20] KVM:VMX: Save host MSR_IA32_S

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZMDdp1A7DOsRNeTd@chao-email>
Date:   Wed, 26 Jul 2023 16:47:35 +0800
From:   Chao Gao <chao.gao@...el.com>
To:     Yang Weijiang <weijiang.yang@...el.com>
CC:     <seanjc@...gle.com>, <pbonzini@...hat.com>, <peterz@...radead.org>,
        <john.allen@....com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <rick.p.edgecombe@...el.com>,
        <binbin.wu@...ux.intel.com>
Subject: Re: [PATCH v4 15/20] KVM:VMX: Save host MSR_IA32_S_CET to VMCS field

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@...gle.com>
>Signed-off-by: Yang Weijiang <weijiang.yang@...el.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
>