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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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 80000021.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 80000021.eax.
> > 
> > The bit is exposed to guests by default if the underlying hardware
> > supports leaf 80000021, 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 <andrew.cooper3@xxxxxxxxxx>
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> > ---
> > Note this doesn't yet expose the bit on hardware that doesn't support
> > leaf 80000021. 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 = 0x80000000 | min_t(uint32_t, p->extd.max_leaf & 0xffff,
-                                          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 = 0x80000000 | ARRAY_SIZE(p->extd.raw) - 1;
 
     cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p);
     recalculate_xstate(p);