Re: [PATCH 5/6] target/i386/mshv: use the register page to get registers

2026-04-29 Thread Magnus Kulke
On Tue, Apr 28, 2026 at 04:50:52PM +0300, Doru Blânzeanu wrote:
> Change the mshv_load_regs to use the register page when it is mmapped
> and is valid.
> Otherwise use the existing logic that uses ioctls to fetch registers.
> 
> When retrieving the special registers, there are some registers that are
> not present in the register page: TR, LDTR, GDTR, IDTR, CR2, APIC_BASE.
> For this ones we still need to use ioctls to correctly fetch.
> 
> Signed-off-by: Doru Blânzeanu 
> ---
>  target/i386/mshv/mshv-cpu.c | 137 +---
>  1 file changed, 128 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 42b6fb1912..7949493e97 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -107,6 +107,15 @@ static enum hv_register_name FPU_REGISTER_NAMES[26] = {
>  HV_X64_REGISTER_XMM_CONTROL_STATUS,
>  };
>  
> +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)
>  {
> @@ -401,6 +410,105 @@ 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)
> +{
> +X86CPU *x86cpu = X86_CPU(cpu);
> +CPUX86State *env = &x86cpu->env;
> +
> +/* General Purpose Registers  */
> +env->regs[R_EAX] = env->regs_page->rax;
> +env->regs[R_EBX] = env->regs_page->rbx;
> +env->regs[R_ECX] = env->regs_page->rcx;
> +env->regs[R_EDX] = env->regs_page->rdx;
> +env->regs[R_ESI] = env->regs_page->rsi;
> +env->regs[R_EDI] = env->regs_page->rdi;
> +env->regs[R_ESP] = env->regs_page->rsp;
> +env->regs[R_EBP] = env->regs_page->rbp;
> +env->regs[R_R8]  = env->regs_page->r8;
> +env->regs[R_R9]  = env->regs_page->r9;
> +env->regs[R_R10] = env->regs_page->r10;
> +env->regs[R_R11] = env->regs_page->r11;
> +env->regs[R_R12] = env->regs_page->r12;
> +env->regs[R_R13] = env->regs_page->r13;
> +env->regs[R_R14] = env->regs_page->r14;
> +env->regs[R_R15] = env->regs_page->r15;
> +
> +env->eip = env->regs_page->rip;
> +env->eflags = env->regs_page->rflags;
> +rflags_to_lflags(env);
> +}
> +
> +static int mshv_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 */
> +env->cr[0] = env->regs_page->cr0;
> +env->cr[3] = env->regs_page->cr3;
> +env->cr[4] = env->regs_page->cr4;
> +env->efer = env->regs_page->efer;
> +cpu_set_apic_tpr(x86cpu->apic_state, env->regs_page->cr8);
> +
> +/* Segment Registers - copy from packed struct to avoid unaligned access 
> */
> +memcpy(&seg, &env->regs_page->es, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_ES]);
> +memcpy(&seg, &env->regs_page->cs, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_CS]);
> +memcpy(&seg, &env->regs_page->ss, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_SS]);
> +memcpy(&seg, &env->regs_page->ds, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_DS]);
> +memcpy(&seg, &env->regs_page->fs, sizeof(hv_x64_segment_register));
> +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 = 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);

Do we know whether MMIO emulation requires the
NON_VP_PAGE_REGISTER_NAMES to be roundtripped? I understand this is

Re: [PATCH 5/6] target/i386/mshv: use the register page to get registers

2026-04-28 Thread Mohamed Mediouni



> On 28. Apr 2026, at 15:50, Doru Blânzeanu  
> wrote:
> 
> Change the mshv_load_regs to use the register page when it is mmapped
> and is valid.
> Otherwise use the existing logic that uses ioctls to fetch registers.
> 
> When retrieving the special registers, there are some registers that are
> not present in the register page: TR, LDTR, GDTR, IDTR, CR2, APIC_BASE.
> For this ones we still need to use ioctls to correctly fetch.
> 
> Signed-off-by: Doru Blânzeanu 

Reviewed-by: Mohamed Mediouni 

But with comments below...

> ---
> target/i386/mshv/mshv-cpu.c | 137 +---
> 1 file changed, 128 insertions(+), 9 deletions(-)
> 
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 42b6fb1912..7949493e97 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -107,6 +107,15 @@ static enum hv_register_name FPU_REGISTER_NAMES[26] = {
> HV_X64_REGISTER_XMM_CONTROL_STATUS,
> };
> 
> +static enum hv_register_name NON_VP_PAGE_REGISTER_NAMES[6] = {
> +HV_X64_REGISTER_TR,
> +HV_X64_REGISTER_LDTR,
> +HV_X64_REGISTER_GDTR,

Those two _not_ being in a shared register page does cause quite a bit of
pain for perf if not dealt with carefully which leads to…

> +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)
> {
> @@ -401,6 +410,105 @@ 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)
> +{
> +X86CPU *x86cpu = X86_CPU(cpu);
> +CPUX86State *env = &x86cpu->env;
> +
> +/* General Purpose Registers  */
> +env->regs[R_EAX] = env->regs_page->rax;
> +env->regs[R_EBX] = env->regs_page->rbx;
> +env->regs[R_ECX] = env->regs_page->rcx;
> +env->regs[R_EDX] = env->regs_page->rdx;
> +env->regs[R_ESI] = env->regs_page->rsi;
> +env->regs[R_EDI] = env->regs_page->rdi;
> +env->regs[R_ESP] = env->regs_page->rsp;
> +env->regs[R_EBP] = env->regs_page->rbp;
> +env->regs[R_R8]  = env->regs_page->r8;
> +env->regs[R_R9]  = env->regs_page->r9;
> +env->regs[R_R10] = env->regs_page->r10;
> +env->regs[R_R11] = env->regs_page->r11;
> +env->regs[R_R12] = env->regs_page->r12;
> +env->regs[R_R13] = env->regs_page->r13;
> +env->regs[R_R14] = env->regs_page->r14;
> +env->regs[R_R15] = env->regs_page->r15;
> +
> +env->eip = env->regs_page->rip;
> +env->eflags = env->regs_page->rflags;
> +rflags_to_lflags(env);
> +}
> +
> +static int mshv_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 */
> +env->cr[0] = env->regs_page->cr0;
> +env->cr[3] = env->regs_page->cr3;
> +env->cr[4] = env->regs_page->cr4;
> +env->efer = env->regs_page->efer;
> +cpu_set_apic_tpr(x86cpu->apic_state, env->regs_page->cr8);
> +
> +/* Segment Registers - copy from packed struct to avoid unaligned access 
> */
> +memcpy(&seg, &env->regs_page->es, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_ES]);
> +memcpy(&seg, &env->regs_page->cs, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_CS]);
> +memcpy(&seg, &env->regs_page->ss, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_SS]);
> +memcpy(&seg, &env->regs_page->ds, sizeof(hv_x64_segment_register));
> +populate_segment_reg(&seg, &env->segs[R_DS]);
> +memcpy(&seg, &env->regs_page->fs, sizeof(hv_x64_segment_register));
> +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]);

It’s probably best to split this function.

A “regular” MMIO access is going to need segment info, but not GDTR/LDTR.

Otherwise you’d effectively defeat the point of the shared register page. 
Ideally
0 register fetch/writes outside of the shared register page would happen on an 
MMIO
access.

> +
> +/* 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 = 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 */
> +popul