Re: [PATCH 2/2] x86/cpuid: support LFENCE always serializing CPUID bit

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 14/04/2021 12:04, Roger Pau Monne wrote:
> Milan AMD CPUs

I'd suggest "AMD Milan (Zen3) CPUs" for people who don't know the AMD
codenames inside/out.

>  have an LFENCE Always Serializing CPUID bit in leaf
> 80000021.eax.

Its probably worth noting that this is because of SEV-SNP, which is a
headline feature in Milan.

There must not be anything the VMM can do to impact the confidentiality
of an SEV-SNP VM, and breaking Spectre protections is definitely a
problem.  In a post-Spectre world, no system should be running without
LFENCE set to dispatch serialising.

>  Previous AMD versions used to have a user settable bit
> in DE_CFG MSR to select whether LFENCE was dispatch serializing, which
> Xen always attempts to set.
>
> In order to support this new CPUID bit move the LFENCE_DISPATCH
> synthetic CPUID bit to map the hardware bit (leaving a hole in the
> synthetic range) and either rely on the bit already being set by the
> native CPUID output, or attempt to fake it in Xen by modifying the
> DE_CFG MSR. This requires adding one more entry to the featureset to
> support leaf 80000021.eax.
>
> The bit is exposed to guests by default, as a way to signal that
> LFENCE is always serializing, either because it's mandated by
> hardware, or because Xen has performed the necessary arrangements.
> Note that Xen doesn't allow guests to change the DE_CFG value, so once
> set by Xen LFENCE will always be serializing.
>
> Note that the access to DE_CFG by guests is left as-is: reads will
> unconditionally return LFENCE_SERIALISE bit set, while writes are
> silently dropped. The MSR is not present on AMD Milan hardware

Not all MSRs are listed in the PPR.

This MSR does exist, and is my understanding that the bit still exists,
and is read-as-1/write-discard, so the past 3 years of software will
still conclude "lfence safe" when inspecting the MSR.

One of the speculation whitepapers says that this bit is going to remain
fixed in the future to simplify software needing to interact with it.

> , but
> I'm not sure there's any value in adding logic in Xen to also hide it
> from guests when running on such hardware.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  tools/misc/xen-cpuid.c                      |  6 ++++

Need to patch the translation table in libxl_cpuid.c too.  See c/s
23ccf530431561 for a reference.

>  xen/arch/x86/cpu/amd.c                      |  7 ++++
>  xen/arch/x86/cpu/common.c                   |  3 ++
>  xen/include/asm-x86/cpufeatures.h           |  2 +-
>  xen/include/public/arch-x86/cpufeatureset.h |  3 ++
>  xen/include/xen/lib/x86/cpuid.h             | 37 ++++++++++++++++++++-
>  6 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> index 2d04162d8d8..38406baadad 100644
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -178,6 +178,11 @@ static const char *const str_7a1[32] =
>      [ 4] = "avx-vnni",      [ 5] = "avx512-bf16",
>  };
>  
> +static const char *const str_e21a[32] =
> +{
> +    [ 2] = "lfence-always-serializing",

This is a bit of a mouthful.  One problem is the fact that "serialising"
is an ambiguous term, because neither Intel nor AMD formally specify
what it means in the architecture.

There is a description of what "architecturally serialising" does, which
does occasionally move over time, and the name of this CPUID bit in the
PPR confusing at best, because it very much isn't "architecturally
serialising", and the term "dispatch serialising" isn't actually defined
anywhere.

Intel have now started referring to LFENCE as a "speculative execution
barrier", but this still doesn't have a precise definition.

I'm afraid I don't have a useful suggestion for something short and
concise, which also conveys the precise meaning.

~Andrew