Re: [PATCH] KVM: x86: Expose Predictive Store Forwarding Disable

[Jim Mattson <jmattson@google.com>]

On Thu, Sep 23, 2021 at 6:15 PM Babu Moger <babu.moger@amd.com> wrote:
>
> From: Babu Moger <Babu.Moger@amd.com>
>
> Predictive Store Forwarding: AMD Zen3 processors feature a new
> technology called Predictive Store Forwarding (PSF).
>
> PSF is a hardware-based micro-architectural optimization designed
> to improve the performance of code execution by predicting address
> dependencies between loads and stores.
>
> How PSF works:
>
> It is very common for a CPU to execute a load instruction to an address
> that was recently written by a store. Modern CPUs implement a technique
> known as Store-To-Load-Forwarding (STLF) to improve performance in such
> cases. With STLF, data from the store is forwarded directly to the load
> without having to wait for it to be written to memory. In a typical CPU,
> STLF occurs after the address of both the load and store are calculated
> and determined to match.
>
> PSF expands on this by speculating on the relationship between loads and
> stores without waiting for the address calculation to complete. With PSF,
> the CPU learns over time the relationship between loads and stores. If
> STLF typically occurs between a particular store and load, the CPU will
> remember this.
>
> In typical code, PSF provides a performance benefit by speculating on
> the load result and allowing later instructions to begin execution
> sooner than they otherwise would be able to.
>
> The details of security analysis of AMD predictive store forwarding is
> documented here.
> https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf
>
> Predictive Store Forwarding controls:
> There are two hardware control bits which influence the PSF feature:
> - MSR 48h bit 2 – Speculative Store Bypass (SSBD)
> - MSR 48h bit 7 – Predictive Store Forwarding Disable (PSFD)
>
> The PSF feature is disabled if either of these bits are set.  These bits
> are controllable on a per-thread basis in an SMT system. By default, both
> SSBD and PSFD are 0 meaning that the speculation features are enabled.
>
> While the SSBD bit disables PSF and speculative store bypass, PSFD only
> disables PSF.
>
> PSFD may be desirable for software which is concerned with the
> speculative behavior of PSF but desires a smaller performance impact than
> setting SSBD.
>
> Support for PSFD is indicated in CPUID Fn8000_0008 EBX[28].
> All processors that support PSF will also support PSFD.
>
> Linux kernel does not have the interface to enable/disable PSFD yet. Plan
> here is to expose the PSFD technology to KVM so that the guest kernel can
> make use of it if they wish to.
>
> Signed-off-by: Babu Moger <Babu.Moger@amd.com>
> ---
> NOTE: Per earlier discussions, Linux kernel interface for PSF mitigation
> is not included in this series. This series only exposes the PSFD technology
> to KVM guests. Here is the link for earlier discussion.
> https://lore.kernel.org/lkml/20210517220059.6452-1-rsaripal@amd.com/
> https://lore.kernel.org/lkml/20210505190923.276051-1-rsaripal@amd.com/
> https://lore.kernel.org/lkml/20210430131733.192414-1-rsaripal@amd.com/
> https://lore.kernel.org/lkml/20210428160349.158774-1-rsaripal@amd.com/
> https://lore.kernel.org/lkml/20210422171013.50207-1-rsaripal@amd.com/
> https://lore.kernel.org/lkml/20210421090117.22315-1-rsaripal@amd.com/
>
>  arch/x86/include/asm/cpufeatures.h |    1 +
>  arch/x86/kvm/cpuid.c               |    2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index d0ce5cfd3ac1..7d6268ede35a 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -313,6 +313,7 @@
>  #define X86_FEATURE_AMD_SSBD           (13*32+24) /* "" Speculative Store Bypass Disable */
>  #define X86_FEATURE_VIRT_SSBD          (13*32+25) /* Virtualized Speculative Store Bypass Disable */
>  #define X86_FEATURE_AMD_SSB_NO         (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */
> +#define X86_FEATURE_PSFD               (13*32+28) /* Predictive Store Forwarding Disable */
>
>  /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
>  #define X86_FEATURE_DTHERM             (14*32+ 0) /* Digital Thermal Sensor */
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index fe03bd978761..ba773919f21d 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -500,7 +500,7 @@ 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)
> +               F(AMD_SSB_NO) | F(AMD_STIBP) | F(AMD_STIBP_ALWAYS_ON) | F(PSFD)
>         );
>
>         /*
>
>

For consistency, should this feature be renamed AMD_PSFD, now that
Intel is enumerating PSFD with CPUID.(EAX=7,ECX=2):EDX.PSFD[bit 0]?
See https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/cpuid-enumeration-and-architectural-msrs.html.

And, Paolo, why are we carrying X86_FEATURE_PSFD as a private #define
in KVM rather than putting it where it belongs in cpufeatures.h?