Re: [PATCH 4/7] x86/xstate: Rework xstate_ctxt_size() as xstate_uncompressed_size()

[Andrew Cooper <andrew.cooper3@citrix.com>]

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