On 23/05/2024 5:09 pm, Jan Beulich wrote:
> On 23.05.2024 13:16, Andrew Cooper wrote:
>> @@ -611,6 +587,40 @@ static bool valid_xcr0(uint64_t xcr0)
>> return true;
>> }
>>
>> +unsigned int xstate_uncompressed_size(uint64_t xcr0)
>> +{
>> + unsigned int size = XSTATE_AREA_MIN_SIZE, i;
>> +
>> + ASSERT((xcr0 & ~X86_XCR0_STATES) == 0);
> I'm puzzled by the combination of this assertion and ...
>
>> + if ( xcr0 == xfeature_mask )
>> + return xsave_cntxt_size;
> ... this conditional return. Yes, right now we don't support/use any XSS
> components, but without any comment the assertion looks overly restrictive
> to me.
The ASSERT() is to cover a bug I found during testing.
It is a hard error to pass in non-XCR0 states. XSS states do not exist
in an uncompressed image, and have a base of 0, which ends up hitting a
later assertion.
This snippet with xfeature_mask is just code motion from
xstate_ctxt_size(), expressed as an addition because of diff. It was to
save a double XCR0 write in a perceived common case.
But, your AMX series makes both xfeature_mask and xsave_cntxt_size bogus
by there no longer being a uniform size of the save area, so I can
probably get away with simply dropping it here.
~Andrew
