target/i386/mshv: Fix segment regression in MMIO emu

[PATCH] target/i386/mshv: Fix segment regression in MMIO emu

Posted by Magnus Kulke 1 day, 1 hour ago

When the segmentation code has been reworked, there is now an
unconditional call to emul_ops->read_segment_descriptor(). The MSHV impl
was delegating this to x86_read_segement_descriptor(), which read from
the GDT in guest memory. This fails for selector.idx == 0 and when no
GDT is set up (which is the case in real mode).

In the fix we change the MSHV impl to fill segment descriptor from
SegmentCache, that was populated from the hypervisor by mshv_load_regs()
before instruction emulation.

Fixes: 09442d98ab (target/i386: emulate: segmentation rework)

Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
---
 target/i386/mshv/mshv-cpu.c | 39 ++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index 2bc978deb2..4ed6e7548f 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -1552,17 +1552,42 @@ static void read_segment_descriptor(CPUState *cpu,
                                     struct x86_segment_descriptor *desc,
                                     enum X86Seg seg_idx)
 {
-    bool ret;
     X86CPU *x86_cpu = X86_CPU(cpu);
     CPUX86State *env = &x86_cpu->env;
     SegmentCache *seg = &env->segs[seg_idx];
-    x86_segment_selector sel = { .sel = seg->selector & 0xFFFF };
-
-    ret = x86_read_segment_descriptor(cpu, desc, sel);
-    if (ret == false) {
-        error_report("failed to read segment descriptor");
-        abort();
+    uint32_t limit;
+
+    memset(desc, 0, sizeof(struct x86_segment_descriptor));
+
+    desc->type = (seg->flags & DESC_TYPE_MASK) >> DESC_TYPE_SHIFT;
+    desc->s    = (seg->flags & DESC_S_MASK)    >> DESC_S_SHIFT;
+    desc->dpl  = (seg->flags & DESC_DPL_MASK)  >> DESC_DPL_SHIFT;
+    desc->p    = (seg->flags & DESC_P_MASK)    >> DESC_P_SHIFT;
+    desc->avl  = (seg->flags & DESC_AVL_MASK)  >> DESC_AVL_SHIFT;
+    desc->l    = (seg->flags & DESC_L_MASK)    >> DESC_L_SHIFT;
+    desc->db   = (seg->flags & DESC_B_MASK)    >> DESC_B_SHIFT;
+    desc->g    = (seg->flags & DESC_G_MASK)    >> DESC_G_SHIFT;
+
+    /*
+     * SegmentCache stores the hypervisor-provided value verbatim (populated by
+     * mshv_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
+     * (4k granularity) and shift the limit accordingly.
+     *
+     * This quirk has been adopted from "whpx_segment_to_x86_description()"
+     */
+
+    if (!desc->g && seg->limit <= 0xfffff) {
+        limit = seg->limit;
+    } else {
+        limit = seg->limit >> 12;
+        desc->g = 1;
     }
+
+    x86_set_segment_limit(desc, limit);
+    x86_set_segment_base(desc, seg->base);
 }
 
 static const struct x86_emul_ops mshv_x86_emul_ops = {
-- 
2.34.1

Re: [PATCH] target/i386/mshv: Fix segment regression in MMIO emu

Posted by Paolo Bonzini 3 hours ago

Queued, thanks.

Paolo

Re: [PATCH] target/i386/mshv: Fix segment regression in MMIO emu

Posted by Mohamed Mediouni 23 hours ago


> On 10. Apr 2026, at 16:26, Magnus Kulke <magnuskulke@linux.microsoft.com> wrote:
> 
> When the segmentation code has been reworked, there is now an
> unconditional call to emul_ops->read_segment_descriptor(). The MSHV impl
> was delegating this to x86_read_segement_descriptor(), which read from
> the GDT in guest memory. This fails for selector.idx == 0 and when no
> GDT is set up (which is the case in real mode).
> 
> In the fix we change the MSHV impl to fill segment descriptor from
> SegmentCache, that was populated from the hypervisor by mshv_load_regs()
> before instruction emulation.
> 
> Fixes: 09442d98ab (target/i386: emulate: segmentation rework)
> 
> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr>

> ---
> target/i386/mshv/mshv-cpu.c | 39 ++++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
> index 2bc978deb2..4ed6e7548f 100644
> --- a/target/i386/mshv/mshv-cpu.c
> +++ b/target/i386/mshv/mshv-cpu.c
> @@ -1552,17 +1552,42 @@ static void read_segment_descriptor(CPUState *cpu,
>                                     struct x86_segment_descriptor *desc,
>                                     enum X86Seg seg_idx)
> {
> -    bool ret;
>     X86CPU *x86_cpu = X86_CPU(cpu);
>     CPUX86State *env = &x86_cpu->env;
>     SegmentCache *seg = &env->segs[seg_idx];
> -    x86_segment_selector sel = { .sel = seg->selector & 0xFFFF };
> -
> -    ret = x86_read_segment_descriptor(cpu, desc, sel);
> -    if (ret == false) {
> -        error_report("failed to read segment descriptor");
> -        abort();
> +    uint32_t limit;
> +
> +    memset(desc, 0, sizeof(struct x86_segment_descriptor));
> +
> +    desc->type = (seg->flags & DESC_TYPE_MASK) >> DESC_TYPE_SHIFT;
> +    desc->s    = (seg->flags & DESC_S_MASK)    >> DESC_S_SHIFT;
> +    desc->dpl  = (seg->flags & DESC_DPL_MASK)  >> DESC_DPL_SHIFT;
> +    desc->p    = (seg->flags & DESC_P_MASK)    >> DESC_P_SHIFT;
> +    desc->avl  = (seg->flags & DESC_AVL_MASK)  >> DESC_AVL_SHIFT;
> +    desc->l    = (seg->flags & DESC_L_MASK)    >> DESC_L_SHIFT;
> +    desc->db   = (seg->flags & DESC_B_MASK)    >> DESC_B_SHIFT;
> +    desc->g    = (seg->flags & DESC_G_MASK)    >> DESC_G_SHIFT;
> +
> +    /*
> +     * SegmentCache stores the hypervisor-provided value verbatim (populated by
> +     * mshv_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
> +     * (4k granularity) and shift the limit accordingly.
> +     *
> +     * This quirk has been adopted from "whpx_segment_to_x86_description()"
> +     */
> +
> +    if (!desc->g && seg->limit <= 0xfffff) {
> +        limit = seg->limit;
> +    } else {
> +        limit = seg->limit >> 12;
> +        desc->g = 1;
>     }
> +
> +    x86_set_segment_limit(desc, limit);
> +    x86_set_segment_base(desc, seg->base);
> }
> 
> static const struct x86_emul_ops mshv_x86_emul_ops = {
> -- 
> 2.34.1
> 
>