Re: [PATCH] x86/cpuid: Clobber CPUID leaves 0x800000{1d..20}

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.04.2022 03:01, Andrew Cooper wrote:
> c/s 1a914256dca5 increased the AMD max leaf from 0x8000001c to 0x80000021, but
> did not adjust anything in the calculate_*_policy() chain.  As a result, on
> hardware supporting these leaves, we read the real hardware values into the
> raw policy, then copy into host, and all the way into the PV/HVM default
> policies.
> 
> All 4 of these leaves have enable bits (first two by TopoExt, next by SEV,
> next by PQOS), so any software following the rules is fine and will leave them
> alone.  However, leaf 0x8000001d takes a subleaf input and at least two
> userspace utilities have been observed to loop indefinitely under Xen (clearly
> waiting for eax to report "no more cache levels").
> 
> Such userspace is buggy, but Xen's behaviour isn't great either.
> 
> In the short term, clobber all information in these leaves.  This is a giant
> bodge, but there are complexities with implementing all of these leaves
> properly.
> 
> Fixes: 1a914256dca5 ("x86/cpuid: support LFENCE always serialising CPUID bit")
> Link: https://github.com/QubesOS/qubes-issues/issues/7392
> Reported-by: fosslinux <fosslinux@aussies.space>
> Reported-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> It turns out that Intel leaf 4 and AMD leaf 0x8000001d are *almost* identical.
> They differ by the "complex" bit in edx, and the $X-per-cache fields in the
> top of eax (Intel is threads-per-cache, AMD is cores-per-cache and lacks the
> cores-per-package field).
> 
> As neither vendor implement each others version, I'm incredibly tempted to
> reuse p->cache for both, rather than doubling the storage space.  Reading the
> data out is easy to key on p->extd.topoext.  Writing the data can be done
> without any further complexity if we simply trust the sending side to have its
> indices the proper way around.  Particularly, this avoids needing to ensure
> that p->extd.topoext is out of order and at the head of the stream.  Thoughts?

Sounds quite reasonable to me. I guess the main risk is for new things
to appear on either vendor's side in a way breaking the overlaying
approach. But I guess that's not a significant risk.

As to ordering dependencies: Are there any in reality? Neither vendor
implements the other vendor's leaf, so there's only going to be one in
the stream anyway, and which one it is can be disambiguated by having
seen leaf 0 alone.

Jan