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]

Date:   Thu, 27 Jul 2023 15:29:15 +0800
From:   "Yang, Weijiang" <weijiang.yang@...el.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Chao Gao <chao.gao@...el.com>
CC:     <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 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@...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.
> 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!