Re: [PATCH 1/8] hv: add lock prefix check for exception


Xu, Anthony
 

Does this trigger GP in guest before triggering VM_Exit?

Anthony

-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Yin, Fengwei
Sent: Tuesday, August 14, 2018 6:04 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH 1/8] hv: add lock prefix check for exception

According to SDM Vol2 - Instruction Set Reference:
Some instructions can't work with prefix lock or can't work in
some situations (destination is not a memory operand).

We add the lock prefix check for the instructions we support and
inject UD if lock shouldn't be used.

Signed-off-by: Yin Fengwei <fengwei.yin@...>
---
hypervisor/arch/x86/guest/instr_emul.c | 34 ++++++++++++++++++++++----
hypervisor/arch/x86/guest/instr_emul.h | 3 ++-
2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/hypervisor/arch/x86/guest/instr_emul.c
b/hypervisor/arch/x86/guest/instr_emul.c
index 521f0d36..222fa033 100644
--- a/hypervisor/arch/x86/guest/instr_emul.c
+++ b/hypervisor/arch/x86/guest/instr_emul.c
@@ -56,20 +56,24 @@
#define VIE_OP_F_MOFFSET (1U << 2) /* 16/32/64-bit immediate
moffset */
#define VIE_OP_F_NO_MODRM (1U << 3)
#define VIE_OP_F_NO_GLA_VERIFICATION (1U << 4)
+#define VIE_OP_F_NO_LOCK (1U << 5) /* UD if lock prefix is used
*/

static const struct instr_emul_vie_op two_byte_opcodes[256] = {
[0xB6] = {
.op_type = VIE_OP_TYPE_MOVZX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xB7] = {
.op_type = VIE_OP_TYPE_MOVZX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xBA] = {
.op_type = VIE_OP_TYPE_BITTEST,
- .op_flags = VIE_OP_F_IMM8,
+ .op_flags = VIE_OP_F_IMM8 | VIE_OP_F_NO_LOCK,
},
[0xBE] = {
.op_type = VIE_OP_TYPE_MOVSX,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
};

@@ -79,32 +83,41 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
},
[0x2B] = {
.op_type = VIE_OP_TYPE_SUB,
+ .op_flags = VIE_OP_F_NO_LOCK, /* 0x2B has register as dst */
},
[0x39] = {
.op_type = VIE_OP_TYPE_CMP,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x3B] = {
.op_type = VIE_OP_TYPE_CMP,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x88] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x89] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x8A] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x8B] = {
.op_type = VIE_OP_TYPE_MOV,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0xA1] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM,
+ .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM |
+ VIE_OP_F_NO_LOCK,
},
[0xA3] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM,
+ .op_flags = VIE_OP_F_MOFFSET | VIE_OP_F_NO_MODRM |
+ VIE_OP_F_NO_LOCK,
},
[0xA4] = {
.op_type = VIE_OP_TYPE_MOVS,
@@ -125,14 +138,15 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
[0xC6] = {
/* XXX Group 11 extended opcode - not just MOV */
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_IMM8,
+ .op_flags = VIE_OP_F_IMM8 | VIE_OP_F_NO_LOCK,
},
[0xC7] = {
.op_type = VIE_OP_TYPE_MOV,
- .op_flags = VIE_OP_F_IMM,
+ .op_flags = VIE_OP_F_IMM | VIE_OP_F_NO_LOCK,
},
[0x23] = {
.op_type = VIE_OP_TYPE_AND,
+ .op_flags = VIE_OP_F_NO_LOCK, /* 0x23 has register as dst */
},
[0x80] = {
/* Group 1 extended opcode */
@@ -151,9 +165,11 @@ static const struct instr_emul_vie_op
one_byte_opcodes[256] = {
},
[0x84] = {
.op_type = VIE_OP_TYPE_TEST,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x85] = {
.op_type = VIE_OP_TYPE_TEST,
+ .op_flags = VIE_OP_F_NO_LOCK,
},
[0x08] = {
.op_type = VIE_OP_TYPE_OR,
@@ -1714,6 +1730,8 @@ static int decode_prefixes(struct instr_emul_vie *vie,
vie->repz_present = 1U;
} else if (x == 0xF2U) {
vie->repnz_present = 1U;
+ } else if (x == 0xF0U) {
+ vie->lock_prefix = 1U;
} else if (segment_override(x, &vie->segment_register)) {
vie->seg_override = 1U;
} else {
@@ -2114,6 +2132,12 @@ static int local_decode_instruction(enum
vm_cpu_mode cpu_mode,
return -1;
}

+ /* If lock is used with instruction doesn't allow lock,
+ * inject invalid opcode exception UD to guest.
+ */
+ if (vie->lock_prefix && (vie->op.op_flags & VIE_OP_F_NO_LOCK))
+ return -1;
+
vie->decoded = 1U; /* success */

return 0;
diff --git a/hypervisor/arch/x86/guest/instr_emul.h
b/hypervisor/arch/x86/guest/instr_emul.h
index 6132332e..a7a20bdb 100644
--- a/hypervisor/arch/x86/guest/instr_emul.h
+++ b/hypervisor/arch/x86/guest/instr_emul.h
@@ -103,7 +103,8 @@ struct instr_emul_vie {
repnz_present:1, /* REPNE/REPNZ prefix */
opsize_override:1, /* Operand size override */
addrsize_override:1, /* Address size override */
- seg_override:1; /* Segment override */
+ seg_override:1, /* Segment override */
+ lock_prefix:1; /* has LOCK prefix */

uint8_t mod:2, /* ModRM byte */
reg:4,
--
2.17.0


Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.