mov (%rax), %ax is a 16-bit data MOV_FROM_MEM that will emit 0x66 0x8b 0x00.
0x66 is the operand-size override prefix which we currently do not support. We should support it, as we can find this opcode, for example, for some mmconfig space access from Linux (e.g., pci_generic_config_read). This also adds appropriate mmio-access tests. Tested in QEMU virtual target. Signed-off-by: Ralf Ramsauer <[email protected]> --- hypervisor/arch/x86/include/asm/processor.h | 1 + hypervisor/arch/x86/mmio.c | 47 +++++++++++++++------ inmates/tests/x86/mmio-access-32.c | 21 ++++++++- inmates/tests/x86/mmio-access.c | 21 ++++++++- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/hypervisor/arch/x86/include/asm/processor.h b/hypervisor/arch/x86/include/asm/processor.h index 70a6c3ff..d8111690 100644 --- a/hypervisor/arch/x86/include/asm/processor.h +++ b/hypervisor/arch/x86/include/asm/processor.h @@ -145,6 +145,7 @@ #define X86_REX_CODE 4 +#define X86_PREFIX_OP_SZ 0x66 #define X86_PREFIX_ADDR_SZ 0x67 #define X86_OP_MOVZX_OPC1 0x0f diff --git a/hypervisor/arch/x86/mmio.c b/hypervisor/arch/x86/mmio.c index df8c97a1..b836f43c 100644 --- a/hypervisor/arch/x86/mmio.c +++ b/hypervisor/arch/x86/mmio.c @@ -54,6 +54,7 @@ struct parse_context { bool has_rex_w; bool has_rex_r; bool has_addrsz_prefix; + bool has_opsz_prefix; }; static bool ctx_update(struct parse_context *ctx, u64 *pc, unsigned int advance, @@ -74,14 +75,33 @@ static bool ctx_update(struct parse_context *ctx, u64 *pc, unsigned int advance, return true; } -static unsigned int get_address_width(bool has_addrsz_prefix) +static void parse_widths(struct parse_context *ctx, + struct mmio_instruction *inst, bool parse_addr_width) { u16 cs_attr = vcpu_vendor_get_cs_attr(); - bool long_mode = (vcpu_vendor_get_efer() & EFER_LMA) && - (cs_attr & VCPU_CS_L); + bool cs_db = !!(cs_attr & VCPU_CS_DB); + bool long_mode = + (vcpu_vendor_get_efer() & EFER_LMA) && (cs_attr & VCPU_CS_L); - return long_mode ? (has_addrsz_prefix ? 4 : 8) : - (!!(cs_attr & VCPU_CS_DB) ^ has_addrsz_prefix) ? 4 : 2; + /* Op size prefix is ignored if rex.w = 1 */ + if (ctx->has_rex_w) { + inst->access_size = 8; + } else { + if (long_mode) + /* CS.d is ignored in long mode */ + inst->access_size = ctx->has_opsz_prefix ? 2 : 4; + else + inst->access_size = + (cs_db ^ ctx->has_opsz_prefix) ? 4 : 2; + } + + if (parse_addr_width) { + if (long_mode) + inst->inst_len += ctx->has_addrsz_prefix ? 4 : 8; + else + inst->inst_len += + (cs_db ^ ctx->has_addrsz_prefix) ? 4 : 2; + } } struct mmio_instruction @@ -118,6 +138,11 @@ restart: goto error_noinst; ctx.has_addrsz_prefix = true; goto restart; + case X86_PREFIX_OP_SZ: + if (!ctx_update(&ctx, &pc, 1, pg_structs)) + goto error_noinst; + ctx.has_opsz_prefix = true; + goto restart; case X86_OP_MOVZX_OPC1: if (!ctx_update(&ctx, &pc, 1, pg_structs)) goto error_noinst; @@ -134,28 +159,26 @@ restart: ctx.does_write = true; break; case X86_OP_MOV_TO_MEM: - inst.access_size = ctx.has_rex_w ? 8 : 4; + parse_widths(&ctx, &inst, false); ctx.does_write = true; break; case X86_OP_MOVB_FROM_MEM: inst.access_size = 1; break; case X86_OP_MOV_FROM_MEM: - inst.access_size = ctx.has_rex_w ? 8 : 4; + parse_widths(&ctx, &inst, false); break; case X86_OP_MOV_IMMEDIATE_TO_MEM: - inst.access_size = ctx.has_rex_w ? 8 : 4; + parse_widths(&ctx, &inst, false); ctx.has_immediate = true; ctx.does_write = true; break; case X86_OP_MOV_MEM_TO_AX: - inst.inst_len += get_address_width(ctx.has_addrsz_prefix); - inst.access_size = ctx.has_rex_w ? 8 : 4; + parse_widths(&ctx, &inst, true); inst.in_reg_num = 15; goto final; case X86_OP_MOV_AX_TO_MEM: - inst.inst_len += get_address_width(ctx.has_addrsz_prefix); - inst.access_size = ctx.has_rex_w ? 8 : 4; + parse_widths(&ctx, &inst, true); inst.out_val = guest_regs->by_index[15]; ctx.does_write = true; goto final; diff --git a/inmates/tests/x86/mmio-access-32.c b/inmates/tests/x86/mmio-access-32.c index be1d470f..9c1db1d8 100644 --- a/inmates/tests/x86/mmio-access-32.c +++ b/inmates/tests/x86/mmio-access-32.c @@ -41,6 +41,10 @@ void inmate_main(void) mmio_write32(mmio_reg, pattern); EXPECT_EQUAL(*comm_page_reg, pattern); + /* MOV_FROM_MEM (8b), 16-bit data, 32-bit address, OP size prefix */ + asm volatile("mov (%%eax), %%ax" : "=a" (reg32) : "a" (mmio_reg)); + EXPECT_EQUAL((u16)reg32, (u16)pattern); + /* MOV_FROM_MEM (8b), 32-bit data, 32-bit address */ asm volatile("movl (%%ebx), %%eax" : "=a" (reg32) : "a" (0), "b" (mmio_reg)); @@ -55,6 +59,13 @@ void inmate_main(void) EXPECT_EQUAL(reg32, ((unsigned long)mmio_reg & ~0xffUL) | (pattern & 0xff)); + /* MOV_FROM_MEM (8a), 8-bit data, 32-bit address, OP size prefix */ + asm volatile("data16 mov (%%eax), %%al" + : "=a" (reg32) : "a" (mmio_reg)); + EXPECT_EQUAL((u8)reg32, (u8)pattern); + EXPECT_EQUAL(reg32, + ((unsigned long)mmio_reg & ~0xffUL) | (pattern & 0xff)); + /* MOVZXB (0f b6), 32-bit data, 32-bit address */ asm volatile("movzxb (%%ebx), %%eax" : "=a" (reg32) : "a" (0), "b" (mmio_reg)); @@ -87,7 +98,15 @@ void inmate_main(void) /* MOV_TO_MEM (88), 8-bit data */ asm volatile("movb %%al, (%%ebx)" : : "a" (0x42), "b" (mmio_reg)); - EXPECT_EQUAL(*comm_page_reg, (pattern & 0xffffff00) | 0x42); + EXPECT_EQUAL(*comm_page_reg, (pattern & ~0xffUL) | 0x42); + + /* MOV_TO_MEM (88), 8-bit data, OP size prefix */ + asm volatile("data16 mov %%al, (%%ebx)" : : "a" (0x23), "b" (mmio_reg)); + EXPECT_EQUAL(*comm_page_reg, (pattern & ~0xffUL) | 0x23); + + /* MOV_TO_MEM (89), 16-bit data, OP size prefix */ + asm volatile("mov %%ax, (%%ebx)" : : "a" (0x2342), "b" (mmio_reg)); + EXPECT_EQUAL(*comm_page_reg, (pattern & ~0xffffUL) | 0x2342); /* IMMEDIATE_TO_MEM (c7), 32-bit data, mod=0, reg=0, rm=3 */ asm volatile("movl %0, (%%ebx)" diff --git a/inmates/tests/x86/mmio-access.c b/inmates/tests/x86/mmio-access.c index a9d2fcaf..3794555f 100644 --- a/inmates/tests/x86/mmio-access.c +++ b/inmates/tests/x86/mmio-access.c @@ -51,6 +51,10 @@ void inmate_main(void) mmio_write64(mmio_reg, pattern); EXPECT_EQUAL(*comm_page_reg, pattern); + /* MOV_FROM_MEM (8b), 16-bit data, Ox66 OP size prefix */ + asm volatile("mov (%%rax), %%ax" : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL((u16)reg64, (u16)pattern); + /* MOV_FROM_MEM (8b), 64-bit data, mod=0, reg=0, rm=3 */ asm volatile("movq (%%rbx), %%rax" : "=a" (reg64) : "a" (0), "b" (mmio_reg)); @@ -75,6 +79,13 @@ void inmate_main(void) EXPECT_EQUAL(reg64, ((unsigned long)mmio_reg & ~0xffUL) | (pattern & 0xff)); + /* MOV_FROM_MEM (8a), 8-bit data */ + asm volatile("data16 mov (%%rax), %%al" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL((u8)reg64, (u8)pattern); + EXPECT_EQUAL(reg64, + ((unsigned long)mmio_reg & ~0xffUL) | (pattern & 0xff)); + /* MOVZXB (0f b6), to 64-bit, mod=0, reg=0, rm=3 */ asm volatile("movzxb (%%rbx), %%rax" : "=a" (reg64) : "a" (0), "b" (mmio_reg)); @@ -129,7 +140,15 @@ void inmate_main(void) /* MOV_TO_MEM (88), 8-bit data */ asm volatile("movb %%al, (%%rbx)" : : "a" (0x42), "b" (mmio_reg)); - EXPECT_EQUAL(*comm_page_reg, (pattern & 0xffffffffffffff00) | 0x42); + EXPECT_EQUAL(*comm_page_reg, (pattern & ~0xffUL) | 0x42); + + /* MOV_TO_MEM (88), 8-bit data, OP size prefix */ + asm volatile("data16 mov %%al, (%%ebx)" : : "a" (0x23), "b" (mmio_reg)); + EXPECT_EQUAL(*comm_page_reg, (pattern & ~0xffUL) | 0x23); + + /* MOV_TO_MEM (89), 16-bit data, OP size prefix */ + asm volatile("mov %%ax, (%%ebx)" : : "a" (0x2342), "b" (mmio_reg)); + EXPECT_EQUAL(*comm_page_reg, (pattern & ~0xffffUL) | 0x2342); /* IMMEDIATE_TO_MEM (c7), 64-bit data, mod=0, reg=0, rm=3 */ asm volatile("movq %0, (%%rbx)" -- 2.21.0 -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190606224457.1053-4-ralf.ramsauer%40oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
