* [PATCH 1/5] include/hw/hyperv: move hv_vp_register_page struct definition
2026-07-07 22:16 [PATCH 0/5] Fix register page use for guests using mshv accel Doru Blânzeanu
@ 2026-07-07 22:16 ` Doru Blânzeanu
2026-07-08 16:36 ` Magnus Kulke
2026-07-07 22:16 ` [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails Doru Blânzeanu
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Doru Blânzeanu @ 2026-07-07 22:16 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Doru Blânzeanu, Magnus Kulke, Wei Liu,
Magnus Kulke, Doru Blânzeanu, Wei Liu
Move the `hv_vp_register_page` structure definition from `hvgdk_mini.h` to
`hvhdk.h`.
Fixes: 8afb40b7fe
Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
include/hw/hyperv/hvgdk.h | 2 +
include/hw/hyperv/hvgdk_mini.h | 103 --------------------------------
include/hw/hyperv/hvhdk.h | 106 +++++++++++++++++++++++++++++++++
3 files changed, 108 insertions(+), 103 deletions(-)
diff --git a/include/hw/hyperv/hvgdk.h b/include/hw/hyperv/hvgdk.h
index 71161f477c..e4be861716 100644
--- a/include/hw/hyperv/hvgdk.h
+++ b/include/hw/hyperv/hvgdk.h
@@ -9,6 +9,8 @@
#ifndef HW_HYPERV_HVGDK_H
#define HW_HYPERV_HVGDK_H
+#include "hvgdk_mini.h"
+
#define HVGDK_H_VERSION (25125)
enum hv_unimplemented_msr_action {
diff --git a/include/hw/hyperv/hvgdk_mini.h b/include/hw/hyperv/hvgdk_mini.h
index f8838a31bb..c6749a9525 100644
--- a/include/hw/hyperv/hvgdk_mini.h
+++ b/include/hw/hyperv/hvgdk_mini.h
@@ -504,109 +504,6 @@ struct hv_input_assert_virtual_interrupt {
uint16_t rsvd_z1;
} QEMU_PACKED;
-/* Flags for dirty mask of hv_vp_register_page */
-enum hv_x64_register_class_type {
- HV_X64_REGISTER_CLASS_GENERAL = 0,
- HV_X64_REGISTER_CLASS_IP = 1,
- HV_X64_REGISTER_CLASS_XMM = 2,
- HV_X64_REGISTER_CLASS_SEGMENT = 3,
- HV_X64_REGISTER_CLASS_FLAGS = 4,
-};
-
-#define HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT 7
-
-union hv_vp_register_page_interrupt_vectors {
- uint64_t as_uint64;
- struct {
- uint8_t vector_count;
- uint8_t vector[HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT];
- };
-};
-
-struct hv_vp_register_page {
- uint16_t version;
- uint8_t isvalid;
- uint8_t rsvdz;
- uint32_t dirty;
-
- union {
- struct {
- /* General purpose registers (HV_X64_REGISTER_CLASS_GENERAL) */
- union {
- struct {
- uint64_t rax;
- uint64_t rcx;
- uint64_t rdx;
- uint64_t rbx;
- uint64_t rsp;
- uint64_t rbp;
- uint64_t rsi;
- uint64_t rdi;
- uint64_t r8;
- uint64_t r9;
- uint64_t r10;
- uint64_t r11;
- uint64_t r12;
- uint64_t r13;
- uint64_t r14;
- uint64_t r15;
- } QEMU_PACKED;
-
- uint64_t gp_registers[16];
- };
- /* Instruction pointer (HV_X64_REGISTER_CLASS_IP) */
- uint64_t rip;
- /* Flags (HV_X64_REGISTER_CLASS_FLAGS) */
- uint64_t rflags;
- } QEMU_PACKED;
-
- uint64_t registers[18];
- };
- uint8_t reserved[8];
- /* Volatile XMM registers (HV_X64_REGISTER_CLASS_XMM) */
- union {
- struct {
- struct hv_u128 xmm0;
- struct hv_u128 xmm1;
- struct hv_u128 xmm2;
- struct hv_u128 xmm3;
- struct hv_u128 xmm4;
- struct hv_u128 xmm5;
- } QEMU_PACKED;
-
- struct hv_u128 xmm_registers[6];
- };
- /* Segment registers (HV_X64_REGISTER_CLASS_SEGMENT) */
- union {
- struct {
- struct hv_x64_segment_register es;
- struct hv_x64_segment_register cs;
- struct hv_x64_segment_register ss;
- struct hv_x64_segment_register ds;
- struct hv_x64_segment_register fs;
- struct hv_x64_segment_register gs;
- } QEMU_PACKED;
-
- struct hv_x64_segment_register segment_registers[6];
- };
- /* Misc. control registers (cannot be set via this interface) */
- uint64_t cr0;
- uint64_t cr3;
- uint64_t cr4;
- uint64_t cr8;
- uint64_t efer;
- uint64_t dr7;
- union hv_x64_pending_interruption_register pending_interruption;
- union hv_x64_interrupt_state_register interrupt_state;
- uint64_t instruction_emulation_hints;
- uint64_t xfem;
-
- uint8_t reserved1[0x100];
-
- /* Interrupts injected as part of HvCallDispatchVp. */
- union hv_vp_register_page_interrupt_vectors interrupt_vectors;
-} QEMU_PACKED;
-
/* /dev/mshv */
#define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct mshv_create_partition)
#define MSHV_CREATE_VP _IOW(MSHV_IOCTL, 0x01, struct mshv_create_vp)
diff --git a/include/hw/hyperv/hvhdk.h b/include/hw/hyperv/hvhdk.h
index 9e6dcb22f6..b9b76cf93c 100644
--- a/include/hw/hyperv/hvhdk.h
+++ b/include/hw/hyperv/hvhdk.h
@@ -9,7 +9,12 @@
#ifndef HW_HYPERV_HVHDK_H
#define HW_HYPERV_HVHDK_H
+#include "hvgdk.h"
+#include "hvhdk_mini.h"
+
#define HV_PARTITION_SYNTHETIC_PROCESSOR_FEATURES_BANKS 1
+#define HV_VP_REGISTER_PAGE_VERSION_1 1u
+#define HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT 7
struct hv_input_get_partition_property {
uint64_t partition_id;
@@ -441,4 +446,105 @@ typedef struct hv_input_register_intercept_result {
union hv_register_intercept_result_parameters parameters;
} QEMU_PACKED hv_input_register_intercept_result;
+/* Flags for dirty mask of hv_vp_register_page */
+enum hv_x64_register_class_type {
+ HV_X64_REGISTER_CLASS_GENERAL = 0,
+ HV_X64_REGISTER_CLASS_IP = 1,
+ HV_X64_REGISTER_CLASS_XMM = 2,
+ HV_X64_REGISTER_CLASS_SEGMENT = 3,
+ HV_X64_REGISTER_CLASS_FLAGS = 4,
+};
+
+union hv_vp_register_page_interrupt_vectors {
+ uint64_t as_uint64;
+ struct {
+ uint8_t vector_count;
+ uint8_t vector[HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT];
+ };
+};
+
+struct hv_vp_register_page {
+ uint16_t version;
+ uint8_t isvalid;
+ uint8_t rsvdz;
+ uint32_t dirty;
+
+ union {
+ struct {
+ /* General purpose registers (HV_X64_REGISTER_CLASS_GENERAL) */
+ union {
+ struct {
+ uint64_t rax;
+ uint64_t rcx;
+ uint64_t rdx;
+ uint64_t rbx;
+ uint64_t rsp;
+ uint64_t rbp;
+ uint64_t rsi;
+ uint64_t rdi;
+ uint64_t r8;
+ uint64_t r9;
+ uint64_t r10;
+ uint64_t r11;
+ uint64_t r12;
+ uint64_t r13;
+ uint64_t r14;
+ uint64_t r15;
+ } QEMU_PACKED;
+
+ uint64_t gp_registers[16];
+ };
+ /* Instruction pointer (HV_X64_REGISTER_CLASS_IP) */
+ uint64_t rip;
+ /* Flags (HV_X64_REGISTER_CLASS_FLAGS) */
+ uint64_t rflags;
+ } QEMU_PACKED;
+
+ uint64_t registers[18];
+ };
+ uint8_t reserved[8];
+ /* Volatile XMM registers (HV_X64_REGISTER_CLASS_XMM) */
+ union {
+ struct {
+ struct hv_u128 xmm0;
+ struct hv_u128 xmm1;
+ struct hv_u128 xmm2;
+ struct hv_u128 xmm3;
+ struct hv_u128 xmm4;
+ struct hv_u128 xmm5;
+ } QEMU_PACKED;
+
+ struct hv_u128 xmm_registers[6];
+ };
+ /* Segment registers (HV_X64_REGISTER_CLASS_SEGMENT) */
+ union {
+ struct {
+ struct hv_x64_segment_register es;
+ struct hv_x64_segment_register cs;
+ struct hv_x64_segment_register ss;
+ struct hv_x64_segment_register ds;
+ struct hv_x64_segment_register fs;
+ struct hv_x64_segment_register gs;
+ } QEMU_PACKED;
+
+ struct hv_x64_segment_register segment_registers[6];
+ };
+ /* Misc. control registers (cannot be set via this interface) */
+ uint64_t cr0;
+ uint64_t cr3;
+ uint64_t cr4;
+ uint64_t cr8;
+ uint64_t efer;
+ uint64_t dr7;
+ union hv_x64_pending_interruption_register pending_interruption;
+ union hv_x64_interrupt_state_register interrupt_state;
+ uint64_t instruction_emulation_hints;
+ uint64_t xfem;
+
+ uint8_t reserved1[0x100];
+
+ /* Interrupts injected as part of HvCallDispatchVp. */
+ union hv_vp_register_page_interrupt_vectors interrupt_vectors;
+} QEMU_PACKED;
+
#endif /* HW_HYPERV_HVHDK_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/5] include/hw/hyperv: move hv_vp_register_page struct definition
2026-07-07 22:16 ` [PATCH 1/5] include/hw/hyperv: move hv_vp_register_page struct definition Doru Blânzeanu
@ 2026-07-08 16:36 ` Magnus Kulke
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kulke @ 2026-07-08 16:36 UTC (permalink / raw)
To: Doru Blânzeanu
Cc: qemu-devel, Paolo Bonzini, Wei Liu, Magnus Kulke,
Doru Blânzeanu, Wei Liu
On Wed, Jul 08, 2026 at 01:16:41AM +0300, Doru Blânzeanu wrote:
> Move the `hv_vp_register_page` structure definition from `hvgdk_mini.h` to
> `hvhdk.h`.
>
> Fixes: 8afb40b7fe
> Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
> ---
> include/hw/hyperv/hvgdk.h | 2 +
> include/hw/hyperv/hvgdk_mini.h | 103 --------------------------------
> include/hw/hyperv/hvhdk.h | 106 +++++++++++++++++++++++++++++++++
> 3 files changed, 108 insertions(+), 103 deletions(-)
>
> diff --git a/include/hw/hyperv/hvgdk.h b/include/hw/hyperv/hvgdk.h
> index 71161f477c..e4be861716 100644
> --- a/include/hw/hyperv/hvgdk.h
> +++ b/include/hw/hyperv/hvgdk.h
> @@ -9,6 +9,8 @@
> #ifndef HW_HYPERV_HVGDK_H
> #define HW_HYPERV_HVGDK_H
>
> +#include "hvgdk_mini.h"
> +
> #define HVGDK_H_VERSION (25125)
>
> enum hv_unimplemented_msr_action {
> diff --git a/include/hw/hyperv/hvgdk_mini.h b/include/hw/hyperv/hvgdk_mini.h
> index f8838a31bb..c6749a9525 100644
> --- a/include/hw/hyperv/hvgdk_mini.h
> +++ b/include/hw/hyperv/hvgdk_mini.h
> @@ -504,109 +504,6 @@ struct hv_input_assert_virtual_interrupt {
> uint16_t rsvd_z1;
> } QEMU_PACKED;
>
> -/* Flags for dirty mask of hv_vp_register_page */
> -enum hv_x64_register_class_type {
> - HV_X64_REGISTER_CLASS_GENERAL = 0,
> - HV_X64_REGISTER_CLASS_IP = 1,
> - HV_X64_REGISTER_CLASS_XMM = 2,
> - HV_X64_REGISTER_CLASS_SEGMENT = 3,
> - HV_X64_REGISTER_CLASS_FLAGS = 4,
> -};
> -
> -#define HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT 7
> -
> -union hv_vp_register_page_interrupt_vectors {
> - uint64_t as_uint64;
> - struct {
> - uint8_t vector_count;
> - uint8_t vector[HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT];
> - };
> -};
> -
> -struct hv_vp_register_page {
> - uint16_t version;
> - uint8_t isvalid;
> - uint8_t rsvdz;
> - uint32_t dirty;
> -
> - union {
> - struct {
> - /* General purpose registers (HV_X64_REGISTER_CLASS_GENERAL) */
> - union {
> - struct {
> - uint64_t rax;
> - uint64_t rcx;
> - uint64_t rdx;
> - uint64_t rbx;
> - uint64_t rsp;
> - uint64_t rbp;
> - uint64_t rsi;
> - uint64_t rdi;
> - uint64_t r8;
> - uint64_t r9;
> - uint64_t r10;
> - uint64_t r11;
> - uint64_t r12;
> - uint64_t r13;
> - uint64_t r14;
> - uint64_t r15;
> - } QEMU_PACKED;
> -
> - uint64_t gp_registers[16];
> - };
> - /* Instruction pointer (HV_X64_REGISTER_CLASS_IP) */
> - uint64_t rip;
> - /* Flags (HV_X64_REGISTER_CLASS_FLAGS) */
> - uint64_t rflags;
> - } QEMU_PACKED;
> -
> - uint64_t registers[18];
> - };
> - uint8_t reserved[8];
> - /* Volatile XMM registers (HV_X64_REGISTER_CLASS_XMM) */
> - union {
> - struct {
> - struct hv_u128 xmm0;
> - struct hv_u128 xmm1;
> - struct hv_u128 xmm2;
> - struct hv_u128 xmm3;
> - struct hv_u128 xmm4;
> - struct hv_u128 xmm5;
> - } QEMU_PACKED;
> -
> - struct hv_u128 xmm_registers[6];
> - };
> - /* Segment registers (HV_X64_REGISTER_CLASS_SEGMENT) */
> - union {
> - struct {
> - struct hv_x64_segment_register es;
> - struct hv_x64_segment_register cs;
> - struct hv_x64_segment_register ss;
> - struct hv_x64_segment_register ds;
> - struct hv_x64_segment_register fs;
> - struct hv_x64_segment_register gs;
> - } QEMU_PACKED;
> -
> - struct hv_x64_segment_register segment_registers[6];
> - };
> - /* Misc. control registers (cannot be set via this interface) */
> - uint64_t cr0;
> - uint64_t cr3;
> - uint64_t cr4;
> - uint64_t cr8;
> - uint64_t efer;
> - uint64_t dr7;
> - union hv_x64_pending_interruption_register pending_interruption;
> - union hv_x64_interrupt_state_register interrupt_state;
> - uint64_t instruction_emulation_hints;
> - uint64_t xfem;
> -
> - uint8_t reserved1[0x100];
> -
> - /* Interrupts injected as part of HvCallDispatchVp. */
> - union hv_vp_register_page_interrupt_vectors interrupt_vectors;
> -} QEMU_PACKED;
> -
> /* /dev/mshv */
> #define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct mshv_create_partition)
> #define MSHV_CREATE_VP _IOW(MSHV_IOCTL, 0x01, struct mshv_create_vp)
> diff --git a/include/hw/hyperv/hvhdk.h b/include/hw/hyperv/hvhdk.h
> index 9e6dcb22f6..b9b76cf93c 100644
> --- a/include/hw/hyperv/hvhdk.h
> +++ b/include/hw/hyperv/hvhdk.h
> @@ -9,7 +9,12 @@
> #ifndef HW_HYPERV_HVHDK_H
> #define HW_HYPERV_HVHDK_H
>
> +#include "hvgdk.h"
> +#include "hvhdk_mini.h"
> +
> #define HV_PARTITION_SYNTHETIC_PROCESSOR_FEATURES_BANKS 1
> +#define HV_VP_REGISTER_PAGE_VERSION_1 1u
> +#define HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT 7
>
> struct hv_input_get_partition_property {
> uint64_t partition_id;
> @@ -441,4 +446,105 @@ typedef struct hv_input_register_intercept_result {
> union hv_register_intercept_result_parameters parameters;
> } QEMU_PACKED hv_input_register_intercept_result;
>
> +/* Flags for dirty mask of hv_vp_register_page */
> +enum hv_x64_register_class_type {
> + HV_X64_REGISTER_CLASS_GENERAL = 0,
> + HV_X64_REGISTER_CLASS_IP = 1,
> + HV_X64_REGISTER_CLASS_XMM = 2,
> + HV_X64_REGISTER_CLASS_SEGMENT = 3,
> + HV_X64_REGISTER_CLASS_FLAGS = 4,
> +};
> +
> +union hv_vp_register_page_interrupt_vectors {
> + uint64_t as_uint64;
> + struct {
> + uint8_t vector_count;
> + uint8_t vector[HV_VP_REGISTER_PAGE_MAX_VECTOR_COUNT];
> + };
> +};
> +
> +struct hv_vp_register_page {
> + uint16_t version;
> + uint8_t isvalid;
> + uint8_t rsvdz;
> + uint32_t dirty;
> +
> + union {
> + struct {
> + /* General purpose registers (HV_X64_REGISTER_CLASS_GENERAL) */
> + union {
> + struct {
> + uint64_t rax;
> + uint64_t rcx;
> + uint64_t rdx;
> + uint64_t rbx;
> + uint64_t rsp;
> + uint64_t rbp;
> + uint64_t rsi;
> + uint64_t rdi;
> + uint64_t r8;
> + uint64_t r9;
> + uint64_t r10;
> + uint64_t r11;
> + uint64_t r12;
> + uint64_t r13;
> + uint64_t r14;
> + uint64_t r15;
> + } QEMU_PACKED;
> +
> + uint64_t gp_registers[16];
> + };
> + /* Instruction pointer (HV_X64_REGISTER_CLASS_IP) */
> + uint64_t rip;
> + /* Flags (HV_X64_REGISTER_CLASS_FLAGS) */
> + uint64_t rflags;
> + } QEMU_PACKED;
> +
> + uint64_t registers[18];
> + };
> + uint8_t reserved[8];
> + /* Volatile XMM registers (HV_X64_REGISTER_CLASS_XMM) */
> + union {
> + struct {
> + struct hv_u128 xmm0;
> + struct hv_u128 xmm1;
> + struct hv_u128 xmm2;
> + struct hv_u128 xmm3;
> + struct hv_u128 xmm4;
> + struct hv_u128 xmm5;
> + } QEMU_PACKED;
> +
> + struct hv_u128 xmm_registers[6];
> + };
> + /* Segment registers (HV_X64_REGISTER_CLASS_SEGMENT) */
> + union {
> + struct {
> + struct hv_x64_segment_register es;
> + struct hv_x64_segment_register cs;
> + struct hv_x64_segment_register ss;
> + struct hv_x64_segment_register ds;
> + struct hv_x64_segment_register fs;
> + struct hv_x64_segment_register gs;
> + } QEMU_PACKED;
> +
> + struct hv_x64_segment_register segment_registers[6];
> + };
> + /* Misc. control registers (cannot be set via this interface) */
> + uint64_t cr0;
> + uint64_t cr3;
> + uint64_t cr4;
> + uint64_t cr8;
> + uint64_t efer;
> + uint64_t dr7;
> + union hv_x64_pending_interruption_register pending_interruption;
> + union hv_x64_interrupt_state_register interrupt_state;
> + uint64_t instruction_emulation_hints;
> + uint64_t xfem;
> +
> + uint8_t reserved1[0x100];
> +
> + /* Interrupts injected as part of HvCallDispatchVp. */
> + union hv_vp_register_page_interrupt_vectors interrupt_vectors;
> +} QEMU_PACKED;
> +
> #endif /* HW_HYPERV_HVHDK_H */
> --
> 2.53.0
Reviewed-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails
2026-07-07 22:16 [PATCH 0/5] Fix register page use for guests using mshv accel Doru Blânzeanu
2026-07-07 22:16 ` [PATCH 1/5] include/hw/hyperv: move hv_vp_register_page struct definition Doru Blânzeanu
@ 2026-07-07 22:16 ` Doru Blânzeanu
2026-07-08 16:37 ` Magnus Kulke
2026-07-08 16:38 ` Magnus Kulke
2026-07-07 22:16 ` [PATCH 3/5] target/i386/mshv: remove fallback for register page get registers Doru Blânzeanu
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Doru Blânzeanu @ 2026-07-07 22:16 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Doru Blânzeanu, Magnus Kulke, Wei Liu,
Magnus Kulke, Doru Blânzeanu, Wei Liu
In case the call to mmap fails, we log an error and abort to signal
there is something wrong with the system.
Check the register page version and compare with the expected version and
abort in case of a mismatch.
Fixes: a173f8f170
Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
target/i386/mshv/mshv-cpu.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index 1c433c408c..c782715497 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -2049,6 +2049,7 @@ void mshv_arch_init_vcpu(CPUState *cpu)
CPUX86State *env = &x86_cpu->env;
AccelCPUState *state = cpu->accel;
size_t page = HV_HYP_PAGE_SIZE, xsave_len;
+ void *regs_page;
void *mem = qemu_memalign(page, 2 * page);
int ret;
X86XSaveHeader *header;
@@ -2060,15 +2061,20 @@ void mshv_arch_init_vcpu(CPUState *cpu)
> HV_HYP_PAGE_SIZE));
/* mmap the registers page */
- void *rp = mmap(NULL, page, PROT_READ | PROT_WRITE,
+ regs_page = mmap(NULL, page, PROT_READ | PROT_WRITE,
MAP_SHARED, mshv_vcpufd(cpu),
MSHV_VP_MMAP_OFFSET_REGISTERS * page);
- if (rp == MAP_FAILED) {
- warn_report("register page mmap failed, falling back to hypercalls: %s",
- strerror(errno));
- env->regs_page = NULL;
- } else {
- env->regs_page = (struct hv_vp_register_page *) rp;
+ if (regs_page == MAP_FAILED) {
+ /* This shouldn't fail, so we treat it as a fatal error */
+ error_report("register page mmap failed: %s", strerror(errno));
+ abort();
+ }
+ env->regs_page = (struct hv_vp_register_page *) regs_page;
+
+ if (env->regs_page->version != HV_VP_REGISTER_PAGE_VERSION_1) {
+ error_report("register page version mismatch: got %u, expected %u",
+ env->regs_page->version, HV_VP_REGISTER_PAGE_VERSION_1);
+ abort();
}
state->hvcall_args.base = mem;
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails
2026-07-07 22:16 ` [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails Doru Blânzeanu
@ 2026-07-08 16:37 ` Magnus Kulke
2026-07-08 16:38 ` Magnus Kulke
1 sibling, 0 replies; 13+ messages in thread
From: Magnus Kulke @ 2026-07-08 16:37 UTC (permalink / raw)
To: Doru Blânzeanu
Cc: qemu-devel, Paolo Bonzini, Wei Liu, Magnus Kulke,
Doru Blânzeanu, Wei Liu
On Wed, Jul 08, 2026 at 01:16:42AM +0300, Doru Blânzeanu wrote:
> In case the call to mmap fails, we log an error and abort to signal
> there is something wrong with the system.
> Check the register page version and compare with the expected version and
> abort in case of a mismatch.
>
> Fixes: a173f8f170
> Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
> ---
> target/i386/mshv/mshv-cpu.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 1c433c408c..c782715497 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -2049,6 +2049,7 @@ void mshv_arch_init_vcpu(CPUState *cpu)
> CPUX86State *env = &x86_cpu->env;
> AccelCPUState *state = cpu->accel;
> size_t page = HV_HYP_PAGE_SIZE, xsave_len;
> + void *regs_page;
> void *mem = qemu_memalign(page, 2 * page);
> int ret;
> X86XSaveHeader *header;
> @@ -2060,15 +2061,20 @@ void mshv_arch_init_vcpu(CPUState *cpu)
> > HV_HYP_PAGE_SIZE));
>
> /* mmap the registers page */
> - void *rp = mmap(NULL, page, PROT_READ | PROT_WRITE,
> + regs_page = mmap(NULL, page, PROT_READ | PROT_WRITE,
> MAP_SHARED, mshv_vcpufd(cpu),
> MSHV_VP_MMAP_OFFSET_REGISTERS * page);
> - if (rp == MAP_FAILED) {
> - warn_report("register page mmap failed, falling back to hypercalls: %s",
> - strerror(errno));
> - env->regs_page = NULL;
> - } else {
> - env->regs_page = (struct hv_vp_register_page *) rp;
> + if (regs_page == MAP_FAILED) {
> + /* This shouldn't fail, so we treat it as a fatal error */
> + error_report("register page mmap failed: %s", strerror(errno));
> + abort();
> + }
> + env->regs_page = (struct hv_vp_register_page *) regs_page;
> +
> + if (env->regs_page->version != HV_VP_REGISTER_PAGE_VERSION_1) {
> + error_report("register page version mismatch: got %u, expected %u",
> + env->regs_page->version, HV_VP_REGISTER_PAGE_VERSION_1);
> + abort();
> }
>
> state->hvcall_args.base = mem;
> --
> 2.53.0
Reviewed-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails
2026-07-07 22:16 ` [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails Doru Blânzeanu
2026-07-08 16:37 ` Magnus Kulke
@ 2026-07-08 16:38 ` Magnus Kulke
1 sibling, 0 replies; 13+ messages in thread
From: Magnus Kulke @ 2026-07-08 16:38 UTC (permalink / raw)
To: Doru Blânzeanu
Cc: qemu-devel, Paolo Bonzini, Wei Liu, Magnus Kulke,
Doru Blânzeanu, Wei Liu
On Wed, Jul 08, 2026 at 01:16:42AM +0300, Doru Blânzeanu wrote:
> In case the call to mmap fails, we log an error and abort to signal
> there is something wrong with the system.
> Check the register page version and compare with the expected version and
> abort in case of a mismatch.
>
> Fixes: a173f8f170
> Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
> ---
> target/i386/mshv/mshv-cpu.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 1c433c408c..c782715497 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -2049,6 +2049,7 @@ void mshv_arch_init_vcpu(CPUState *cpu)
> CPUX86State *env = &x86_cpu->env;
> AccelCPUState *state = cpu->accel;
> size_t page = HV_HYP_PAGE_SIZE, xsave_len;
> + void *regs_page;
> void *mem = qemu_memalign(page, 2 * page);
> int ret;
> X86XSaveHeader *header;
> @@ -2060,15 +2061,20 @@ void mshv_arch_init_vcpu(CPUState *cpu)
> > HV_HYP_PAGE_SIZE));
>
> /* mmap the registers page */
> - void *rp = mmap(NULL, page, PROT_READ | PROT_WRITE,
> + regs_page = mmap(NULL, page, PROT_READ | PROT_WRITE,
> MAP_SHARED, mshv_vcpufd(cpu),
> MSHV_VP_MMAP_OFFSET_REGISTERS * page);
> - if (rp == MAP_FAILED) {
> - warn_report("register page mmap failed, falling back to hypercalls: %s",
> - strerror(errno));
> - env->regs_page = NULL;
> - } else {
> - env->regs_page = (struct hv_vp_register_page *) rp;
> + if (regs_page == MAP_FAILED) {
> + /* This shouldn't fail, so we treat it as a fatal error */
> + error_report("register page mmap failed: %s", strerror(errno));
> + abort();
> + }
> + env->regs_page = (struct hv_vp_register_page *) regs_page;
> +
> + if (env->regs_page->version != HV_VP_REGISTER_PAGE_VERSION_1) {
> + error_report("register page version mismatch: got %u, expected %u",
> + env->regs_page->version, HV_VP_REGISTER_PAGE_VERSION_1);
> + abort();
> }
>
> state->hvcall_args.base = mem;
> --
> 2.53.0
Reviewed-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/5] target/i386/mshv: remove fallback for register page get registers
2026-07-07 22:16 [PATCH 0/5] Fix register page use for guests using mshv accel Doru Blânzeanu
2026-07-07 22:16 ` [PATCH 1/5] include/hw/hyperv: move hv_vp_register_page struct definition Doru Blânzeanu
2026-07-07 22:16 ` [PATCH 2/5] target/i386/mshv: abort when hv_vp_register_page setup fails Doru Blânzeanu
@ 2026-07-07 22:16 ` Doru Blânzeanu
2026-07-08 16:49 ` Magnus Kulke
2026-07-07 22:16 ` [PATCH 4/5] target/i386/mshv: remove fallback for register page set registers Doru Blânzeanu
2026-07-07 22:16 ` [PATCH 5/5] target/i386/mshv: fix pio handlers clobbering device-modified registers Doru Blânzeanu
4 siblings, 1 reply; 13+ messages in thread
From: Doru Blânzeanu @ 2026-07-07 22:16 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Doru Blânzeanu, Magnus Kulke, Wei Liu,
Magnus Kulke, Doru Blânzeanu, Wei Liu
Change `load_regs` to use the register page when it is mmapped
and is valid.
Eliminate the hypercall based logic and fail in case the register page
is found in an unexpected state.
When retrieving the special registers, there are some registers that are
not present in the register page: TR, LDTR, GDTR, IDTR, CR2, APIC_BASE.
As this registers are not likely to be used in an MMIO/PIO operation,
and to avoid a hypercall overhead we do not retrieve them.
Local testing showed no regression when using this logic. To properly
retrieve all the necessary registers for each decoded operation implies
having a mechanism that tracks the state of each register, which is
beyond the scope of this patch series.
Fixes: 40072a7391
Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
target/i386/mshv/mshv-cpu.c | 107 +++++++++++-------------------------
1 file changed, 31 insertions(+), 76 deletions(-)
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index c782715497..9ec5c19c67 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -270,15 +270,6 @@ static int get_xc_reg(CPUState *cpu)
return 0;
}
-static enum hv_register_name NON_VP_PAGE_REGISTER_NAMES[6] = {
- HV_X64_REGISTER_TR,
- HV_X64_REGISTER_LDTR,
- HV_X64_REGISTER_GDTR,
- HV_X64_REGISTER_IDTR,
- HV_X64_REGISTER_CR2,
- HV_X64_REGISTER_APIC_BASE,
-};
-
static int translate_gva(const CPUState *cpu, uint64_t gva, uint64_t *gpa,
uint64_t flags)
{
@@ -615,7 +606,7 @@ static void populate_special_regs(const hv_register_assoc *assocs,
cpu_set_apic_base(x86cpu->apic_state, assocs[16].value.reg64);
}
-static void mshv_get_standard_regs_vp_page(CPUState *cpu)
+static void get_standard_regs_vp_page(CPUState *cpu)
{
X86CPU *x86cpu = X86_CPU(cpu);
CPUX86State *env = &x86cpu->env;
@@ -643,13 +634,19 @@ static void mshv_get_standard_regs_vp_page(CPUState *cpu)
rflags_to_lflags(env);
}
-static int mshv_get_special_regs_vp_page(CPUState *cpu)
+/*
+ * This function synchronizes the special registers present in the
+ * register vp page, which are not all the special registers.
+ * The rest of the special registers (LD, TR, GDT, IDT, CR2, APIC_BASE)
+ * are not synchronized to avoid the overhead of a hypercall.
+ *
+ * These special registers are not normally used by the guest,
+ * and are only used in some specific cases.
+ */
+static void get_special_regs_vp_page(CPUState *cpu)
{
X86CPU *x86cpu = X86_CPU(cpu);
CPUX86State *env = &x86cpu->env;
- struct hv_register_assoc assocs[ARRAY_SIZE(NON_VP_PAGE_REGISTER_NAMES)];
- int ret;
- size_t n_regs = ARRAY_SIZE(NON_VP_PAGE_REGISTER_NAMES);
hv_x64_segment_register seg;
/* Populate special registers that are in the VP register page */
@@ -672,46 +669,15 @@ static int mshv_get_special_regs_vp_page(CPUState *cpu)
populate_segment_reg(&seg, &env->segs[R_FS]);
memcpy(&seg, &env->regs_page->gs, sizeof(hv_x64_segment_register));
populate_segment_reg(&seg, &env->segs[R_GS]);
-
- /* The rest of the special registers that are not in the VP register page */
- for (size_t i = 0; i < n_regs; i++) {
- assocs[i].name = NON_VP_PAGE_REGISTER_NAMES[i];
- }
-
- ret = mshv_get_generic_regs(cpu, assocs, n_regs);
- if (ret < 0) {
- error_report("failed to get non-vp-page special registers");
- return -1;
- }
-
- /* Non-VP page registers - TR, LDTR, GDTR, IDTR, CR2, APIC_BASE */
- populate_segment_reg(&assocs[0].value.segment, &env->tr);
- populate_segment_reg(&assocs[1].value.segment, &env->ldt);
-
- populate_table_reg(&assocs[2].value.table, &env->gdt);
- populate_table_reg(&assocs[3].value.table, &env->idt);
- env->cr[2] = assocs[4].value.reg64;
-
- cpu_set_apic_base(x86cpu->apic_state, assocs[5].value.reg64);
-
- return ret;
}
-static int mshv_get_registers_vp_page(CPUState *cpu)
+static void get_registers_vp_page(CPUState *cpu)
{
- int ret;
-
/* General Purpose Registers */
- mshv_get_standard_regs_vp_page(cpu);
+ get_standard_regs_vp_page(cpu);
- /* Special Registers - makes a hypercall */
- ret = mshv_get_special_regs_vp_page(cpu);
- if (ret < 0) {
- error_report("failed to get special registers for vp page");
- return -1;
- }
-
- return 0;
+ /* Special Registers */
+ get_special_regs_vp_page(cpu);
}
@@ -735,29 +701,26 @@ static int get_special_regs(CPUState *cpu)
return 0;
}
-static int load_regs(CPUState *cpu)
+static void load_regs(CPUState *cpu)
{
X86CPU *x86_cpu = X86_CPU(cpu);
CPUX86State *env = &x86_cpu->env;
- int ret;
-
- /* Use register vp page to optimize registers access */
- if (env->regs_page && env->regs_page->isvalid != 0) {
- ret = mshv_get_registers_vp_page(cpu);
- return ret;
- }
- ret = get_standard_regs(cpu);
- if (ret < 0) {
- return ret;
+ /* Check register page pointer and abort if in unexpected state */
+ if (!env->regs_page) {
+ error_report(
+ "load regs: register page not set for vcpu %d",
+ cpu->cpu_index);
+ abort();
}
-
- ret = get_special_regs(cpu);
- if (ret < 0) {
- return ret;
+ if (env->regs_page->isvalid == 0) {
+ error_report(
+ "load regs: register page invalid for vcpu %d",
+ cpu->cpu_index);
+ abort();
}
- return 0;
+ get_registers_vp_page(cpu);
}
static int get_vcpu_events(CPUState *cpu)
@@ -1565,11 +1528,7 @@ static int emulate_instruction(CPUState *cpu,
int ret;
x86_insn_stream stream = { .bytes = insn_bytes, .len = insn_len };
- ret = load_regs(cpu);
- if (ret < 0) {
- error_report("Failed to load registers");
- return -1;
- }
+ load_regs(cpu);
decode_instruction_stream(env, &decode, &stream);
exec_instruction(env, &decode);
@@ -1872,11 +1831,7 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
X86CPU *x86_cpu = X86_CPU(cpu);
CPUX86State *env = &x86_cpu->env;
- ret = load_regs(cpu);
- if (ret < 0) {
- error_report("Failed to load registers");
- return -1;
- }
+ load_regs(cpu);
direction_flag = (env->eflags & DESC_E_MASK) != 0;
@@ -2013,7 +1968,7 @@ static void read_segment_descriptor(CPUState *cpu,
/*
* SegmentCache stores the hypervisor-provided value verbatim (populated by
- * mshv_load_regs). We need to convert it to format expected by the
+ * load_regs). We need to convert it to format expected by the
* instruction emulator. We can have a limit value > 0xfffff with
* granularity of 0 (byte granularity), which is not representable
* in real x86_segment_descriptor. In this case we set granularity to 1
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/5] target/i386/mshv: remove fallback for register page get registers
2026-07-07 22:16 ` [PATCH 3/5] target/i386/mshv: remove fallback for register page get registers Doru Blânzeanu
@ 2026-07-08 16:49 ` Magnus Kulke
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kulke @ 2026-07-08 16:49 UTC (permalink / raw)
To: Doru Blânzeanu
Cc: qemu-devel, Paolo Bonzini, Wei Liu, Magnus Kulke,
Doru Blânzeanu, Wei Liu
On Wed, Jul 08, 2026 at 01:16:43AM +0300, Doru Blânzeanu wrote:
> Change `load_regs` to use the register page when it is mmapped
> and is valid.
> Eliminate the hypercall based logic and fail in case the register page
> is found in an unexpected state.
>
> When retrieving the special registers, there are some registers that are
> not present in the register page: TR, LDTR, GDTR, IDTR, CR2, APIC_BASE.
> As this registers are not likely to be used in an MMIO/PIO operation,
> and to avoid a hypercall overhead we do not retrieve them.
>
> Local testing showed no regression when using this logic. To properly
> retrieve all the necessary registers for each decoded operation implies
> having a mechanism that tracks the state of each register, which is
> beyond the scope of this patch series.
>
> Fixes: 40072a7391
> Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
> ---
> target/i386/mshv/mshv-cpu.c | 107 +++++++++++-------------------------
> 1 file changed, 31 insertions(+), 76 deletions(-)
>
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index c782715497..9ec5c19c67 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -270,15 +270,6 @@ static int get_xc_reg(CPUState *cpu)
> return 0;
> }
>
> -static enum hv_register_name NON_VP_PAGE_REGISTER_NAMES[6] = {
> - HV_X64_REGISTER_TR,
> - HV_X64_REGISTER_LDTR,
> - HV_X64_REGISTER_GDTR,
> - HV_X64_REGISTER_IDTR,
> - HV_X64_REGISTER_CR2,
> - HV_X64_REGISTER_APIC_BASE,
> -};
> -
> static int translate_gva(const CPUState *cpu, uint64_t gva, uint64_t *gpa,
> uint64_t flags)
> {
> @@ -615,7 +606,7 @@ static void populate_special_regs(const hv_register_assoc *assocs,
> cpu_set_apic_base(x86cpu->apic_state, assocs[16].value.reg64);
> }
>
> -static void mshv_get_standard_regs_vp_page(CPUState *cpu)
> +static void get_standard_regs_vp_page(CPUState *cpu)
> {
> X86CPU *x86cpu = X86_CPU(cpu);
> CPUX86State *env = &x86cpu->env;
> @@ -643,13 +634,19 @@ static void mshv_get_standard_regs_vp_page(CPUState *cpu)
> rflags_to_lflags(env);
> }
>
> -static int mshv_get_special_regs_vp_page(CPUState *cpu)
> +/*
> + * This function synchronizes the special registers present in the
> + * register vp page, which are not all the special registers.
> + * The rest of the special registers (LD, TR, GDT, IDT, CR2, APIC_BASE)
> + * are not synchronized to avoid the overhead of a hypercall.
> + *
> + * These special registers are not normally used by the guest,
> + * and are only used in some specific cases.
> + */
> +static void get_special_regs_vp_page(CPUState *cpu)
> {
> X86CPU *x86cpu = X86_CPU(cpu);
> CPUX86State *env = &x86cpu->env;
> - struct hv_register_assoc assocs[ARRAY_SIZE(NON_VP_PAGE_REGISTER_NAMES)];
> - int ret;
> - size_t n_regs = ARRAY_SIZE(NON_VP_PAGE_REGISTER_NAMES);
> hv_x64_segment_register seg;
>
> /* Populate special registers that are in the VP register page */
> @@ -672,46 +669,15 @@ static int mshv_get_special_regs_vp_page(CPUState *cpu)
> populate_segment_reg(&seg, &env->segs[R_FS]);
> memcpy(&seg, &env->regs_page->gs, sizeof(hv_x64_segment_register));
> populate_segment_reg(&seg, &env->segs[R_GS]);
> -
> - /* The rest of the special registers that are not in the VP register page */
> - for (size_t i = 0; i < n_regs; i++) {
> - assocs[i].name = NON_VP_PAGE_REGISTER_NAMES[i];
> - }
> -
> - ret = mshv_get_generic_regs(cpu, assocs, n_regs);
> - if (ret < 0) {
> - error_report("failed to get non-vp-page special registers");
> - return -1;
> - }
> -
> - /* Non-VP page registers - TR, LDTR, GDTR, IDTR, CR2, APIC_BASE */
> - populate_segment_reg(&assocs[0].value.segment, &env->tr);
> - populate_segment_reg(&assocs[1].value.segment, &env->ldt);
> -
> - populate_table_reg(&assocs[2].value.table, &env->gdt);
> - populate_table_reg(&assocs[3].value.table, &env->idt);
> - env->cr[2] = assocs[4].value.reg64;
> -
> - cpu_set_apic_base(x86cpu->apic_state, assocs[5].value.reg64);
> -
> - return ret;
> }
>
> -static int mshv_get_registers_vp_page(CPUState *cpu)
> +static void get_registers_vp_page(CPUState *cpu)
> {
> - int ret;
> -
> /* General Purpose Registers */
> - mshv_get_standard_regs_vp_page(cpu);
> + get_standard_regs_vp_page(cpu);
>
> - /* Special Registers - makes a hypercall */
> - ret = mshv_get_special_regs_vp_page(cpu);
> - if (ret < 0) {
> - error_report("failed to get special registers for vp page");
> - return -1;
> - }
> -
> - return 0;
> + /* Special Registers */
> + get_special_regs_vp_page(cpu);
> }
>
>
> @@ -735,29 +701,26 @@ static int get_special_regs(CPUState *cpu)
> return 0;
> }
>
> -static int load_regs(CPUState *cpu)
> +static void load_regs(CPUState *cpu)
> {
> X86CPU *x86_cpu = X86_CPU(cpu);
> CPUX86State *env = &x86_cpu->env;
> - int ret;
> -
> - /* Use register vp page to optimize registers access */
> - if (env->regs_page && env->regs_page->isvalid != 0) {
> - ret = mshv_get_registers_vp_page(cpu);
> - return ret;
> - }
>
> - ret = get_standard_regs(cpu);
> - if (ret < 0) {
> - return ret;
> + /* Check register page pointer and abort if in unexpected state */
> + if (!env->regs_page) {
> + error_report(
> + "load regs: register page not set for vcpu %d",
> + cpu->cpu_index);
> + abort();
> }
> -
> - ret = get_special_regs(cpu);
> - if (ret < 0) {
> - return ret;
> + if (env->regs_page->isvalid == 0) {
> + error_report(
> + "load regs: register page invalid for vcpu %d",
> + cpu->cpu_index);
> + abort();
> }
>
> - return 0;
> + get_registers_vp_page(cpu);
> }
>
> static int get_vcpu_events(CPUState *cpu)
> @@ -1565,11 +1528,7 @@ static int emulate_instruction(CPUState *cpu,
> int ret;
> x86_insn_stream stream = { .bytes = insn_bytes, .len = insn_len };
>
> - ret = load_regs(cpu);
> - if (ret < 0) {
> - error_report("Failed to load registers");
> - return -1;
> - }
> + load_regs(cpu);
>
> decode_instruction_stream(env, &decode, &stream);
> exec_instruction(env, &decode);
> @@ -1872,11 +1831,7 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
> X86CPU *x86_cpu = X86_CPU(cpu);
> CPUX86State *env = &x86_cpu->env;
>
> - ret = load_regs(cpu);
> - if (ret < 0) {
> - error_report("Failed to load registers");
> - return -1;
> - }
> + load_regs(cpu);
>
> direction_flag = (env->eflags & DESC_E_MASK) != 0;
>
> @@ -2013,7 +1968,7 @@ static void read_segment_descriptor(CPUState *cpu,
>
> /*
> * SegmentCache stores the hypervisor-provided value verbatim (populated by
> - * mshv_load_regs). We need to convert it to format expected by the
> + * load_regs). We need to convert it to format expected by the
> * instruction emulator. We can have a limit value > 0xfffff with
> * granularity of 0 (byte granularity), which is not representable
> * in real x86_segment_descriptor. In this case we set granularity to 1
> --
> 2.53.0
Reviewed-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/5] target/i386/mshv: remove fallback for register page set registers
2026-07-07 22:16 [PATCH 0/5] Fix register page use for guests using mshv accel Doru Blânzeanu
` (2 preceding siblings ...)
2026-07-07 22:16 ` [PATCH 3/5] target/i386/mshv: remove fallback for register page get registers Doru Blânzeanu
@ 2026-07-07 22:16 ` Doru Blânzeanu
2026-07-08 16:44 ` Magnus Kulke
2026-07-07 22:16 ` [PATCH 5/5] target/i386/mshv: fix pio handlers clobbering device-modified registers Doru Blânzeanu
4 siblings, 1 reply; 13+ messages in thread
From: Doru Blânzeanu @ 2026-07-07 22:16 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Doru Blânzeanu, Magnus Kulke, Wei Liu,
Magnus Kulke, Doru Blânzeanu, Wei Liu
Update `store_regs` to use the register page when it is mmapped and
valid to set registers.
Remove the ioctl based register retrieval and fail in case the register
page is not correctly set or valid.
Fixes: 80c7f8e9cd
Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
target/i386/mshv/mshv-cpu.c | 48 +++++++++++++------------------------
1 file changed, 17 insertions(+), 31 deletions(-)
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index 9ec5c19c67..8a59a0b40e 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -448,7 +448,7 @@ static int set_standard_regs(const CPUState *cpu)
return 0;
}
-static void mshv_set_standard_regs_vp_page(CPUState *cpu)
+static void set_standard_regs_vp_page(CPUState *cpu)
{
X86CPU *x86cpu = X86_CPU(cpu);
CPUX86State *env = &x86cpu->env;
@@ -478,29 +478,26 @@ static void mshv_set_standard_regs_vp_page(CPUState *cpu)
| (1u << HV_X64_REGISTER_CLASS_FLAGS);
}
-static int store_regs(CPUState *cpu)
+static void store_regs(CPUState *cpu)
{
X86CPU *x86cpu = X86_CPU(cpu);
CPUX86State *env = &x86cpu->env;
- int ret;
- /* Use register vp page to optimize registers access */
- if (env->regs_page && env->regs_page->isvalid != 0) {
- mshv_set_standard_regs_vp_page(cpu);
- } else {
- ret = set_standard_regs(cpu);
- if (ret < 0) {
- return ret;
- }
+ /* Check register page pointer and abort if in unexpected state */
+ if (!env->regs_page) {
+ error_report(
+ "store regs: register page not set for vcpu %d",
+ cpu->cpu_index);
+ abort();
}
-
- ret = set_special_regs(cpu);
- if (ret < 0) {
- error_report("Failed to store speical registers");
- return ret;
+ if (env->regs_page->isvalid == 0) {
+ error_report(
+ "store regs: register page invalid for vcpu %d",
+ cpu->cpu_index);
+ abort();
}
- return 0;
+ set_standard_regs_vp_page(cpu);
}
static void populate_standard_regs(const hv_register_assoc *assocs,
@@ -1518,14 +1515,13 @@ static int set_memory_info(const struct hyperv_message *msg,
return 0;
}
-static int emulate_instruction(CPUState *cpu,
+static void emulate_instruction(CPUState *cpu,
const uint8_t *insn_bytes, size_t insn_len,
uint64_t gva, uint64_t gpa)
{
X86CPU *x86_cpu = X86_CPU(cpu);
CPUX86State *env = &x86_cpu->env;
struct x86_decode decode = { 0 };
- int ret;
x86_insn_stream stream = { .bytes = insn_bytes, .len = insn_len };
load_regs(cpu);
@@ -1533,13 +1529,7 @@ static int emulate_instruction(CPUState *cpu,
decode_instruction_stream(env, &decode, &stream);
exec_instruction(env, &decode);
- ret = store_regs(cpu);
- if (ret < 0) {
- error_report("failed to store registers");
- return -1;
- }
-
- return 0;
+ store_regs(cpu);
}
static int handle_mmio(CPUState *cpu, const struct hyperv_message *msg,
@@ -1575,13 +1565,9 @@ static int handle_mmio(CPUState *cpu, const struct hyperv_message *msg,
instruction_bytes = info.instruction_bytes;
- ret = emulate_instruction(cpu, instruction_bytes, insn_len,
+ emulate_instruction(cpu, instruction_bytes, insn_len,
info.guest_virtual_address,
info.guest_physical_address);
- if (ret < 0) {
- error_report("failed to emulate mmio");
- return -1;
- }
*exit_reason = MshvVmExitIgnore;
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/5] target/i386/mshv: remove fallback for register page set registers
2026-07-07 22:16 ` [PATCH 4/5] target/i386/mshv: remove fallback for register page set registers Doru Blânzeanu
@ 2026-07-08 16:44 ` Magnus Kulke
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kulke @ 2026-07-08 16:44 UTC (permalink / raw)
To: Doru Blânzeanu
Cc: qemu-devel, Paolo Bonzini, Wei Liu, Magnus Kulke,
Doru Blânzeanu, Wei Liu
On Wed, Jul 08, 2026 at 01:16:44AM +0300, Doru Blânzeanu wrote:
> Update `store_regs` to use the register page when it is mmapped and
> valid to set registers.
> Remove the ioctl based register retrieval and fail in case the register
> page is not correctly set or valid.
>
> Fixes: 80c7f8e9cd
> Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
> ---
> target/i386/mshv/mshv-cpu.c | 48 +++++++++++++------------------------
> 1 file changed, 17 insertions(+), 31 deletions(-)
>
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 9ec5c19c67..8a59a0b40e 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -448,7 +448,7 @@ static int set_standard_regs(const CPUState *cpu)
> return 0;
> }
>
> -static void mshv_set_standard_regs_vp_page(CPUState *cpu)
> +static void set_standard_regs_vp_page(CPUState *cpu)
> {
> X86CPU *x86cpu = X86_CPU(cpu);
> CPUX86State *env = &x86cpu->env;
> @@ -478,29 +478,26 @@ static void mshv_set_standard_regs_vp_page(CPUState *cpu)
> | (1u << HV_X64_REGISTER_CLASS_FLAGS);
> }
>
> -static int store_regs(CPUState *cpu)
> +static void store_regs(CPUState *cpu)
> {
> X86CPU *x86cpu = X86_CPU(cpu);
> CPUX86State *env = &x86cpu->env;
> - int ret;
>
> - /* Use register vp page to optimize registers access */
> - if (env->regs_page && env->regs_page->isvalid != 0) {
> - mshv_set_standard_regs_vp_page(cpu);
> - } else {
> - ret = set_standard_regs(cpu);
> - if (ret < 0) {
> - return ret;
> - }
> + /* Check register page pointer and abort if in unexpected state */
> + if (!env->regs_page) {
> + error_report(
> + "store regs: register page not set for vcpu %d",
> + cpu->cpu_index);
> + abort();
> }
> -
> - ret = set_special_regs(cpu);
> - if (ret < 0) {
> - error_report("Failed to store speical registers");
> - return ret;
> + if (env->regs_page->isvalid == 0) {
> + error_report(
> + "store regs: register page invalid for vcpu %d",
> + cpu->cpu_index);
> + abort();
> }
>
> - return 0;
> + set_standard_regs_vp_page(cpu);
> }
>
> static void populate_standard_regs(const hv_register_assoc *assocs,
> @@ -1518,14 +1515,13 @@ static int set_memory_info(const struct hyperv_message *msg,
> return 0;
> }
>
> -static int emulate_instruction(CPUState *cpu,
> +static void emulate_instruction(CPUState *cpu,
> const uint8_t *insn_bytes, size_t insn_len,
> uint64_t gva, uint64_t gpa)
> {
> X86CPU *x86_cpu = X86_CPU(cpu);
> CPUX86State *env = &x86_cpu->env;
> struct x86_decode decode = { 0 };
> - int ret;
> x86_insn_stream stream = { .bytes = insn_bytes, .len = insn_len };
>
> load_regs(cpu);
> @@ -1533,13 +1529,7 @@ static int emulate_instruction(CPUState *cpu,
> decode_instruction_stream(env, &decode, &stream);
> exec_instruction(env, &decode);
>
> - ret = store_regs(cpu);
> - if (ret < 0) {
> - error_report("failed to store registers");
> - return -1;
> - }
> -
> - return 0;
> + store_regs(cpu);
> }
>
> static int handle_mmio(CPUState *cpu, const struct hyperv_message *msg,
> @@ -1575,13 +1565,9 @@ static int handle_mmio(CPUState *cpu, const struct hyperv_message *msg,
>
> instruction_bytes = info.instruction_bytes;
>
> - ret = emulate_instruction(cpu, instruction_bytes, insn_len,
> + emulate_instruction(cpu, instruction_bytes, insn_len,
> info.guest_virtual_address,
> info.guest_physical_address);
> - if (ret < 0) {
> - error_report("failed to emulate mmio");
> - return -1;
> - }
>
> *exit_reason = MshvVmExitIgnore;
>
> --
> 2.53.0
Reviewed-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/5] target/i386/mshv: fix pio handlers clobbering device-modified registers
2026-07-07 22:16 [PATCH 0/5] Fix register page use for guests using mshv accel Doru Blânzeanu
` (3 preceding siblings ...)
2026-07-07 22:16 ` [PATCH 4/5] target/i386/mshv: remove fallback for register page set registers Doru Blânzeanu
@ 2026-07-07 22:16 ` Doru Blânzeanu
2026-07-08 16:50 ` Magnus Kulke
4 siblings, 1 reply; 13+ messages in thread
From: Doru Blânzeanu @ 2026-07-07 22:16 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Doru Blânzeanu, Magnus Kulke, Wei Liu,
Magnus Kulke, Doru Blânzeanu, Wei Liu
When a device handler (e.g. vmport) calls cpu_synchronize_state() during
I/O port dispatch, it sets cpu->accel->dirty = true and may modify
registers directly in env. The old PIO code ignored this: it
unconditionally wrote the stale info->rax from the VM-exit intercept
message back to the hypervisor and then cleared dirty, discarding any
register changes made by the device.
Bifurcate both handlers on cpu->accel->dirty:
handle_pio_non_str:
- dirty path: update env->eip directly. For reads (IN), merge the I/O
result into env->regs[R_EAX] (which may have been modified by the
device) rather than info->rax. For writes (OUT), leave RAX untouched.
Flush all registers via mshv_store_regs() and clear dirty.
- non-dirty path: write RIP and RAX via set_x64_registers hypercall as
before.
handle_pio_str:
- dirty path: update env->eip and the appropriate index register
(RSI for OUTS, RDI for INS) directly. Flush via mshv_store_regs()
and clear dirty.
- non-dirty path: write the index register and RIP via
set_x64_registers. Drop the RAX assignment that was here before;
string I/O does not modify RAX, and set_x64_registers is hardcoded
to write only 2 registers so the third slot was silently ignored
anyway.
Remove the unconditional "cpu->accel->dirty = false" at the end of both
handlers. In the non-dirty fast path it was redundant (already false).
In the dirty path it was actively harmful: it told the vcpu run loop
that env was clean when it was not, losing the device's modifications.
Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
target/i386/mshv/mshv-cpu.c | 72 +++++++++++++++++++++++++------------
1 file changed, 50 insertions(+), 22 deletions(-)
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index 8a59a0b40e..5e680351cd 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -1646,10 +1646,12 @@ static int handle_pio_non_str(CPUState *cpu,
uint32_t val, eax;
const uint32_t eax_mask = 0xffffffffu >> (32 - len * 8);
size_t insn_len;
- uint64_t rip, rax;
+ uint64_t rip;
uint32_t reg_names[2];
uint64_t reg_values[2];
uint16_t port = info->port_number;
+ X86CPU *x86_cpu = X86_CPU(cpu);
+ CPUX86State *env = &x86_cpu->env;
if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
union {
@@ -1680,21 +1682,36 @@ static int handle_pio_non_str(CPUState *cpu,
/* Advance RIP and update RAX */
rip = info->header.rip + insn_len;
- rax = info->rax;
- reg_names[0] = HV_X64_REGISTER_RIP;
- reg_values[0] = rip;
- reg_names[1] = HV_X64_REGISTER_RAX;
- reg_values[1] = rax;
+ if (cpu->vcpu_dirty) {
+ env->eip = rip;
+ if (access_type != HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
+ /*
+ * For reads, merge the I/O result into the current RAX.
+ * Use env->regs[R_EAX] as the base since a device handler
+ * (e.g. vmport) may have called cpu_synchronize_state()
+ * and modified registers.
+ */
+ eax = (((uint32_t)env->regs[R_EAX]) & ~eax_mask)
+ | (val & eax_mask);
+ env->regs[R_EAX] = (uint64_t)eax;
+ }
+ /* Sync modified standard registers back and clear dirty. */
+ store_regs(cpu);
+ cpu->vcpu_dirty = false;
+ } else {
+ reg_names[0] = HV_X64_REGISTER_RIP;
+ reg_values[0] = rip;
+ reg_names[1] = HV_X64_REGISTER_RAX;
+ reg_values[1] = info->rax;
- ret = set_x64_registers(cpu, reg_names, reg_values);
- if (ret < 0) {
- error_report("Failed to set x64 registers");
- return -1;
+ ret = set_x64_registers(cpu, reg_names, reg_values);
+ if (ret < 0) {
+ error_report("Failed to set x64 registers");
+ return -1;
+ }
}
- cpu->vcpu_dirty = false;
-
return 0;
}
@@ -1810,6 +1827,7 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
bool repop = info->access_info.rep_prefix == 1;
size_t repeat = repop ? info->rcx : 1;
size_t insn_len = info->header.instruction_length;
+ uint64_t rip;
bool direction_flag;
uint32_t reg_names[3];
uint64_t reg_values[3];
@@ -1839,18 +1857,28 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
reg_values[0] = info->rdi;
}
- reg_names[1] = HV_X64_REGISTER_RIP;
- reg_values[1] = info->header.rip + insn_len;
- reg_names[2] = HV_X64_REGISTER_RAX;
- reg_values[2] = info->rax;
+ rip = info->header.rip + insn_len;
- ret = set_x64_registers(cpu, reg_names, reg_values);
- if (ret < 0) {
- error_report("Failed to set RIP and RAX registers");
- return -1;
- }
+ if (cpu->vcpu_dirty) {
+ env->eip = rip;
+ if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
+ env->regs[R_ESI] = info->rsi;
+ } else {
+ env->regs[R_EDI] = info->rdi;
+ }
+ /* Sync modified standard registers back and clear dirty. */
+ store_regs(cpu);
+ cpu->vcpu_dirty = false;
+ } else {
+ reg_names[1] = HV_X64_REGISTER_RIP;
+ reg_values[1] = rip;
- cpu->vcpu_dirty = false;
+ ret = set_x64_registers(cpu, reg_names, reg_values);
+ if (ret < 0) {
+ error_report("Failed to set x64 registers");
+ return -1;
+ }
+ }
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/5] target/i386/mshv: fix pio handlers clobbering device-modified registers
2026-07-07 22:16 ` [PATCH 5/5] target/i386/mshv: fix pio handlers clobbering device-modified registers Doru Blânzeanu
@ 2026-07-08 16:50 ` Magnus Kulke
0 siblings, 0 replies; 13+ messages in thread
From: Magnus Kulke @ 2026-07-08 16:50 UTC (permalink / raw)
To: Doru Blânzeanu
Cc: qemu-devel, Paolo Bonzini, Wei Liu, Magnus Kulke,
Doru Blânzeanu, Wei Liu
On Wed, Jul 08, 2026 at 01:16:45AM +0300, Doru Blânzeanu wrote:
> When a device handler (e.g. vmport) calls cpu_synchronize_state() during
> I/O port dispatch, it sets cpu->accel->dirty = true and may modify
> registers directly in env. The old PIO code ignored this: it
> unconditionally wrote the stale info->rax from the VM-exit intercept
> message back to the hypervisor and then cleared dirty, discarding any
> register changes made by the device.
>
> Bifurcate both handlers on cpu->accel->dirty:
>
> handle_pio_non_str:
> - dirty path: update env->eip directly. For reads (IN), merge the I/O
> result into env->regs[R_EAX] (which may have been modified by the
> device) rather than info->rax. For writes (OUT), leave RAX untouched.
> Flush all registers via mshv_store_regs() and clear dirty.
> - non-dirty path: write RIP and RAX via set_x64_registers hypercall as
> before.
>
> handle_pio_str:
> - dirty path: update env->eip and the appropriate index register
> (RSI for OUTS, RDI for INS) directly. Flush via mshv_store_regs()
> and clear dirty.
> - non-dirty path: write the index register and RIP via
> set_x64_registers. Drop the RAX assignment that was here before;
> string I/O does not modify RAX, and set_x64_registers is hardcoded
> to write only 2 registers so the third slot was silently ignored
> anyway.
>
> Remove the unconditional "cpu->accel->dirty = false" at the end of both
> handlers. In the non-dirty fast path it was redundant (already false).
> In the dirty path it was actively harmful: it told the vcpu run loop
> that env was clean when it was not, losing the device's modifications.
>
> Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
> ---
> target/i386/mshv/mshv-cpu.c | 72 +++++++++++++++++++++++++------------
> 1 file changed, 50 insertions(+), 22 deletions(-)
>
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 8a59a0b40e..5e680351cd 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -1646,10 +1646,12 @@ static int handle_pio_non_str(CPUState *cpu,
> uint32_t val, eax;
> const uint32_t eax_mask = 0xffffffffu >> (32 - len * 8);
> size_t insn_len;
> - uint64_t rip, rax;
> + uint64_t rip;
> uint32_t reg_names[2];
> uint64_t reg_values[2];
> uint16_t port = info->port_number;
> + X86CPU *x86_cpu = X86_CPU(cpu);
> + CPUX86State *env = &x86_cpu->env;
>
> if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
> union {
> @@ -1680,21 +1682,36 @@ static int handle_pio_non_str(CPUState *cpu,
>
> /* Advance RIP and update RAX */
> rip = info->header.rip + insn_len;
> - rax = info->rax;
>
> - reg_names[0] = HV_X64_REGISTER_RIP;
> - reg_values[0] = rip;
> - reg_names[1] = HV_X64_REGISTER_RAX;
> - reg_values[1] = rax;
> + if (cpu->vcpu_dirty) {
> + env->eip = rip;
> + if (access_type != HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
> + /*
> + * For reads, merge the I/O result into the current RAX.
> + * Use env->regs[R_EAX] as the base since a device handler
> + * (e.g. vmport) may have called cpu_synchronize_state()
> + * and modified registers.
> + */
> + eax = (((uint32_t)env->regs[R_EAX]) & ~eax_mask)
> + | (val & eax_mask);
> + env->regs[R_EAX] = (uint64_t)eax;
> + }
> + /* Sync modified standard registers back and clear dirty. */
> + store_regs(cpu);
> + cpu->vcpu_dirty = false;
> + } else {
> + reg_names[0] = HV_X64_REGISTER_RIP;
> + reg_values[0] = rip;
> + reg_names[1] = HV_X64_REGISTER_RAX;
> + reg_values[1] = info->rax;
>
> - ret = set_x64_registers(cpu, reg_names, reg_values);
> - if (ret < 0) {
> - error_report("Failed to set x64 registers");
> - return -1;
> + ret = set_x64_registers(cpu, reg_names, reg_values);
> + if (ret < 0) {
> + error_report("Failed to set x64 registers");
> + return -1;
> + }
> }
>
> - cpu->vcpu_dirty = false;
> -
> return 0;
> }
>
> @@ -1810,6 +1827,7 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
> bool repop = info->access_info.rep_prefix == 1;
> size_t repeat = repop ? info->rcx : 1;
> size_t insn_len = info->header.instruction_length;
> + uint64_t rip;
> bool direction_flag;
> uint32_t reg_names[3];
> uint64_t reg_values[3];
> @@ -1839,18 +1857,28 @@ static int handle_pio_str(CPUState *cpu, hv_x64_io_port_intercept_message *info)
> reg_values[0] = info->rdi;
> }
>
> - reg_names[1] = HV_X64_REGISTER_RIP;
> - reg_values[1] = info->header.rip + insn_len;
> - reg_names[2] = HV_X64_REGISTER_RAX;
> - reg_values[2] = info->rax;
> + rip = info->header.rip + insn_len;
>
> - ret = set_x64_registers(cpu, reg_names, reg_values);
> - if (ret < 0) {
> - error_report("Failed to set RIP and RAX registers");
> - return -1;
> - }
> + if (cpu->vcpu_dirty) {
> + env->eip = rip;
> + if (access_type == HV_X64_INTERCEPT_ACCESS_TYPE_WRITE) {
> + env->regs[R_ESI] = info->rsi;
> + } else {
> + env->regs[R_EDI] = info->rdi;
> + }
> + /* Sync modified standard registers back and clear dirty. */
> + store_regs(cpu);
> + cpu->vcpu_dirty = false;
> + } else {
> + reg_names[1] = HV_X64_REGISTER_RIP;
> + reg_values[1] = rip;
>
> - cpu->vcpu_dirty = false;
> + ret = set_x64_registers(cpu, reg_names, reg_values);
> + if (ret < 0) {
> + error_report("Failed to set x64 registers");
> + return -1;
> + }
> + }
>
> return 0;
> }
> --
> 2.53.0
Reviewed-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
^ permalink raw reply [flat|nested] 13+ messages in thread