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