[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86/cpuid: Clobber CPUID leaves 0x800000{1d..20}
On 07/04/2022 07:26, Jan Beulich wrote: > 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> Thanks. > >> 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. Neither of the vendors are going to change it in incompatible ways to how they currently expose it, and it's data that Xen doesn't particularly care about it - we never interpret it on behalf of the guest. When we fix the toolstack side of things to calculate topology properly, the $foo-per-cache fields need adjusting, but that logic will be fine to switch ( vendor ) on. Since writing this, I found AMD's cores-per-package and it's in the adjacent leaf with a wider field. > 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. The complexity is what (if anything) we do in x86_cpuid_copy_from_buffer(). I've done some prototyping, and the easiest option is to accept both 4 and e1Dd, in a latest-takes-precedent manner precedent, and that we don't create interlinkage with the topoext bit. I've also got a pile of fixes to the unit tests so we hopefully can't make mistakes like this again, although that will depend on getting test-cpuid-policy running in OSSTest which is a todo list item which really needs to get done. ~Andrew
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |