Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 24 Mar 2023 21:15:33 +0000
In-Reply-To: <20230301185838.21659-2-itazur@amazon.com>
Mime-Version: 1.0
References: <20230301185838.21659-1-itazur@amazon.com> <20230301185838.21659-2-itazur@amazon.com>
Message-ID: <ZB4S9eP0tGbGUTSC@google.com>
Subject: Re: [PATCH v2 1/1] KVM: x86: Propagate AMD-specific IBRS bits to guests
From:   Sean Christopherson <seanjc@google.com>
To:     Takahiro Itazuri <itazur@amazon.com>
Cc:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        linux-kernel@vger.kernel.org, Takahiro Itazuri <zulinx86@gmail.com>
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Wed, Mar 01, 2023, Takahiro Itazuri wrote:
> VMMs retrieve supported CPUID features via KVM_GET_SUPPORTED_CPUID to
> construct CPUID information to be passed to KVM_SET_CPUID2. Most CPUID
> feature bits related to speculative attacks are propagated from host
> CPUID. AMD processors have AMD-specific IBRS related bits in CPUID
> Fn8000_0008_EBX (ref: AMD64 Architecture Programmer's Manual Volume 3:
> General-Purpose and System Instructions) and some bits are not
> propagated to guests.
> 
> Enable propagation of these bits to guests, so that guests can see the
> same security information as the host without VMM action.

Please provide some description on what these bits do, and more importantly why
no action is required to virtualize them in KVM, even if it seems obvious.  We've
had a few goofs with respect to mitigations across guest domains, I just want to
be extra paranoid that we document upfront why it's ok (recommended?) to advertise
this information to the guest.

> Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
> ---
>  arch/x86/kvm/cpuid.c         | 5 +++--
>  arch/x86/kvm/reverse_cpuid.h | 5 +++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 596061c1610e..c297064208dd 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -704,8 +704,9 @@ void kvm_set_cpu_caps(void)
>  	kvm_cpu_cap_mask(CPUID_8000_0008_EBX,
>  		F(CLZERO) | F(XSAVEERPTR) |
>  		F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
> -		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) |
> -		__feature_bit(KVM_X86_FEATURE_AMD_PSFD)
> +		F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_IBRS_ALWAYS_ON) |
> +		F(AMD_STIBP_ALWAYS_ON) | F(AMD_IBRS_PREFERRED) |
> +		F(AMD_IBRS_SAME_MODE) | __feature_bit(KVM_X86_FEATURE_AMD_PSFD)
>  	);
>  
>  	/*
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index 042d0aca3c92..1e538e29b117 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -43,6 +43,11 @@ enum kvm_only_cpuid_leafs {
>  #define X86_FEATURE_AVX_NE_CONVERT      KVM_X86_FEATURE(CPUID_7_1_EDX, 5)
>  #define X86_FEATURE_PREFETCHITI         KVM_X86_FEATURE(CPUID_7_1_EDX, 14)
>  
> +/* AMD-specific IBRS hint bits, CPUID level 0x80000008 (EBX) */
> +#define X86_FEATURE_AMD_IBRS_ALWAYS_ON	KVM_X86_FEATURE(CPUID_8000_0008_EBX, 16)
> +#define X86_FEATURE_AMD_IBRS_PREFERRED	KVM_X86_FEATURE(CPUID_8000_0008_EBX, 18)
> +#define X86_FEATURE_AMD_IBRS_SAME_MODE	KVM_X86_FEATURE(CPUID_8000_0008_EBX, 19)

These belong in cpufeatures.h, see the rest of the discussion in v1[*].  Sorry for
the runaround :-(

[*] https://lore.kernel.org/all/e5bf7da5-df29-31c6-6d33-81bbecb849ba@redhat.com