[PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation

Magnus Kulke posted 27 patches 1 month, 4 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Magnus Kulke <magnus.kulke@linux.microsoft.com>, Wei Liu <wei.liu@kernel.org>, "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Dr. David Alan Gilbert" <dave@treblig.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eric Blake <eblake@redhat.com>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>
There is a newer version of this series
[PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Magnus Kulke 1 month, 4 weeks ago
The MSHV accelerator requires a x86 decoder/emulator in userland to
emulate MMIO instructions. This change contains the implementations for
the generalized i386 instruction decoder/emulator.

Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
---
 include/system/mshv.h           |  25 +++
 target/i386/cpu.h               |   2 +-
 target/i386/emulate/meson.build |   7 +-
 target/i386/meson.build         |   2 +
 target/i386/mshv/meson.build    |   7 +
 target/i386/mshv/x86.c          | 297 ++++++++++++++++++++++++++++++++
 6 files changed, 337 insertions(+), 3 deletions(-)
 create mode 100644 include/system/mshv.h
 create mode 100644 target/i386/mshv/meson.build
 create mode 100644 target/i386/mshv/x86.c

diff --git a/include/system/mshv.h b/include/system/mshv.h
new file mode 100644
index 0000000000..a971982b52
--- /dev/null
+++ b/include/system/mshv.h
@@ -0,0 +1,25 @@
+/*
+ * QEMU MSHV support
+ *
+ * Copyright Microsoft, Corp. 2025
+ *
+ * Authors: Ziqiao Zhou  <ziqiaozhou@microsoft.com>
+ *          Magnus Kulke <magnuskulke@microsoft.com>
+ *          Jinank Jain  <jinankjain@microsoft.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ */
+
+#ifndef QEMU_MSHV_INT_H
+#define QEMU_MSHV_INT_H
+
+#ifdef COMPILING_PER_TARGET
+#ifdef CONFIG_MSHV
+#define CONFIG_MSHV_IS_POSSIBLE
+#endif
+#else
+#define CONFIG_MSHV_IS_POSSIBLE
+#endif
+
+#endif
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f977fc49a7..6d3d2b1440 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2126,7 +2126,7 @@ typedef struct CPUArchState {
     QEMUTimer *xen_periodic_timer;
     QemuMutex xen_timers_lock;
 #endif
-#if defined(CONFIG_HVF)
+#if defined(CONFIG_HVF) || defined(CONFIG_MSHV)
     void *emu_mmio_buf;
 #endif
 
diff --git a/target/i386/emulate/meson.build b/target/i386/emulate/meson.build
index 4edd4f462f..b6dafb6a5b 100644
--- a/target/i386/emulate/meson.build
+++ b/target/i386/emulate/meson.build
@@ -1,5 +1,8 @@
-i386_system_ss.add(when: [hvf, 'CONFIG_HVF'], if_true: files(
+emulator_files = files(
   'x86_decode.c',
   'x86_emu.c',
   'x86_flags.c',
-))
+)
+
+i386_system_ss.add(when: [hvf, 'CONFIG_HVF'], if_true: emulator_files)
+i386_system_ss.add(when: 'CONFIG_MSHV', if_true: emulator_files)
diff --git a/target/i386/meson.build b/target/i386/meson.build
index 092af34e2d..89ba4912aa 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -13,6 +13,7 @@ i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
 i386_ss.add(when: 'CONFIG_HVF', if_true: files('host-cpu.c'))
 i386_ss.add(when: 'CONFIG_WHPX', if_true: files('host-cpu.c'))
 i386_ss.add(when: 'CONFIG_NVMM', if_true: files('host-cpu.c'))
+i386_ss.add(when: 'CONFIG_MSHV', if_true: files('host-cpu.c'))
 
 i386_system_ss = ss.source_set()
 i386_system_ss.add(files(
@@ -34,6 +35,7 @@ subdir('nvmm')
 subdir('hvf')
 subdir('tcg')
 subdir('emulate')
+subdir('mshv')
 
 target_arch += {'i386': i386_ss}
 target_system_arch += {'i386': i386_system_ss}
diff --git a/target/i386/mshv/meson.build b/target/i386/mshv/meson.build
new file mode 100644
index 0000000000..8ddaa7c11d
--- /dev/null
+++ b/target/i386/mshv/meson.build
@@ -0,0 +1,7 @@
+i386_mshv_ss = ss.source_set()
+
+i386_mshv_ss.add(files(
+  'x86.c',
+))
+
+i386_system_ss.add_all(when: 'CONFIG_MSHV', if_true: i386_mshv_ss)
diff --git a/target/i386/mshv/x86.c b/target/i386/mshv/x86.c
new file mode 100644
index 0000000000..d574b3bc52
--- /dev/null
+++ b/target/i386/mshv/x86.c
@@ -0,0 +1,297 @@
+/*
+ * QEMU MSHV support
+ *
+ * Copyright Microsoft, Corp. 2025
+ *
+ * Authors: Magnus Kulke <magnuskulke@microsoft.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+
+#include "cpu.h"
+#include "emulate/x86_decode.h"
+#include "emulate/x86_emu.h"
+#include "qemu/typedefs.h"
+#include "qemu/error-report.h"
+#include "system/mshv.h"
+
+/* RW or Exec segment */
+static const uint8_t RWRX_SEGMENT_TYPE        = 0x2;
+static const uint8_t CODE_SEGMENT_TYPE        = 0x8;
+static const uint8_t EXPAND_DOWN_SEGMENT_TYPE = 0x4;
+
+typedef enum CpuMode {
+    REAL_MODE,
+    PROTECTED_MODE,
+    LONG_MODE,
+} CpuMode;
+
+static CpuMode cpu_mode(CPUState *cpu)
+{
+    enum CpuMode m = REAL_MODE;
+
+    if (x86_is_protected(cpu)) {
+        m = PROTECTED_MODE;
+
+        if (x86_is_long_mode(cpu)) {
+            m = LONG_MODE;
+        }
+    }
+
+    return m;
+}
+
+static bool segment_type_ro(const SegmentCache *seg)
+{
+    uint32_t type_ = (seg->flags >> DESC_TYPE_SHIFT) & 15;
+    return (type_ & (~RWRX_SEGMENT_TYPE)) == 0;
+}
+
+static bool segment_type_code(const SegmentCache *seg)
+{
+    uint32_t type_ = (seg->flags >> DESC_TYPE_SHIFT) & 15;
+    return (type_ & CODE_SEGMENT_TYPE) != 0;
+}
+
+static bool segment_expands_down(const SegmentCache *seg)
+{
+    uint32_t type_ = (seg->flags >> DESC_TYPE_SHIFT) & 15;
+
+    if (segment_type_code(seg)) {
+        return false;
+    }
+
+    return (type_ & EXPAND_DOWN_SEGMENT_TYPE) != 0;
+}
+
+static uint32_t segment_limit(const SegmentCache *seg)
+{
+    uint32_t limit = seg->limit;
+    uint32_t granularity = (seg->flags & DESC_G_MASK) != 0;
+
+    if (granularity != 0) {
+        limit = (limit << 12) | 0xFFF;
+    }
+
+    return limit;
+}
+
+static uint8_t segment_db(const SegmentCache *seg)
+{
+    return (seg->flags >> DESC_B_SHIFT) & 1;
+}
+
+static uint32_t segment_max_limit(const SegmentCache *seg)
+{
+    if (segment_db(seg) != 0) {
+        return 0xFFFFFFFF;
+    }
+    return 0xFFFF;
+}
+
+static int linearize(CPUState *cpu,
+                     target_ulong logical_addr, target_ulong *linear_addr,
+                     X86Seg seg_idx)
+{
+    enum CpuMode mode;
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    SegmentCache *seg = &env->segs[seg_idx];
+    target_ulong base = seg->base;
+    target_ulong logical_addr_32b;
+    uint32_t limit;
+    /* TODO: the emulator will not pass us "write" indicator yet */
+    bool write = false;
+
+    mode = cpu_mode(cpu);
+
+    switch (mode) {
+    case LONG_MODE:
+        if (__builtin_add_overflow(logical_addr, base, linear_addr)) {
+            error_report("Address overflow");
+            return -1;
+        }
+        break;
+    case PROTECTED_MODE:
+    case REAL_MODE:
+        if (segment_type_ro(seg) && write) {
+            error_report("Cannot write to read-only segment");
+            return -1;
+        }
+
+        logical_addr_32b = logical_addr & 0xFFFFFFFF;
+        limit = segment_limit(seg);
+
+        if (segment_expands_down(seg)) {
+            if (logical_addr_32b >= limit) {
+                error_report("Address exceeds limit (expands down)");
+                return -1;
+            }
+
+            limit = segment_max_limit(seg);
+        }
+
+        if (logical_addr_32b > limit) {
+            error_report("Address exceeds limit %u", limit);
+            return -1;
+        }
+        *linear_addr = logical_addr_32b + base;
+        break;
+    default:
+        error_report("Unknown cpu mode: %d", mode);
+        return -1;
+    }
+
+    return 0;
+}
+
+bool x86_read_segment_descriptor(CPUState *cpu,
+                                 struct x86_segment_descriptor *desc,
+                                 x86_segment_selector sel)
+{
+    target_ulong base;
+    uint32_t limit;
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    target_ulong gva;
+
+    memset(desc, 0, sizeof(*desc));
+
+    /* valid gdt descriptors start from index 1 */
+    if (!sel.index && GDT_SEL == sel.ti) {
+        return false;
+    }
+
+    if (GDT_SEL == sel.ti) {
+        base = env->gdt.base;
+        limit = env->gdt.limit;
+    } else {
+        base = env->ldt.base;
+        limit = env->ldt.limit;
+    }
+
+    if (sel.index * 8 >= limit) {
+        return false;
+    }
+
+    gva = base + sel.index * 8;
+    emul_ops->read_mem(cpu, desc, gva, sizeof(*desc));
+
+    return true;
+}
+
+bool x86_read_call_gate(CPUState *cpu, struct x86_call_gate *idt_desc,
+                        int gate)
+{
+    target_ulong base;
+    uint32_t limit;
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    target_ulong gva;
+
+    base = env->idt.base;
+    limit = env->idt.limit;
+
+    memset(idt_desc, 0, sizeof(*idt_desc));
+    if (gate * 8 >= limit) {
+        perror("call gate exceeds idt limit");
+        return false;
+    }
+
+    gva = base + gate * 8;
+    emul_ops->read_mem(cpu, idt_desc, gva, sizeof(*idt_desc));
+
+    return true;
+}
+
+bool x86_is_protected(CPUState *cpu)
+{
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    uint64_t cr0 = env->cr[0];
+
+    return cr0 & CR0_PE_MASK;
+}
+
+bool x86_is_real(CPUState *cpu)
+{
+    return !x86_is_protected(cpu);
+}
+
+bool x86_is_v8086(CPUState *cpu)
+{
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    return x86_is_protected(cpu) && (env->eflags & VM_MASK);
+}
+
+bool x86_is_long_mode(CPUState *cpu)
+{
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    uint64_t efer = env->efer;
+    uint64_t lme_lma = (MSR_EFER_LME | MSR_EFER_LMA);
+
+    return ((efer & lme_lma) == lme_lma);
+}
+
+bool x86_is_long64_mode(CPUState *cpu)
+{
+    error_report("unimplemented: is_long64_mode()");
+    abort();
+}
+
+bool x86_is_paging_mode(CPUState *cpu)
+{
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    uint64_t cr0 = env->cr[0];
+
+    return cr0 & CR0_PG_MASK;
+}
+
+bool x86_is_pae_enabled(CPUState *cpu)
+{
+    X86CPU *x86_cpu = X86_CPU(cpu);
+    CPUX86State *env = &x86_cpu->env;
+    uint64_t cr4 = env->cr[4];
+
+    return cr4 & CR4_PAE_MASK;
+}
+
+target_ulong linear_addr(CPUState *cpu, target_ulong addr, X86Seg seg)
+{
+    int ret;
+    target_ulong linear_addr;
+
+    ret = linearize(cpu, addr, &linear_addr, seg);
+    if (ret < 0) {
+        error_report("failed to linearize address");
+        abort();
+    }
+
+    return linear_addr;
+}
+
+target_ulong linear_addr_size(CPUState *cpu, target_ulong addr, int size,
+                              X86Seg seg)
+{
+    switch (size) {
+    case 2:
+        addr = (uint16_t)addr;
+        break;
+    case 4:
+        addr = (uint32_t)addr;
+        break;
+    default:
+        break;
+    }
+    return linear_addr(cpu, addr, seg);
+}
+
+target_ulong linear_rip(CPUState *cpu, target_ulong rip)
+{
+    return linear_addr(cpu, rip, R_CS);
+}
-- 
2.34.1
Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Daniel P. Berrangé 1 month, 2 weeks ago
On Tue, Sep 16, 2025 at 06:48:23PM +0200, Magnus Kulke wrote:
> The MSHV accelerator requires a x86 decoder/emulator in userland to
> emulate MMIO instructions. This change contains the implementations for
> the generalized i386 instruction decoder/emulator.
> 
> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
> ---
>  include/system/mshv.h           |  25 +++
>  target/i386/cpu.h               |   2 +-
>  target/i386/emulate/meson.build |   7 +-
>  target/i386/meson.build         |   2 +
>  target/i386/mshv/meson.build    |   7 +
>  target/i386/mshv/x86.c          | 297 ++++++++++++++++++++++++++++++++
>  6 files changed, 337 insertions(+), 3 deletions(-)
>  create mode 100644 include/system/mshv.h
>  create mode 100644 target/i386/mshv/meson.build
>  create mode 100644 target/i386/mshv/x86.c
> 

> diff --git a/target/i386/mshv/meson.build b/target/i386/mshv/meson.build
> new file mode 100644
> index 0000000000..8ddaa7c11d
> --- /dev/null
> +++ b/target/i386/mshv/meson.build
> @@ -0,0 +1,7 @@
> +i386_mshv_ss = ss.source_set()
> +
> +i386_mshv_ss.add(files(
> +  'x86.c',
> +))
> +
> +i386_system_ss.add_all(when: 'CONFIG_MSHV', if_true: i386_mshv_ss)

FYI, we expect the SPDX-License-Identifier to be present on
all new files[1] added to git, even if they're relatively simple
in some cases like this meson.build. 

If you run the series though checkpatch.pl it should tell
you which files you've missed.

With regards,
Daniel

[1] exception for non-plain text files, or if there's some reason it
    would cause problems.
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Paolo Bonzini 1 month, 2 weeks ago
On 10/1/25 12:59, Daniel P. Berrangé wrote:
> On Tue, Sep 16, 2025 at 06:48:23PM +0200, Magnus Kulke wrote:
>> The MSHV accelerator requires a x86 decoder/emulator in userland to
>> emulate MMIO instructions. This change contains the implementations for
>> the generalized i386 instruction decoder/emulator.
>>
>> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
>> ---
>>   include/system/mshv.h           |  25 +++
>>   target/i386/cpu.h               |   2 +-
>>   target/i386/emulate/meson.build |   7 +-
>>   target/i386/meson.build         |   2 +
>>   target/i386/mshv/meson.build    |   7 +
>>   target/i386/mshv/x86.c          | 297 ++++++++++++++++++++++++++++++++
>>   6 files changed, 337 insertions(+), 3 deletions(-)
>>   create mode 100644 include/system/mshv.h
>>   create mode 100644 target/i386/mshv/meson.build
>>   create mode 100644 target/i386/mshv/x86.c
>>
> 
>> diff --git a/target/i386/mshv/meson.build b/target/i386/mshv/meson.build
>> new file mode 100644
>> index 0000000000..8ddaa7c11d
>> --- /dev/null
>> +++ b/target/i386/mshv/meson.build
>> @@ -0,0 +1,7 @@
>> +i386_mshv_ss = ss.source_set()
>> +
>> +i386_mshv_ss.add(files(
>> +  'x86.c',
>> +))
>> +
>> +i386_system_ss.add_all(when: 'CONFIG_MSHV', if_true: i386_mshv_ss)
> 
> FYI, we expect the SPDX-License-Identifier to be present on
> all new files[1] added to git, even if they're relatively simple
> in some cases like this meson.build.
> 
> If you run the series though checkpatch.pl it should tell
> you which files you've missed.

I'll handle this when applying.

Paolo


Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Dr. David Alan Gilbert 1 month, 4 weeks ago
* Magnus Kulke (magnuskulke@linux.microsoft.com) wrote:
> The MSHV accelerator requires a x86 decoder/emulator in userland to
> emulate MMIO instructions. This change contains the implementations for
> the generalized i386 instruction decoder/emulator.
> 
> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>

Hi Magnus,
  Just some things I noticed.

> ---
>  include/system/mshv.h           |  25 +++
>  target/i386/cpu.h               |   2 +-
>  target/i386/emulate/meson.build |   7 +-
>  target/i386/meson.build         |   2 +
>  target/i386/mshv/meson.build    |   7 +
>  target/i386/mshv/x86.c          | 297 ++++++++++++++++++++++++++++++++
>  6 files changed, 337 insertions(+), 3 deletions(-)
>  create mode 100644 include/system/mshv.h
>  create mode 100644 target/i386/mshv/meson.build
>  create mode 100644 target/i386/mshv/x86.c
> 

<snip>

> diff --git a/target/i386/mshv/x86.c b/target/i386/mshv/x86.c
> new file mode 100644
> index 0000000000..d574b3bc52
> --- /dev/null
> +++ b/target/i386/mshv/x86.c
> @@ -0,0 +1,297 @@
> +/*
> + * QEMU MSHV support
> + *
> + * Copyright Microsoft, Corp. 2025
> + *
> + * Authors: Magnus Kulke <magnuskulke@microsoft.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "cpu.h"
> +#include "emulate/x86_decode.h"
> +#include "emulate/x86_emu.h"
> +#include "qemu/typedefs.h"
> +#include "qemu/error-report.h"
> +#include "system/mshv.h"
> +
> +/* RW or Exec segment */
> +static const uint8_t RWRX_SEGMENT_TYPE        = 0x2;
> +static const uint8_t CODE_SEGMENT_TYPE        = 0x8;
> +static const uint8_t EXPAND_DOWN_SEGMENT_TYPE = 0x4;
> +
> +typedef enum CpuMode {
> +    REAL_MODE,
> +    PROTECTED_MODE,
> +    LONG_MODE,
> +} CpuMode;

These consts and enum look like things that should be in some
shared x86 thing; in fact there are some existing routines
that partially map to some of this.

For example 'x86_is_real' is declared in target/i386/emulate/x86.h
and defined in target/i386/hvf/x86.c  (hmm that's a bit weird).
So it's probably best to check if what you want already exists,
move it into target/i386 somewhere I guess, and everyone shares it.

(Don't worry, I'm not suggesting you fix all the zillions of places
that open code this type of stuff, but once it's in somewhere shared
that can happen).

> +static CpuMode cpu_mode(CPUState *cpu)
> +{
> +    enum CpuMode m = REAL_MODE;
> +
> +    if (x86_is_protected(cpu)) {
> +        m = PROTECTED_MODE;
> +
> +        if (x86_is_long_mode(cpu)) {
> +            m = LONG_MODE;
> +        }
> +    }
> +
> +    return m;
> +}
> +
> +static bool segment_type_ro(const SegmentCache *seg)
> +{
> +    uint32_t type_ = (seg->flags >> DESC_TYPE_SHIFT) & 15;

A variable ending in '_' is a bit odd for qemu.

Dave

> +    return (type_ & (~RWRX_SEGMENT_TYPE)) == 0;
> +}
> +
> +static bool segment_type_code(const SegmentCache *seg)
> +{
> +    uint32_t type_ = (seg->flags >> DESC_TYPE_SHIFT) & 15;
> +    return (type_ & CODE_SEGMENT_TYPE) != 0;
> +}
> +
> +static bool segment_expands_down(const SegmentCache *seg)
> +{
> +    uint32_t type_ = (seg->flags >> DESC_TYPE_SHIFT) & 15;
> +
> +    if (segment_type_code(seg)) {
> +        return false;
> +    }
> +
> +    return (type_ & EXPAND_DOWN_SEGMENT_TYPE) != 0;
> +}
> +
> +static uint32_t segment_limit(const SegmentCache *seg)
> +{
> +    uint32_t limit = seg->limit;
> +    uint32_t granularity = (seg->flags & DESC_G_MASK) != 0;
> +
> +    if (granularity != 0) {
> +        limit = (limit << 12) | 0xFFF;
> +    }
> +
> +    return limit;
> +}
> +
> +static uint8_t segment_db(const SegmentCache *seg)
> +{
> +    return (seg->flags >> DESC_B_SHIFT) & 1;
> +}
> +
> +static uint32_t segment_max_limit(const SegmentCache *seg)
> +{
> +    if (segment_db(seg) != 0) {
> +        return 0xFFFFFFFF;
> +    }
> +    return 0xFFFF;
> +}
> +
> +static int linearize(CPUState *cpu,
> +                     target_ulong logical_addr, target_ulong *linear_addr,
> +                     X86Seg seg_idx)
> +{
> +    enum CpuMode mode;
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    SegmentCache *seg = &env->segs[seg_idx];
> +    target_ulong base = seg->base;
> +    target_ulong logical_addr_32b;
> +    uint32_t limit;
> +    /* TODO: the emulator will not pass us "write" indicator yet */
> +    bool write = false;
> +
> +    mode = cpu_mode(cpu);
> +
> +    switch (mode) {
> +    case LONG_MODE:
> +        if (__builtin_add_overflow(logical_addr, base, linear_addr)) {
> +            error_report("Address overflow");
> +            return -1;
> +        }
> +        break;
> +    case PROTECTED_MODE:
> +    case REAL_MODE:
> +        if (segment_type_ro(seg) && write) {
> +            error_report("Cannot write to read-only segment");
> +            return -1;
> +        }
> +
> +        logical_addr_32b = logical_addr & 0xFFFFFFFF;
> +        limit = segment_limit(seg);
> +
> +        if (segment_expands_down(seg)) {
> +            if (logical_addr_32b >= limit) {
> +                error_report("Address exceeds limit (expands down)");
> +                return -1;
> +            }
> +
> +            limit = segment_max_limit(seg);
> +        }
> +
> +        if (logical_addr_32b > limit) {
> +            error_report("Address exceeds limit %u", limit);
> +            return -1;
> +        }
> +        *linear_addr = logical_addr_32b + base;
> +        break;
> +    default:
> +        error_report("Unknown cpu mode: %d", mode);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +bool x86_read_segment_descriptor(CPUState *cpu,
> +                                 struct x86_segment_descriptor *desc,
> +                                 x86_segment_selector sel)
> +{
> +    target_ulong base;
> +    uint32_t limit;
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    target_ulong gva;
> +
> +    memset(desc, 0, sizeof(*desc));
> +
> +    /* valid gdt descriptors start from index 1 */
> +    if (!sel.index && GDT_SEL == sel.ti) {
> +        return false;
> +    }
> +
> +    if (GDT_SEL == sel.ti) {
> +        base = env->gdt.base;
> +        limit = env->gdt.limit;
> +    } else {
> +        base = env->ldt.base;
> +        limit = env->ldt.limit;
> +    }
> +
> +    if (sel.index * 8 >= limit) {
> +        return false;
> +    }
> +
> +    gva = base + sel.index * 8;
> +    emul_ops->read_mem(cpu, desc, gva, sizeof(*desc));
> +
> +    return true;
> +}
> +
> +bool x86_read_call_gate(CPUState *cpu, struct x86_call_gate *idt_desc,
> +                        int gate)
> +{
> +    target_ulong base;
> +    uint32_t limit;
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    target_ulong gva;
> +
> +    base = env->idt.base;
> +    limit = env->idt.limit;
> +
> +    memset(idt_desc, 0, sizeof(*idt_desc));
> +    if (gate * 8 >= limit) {
> +        perror("call gate exceeds idt limit");
> +        return false;
> +    }
> +
> +    gva = base + gate * 8;
> +    emul_ops->read_mem(cpu, idt_desc, gva, sizeof(*idt_desc));
> +
> +    return true;
> +}
> +
> +bool x86_is_protected(CPUState *cpu)
> +{
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    uint64_t cr0 = env->cr[0];
> +
> +    return cr0 & CR0_PE_MASK;
> +}
> +
> +bool x86_is_real(CPUState *cpu)
> +{
> +    return !x86_is_protected(cpu);
> +}
> +
> +bool x86_is_v8086(CPUState *cpu)
> +{
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    return x86_is_protected(cpu) && (env->eflags & VM_MASK);
> +}
> +
> +bool x86_is_long_mode(CPUState *cpu)
> +{
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    uint64_t efer = env->efer;
> +    uint64_t lme_lma = (MSR_EFER_LME | MSR_EFER_LMA);
> +
> +    return ((efer & lme_lma) == lme_lma);
> +}
> +
> +bool x86_is_long64_mode(CPUState *cpu)
> +{
> +    error_report("unimplemented: is_long64_mode()");
> +    abort();
> +}
> +
> +bool x86_is_paging_mode(CPUState *cpu)
> +{
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    uint64_t cr0 = env->cr[0];
> +
> +    return cr0 & CR0_PG_MASK;
> +}
> +
> +bool x86_is_pae_enabled(CPUState *cpu)
> +{
> +    X86CPU *x86_cpu = X86_CPU(cpu);
> +    CPUX86State *env = &x86_cpu->env;
> +    uint64_t cr4 = env->cr[4];
> +
> +    return cr4 & CR4_PAE_MASK;
> +}
> +
> +target_ulong linear_addr(CPUState *cpu, target_ulong addr, X86Seg seg)
> +{
> +    int ret;
> +    target_ulong linear_addr;
> +
> +    ret = linearize(cpu, addr, &linear_addr, seg);
> +    if (ret < 0) {
> +        error_report("failed to linearize address");
> +        abort();
> +    }
> +
> +    return linear_addr;
> +}
> +
> +target_ulong linear_addr_size(CPUState *cpu, target_ulong addr, int size,
> +                              X86Seg seg)
> +{
> +    switch (size) {
> +    case 2:
> +        addr = (uint16_t)addr;
> +        break;
> +    case 4:
> +        addr = (uint32_t)addr;
> +        break;
> +    default:
> +        break;
> +    }
> +    return linear_addr(cpu, addr, seg);
> +}
> +
> +target_ulong linear_rip(CPUState *cpu, target_ulong rip)
> +{
> +    return linear_addr(cpu, rip, R_CS);
> +}
> -- 
> 2.34.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/
Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Mohamed Mediouni 1 month, 4 weeks ago

> On 16. Sep 2025, at 19:40, Dr. David Alan Gilbert <dave@treblig.org> wrote:
> 
> For example 'x86_is_real' is declared in target/i386/emulate/x86.h
> and defined in target/i386/hvf/x86.c  (hmm that's a bit weird).
> So it's probably best to check if what you want already exists,
> move it into target/i386 somewhere I guess, and everyone shares it.
Currently there isn’t a backend-agnostic interface for this. 
It was part of the import of HVF support to qemu from downstream.

Notably means that the emulate infrastructure isn’t usable by multiple
backends in the same build. It might be possible to get away without that
however as HVF is macOS specific, MSHV is Linux-specific and WHPX
(which is currently using winhvemulator instead of this) is Windows-specific...
 

Re: [CRM114spam]: Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Dr. David Alan Gilbert 1 month, 4 weeks ago
* Mohamed Mediouni (mohamed@unpredictable.fr) wrote:
> 
> 
> > On 16. Sep 2025, at 19:40, Dr. David Alan Gilbert <dave@treblig.org> wrote:
> > 
> > For example 'x86_is_real' is declared in target/i386/emulate/x86.h
> > and defined in target/i386/hvf/x86.c  (hmm that's a bit weird).
> > So it's probably best to check if what you want already exists,
> > move it into target/i386 somewhere I guess, and everyone shares it.

> Currently there isn’t a backend-agnostic interface for this. 
> It was part of the import of HVF support to qemu from downstream.

Hmm.

> Notably means that the emulate infrastructure isn’t usable by multiple
> backends in the same build. It might be possible to get away without that
> however as HVF is macOS specific, MSHV is Linux-specific and WHPX
> (which is currently using winhvemulator instead of this) is Windows-specific...

It does scare me a bit having 2 different functions with the same name like that;
but hmm OK...

But - x86_is_real() is identical in these cases - the x86_is_protected() is the
implementation dependent bit.
The only bit that seems different is the reading of the register, eg. CR0, so there
could be a:

   x86_read_CR0(CPUState *cpu)
and the x86_is_real and x86_is_paging_mode all be shared (as inline's ???).

But this does all get messy - I mean, even if MSHV is Linux specific, and HVF
is macos specific; it's a shame that the logic behind x86_is_* is open coded
in all of tcg, kvm and copied twice in hvf and mshv.

(I don't understand the structure as to why some stuff needs backend specific
reads and some are in env).

Dave

> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [CRM114spam]: Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Magnus Kulke 1 month, 4 weeks ago
On Tue, Sep 16, 2025 at 11:47:02PM +0000, Dr. David Alan Gilbert wrote:
> (I don't understand the structure as to why some stuff needs backend specific
> reads and some are in env).

Ah, that's because HVF is working on a slightly different level of
abstraction I suppose: the HVF impl will read/write values to VMCS via
VMREAD/VMWRITE, which would be Intel VMX specific.
Re: [CRM114spam]: Re: [PATCH v4 03/27] target/i386/mshv: Add x86 decoder/emu implementation
Posted by Magnus Kulke 1 month, 4 weeks ago
On Tue, Sep 16, 2025 at 11:47:02PM +0000, Dr. David Alan Gilbert wrote:
> * Mohamed Mediouni (mohamed@unpredictable.fr) wrote:
> > 
> > 
> > > On 16. Sep 2025, at 19:40, Dr. David Alan Gilbert <dave@treblig.org> wrote:
> > > 
> > > For example 'x86_is_real' is declared in target/i386/emulate/x86.h
> > > and defined in target/i386/hvf/x86.c  (hmm that's a bit weird).
> > > So it's probably best to check if what you want already exists,
> > > move it into target/i386 somewhere I guess, and everyone shares it.
> 
> > Currently there isn’t a backend-agnostic interface for this. 
> > It was part of the import of HVF support to qemu from downstream.
> 
> Hmm.
> 
> > Notably means that the emulate infrastructure isn’t usable by multiple
> > backends in the same build. It might be possible to get away without that
> > however as HVF is macOS specific, MSHV is Linux-specific and WHPX
> > (which is currently using winhvemulator instead of this) is Windows-specific...
> 
> It does scare me a bit having 2 different functions with the same name like that;
> but hmm OK...
> 
> But - x86_is_real() is identical in these cases - the x86_is_protected() is the
> implementation dependent bit.
> The only bit that seems different is the reading of the register, eg. CR0, so there
> could be a:
> 
>    x86_read_CR0(CPUState *cpu)
> and the x86_is_real and x86_is_paging_mode all be shared (as inline's ???).
> 
> But this does all get messy - I mean, even if MSHV is Linux specific, and HVF
> is macos specific; it's a shame that the logic behind x86_is_* is open coded
> in all of tcg, kvm and copied twice in hvf and mshv.
> 
> (I don't understand the structure as to why some stuff needs backend specific
> reads and some are in env).
> 
> Dave

Hey Dave,

in preperation for the MSHV patches Wei Liu did rework the HVF x86
emulator code and abstracted it into target/i386/emulate, so we could
use prior art as much as possible. Since HVF had to perform similar
userland decoding/emulation of instruction on MMIO exits that was a
good fit. However, the requirements are not exactly the same.

Thus, in the current approach there is some duplication, like
`x86_is_real()` (which is sort of trivial and arguably could indeed
be inlined), but e.g. `x86_is_paging_mode()` would have different
implementations per accelerator target.

best,

magnus