On Thu, Nov 21, 2019 at 01:50:17PM -0800, Sean Christopherson wrote:
I actually don't want to use cpu_has() for the VMX features, which is why I put these in a separate array (one of the future patches). The motivation is purely for /proc/cpuinfo. Currently there is no sane way for a user to query the capabilities of their platform. The issue comes up on a fairly regular basis, e.g. when trying to find a platform to test a new feature or to debug an issue that has a hardware dependency.
Lack of reporting is especially annoying when the end user isn't familiar with VMX, e.g. the format of the MSRs is non-standard, existence of some MSRs is reported by bits in other MSRs, several features from KVM's point of view are actually enumerated as 3+ separate features by hardware, etc...
Punting to a userspace utility isn't a viable option because all of the capabilities are reported via MSRs.
Ok, this justification was missing in the commit message, please add the gist of it so that it is clear why we're doing it. And yes, I agree that having a single concentrated place for feature bits which you otherwise have to painstakingly extract from a bunch of MSRs make sense.
As for why I want to keep these out of cpu_has()... VMX has a concept of features being fixed "on", e.g. early CPUs don't allow disabling off CR3 interception. A cpu_has() approach doesn't work well since it loses the information regarding which bits are fixed-1. KVM also has several module params that can be used to disable use of features, i.e. we don't want cpu_has() for VMX features because the KVM-specific variables need to be the canonical reference.
Well, you can use the cpu_has() machinery for stuff like that too - we can clear bits there too: clear_cpu_cap() - and since clearing those bits are only for /proc/cpuinfo reporting, it's not like anything would break if that flag is gone. Just saying, in case you want to use the machinery for that.
And that would avoid some of the duplication of having KVM-specific variables *and* VMX_FEATURE_* flags, where latter are not really toggleable but only for /proc/cpuinfo. Especially if you wanna enforce "developers to define a VMX_FEATURE flag when adding support for a new hardware feature."
Alternatively, what about adding "vmx flags" but keeping the existing synthetic flags? That'd mean having duplicates for tpr_shadow, vnmi, ept, flexpriority, vpi and ept_ad. On the plus side, we'd cap the pollution of "flags" at those six features.
Yap, this is how you avoid ABI breakage. Makes sense to me.
Thx.