[RFC PATCH 17/25] target/i386/mshv: Implement mshv_get_special_regs()

Magnus Kulke posted 25 patches 5 months, 4 weeks ago
There is a newer version of this series
[RFC PATCH 17/25] target/i386/mshv: Implement mshv_get_special_regs()
Posted by Magnus Kulke 5 months, 4 weeks ago
Retrieve special registers (e.g. segment, control, and descriptor
table registers) from MSHV vCPUs.

Various helper functions to map register state representations between
Qemu and MSHV are introduced.

Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
---
 include/system/mshv.h       |   1 +
 target/i386/mshv/mshv-cpu.c | 118 +++++++++++++++++++++++++++++++++++-
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/include/system/mshv.h b/include/system/mshv.h
index 9b78b66a24..055489a6f3 100644
--- a/include/system/mshv.h
+++ b/include/system/mshv.h
@@ -109,6 +109,7 @@ void mshv_init_cpu_logic(void);
 int mshv_create_vcpu(int vm_fd, uint8_t vp_index, int *cpu_fd);
 void mshv_remove_vcpu(int vm_fd, int cpu_fd);
 int mshv_get_standard_regs(CPUState *cpu);
+int mshv_get_special_regs(CPUState *cpu);
 int mshv_run_vcpu(int vm_fd, CPUState *cpu, hv_message *msg, MshvVmExit *exit);
 int mshv_load_regs(CPUState *cpu);
 int mshv_store_regs(CPUState *cpu);
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index 41584c3f8e..979ee5b8c3 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -58,6 +58,27 @@ static enum hv_register_name STANDARD_REGISTER_NAMES[18] = {
     HV_X64_REGISTER_RFLAGS,
 };
 
+static enum hv_register_name SPECIAL_REGISTER_NAMES[18] = {
+    HV_X64_REGISTER_CS,
+    HV_X64_REGISTER_DS,
+    HV_X64_REGISTER_ES,
+    HV_X64_REGISTER_FS,
+    HV_X64_REGISTER_GS,
+    HV_X64_REGISTER_SS,
+    HV_X64_REGISTER_TR,
+    HV_X64_REGISTER_LDTR,
+    HV_X64_REGISTER_GDTR,
+    HV_X64_REGISTER_IDTR,
+    HV_X64_REGISTER_CR0,
+    HV_X64_REGISTER_CR2,
+    HV_X64_REGISTER_CR3,
+    HV_X64_REGISTER_CR4,
+    HV_X64_REGISTER_CR8,
+    HV_X64_REGISTER_EFER,
+    HV_X64_REGISTER_APIC_BASE,
+    HV_REGISTER_PENDING_INTERRUPTION,
+};
+
 static void add_cpu_guard(int cpu_fd)
 {
     QemuMutex *guard;
@@ -215,6 +236,94 @@ int mshv_get_standard_regs(CPUState *cpu)
     return 0;
 }
 
+static void populate_segment_reg(const hv_x64_segment_register *hv_seg,
+                                 SegmentCache *seg)
+{
+    memset(seg, 0, sizeof(SegmentCache));
+
+    seg->base = hv_seg->base;
+    seg->limit = hv_seg->limit;
+    seg->selector = hv_seg->selector;
+
+    seg->flags = (hv_seg->segment_type << DESC_TYPE_SHIFT)
+                 | (hv_seg->present * DESC_P_MASK)
+                 | (hv_seg->descriptor_privilege_level << DESC_DPL_SHIFT)
+                 | (hv_seg->_default << DESC_B_SHIFT)
+                 | (hv_seg->non_system_segment * DESC_S_MASK)
+                 | (hv_seg->_long << DESC_L_SHIFT)
+                 | (hv_seg->granularity * DESC_G_MASK)
+                 | (hv_seg->available * DESC_AVL_MASK);
+
+}
+
+static void populate_table_reg(const hv_x64_table_register *hv_seg,
+                               SegmentCache *tbl)
+{
+    memset(tbl, 0, sizeof(SegmentCache));
+
+    tbl->base = hv_seg->base;
+    tbl->limit = hv_seg->limit;
+}
+
+static void populate_special_regs(const hv_register_assoc *assocs,
+                                  X86CPU *x86cpu)
+{
+    CPUX86State *env = &x86cpu->env;
+
+    populate_segment_reg(&assocs[0].value.segment, &env->segs[R_CS]);
+    populate_segment_reg(&assocs[1].value.segment, &env->segs[R_DS]);
+    populate_segment_reg(&assocs[2].value.segment, &env->segs[R_ES]);
+    populate_segment_reg(&assocs[3].value.segment, &env->segs[R_FS]);
+    populate_segment_reg(&assocs[4].value.segment, &env->segs[R_GS]);
+    populate_segment_reg(&assocs[5].value.segment, &env->segs[R_SS]);
+
+    /* TODO: should we set TR + LDT? */
+    /* populate_segment_reg(&assocs[6].value.segment, &regs->tr); */
+    /* populate_segment_reg(&assocs[7].value.segment, &regs->ldt); */
+
+    populate_table_reg(&assocs[8].value.table, &env->gdt);
+    populate_table_reg(&assocs[9].value.table, &env->idt);
+
+    env->cr[0] = assocs[10].value.reg64;
+    env->cr[2] = assocs[11].value.reg64;
+    env->cr[3] = assocs[12].value.reg64;
+    env->cr[4] = assocs[13].value.reg64;
+
+    cpu_set_apic_tpr(x86cpu->apic_state, assocs[14].value.reg64);
+    env->efer = assocs[15].value.reg64;
+    cpu_set_apic_base(x86cpu->apic_state, assocs[16].value.reg64);
+
+    /* TODO: should we set those? */
+    /* pending_reg = assocs[17].value.pending_interruption.as_uint64; */
+    /* populate_interrupt_bitmap(pending_reg, regs->interrupt_bitmap); */
+}
+
+
+int mshv_get_special_regs(CPUState *cpu)
+{
+    size_t n_regs = sizeof(SPECIAL_REGISTER_NAMES) / sizeof(hv_register_name);
+    struct hv_register_assoc *assocs;
+    int ret;
+    X86CPU *x86cpu = X86_CPU(cpu);
+    int cpu_fd = mshv_vcpufd(cpu);
+
+    assocs = g_new0(hv_register_assoc, n_regs);
+    for (size_t i = 0; i < n_regs; i++) {
+        assocs[i].name = SPECIAL_REGISTER_NAMES[i];
+    }
+    ret = get_generic_regs(cpu_fd, assocs, n_regs);
+    if (ret < 0) {
+        error_report("failed to get special registers");
+        g_free(assocs);
+        return -errno;
+    }
+
+    populate_special_regs(assocs, x86cpu);
+
+    g_free(assocs);
+    return 0;
+}
+
 int mshv_load_regs(CPUState *cpu)
 {
     int ret;
@@ -225,8 +334,13 @@ int mshv_load_regs(CPUState *cpu)
         return -1;
     }
 
-	error_report("unimplemented");
-	abort();
+    ret = mshv_get_special_regs(cpu);
+    if (ret < 0) {
+        error_report("Failed to load special registers");
+        return -1;
+    }
+
+    return 0;
 }
 
 int mshv_arch_put_registers(const CPUState *cpu)
-- 
2.34.1
Re: [RFC PATCH 17/25] target/i386/mshv: Implement mshv_get_special_regs()
Posted by Wei Liu 5 months, 4 weeks ago
On Tue, May 20, 2025 at 01:30:10PM +0200, Magnus Kulke wrote:
> Retrieve special registers (e.g. segment, control, and descriptor
> table registers) from MSHV vCPUs.
> 
> Various helper functions to map register state representations between
> Qemu and MSHV are introduced.
> 
> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
> ---
>  include/system/mshv.h       |   1 +
>  target/i386/mshv/mshv-cpu.c | 118 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 117 insertions(+), 2 deletions(-)
> 
> diff --git a/include/system/mshv.h b/include/system/mshv.h
> index 9b78b66a24..055489a6f3 100644
> --- a/include/system/mshv.h
> +++ b/include/system/mshv.h
> @@ -109,6 +109,7 @@ void mshv_init_cpu_logic(void);
>  int mshv_create_vcpu(int vm_fd, uint8_t vp_index, int *cpu_fd);
>  void mshv_remove_vcpu(int vm_fd, int cpu_fd);
>  int mshv_get_standard_regs(CPUState *cpu);
> +int mshv_get_special_regs(CPUState *cpu);
>  int mshv_run_vcpu(int vm_fd, CPUState *cpu, hv_message *msg, MshvVmExit *exit);
>  int mshv_load_regs(CPUState *cpu);
>  int mshv_store_regs(CPUState *cpu);
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 41584c3f8e..979ee5b8c3 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -58,6 +58,27 @@ static enum hv_register_name STANDARD_REGISTER_NAMES[18] = {
>      HV_X64_REGISTER_RFLAGS,
>  };
>  
> +static enum hv_register_name SPECIAL_REGISTER_NAMES[18] = {
[...]
> +    HV_REGISTER_PENDING_INTERRUPTION,

Why do you think this is needed?

> +};
> +
>  static void add_cpu_guard(int cpu_fd)
>  {
>      QemuMutex *guard;
> @@ -215,6 +236,94 @@ int mshv_get_standard_regs(CPUState *cpu)
>      return 0;
>  }
>  
> +static void populate_segment_reg(const hv_x64_segment_register *hv_seg,
> +                                 SegmentCache *seg)
> +{
> +    memset(seg, 0, sizeof(SegmentCache));
> +
> +    seg->base = hv_seg->base;
> +    seg->limit = hv_seg->limit;
> +    seg->selector = hv_seg->selector;
> +
> +    seg->flags = (hv_seg->segment_type << DESC_TYPE_SHIFT)
> +                 | (hv_seg->present * DESC_P_MASK)
> +                 | (hv_seg->descriptor_privilege_level << DESC_DPL_SHIFT)
> +                 | (hv_seg->_default << DESC_B_SHIFT)
> +                 | (hv_seg->non_system_segment * DESC_S_MASK)
> +                 | (hv_seg->_long << DESC_L_SHIFT)
> +                 | (hv_seg->granularity * DESC_G_MASK)
> +                 | (hv_seg->available * DESC_AVL_MASK);
> +
> +}
> +
> +static void populate_table_reg(const hv_x64_table_register *hv_seg,
> +                               SegmentCache *tbl)
> +{
> +    memset(tbl, 0, sizeof(SegmentCache));
> +
> +    tbl->base = hv_seg->base;
> +    tbl->limit = hv_seg->limit;
> +}
> +
> +static void populate_special_regs(const hv_register_assoc *assocs,
> +                                  X86CPU *x86cpu)
> +{
> +    CPUX86State *env = &x86cpu->env;
> +
> +    populate_segment_reg(&assocs[0].value.segment, &env->segs[R_CS]);
> +    populate_segment_reg(&assocs[1].value.segment, &env->segs[R_DS]);
> +    populate_segment_reg(&assocs[2].value.segment, &env->segs[R_ES]);
> +    populate_segment_reg(&assocs[3].value.segment, &env->segs[R_FS]);
> +    populate_segment_reg(&assocs[4].value.segment, &env->segs[R_GS]);
> +    populate_segment_reg(&assocs[5].value.segment, &env->segs[R_SS]);
> +
> +    /* TODO: should we set TR + LDT? */
> +    /* populate_segment_reg(&assocs[6].value.segment, &regs->tr); */
> +    /* populate_segment_reg(&assocs[7].value.segment, &regs->ldt); */
> +
> +    populate_table_reg(&assocs[8].value.table, &env->gdt);
> +    populate_table_reg(&assocs[9].value.table, &env->idt);
> +
> +    env->cr[0] = assocs[10].value.reg64;
> +    env->cr[2] = assocs[11].value.reg64;
> +    env->cr[3] = assocs[12].value.reg64;
> +    env->cr[4] = assocs[13].value.reg64;
> +
> +    cpu_set_apic_tpr(x86cpu->apic_state, assocs[14].value.reg64);
> +    env->efer = assocs[15].value.reg64;
> +    cpu_set_apic_base(x86cpu->apic_state, assocs[16].value.reg64);
> +
> +    /* TODO: should we set those? */
> +    /* pending_reg = assocs[17].value.pending_interruption.as_uint64; */
> +    /* populate_interrupt_bitmap(pending_reg, regs->interrupt_bitmap); */

If QEMU never touches it, then there is no need to set it.

> +}
> +
> +
> +int mshv_get_special_regs(CPUState *cpu)
> +{
> +    size_t n_regs = sizeof(SPECIAL_REGISTER_NAMES) / sizeof(hv_register_name);
> +    struct hv_register_assoc *assocs;
> +    int ret;
> +    X86CPU *x86cpu = X86_CPU(cpu);
> +    int cpu_fd = mshv_vcpufd(cpu);
> +
> +    assocs = g_new0(hv_register_assoc, n_regs);
> +    for (size_t i = 0; i < n_regs; i++) {
> +        assocs[i].name = SPECIAL_REGISTER_NAMES[i];
> +    }
> +    ret = get_generic_regs(cpu_fd, assocs, n_regs);
> +    if (ret < 0) {
> +        error_report("failed to get special registers");
> +        g_free(assocs);
> +        return -errno;
> +    }
> +
> +    populate_special_regs(assocs, x86cpu);
> +
> +    g_free(assocs);
> +    return 0;
> +}
> +
>  int mshv_load_regs(CPUState *cpu)
>  {
>      int ret;
> @@ -225,8 +334,13 @@ int mshv_load_regs(CPUState *cpu)
>          return -1;
>      }
>  
> -	error_report("unimplemented");
> -	abort();
> +    ret = mshv_get_special_regs(cpu);
> +    if (ret < 0) {
> +        error_report("Failed to load special registers");
> +        return -1;
> +    }
> +
> +    return 0;

Ah so you changed the code in this patch.

Thanks,
Wei.

>  }
>  
>  int mshv_arch_put_registers(const CPUState *cpu)
> -- 
> 2.34.1
>
Re: [RFC PATCH 17/25] target/i386/mshv: Implement mshv_get_special_regs()
Posted by Magnus Kulke 5 months, 3 weeks ago
On Tue, May 20, 2025 at 10:15:23PM +0000, Wei Liu wrote:
> On Tue, May 20, 2025 at 01:30:10PM +0200, Magnus Kulke wrote:
> >  
> > +static enum hv_register_name SPECIAL_REGISTER_NAMES[18] = {
> [...]
> > +    HV_REGISTER_PENDING_INTERRUPTION,
> 
> Why do you think this is needed?
> 

It's not, that's a leftover. we can remove it, since we don't use it in
QEMU currently.
Re: [RFC PATCH 17/25] target/i386/mshv: Implement mshv_get_special_regs()
Posted by Paolo Bonzini 5 months, 4 weeks ago
On 5/20/25 13:30, Magnus Kulke wrote:
> +static void populate_special_regs(const hv_register_assoc *assocs,
> +                                  X86CPU *x86cpu)
> +{
> +    CPUX86State *env = &x86cpu->env;
> +
> +    populate_segment_reg(&assocs[0].value.segment, &env->segs[R_CS]);
> +    populate_segment_reg(&assocs[1].value.segment, &env->segs[R_DS]);
> +    populate_segment_reg(&assocs[2].value.segment, &env->segs[R_ES]);
> +    populate_segment_reg(&assocs[3].value.segment, &env->segs[R_FS]);
> +    populate_segment_reg(&assocs[4].value.segment, &env->segs[R_GS]);
> +    populate_segment_reg(&assocs[5].value.segment, &env->segs[R_SS]);
> +
> +    /* TODO: should we set TR + LDT? */
> +    /* populate_segment_reg(&assocs[6].value.segment, &regs->tr); */
> +    /* populate_segment_reg(&assocs[7].value.segment, &regs->ldt); */

Yes :)

Paolo