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

[Jan Beulich <jbeulich@xxxxxxxx>]

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

Jan