Re: [PATCH 3/5] x86/xstate: Rework xstate_ctxt_size() as xstate_uncompressed_size()

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

On 04/05/2021 13:08, Jan Beulich wrote:
> On 03.05.2021 20:17, Andrew Cooper wrote:
>> On 03/05/2021 16:39, Andrew Cooper wrote:
>>> We're soon going to need a compressed helper of the same form.
>>>
>>> The size of the uncompressed image is a strictly a property of the highest
>>> user state.  This can be calculated trivially with xstate_offsets/sizes, and
>>> is much faster than a CPUID instruction in the first place, let alone the two
>>> XCR0 writes surrounding it.
>>>
>>> Retain the cross-check with hardware in debug builds, but forgo it normal
>>> builds.  In particular, this means that the migration paths don't need to mess
>>> with XCR0 just to sanity check the buffer size.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> The Qemu smoketests have actually found a bug here.
>>
>> https://gitlab.com/xen-project/patchew/xen/-/jobs/1232118510/artifacts/file/smoke.serial
>>
>> We call into xstate_uncompressed_size() from
>> hvm_register_CPU_save_and_restore() so the previous "xcr0 == 0" path was
>> critical to Xen not exploding on non-xsave platforms.
>>
>> This is straight up buggy - we shouldn't be registering xsave handlers
>> on non-xsave platforms, but the calculation is also wrong (in the safe
>> directly at least) when we use compressed formats.  Yet another
>> unexpected surprise for the todo list.
> I don't view this as buggy at all - it was an implementation choice.
> Perhaps not the best one, but still correct afaict. Then again I'm
> afraid I don't understand "in the safe directly at least", so I may
> well be overlooking something. Will wait for your updated patch ...

For now, it is a patch 2.5/5 which just puts a cpu_has_xsave guard
around the registration.  Everything to do with xsave record processing
is unnecessary overhead on a non-xsave platform.

I don't intend to alter patch 3 as a consequence.

~Andrew