movzx is a move with zero-extend. It will either move 1 byte (0f b6) or 2 bytes (0f b7). The destination are lower 8 or 16 bits. Zero-extend means that upper bits need to be cleared. The definition of upper bits depends on the destination register.
We already have a preserve mask that allows us for to clear or preserve bits when emulating the instruction. In case of movzx, the preserve mask only depends on the width of the destination register. For the destination register, we have the following cases: - rax -- instruction has REX prefix 0x48 (rex_w set) - eax -- default case, no prefix, nothing - ax -- instruction has OP SZ override prefix 0x66 - al -- not possible, and doesn't make sense for movzx Now, rax and eax have the same effect: Always clear all upper bits (IOW, bits 8-63 if access_size is 1 or bits 16-63 if access_size is 2). Solution for rax and eax is easy: Simply don't set the preserve mask at all. The fun part is if we have the 0x66 operand override size prefix. This means that the 'visibility' of the destination register is narrowed to 16 bit. In case of a 1 byte move (0f b6), copy the source to bits 0-7, clear bits 8-15 and preserve bits 16-63. access_width ensures that we only copy 8 bit to bits 0-7, and the preserve_mask 0xffff.ffff.ffff.0000 does the rest: preserve bits 16-63 and implicitely clear bits 8-15. In case of a 2 byte move (0f b7), copy the source to bits 0-15, clear nothing and preserve bits 16-63. access_width ensures that we only copy 16 bit to bits 0-15, and the preserve_mask 0xffff.ffff.ffff.0000 does the rest: preserve bits 16-63. Once this pattern is clear, the fix in the hypervisor is straight forward. Amend existing and add new test cases that test correct behaviour. Signed-off-by: Ralf Ramsauer <[email protected]> --- Since v1: - Rewrite commit message - Set the correct preserve_mask - Systematically, add tests for all possible combinations - Adress Jan's comments hypervisor/arch/x86/mmio.c | 36 +++++++++- inmates/tests/x86/mmio-access-32.c | 30 +++++--- inmates/tests/x86/mmio-access.c | 108 +++++++++++++++++++++++++---- 3 files changed, 152 insertions(+), 22 deletions(-) diff --git a/hypervisor/arch/x86/mmio.c b/hypervisor/arch/x86/mmio.c index 124f9966..1fcd810a 100644 --- a/hypervisor/arch/x86/mmio.c +++ b/hypervisor/arch/x86/mmio.c @@ -55,6 +55,7 @@ struct parse_context { bool has_rex_r; bool has_addrsz_prefix; bool has_opsz_prefix; + bool zero_extend; }; static bool ctx_update(struct parse_context *ctx, u64 *pc, unsigned int advance, @@ -144,6 +145,7 @@ restart: ctx.has_opsz_prefix = true; goto restart; case X86_OP_MOVZX_OPC1: + ctx.zero_extend = true; if (!ctx_update(&ctx, &pc, 1, pg_structs)) goto error_noinst; op[1].raw = *ctx.inst; @@ -191,8 +193,38 @@ restart: op[2].raw = *ctx.inst; - if (!ctx.does_write && inst.access_size < 4) - inst.reg_preserve_mask = ~BYTE_MASK(inst.access_size); + /* + * reg_preserve_mask defaults to 0, and only needs to be set in case of + * reads + */ + if (!ctx.does_write) { + /* + * MOVs on 32 or 64 bit destination registers need no + * adjustment of the reg_preserve_mask, all upper bits will + * always be cleared. + * + * In case of 16 or 8 bit registers, the instruction must only + * modify bits within that width. Therefore, reg_preserve_mask + * may be set to preserve upper bits. + * + * For regular instructions, this is the case if access_size < 4. + * + * For zero-extend instructions, this is the case if the + * operand size override prefix is set. + */ + if(!ctx.zero_extend && inst.access_size < 4) + /* + * Restrict access to the width of the access_size, and + * preserve all other bits + */ + inst.reg_preserve_mask = ~BYTE_MASK(inst.access_size); + else if (ctx.zero_extend && ctx.has_opsz_prefix) + /* + * Always preserve bits 16-63. Potential zero-extend of + * bits 8-15 is ensured by access_size + */ + inst.reg_preserve_mask = ~BYTE_MASK(2); + } /* ensure that we are actually talking about mov imm,<mem> */ if (op[0].raw == X86_OP_MOV_IMMEDIATE_TO_MEM && op[2].modrm.reg != 0) diff --git a/inmates/tests/x86/mmio-access-32.c b/inmates/tests/x86/mmio-access-32.c index db743410..566f8eaa 100644 --- a/inmates/tests/x86/mmio-access-32.c +++ b/inmates/tests/x86/mmio-access-32.c @@ -64,15 +64,29 @@ void inmate_main(void) 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)); - EXPECT_EQUAL(reg32, (u8)pattern); + /* MOVZXB (0f b6), 8-bit data, 32-bit address, zero extend bits 8-31 */ + asm volatile("movzxb (%%eax), %%eax" + : "=a" (reg32) : "a" (mmio_reg)); + EXPECT_EQUAL(reg32, pattern & 0xff); - /* MOVZXW (0f b7) */ - asm volatile("movzxw (%%ebx), %%eax" - : "=a" (reg32) : "a" (0), "b" (mmio_reg)); - EXPECT_EQUAL(reg32, (u16)pattern); + /* MOVZXB (66 0f b6), 8-bit data, 32-bit address, zero extend bits 8-15, + * operand size prefix */ + asm volatile("movzxb (%%eax), %%ax" + : "=a" (reg32) : "a" (mmio_reg)); + EXPECT_EQUAL(reg32, + ((unsigned long)mmio_reg & ~0xffff) | (pattern & 0xff)); + + /* MOVZXW (0f b7), 16-bit data, 32-bit address, zero extend bits + * 16-31 */ + asm volatile("movzxw (%%eax), %%eax" + : "=a" (reg32) : "a" (mmio_reg)); + EXPECT_EQUAL(reg32, pattern & 0xffff); + + /* MOVZXW (66 0f b7), 16-bit data, 32-bit address, preserve bits 16-31 */ + asm volatile(".byte 0x66, 0x0f, 0xb7, 0x00" + : "=a" (reg32) : "a" (mmio_reg)); + EXPECT_EQUAL(reg32, ((unsigned long)mmio_reg & ~0xffff) | + (pattern & 0xffff)); /* MEM_TO_AX (a1), 32-bit data, 32-bit address */ asm volatile("mov (0x101ff8), %%eax" diff --git a/inmates/tests/x86/mmio-access.c b/inmates/tests/x86/mmio-access.c index a17455b0..328d19f0 100644 --- a/inmates/tests/x86/mmio-access.c +++ b/inmates/tests/x86/mmio-access.c @@ -84,20 +84,104 @@ void inmate_main(void) 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)); - EXPECT_EQUAL(reg64, (u8)pattern); + /* MOVZX test cases */ - /* MOVZXB (0f b6), 32-bit data, 32-bit address */ - asm volatile("movzxb (%%ebx), %%eax" - : "=a" (reg64) : "a" (0), "b" (mmio_reg)); - EXPECT_EQUAL(reg64, (u8)pattern); + /* + * First three tests: MOVZXB (0f b6) with 64-bit address, varying + * register width (rax, eax, ax) + */ - /* MOVZXW (0f b7) */ - asm volatile("movzxw (%%rbx), %%rax" - : "=a" (reg64) : "a" (0), "b" (mmio_reg)); - EXPECT_EQUAL(reg64, (u16)pattern); + /* MOVZXB (48 0f b6), 8-bit data, 64-bit address, clear bits 8-63 */ + asm volatile("movzxb (%%rax), %%rax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xff); + + /* MOVZXB (0f b6), 8-bit data, 64-bit address, clear bits 8-63 + * Exposes the same behaviour as 48 0f b6. */ + asm volatile("movzxb (%%rax), %%eax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xff); + + /* MOVZXB (66 0f b6), 8-bit data, clear bits 8-15, preserve 16-63, + * operand size prefix */ + asm volatile("movzxb (%%rax), %%ax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, + ((unsigned long)mmio_reg & ~0xffffUL) | (pattern & 0xff)); + + /* + * Second three tests: MOVZXB (0f b6) with 32-bit address, varying + * register width (rax, eax, ax). + * + * These pattern will cover cases, where we have, e.g., both operand + * prefixes (address size override prefix and operand size override + * prefix), and a REX + adress size override prefix. + */ + + /* MOVZXB (67 48 0f b6), 8-bit data, clear bits 8-63, 32-bit address, + * REX_W, AD SZ override prefix */ + asm volatile("movzxb (%%eax), %%rax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xff); + + /* MOVZXB (67 0f b6), 8-bit data, clear bits 8-63, 32-bit address, + * AD SZ override prefix. Exposes the same behaviour as 67 48 0f b6. */ + asm volatile("movzxb (%%eax), %%eax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xff); + + /* MOVZXB (67 66 0f b6), 8-bit data, clear bits 8-15, preserve 16-63, + * 32-bit address, AD SZ override prefix, OP SZ override prefix */ + asm volatile("movzxb (%%eax), %%ax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, + ((unsigned long)mmio_reg & ~0xffffUL) | (pattern & 0xff)); + + /* + * Three tests for: MOVZXW (0f b7) with 64-bit address, varying + * register width (rax, eax, ax). + */ + + /* MOVZXW (48 0f b7), 16-bit data, clear bits 16-63, 64-bit address */ + asm volatile("movzxw (%%rax), %%rax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xffff); + + /* MOVZXW (0f b7), 16-bit data, clear bits 16-63, 64-bit address. + * Exposes the same behaviour as 48 0f b7. */ + asm volatile("movzxw (%%rax), %%eax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xffff); + + /* MOVZWW (66 0f b7), 16-bit data, preserve bits 16-63, OP SZ prefix */ + asm volatile(".byte 0x66, 0x0f, 0xb7, 0x00" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, ((unsigned long)mmio_reg & ~0xffffUL) | + (pattern & 0xffff)); + + /* + * Last but not least: MOVZXW (0f b7) with 32-bit address, varying + * register width (rax, eax, ax). + */ + + /* MOVZXW (67 48 0f b7), 16-bit data, clear bits 16-63, 32-bit address, + * AD SZ prefix, REX_W */ + asm volatile("movzxw (%%eax), %%rax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xffff); + + /* MOVZXW (67 0f b7), 16-bit data, clear bits 16-63, 32-bit address, + * AD SZ prefix. Exposes same behaviour as 67 48 0f b7. */ + asm volatile("movzxw (%%eax), %%eax" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, pattern & 0xffff); + + /* MOVZXW (67 66 0f b7), 16-bit data, preserve bits 16-63, 32-bit address, + * AD SZ prefix, OP SZ prefix */ + asm volatile(".byte 0x67, 0x66, 0x0f, 0xb7, 0x00" + : "=a" (reg64) : "a" (mmio_reg)); + EXPECT_EQUAL(reg64, ((unsigned long)mmio_reg & ~0xffffUL) | + (pattern & 0xffff)); /* MEM_TO_AX (a1), 64-bit data, 64-bit address */ asm volatile("movabs (0x101ff8), %%rax" -- 2.22.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/20190613133052.8347-1-ralf.ramsauer%40oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
