RE: [PATCH] whpx: Added support for saving/restoring VM state
Hi Paolo, Thanks for pointing out the relevant fields of the XSAVE state - it explains what is going on. I did some quick experiments with modifying the buffer manually, so: * Hyper-V doesn't care about the value of MXCSR_MASK. Both 0x and 0x work. * Setting XCOMP_BV to 0 does trigger the error. My best guess is that Hyper-V is emulating XSAVE/XRSTOR programmatically and they only implemented the compacted format. Do you think it would be a reasonable workaround to handle it like this: 1. Pass the XSAVE data received from Hyper-V to x86_cpu_xrstor_all_areas(). 2. Also save it into the snapshot area like we do now. 3. When restoring, first try to pass the data from x86_cpu_xsave_all_areas() to Hyper-V. 4. If it rejects it, pass the original block saved in step 2. If either Microsoft adds support for the regular format, or someone on the Qemu side implements the compacted one, this logic will start properly synchronizing the QEMU-level XSAVE structure with Hyper-V. Until then (or if it breaks again) it will fall back to saving/restoring the data "as is", which will be sufficient for snapshots. Best, Ivan -Original Message- From: Qemu-devel On Behalf Of Paolo Bonzini Sent: Tuesday, May 17, 2022 7:12 AM To: Ivan Shcherbakov ; qemu-devel@nongnu.org Subject: Re: [PATCH] whpx: Added support for saving/restoring VM state On 5/16/22 20:44, Ivan Shcherbakov wrote: > Passing it to x86_cpu_xrstor_all_areas()/x86_cpu_xsave_all_areas() changed > the following values: > > 0x001C: ff ff -> 00 00 > 0x0208: 07 -> 00 > 0x020F: 80 -> 00 0x1C-0x1F is MXCSR_MASK. There's already a field in the x86 CPUState, but it was forgotten in x86_cpu_xsave_all_areas()/x86_cpu_xrstor_all_areas(). The field should also be initialized to 0x in the CPU reset function. 0x208...0x20F is XCOMP_BV and bit 63 in there is indeed signaling compacted format. First of all I'd start with your patch and hack it to check if Hyper-V accepts zero at 0x208..0x20F; in this specific case of 0x208...0x20F have all low consecutive bits set plus bit 63 set, it's fine to do just that. If so, x86_cpu_xrstor_all_areas() needs no support for compacted format. I would be somewhat surprised if Hyper-V needs support in XRSTOR too. For XSAVE, the algorithm to compute the offset (instead of just using x->offset) is given in the Intel manual: If XCOMP_BV[i] = 0, state component i is not in the XSAVE area at all. If XCOMP_BV[i] = 1, state component i is located at a byte offset from the base address of the XSAVE area, which is determined by the following steps: - If i is the first bit set in bits 62:2 of the XCOMP_BV, state component i starts at offset 576 - Otherwise, take CPUID[EAX=0DH,ECX=i].ECX[1]: - If it is 0, state component i starts right after the preceding state component whose bit is set in XCOMP_BV (where the size of component j is enumerated in CPUID[EAX=0DH,ECX=j].EAX). - If it is 1, state component i starts after the preceding state component whose bit is set in XCOMP_BV, but on a 64-byte aligned offset relative to the beginning of the XSAVE area. Paolo
Re: [PATCH] whpx: Added support for saving/restoring VM state
On 5/16/22 20:44, Ivan Shcherbakov wrote: Passing it to x86_cpu_xrstor_all_areas()/x86_cpu_xsave_all_areas() changed the following values: 0x001C: ff ff -> 00 00 0x0208: 07 -> 00 0x020F: 80 -> 00 0x1C-0x1F is MXCSR_MASK. There's already a field in the x86 CPUState, but it was forgotten in x86_cpu_xsave_all_areas()/x86_cpu_xrstor_all_areas(). The field should also be initialized to 0x in the CPU reset function. 0x208...0x20F is XCOMP_BV and bit 63 in there is indeed signaling compacted format. First of all I'd start with your patch and hack it to check if Hyper-V accepts zero at 0x208..0x20F; in this specific case of 0x208...0x20F have all low consecutive bits set plus bit 63 set, it's fine to do just that. If so, x86_cpu_xrstor_all_areas() needs no support for compacted format. I would be somewhat surprised if Hyper-V needs support in XRSTOR too. For XSAVE, the algorithm to compute the offset (instead of just using x->offset) is given in the Intel manual: If XCOMP_BV[i] = 0, state component i is not in the XSAVE area at all. If XCOMP_BV[i] = 1, state component i is located at a byte offset from the base address of the XSAVE area, which is determined by the following steps: - If i is the first bit set in bits 62:2 of the XCOMP_BV, state component i starts at offset 576 - Otherwise, take CPUID[EAX=0DH,ECX=i].ECX[1]: - If it is 0, state component i starts right after the preceding state component whose bit is set in XCOMP_BV (where the size of component j is enumerated in CPUID[EAX=0DH,ECX=j].EAX). - If it is 1, state component i starts after the preceding state component whose bit is set in XCOMP_BV, but on a 64-byte aligned offset relative to the beginning of the XSAVE area. Paolo
RE: [PATCH] whpx: Added support for saving/restoring VM state
Hi Paolo,
>What are the differences? Is it using the XSAVEC/XSAVES ("compacted") format?
I am not very familiar with the format internals, so I briefly checked whether
I could reuse the general logic from the HVF port. Here's what I got on a
booted Linux VM:
WHvGetVirtualProcessorXsaveState() returned this block (844 bytes):
7f 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 a0 1f
00 00 ff ff 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 58 65 c8 34 7f 00 00 04 7f 65 c8 34 7f 00 00 44 e1 68 c8 34 7f
00 00 f8 35 65 c8 34 7f 00 00 ec 4f 65 c8 34 7f 00 00 5c 4f 6a c8 34 7f 00 00
d0 7d 65 c8 34 7f 00 00 3c 6a 67 c8 34 7f 00 00 64 90 67 c8 34 7f 00 00 9c 0c
78 c8 34 7f 00 00 3c 6a 67 c8 34 7f 00 00 40 60 65 c8 34 7f 00 00 5c 4f 6a c8
34 7f 00 00 d0 7d 65 c8 34 7f 00 00 f8 35 65 c8 34 7f 00 00 ec 4f 65 c8 34 7f
00 00 70 3d 66 cb 34 7f 00 00 00 00 00 00 00 00 00 00 69 00 65 00 6f 00 20 00
00 00 00 00 00 00 00 00 ff 00 ff 00 ff 00 ff 00 ff 00 ff 00 ff 00 ff 00 ff 00
00 00 ff ff 00 00 00 00 00 00 00 00 ff ff 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 02 00 00 00 0c 00 00 00 05 00 00 00 22 00 00 00 05 00 00 00 10 00
00 00 05 00 00 00 22 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 03 00 00 00 00 00 00 00
07 00 00 00 00 00 00 80 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Passing it to x86_cpu_xrstor_all_areas()/x86_cpu_xsave_all_areas() changed the
following values:
0x001C: ff ff -> 00 00
0x0208: 07 -> 00
0x020F: 80 -> 00
Trying to pass the updated block to WHvSetVirtualProcessorXsaveState() just
made it reject the entire block with error c0350005 (generic "invalid
parameter").
I haven't looked too deep into it, since just saving the state "as is" is
sufficient to get the snapshots working. If there is something obvious I
missed, I can give it another brief try.
Best,
Ivan
Re: [PATCH] whpx: Added support for saving/restoring VM state
On 5/14/22 03:29, Ivan Shcherbakov wrote:
+/*
+ * As of Windows 10 21H1, the layout of the XSAVE data returned by the WHPX
API
+ * does not match the layout used by QEMU.
+ *
+ * Specifically, trying to pass the state returned by
x86_cpu_xsave_all_areas()
+ * to WHvSetVirtualProcessorXsaveState() causes it to return an error.
+ *
+ * As a result, we do not reflect the captured XSAVE state in the X86CPU
+ * structure, and instead manually save it to the snapshots via the
+ * whpx_xsave_() callbacks.
+ *
+ * Note that unlike the device drivers that can use the new
VMStateDescription
+ * mechanism via 'DeviceClass::vmsd' field, AccelClass objects cannot
easily
+ * do it. Hence, we rely on the legacy state management API.
+ */
+static SaveVMHandlers savevm_whpx = {
+.save_state = whpx_xsave_save,
+.load_state = whpx_xsave_load,
+};
+
What are the differences? Is it using the XSAVEC/XSAVES ("compacted")
format?
Paolo
