Re: [PATCH v2 2/2] x86/cpuid: support LFENCE always serializing CPUID bit
On 15/04/2021 15:47, Roger Pau Monne wrote: > AMD Milan (Zen3) CPUs have an LFENCE Always Serializing CPUID bit in > leaf 8021.eax. 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. The forcefully always on setting is > due to the addition of SEV-SNP so that a VMM cannot break the > confidentiality of a guest. > > 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 8021.eax. > > The bit is exposed to guests by default if the underlying hardware > supports leaf 8021, as a way to signal that LFENCE is always > serializing. Hardware that doesn't have the leaf might also get the > bit set because Xen has performed the necessary arrangements, but > that's not yet exposed to guests. 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. > > Suggested-by: Andrew Cooper > Signed-off-by: Roger Pau Monné > --- > Changes since v1: > - Rename to lfence+. > - Add feature to libxl_cpuid.c. > - Reword commit message. > --- > Note this doesn't yet expose the bit on hardware that doesn't support > leaf 8021. It's still TBD whether we want to hardcode this support > manually, or instead rely on a more general approach like the one > suggested by the shrink max CPUID leaf patch from Jan. I'm going to insist on using the manual approach. Upping max leaf is strictly opposite to shrinking logic. It's very rare that we'll want to extend max leaf beyond what hardware supports, and it wants calling out clearly, along with identifying why it is safe to do so in this specific case. It is not safe or sensible to blindly escalate to the compile time max. The only cases where the differ are bugs needing fixing - the manual approach has the special case clearly called out, while the blindly escalate case has the bug hidden in derivation logic somewhere else. ~Andrew
Re: [PATCH v2 2/2] x86/cpuid: support LFENCE always serializing CPUID bit
On 20.04.2021 12:47, Roger Pau Monné wrote: > On Tue, Apr 20, 2021 at 12:35:54PM +0200, Jan Beulich wrote: >> I'd like to give Andrew a day or two more to respond there in case he >> continues to see an issue, before I commit that with your R-b and this >> one here. I'll assume you'll subsequently take care of that missing >> piece then - if not, i.e. if e.g. I should, please let me know. > > I think it should be something like the above, Right (assuming you meant "below). > in fact I think it > would be perfectly fine to merge that chunk into your patch? I'd rather not, so that this change can have its own reasoning in its description. Jan > diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c > index 050cd5713e2..daf501779fe 100644 > --- a/xen/arch/x86/cpuid.c > +++ b/xen/arch/x86/cpuid.c > @@ -314,12 +314,9 @@ static void __init calculate_host_policy(void) > > *p = raw_cpuid_policy; > > -p->basic.max_leaf = > -min_t(uint32_t, p->basic.max_leaf, ARRAY_SIZE(p->basic.raw) - 1); > -p->feat.max_subleaf = > -min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1); > -p->extd.max_leaf = 0x8000 | min_t(uint32_t, p->extd.max_leaf & > 0x, > - ARRAY_SIZE(p->extd.raw) - 1); > +p->basic.max_leaf = ARRAY_SIZE(p->basic.raw) - 1; > +p->feat.max_subleaf = ARRAY_SIZE(p->feat.raw) - 1; > +p->extd.max_leaf = 0x8000 | ARRAY_SIZE(p->extd.raw) - 1; > > cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p); > recalculate_xstate(p); >
Re: [PATCH v2 2/2] x86/cpuid: support LFENCE always serializing CPUID bit
On Tue, Apr 20, 2021 at 12:35:54PM +0200, Jan Beulich wrote: > On 15.04.2021 16:47, Roger Pau Monne wrote: > > AMD Milan (Zen3) CPUs have an LFENCE Always Serializing CPUID bit in > > leaf 8021.eax. 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. The forcefully always on setting is > > due to the addition of SEV-SNP so that a VMM cannot break the > > confidentiality of a guest. > > > > 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 8021.eax. > > > > The bit is exposed to guests by default if the underlying hardware > > supports leaf 8021, as a way to signal that LFENCE is always > > serializing. Hardware that doesn't have the leaf might also get the > > bit set because Xen has performed the necessary arrangements, but > > that's not yet exposed to guests. 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. > > > > Suggested-by: Andrew Cooper > > Signed-off-by: Roger Pau Monné > > Reviewed-by: Jan Beulich > > > --- > > Note this doesn't yet expose the bit on hardware that doesn't support > > leaf 8021. It's still TBD whether we want to hardcode this support > > manually, or instead rely on a more general approach like the one > > suggested by the shrink max CPUID leaf patch from Jan. > > I'd like to give Andrew a day or two more to respond there in case he > continues to see an issue, before I commit that with your R-b and this > one here. I'll assume you'll subsequently take care of that missing > piece then - if not, i.e. if e.g. I should, please let me know. I think it should be something like the above, in fact I think it would be perfectly fine to merge that chunk into your patch? Thanks, Roger. ---8<--- diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c index 050cd5713e2..daf501779fe 100644 --- a/xen/arch/x86/cpuid.c +++ b/xen/arch/x86/cpuid.c @@ -314,12 +314,9 @@ static void __init calculate_host_policy(void) *p = raw_cpuid_policy; -p->basic.max_leaf = -min_t(uint32_t, p->basic.max_leaf, ARRAY_SIZE(p->basic.raw) - 1); -p->feat.max_subleaf = -min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1); -p->extd.max_leaf = 0x8000 | min_t(uint32_t, p->extd.max_leaf & 0x, - ARRAY_SIZE(p->extd.raw) - 1); +p->basic.max_leaf = ARRAY_SIZE(p->basic.raw) - 1; +p->feat.max_subleaf = ARRAY_SIZE(p->feat.raw) - 1; +p->extd.max_leaf = 0x8000 | ARRAY_SIZE(p->extd.raw) - 1; cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p); recalculate_xstate(p);
Re: [PATCH v2 2/2] x86/cpuid: support LFENCE always serializing CPUID bit
On 15.04.2021 16:47, Roger Pau Monne wrote: > AMD Milan (Zen3) CPUs have an LFENCE Always Serializing CPUID bit in > leaf 8021.eax. 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. The forcefully always on setting is > due to the addition of SEV-SNP so that a VMM cannot break the > confidentiality of a guest. > > 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 8021.eax. > > The bit is exposed to guests by default if the underlying hardware > supports leaf 8021, as a way to signal that LFENCE is always > serializing. Hardware that doesn't have the leaf might also get the > bit set because Xen has performed the necessary arrangements, but > that's not yet exposed to guests. 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. > > Suggested-by: Andrew Cooper > Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich > --- > Note this doesn't yet expose the bit on hardware that doesn't support > leaf 8021. It's still TBD whether we want to hardcode this support > manually, or instead rely on a more general approach like the one > suggested by the shrink max CPUID leaf patch from Jan. I'd like to give Andrew a day or two more to respond there in case he continues to see an issue, before I commit that with your R-b and this one here. I'll assume you'll subsequently take care of that missing piece then - if not, i.e. if e.g. I should, please let me know. Jan
[PATCH v2 2/2] x86/cpuid: support LFENCE always serializing CPUID bit
AMD Milan (Zen3) CPUs have an LFENCE Always Serializing CPUID bit in
leaf 8021.eax. 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. The forcefully always on setting is
due to the addition of SEV-SNP so that a VMM cannot break the
confidentiality of a guest.
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 8021.eax.
The bit is exposed to guests by default if the underlying hardware
supports leaf 8021, as a way to signal that LFENCE is always
serializing. Hardware that doesn't have the leaf might also get the
bit set because Xen has performed the necessary arrangements, but
that's not yet exposed to guests. 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.
Suggested-by: Andrew Cooper
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- Rename to lfence+.
- Add feature to libxl_cpuid.c.
- Reword commit message.
---
Note this doesn't yet expose the bit on hardware that doesn't support
leaf 8021. It's still TBD whether we want to hardcode this support
manually, or instead rely on a more general approach like the one
suggested by the shrink max CPUID leaf patch from Jan.
---
tools/libs/light/libxl_cpuid.c | 2 ++
tools/misc/xen-cpuid.c | 6
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 -
7 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index aee28b0430d..d3ab66b9a71 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -284,6 +284,8 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list
*cpuid, const char* str)
{"svm_decode", 0x800a, NA, CPUID_REG_EDX, 7, 1},
{"svm_pausefilt",0x800a, NA, CPUID_REG_EDX, 10, 1},
+{"lfence+", 0x8021, NA, CPUID_REG_EAX, 2, 1},
+
{"maxhvleaf",0x4000, NA, CPUID_REG_EAX, 0, 8},
{NULL, 0, NA, CPUID_REG_INV, 0, 0}
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index 628e8f5aa25..9a47237f4a8 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -179,6 +179,11 @@ static const char *const str_7a1[32] =
[ 4] = "avx-vnni", [ 5] = "avx512-bf16",
};
+static const char *const str_e21a[32] =
+{
+[ 2] = "lfence+",
+};
+
static const struct {
const char *name;
const char *abbr;
@@ -196,6 +201,7 @@ static const struct {
{ "0x8008.ebx", "e8b", str_e8b },
{ "0x0007:0.edx", "7d0", str_7d0 },
{ "0x0007:1.eax", "7a1", str_7a1 },
+{ "0x8021.eax", "e21a", str_e21a },
};
#define COL_ALIGN "18"
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 9c8dcd91eef..35f22c24762 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -646,6 +646,13 @@ void amd_init_lfence(struct cpuinfo_x86 *c)
{
uint64_t value;
+ /*
+* Some hardware has LFENCE dispatch serializing always enabled,
+* nothing to do on that case.
+*/
+ if (test_bit(X86_FEATURE_LFENCE_DISPATCH, c->x86_capability))
+ return;
+
/*
* Attempt to set lfence to be Dispatch Serialising. This MSR almost
* certainly isn't virtualised (and Xen at least will leak the real
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index e5c3caf41d5..0eb364f8a65 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -412,6 +412,9 @@ static void generic_identify(struct cpuinfo_x86 *c)
if (c->extended_cpuid_level >= 0x8008)
c->x86_capability[cpufeat_word(X86_FEATURE_CLZERO)]
= cpuid_ebx(0x8008);
+ if (c->extended_cpuid_level >= 0x8021)
+ c->x86_capability[cpufeat_word(X86_FEATURE_LFENCE_DISPATCH)]
+ = cpuid_eax(0x8021);
/* Intel-defined flags: level 0x0007 */
if ( c->cpuid_level >= 0x0007 ) {
diff --git a/xen/include/asm-x86/cpufeatures.h
b/xen/include/asm-x86/cpufeatures.h
index d7e42d9bb6a..6c8f432aee4 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -24,7
