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

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

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
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.

>
>> --- a/xen/tools/gen-cpuid.py
>> +++ b/xen/tools/gen-cpuid.py
>> @@ -297,6 +297,19 @@ def crunch_numbers(state):
>>          RTM: [TSXLDTRK],
>>      }
>>  
>> +    #
>> +    # Pseudo feature names.  These don't map to a feature bit, but are
>> +    # inserted into the values dictionary so they can be parsed and handled
>> +    # specially
>> +    #
>> +    pseduo_names = (
>> +    )
>> +
>> +    for n in pseduo_names:
>> +        if n in state.values:
>> +            raise Fail("Pseduo feature name %s aliases real feature" % (n, 
>> ))
>> +        state.values[n] = 0xffffffff
> Throughout this hunk: s/pseduo/pseudo/g.

Oops, yes.  Will fix.

~Andrew