Re: [PATCH 1/2] x86/cpuid: Infrastructure to support pseudo feature identifiers

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 05/10/2022 14:11, Jan Beulich wrote:
> On 05.10.2022 14:58, Andrew Cooper wrote:
>> On 05/10/2022 07:59, Jan Beulich wrote:
>>> On 04.10.2022 18:08, Andrew Cooper wrote:
>>>> A future change will want a cpuid-like identifier which doesn't have a 
>>>> mapping
>>>> to a feature bit.
>>>>
>>>>  * Pass the feature name into the parse callback.
>>>>  * Exclude a feature value of ~0u from falling into the general set/clear 
>>>> bit
>>>>    paths.
>>>>  * In gen-cpuid.py, insert a placeholder to collect all the pseudo feature
>>>>    names.
>>> Hmm, I was envisioning this to be fitted into the existing model in a
>>> less adhoc way: (parts of) MSRs holding feature bits aren't very different
>>> from individual (pairs of) registers of CPUID output (in the case of
>>> ARCH_CAPS there would be a perhaps just abstract mask limiting things to
>>> the subset of bits which actually act as feature indicators). Hence I'd
>>> have expected them to gain proper entries in the public interface
>>> (cpufeatureset.h) and then be represented / processed the same way in
>>> featuresets and policies. All that would be left out at this point would
>>> be the exposing of the bit to guests (in patch 2, assuming the split into
>>> two patches is then actually still warranted), integration into
>>> guest_rdmsr(), and at least some of the tool stack side (xen-cpuid, for
>>> example, could easily learn of such right away).
>>>
>>> However, since I'm pretty sure you've considered such an approach, I guess
>>> I might be overlooking some caveat?
>> I have on multiple occasions considered putting MSR_ARCH_CAPS into the
>> existing X86_FEATURE_* infrastructure.  In the future, it's likely the
>> right course of action to take.
>>
>> However, doing so now will break speculation safety for guests in some
> Considering further text - s/speculation/migration/, I guess?

More "speculation safety on migrate".

Except its not just on migrate.  It can go wrong for suspend/resume on
the same host across a reboot which changes the microcode version.

>> cases.  The featureset API intended to be safe to treat as a regular
>> bitmap, and this is how it is used in practice.
>>
>> Honestly, I didn't imagine that we'd get bits like RSBA and RRSBA that
>> need to be treated with opposite polarity to be levelled safely.
>>
>> Even if we do end up putting MSR_ARCH_CAPS in X86_FEATURE_*, we still
>> need to remove the featureset api (replaced by the cpu policy
>> infrastructure and libx86) to retain migration safety.
> Well, I didn't mean to suggest to put all of the feature-like bits there
> right away. Just the one bit we're after would do for now. Afaict that
> wouldn't affect migration safety, while it would allow doing things in
> Xen in a more "natural" way.

That requires reworking how the MSR policies get set up, patches for
which have been stagnent on xen-devel, as well as a reasonable amount of
plumbing because the featureset<->policy conversions which only have
CPUID data right now.

If I go down this route, realise it is going to have to go out in an XSA
so will be backported, and that you're also committing to taking the VMX
MSRs in due course.  (which is all on my plan, but I don't want to start
the work now and have an argument down the line.)

~Andrew