linux-kernel - Re: [PATCH 03/22] KVM: x86: Support IBPB

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: <ZOOVtYB5GXJT+Fk6@google.com>
Date:   Mon, 21 Aug 2023 16:49:57 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Josh Poimboeuf <jpoimboe@...nel.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Borislav Petkov <bp@...en8.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Babu Moger <babu.moger@....com>,
        Paolo Bonzini <pbonzini@...hat.com>, David.Kaplan@....com,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        Nikolay Borisov <nik.borisov@...e.com>,
        gregkh@...uxfoundation.org, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 03/22] KVM: x86: Support IBPB_BRTYPE and SBPB

On Sun, Aug 20, 2023, Josh Poimboeuf wrote:
> The IBPB_BRTYPE and SBPB CPUID bits aren't set by HW.
> 
> From the AMD SRSO whitepaper:
> 
>   "Hypervisor software should synthesize the value of both the
>   IBPB_BRTYPE and SBPB CPUID bits on these platforms for use by guest
>   software."
> 
> These bits are already set during kernel boot.  Manually propagate them
> to the guest.

Setting the bits in kvm_cpu_caps just advertises them to userspace, i.e. it doesn't
propagate them to the guest, that's up to userspace.

> Also, propagate PRED_CMD_SBPB writes.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> ---
>  arch/x86/kvm/cpuid.c | 4 ++++
>  arch/x86/kvm/x86.c   | 9 +++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index d3432687c9e6..cdf703eec42d 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -729,6 +729,10 @@ void kvm_set_cpu_caps(void)
>  		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */
>  	);
>  
> +	if (cpu_feature_enabled(X86_FEATURE_SBPB))
> +		kvm_cpu_cap_set(X86_FEATURE_SBPB);

This can simply be:

	kvm_cpu_cap_check_and_set(X86_FEATURE_SBPB);

If there's a strong desire to use cpu_feature_enabled() instead of boot_cpu_has(),
then I would rather make than change in kvm_cpu_cap_check_and_set() for all features.

> +	if (cpu_feature_enabled(X86_FEATURE_IBPB_BRTYPE))
> +		kvm_cpu_cap_set(X86_FEATURE_IBPB_BRTYPE);

Assuming IBPB_BRTYPE doesn't require any extra support, it's probably best to add
that one in a separate patch, as SBPB support is likely going to be a bit more
involved.

>  	if (cpu_feature_enabled(X86_FEATURE_SRSO_NO))
>  		kvm_cpu_cap_set(X86_FEATURE_SRSO_NO);

Ah, this snuck in without going through the normal review channels.  This too
can use kvm_cpu_cap_check_and_set().