[PATCH] target/i386/whpx: whpx_get_xsave_state error handling

Doug Cook (WINDOWS) posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/LVXPR21MB70090CD62E78CC3BA2EB7E52ADA42@LVXPR21MB7009.namprd21.prod.outlook.com
Maintainers: Pedro Barbuda <pbarbuda@microsoft.com>, Mohamed Mediouni <mohamed@unpredictable.fr>
include/system/whpx-internal.h |  5 ++-
target/i386/whpx/whpx-all.c    | 73 +++++++++++++++++++++++++---------
2 files changed, 57 insertions(+), 21 deletions(-)
[PATCH] target/i386/whpx: whpx_get_xsave_state error handling
Posted by Doug Cook (WINDOWS) 1 month, 1 week ago
Several problems with error handling in whpx_get_xsave_state:

- Does not handle WHV_E_INSUFFICIENT_BUFFER, which occurs frequently
  in practice for XSAVE state.
- Does not free xsavec_buf when an error occurs.
- Error message and return code use errno where they should use hr.

Fixes: cfaa3b6c9597 ("whpx: xsave support")

Signed-off-by: Doug Cook <dcook@microsoft.com>
---
 include/system/whpx-internal.h |  5 ++-
 target/i386/whpx/whpx-all.c    | 73 +++++++++++++++++++++++++---------
 2 files changed, 57 insertions(+), 21 deletions(-)

diff --git a/include/system/whpx-internal.h b/include/system/whpx-internal.h
index c295c5a529..b15f3d8faf 100644
--- a/include/system/whpx-internal.h
+++ b/include/system/whpx-internal.h
@@ -58,8 +58,9 @@ void whpx_apic_get(APICCommonState *s);
 
 #define WHV_E_UNKNOWN_CAPABILITY 0x80370300L
 
-/* This should eventually come from the Windows SDK */
-#define WHV_E_UNKNOWN_PROPERTY 0x80370302
+/* These should eventually come from the Windows SDK */
+#define WHV_E_INSUFFICIENT_BUFFER 0x80370301L
+#define WHV_E_UNKNOWN_PROPERTY 0x80370302L
 
 #define LIST_WINHVPLATFORM_FUNCTIONS(X) \
   X(HRESULT, WHvGetCapability, (WHV_CAPABILITY_CODE CapabilityCode, VOID* CapabilityBuffer, UINT32 CapabilityBufferSizeInBytes, UINT32* WrittenSizeInBytes)) \
diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index 634d542821..682c85cef1 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -442,8 +442,8 @@ static int whpx_set_xsave_state(const CPUState *cpu)
 
     qemu_vfree(xsavec_buf);
     if (FAILED(hr)) {
-        error_report("WHPX: Failed to get virtual processor context, hr=%08lx",
-                     hr);
+        error_report("WHPX: Failed to set xsave state, hr=%08lx", hr);
+        return -EIO;
     }
 
     return 0;
@@ -810,9 +810,28 @@ static void whpx_get_legacy_fp_registers(CPUState *cpu, WHPXStateLevel level)
     idx += 1;
 }
 
-static int whpx_get_xsave_state(CPUState *cpu)
+static HRESULT whpx_get_xsave_state_buffer(const CPUState *cpu, void *buf,
+                                           size_t buf_len,
+                                           UINT32 *bytes_written)
 {
     struct whpx_state *whpx = &whpx_global;
+
+    if (!whpx_is_legacy_os()) {
+        return whp_dispatch.WHvGetVirtualProcessorState(
+            whpx->partition, cpu->cpu_index,
+            WHvVirtualProcessorStateTypeXsaveState,
+            buf,
+            buf_len, bytes_written);
+    } else {
+        return whp_dispatch.WHvGetVirtualProcessorXsaveState(
+            whpx->partition, cpu->cpu_index,
+            buf,
+            buf_len, bytes_written);
+    }
+}
+
+static int whpx_get_xsave_state(CPUState *cpu)
+{
     X86CPU *x86cpu = X86_CPU(cpu);
     CPUX86State *env = &x86cpu->env;
     int ret;
@@ -825,27 +844,43 @@ static int whpx_get_xsave_state(CPUState *cpu)
     xsavec_buf = qemu_memalign(page, xsavec_buf_len);
     memset(xsavec_buf, 0, xsavec_buf_len);
 
-    if (!whpx_is_legacy_os()) {
-        hr = whp_dispatch.WHvGetVirtualProcessorState(
-            whpx->partition, cpu->cpu_index,
-            WHvVirtualProcessorStateTypeXsaveState,
-            xsavec_buf,
-            xsavec_buf_len, &bytes_written);
-    } else {
-        hr = whp_dispatch.WHvGetVirtualProcessorXsaveState(
-            whpx->partition, cpu->cpu_index,
-            xsavec_buf,
-            xsavec_buf_len, &bytes_written);
+    bytes_written = 0;
+    hr = whpx_get_xsave_state_buffer(cpu, xsavec_buf, xsavec_buf_len,
+                                     &bytes_written);
+
+    /*
+     * whpx_get_xsave_max_len() returns the size of user state components
+     * enabled in XCR0. The hypervisor returns an XSAVES image, which also
+     * contains the supervisor state, so the whpx_get_xsave_max_len() size
+     * may be too small.
+     */
+    if (hr == WHV_E_INSUFFICIENT_BUFFER && bytes_written > xsavec_buf_len) {
+        qemu_vfree(xsavec_buf);
+        xsavec_buf_len = bytes_written;
+        xsavec_buf = qemu_memalign(page, xsavec_buf_len);
+        memset(xsavec_buf, 0, xsavec_buf_len);
+
+        bytes_written = 0;
+        hr = whpx_get_xsave_state_buffer(cpu, xsavec_buf, xsavec_buf_len,
+                                         &bytes_written);
     }
-    if (FAILED(hr) || bytes_written == 0) {
-        error_report("failed to get xsave state: %s", strerror(errno));
-        return -errno;
+
+    if (FAILED(hr)) {
+        error_report("WHPX: Failed to get xsave state, hr=%08lx", hr);
+        qemu_vfree(xsavec_buf);
+        return -EIO;
+    }
+
+    if (bytes_written == 0) {
+        error_report("WHPX: Failed to get xsave state, no data returned");
+        qemu_vfree(xsavec_buf);
+        return -EIO;
     }
 
-    ret = decompact_xsave_area(xsavec_buf, xsavec_buf_len, env);
+    ret = decompact_xsave_area(xsavec_buf, bytes_written, env);
     qemu_vfree(xsavec_buf);
     if (ret < 0) {
-        error_report("failed to decompact xsave area");
+        error_report("WHPX: Failed to decompact xsave area");
         return ret;
     }
     x86_cpu_xrstor_all_areas(x86cpu, env->xsave_buf, env->xsave_buf_len);
-- 
2.55.0.vfs.0.3
Re: [PATCH] target/i386/whpx: whpx_get_xsave_state error handling
Posted by Mohamed Mediouni 1 month, 1 week ago

> On 20. Aug 2026, at 02:56, Doug Cook (WINDOWS) <dcook@microsoft.com> wrote:
> 
> Several problems with error handling in whpx_get_xsave_state:
> 
> - Does not handle WHV_E_INSUFFICIENT_BUFFER, which occurs frequently
>  in practice for XSAVE state.
> - Does not free xsavec_buf when an error occurs.
> - Error message and return code use errno where they should use hr.
> 
> Fixes: cfaa3b6c9597 ("whpx: xsave support")
> 
> Signed-off-by: Doug Cook <dcook@microsoft.com>

Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr>

> ---
> include/system/whpx-internal.h |  5 ++-
> target/i386/whpx/whpx-all.c    | 73 +++++++++++++++++++++++++---------
> 2 files changed, 57 insertions(+), 21 deletions(-)
> 
> diff --git a/include/system/whpx-internal.h b/include/system/whpx-internal.h
> index c295c5a529..b15f3d8faf 100644
> --- a/include/system/whpx-internal.h
> +++ b/include/system/whpx-internal.h
> @@ -58,8 +58,9 @@ void whpx_apic_get(APICCommonState *s);
> 
> #define WHV_E_UNKNOWN_CAPABILITY 0x80370300L
> 
> -/* This should eventually come from the Windows SDK */
> -#define WHV_E_UNKNOWN_PROPERTY 0x80370302
> +/* These should eventually come from the Windows SDK */
> +#define WHV_E_INSUFFICIENT_BUFFER 0x80370301L
> +#define WHV_E_UNKNOWN_PROPERTY 0x80370302L
> 
> #define LIST_WINHVPLATFORM_FUNCTIONS(X) \
>   X(HRESULT, WHvGetCapability, (WHV_CAPABILITY_CODE CapabilityCode, VOID* CapabilityBuffer, UINT32 CapabilityBufferSizeInBytes, UINT32* WrittenSizeInBytes)) \
> diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
> index 634d542821..682c85cef1 100644
> --- a/target/i386/whpx/whpx-all.c
> +++ b/target/i386/whpx/whpx-all.c
> @@ -442,8 +442,8 @@ static int whpx_set_xsave_state(const CPUState *cpu)
> 
>     qemu_vfree(xsavec_buf);
>     if (FAILED(hr)) {
> -        error_report("WHPX: Failed to get virtual processor context, hr=%08lx",
> -                     hr);
> +        error_report("WHPX: Failed to set xsave state, hr=%08lx", hr);
> +        return -EIO;
>     }
> 
>     return 0;
> @@ -810,9 +810,28 @@ static void whpx_get_legacy_fp_registers(CPUState *cpu, WHPXStateLevel level)
>     idx += 1;
> }
> 
> -static int whpx_get_xsave_state(CPUState *cpu)
> +static HRESULT whpx_get_xsave_state_buffer(const CPUState *cpu, void *buf,
> +                                           size_t buf_len,
> +                                           UINT32 *bytes_written)
> {
>     struct whpx_state *whpx = &whpx_global;
> +
> +    if (!whpx_is_legacy_os()) {
> +        return whp_dispatch.WHvGetVirtualProcessorState(
> +            whpx->partition, cpu->cpu_index,
> +            WHvVirtualProcessorStateTypeXsaveState,
> +            buf,
> +            buf_len, bytes_written);
> +    } else {
> +        return whp_dispatch.WHvGetVirtualProcessorXsaveState(
> +            whpx->partition, cpu->cpu_index,
> +            buf,
> +            buf_len, bytes_written);
> +    }
> +}
> +
> +static int whpx_get_xsave_state(CPUState *cpu)
> +{
>     X86CPU *x86cpu = X86_CPU(cpu);
>     CPUX86State *env = &x86cpu->env;
>     int ret;
> @@ -825,27 +844,43 @@ static int whpx_get_xsave_state(CPUState *cpu)
>     xsavec_buf = qemu_memalign(page, xsavec_buf_len);
>     memset(xsavec_buf, 0, xsavec_buf_len);
> 
> -    if (!whpx_is_legacy_os()) {
> -        hr = whp_dispatch.WHvGetVirtualProcessorState(
> -            whpx->partition, cpu->cpu_index,
> -            WHvVirtualProcessorStateTypeXsaveState,
> -            xsavec_buf,
> -            xsavec_buf_len, &bytes_written);
> -    } else {
> -        hr = whp_dispatch.WHvGetVirtualProcessorXsaveState(
> -            whpx->partition, cpu->cpu_index,
> -            xsavec_buf,
> -            xsavec_buf_len, &bytes_written);
> +    bytes_written = 0;
> +    hr = whpx_get_xsave_state_buffer(cpu, xsavec_buf, xsavec_buf_len,
> +                                     &bytes_written);
> +
> +    /*
> +     * whpx_get_xsave_max_len() returns the size of user state components
> +     * enabled in XCR0. The hypervisor returns an XSAVES image, which also
> +     * contains the supervisor state, so the whpx_get_xsave_max_len() size
> +     * may be too small.
> +     */
> +    if (hr == WHV_E_INSUFFICIENT_BUFFER && bytes_written > xsavec_buf_len) {
> +        qemu_vfree(xsavec_buf);
> +        xsavec_buf_len = bytes_written;
> +        xsavec_buf = qemu_memalign(page, xsavec_buf_len);
> +        memset(xsavec_buf, 0, xsavec_buf_len);
> +
> +        bytes_written = 0;
> +        hr = whpx_get_xsave_state_buffer(cpu, xsavec_buf, xsavec_buf_len,
> +                                         &bytes_written);
>     }
> -    if (FAILED(hr) || bytes_written == 0) {
> -        error_report("failed to get xsave state: %s", strerror(errno));
> -        return -errno;
> +
> +    if (FAILED(hr)) {
> +        error_report("WHPX: Failed to get xsave state, hr=%08lx", hr);
> +        qemu_vfree(xsavec_buf);
> +        return -EIO;
> +    }
> +
> +    if (bytes_written == 0) {
> +        error_report("WHPX: Failed to get xsave state, no data returned");
> +        qemu_vfree(xsavec_buf);
> +        return -EIO;
>     }
> 
> -    ret = decompact_xsave_area(xsavec_buf, xsavec_buf_len, env);
> +    ret = decompact_xsave_area(xsavec_buf, bytes_written, env);
>     qemu_vfree(xsavec_buf);
>     if (ret < 0) {
> -        error_report("failed to decompact xsave area");
> +        error_report("WHPX: Failed to decompact xsave area");
>         return ret;
>     }
>     x86_cpu_xrstor_all_areas(x86cpu, env->xsave_buf, env->xsave_buf_len);
> -- 
> 2.55.0.vfs.0.3
>