On 3/14/24 09:29, Borislav Petkov wrote:
>
>> That argument breaks down a bit on the flags though:
>>
>> xc.xfeat_flags = xstate_flags[i];
>>
>> Because it comes _directly_ from CPUID with zero filtering:
>>
>> cpuid_count(XSTATE_CPUID, i, &eax, &ebx, &ecx, &edx);
>> ...
>> xstate_flags[i] = ecx;
>>
>> So this layout is quite dependent on what's in x86's CPUID.
> Yeah, no, this should not be copying CPUID flags - those flags should be
> *translated* to independently defined flags which describe those
> buffers.
Ditto for:
xc.xfeat_type = i;
Right now, that's bound to CPUID and XSAVE. "feat_type==10" can only
ever be PKRU and that's derived from the XSAVE architecture.
If you want this to be extensible to things outside of the XSAVE
architecture, it needs to be something actually extensible and not
entangled with XSAVE.
In other words "xc.xfeat_type" can enumerate XSAVE state components
being in the dump, but it should not be limited to XSAVE. Just as an
example:
enum feat_type {
FEATURE_XSAVE_PKRU,
FEATURE_XSAVE__YMM,
FEATURE_XSAVE_BNDREGS,
FEATURE_XSAVE_BNDCSR,
...
RANDOM_STATE_NOT_XSAVE
};
See how feat_type==1 is PKRU and *NOT* feat_type==10? That opens the
door to RANDOM_STATE_NOT_XSAVE or anything else you want. This would be
_actually_ extensible.
