Skip to content

Commit d958d31

Browse files
fyin1lijinxia
authored andcommitted
hv: fix the issue of movs emulation
The current movs emulation has issues: 1. it use gva to get/put data. 2. it only support src and dst operand are memory which does not apply to our case (one of them should be mmio and triggers EPT voilation). This patch fix the issue by: 1. convert the address from gva to hva before access it. 2. handle mmio emulation. Also fix the issue introduced by previous instruction reshuffle patchset: 1. the desc validation should be only applied to none-64bit mode. 2. gva2gpa should be given correct guest virtual address. Specailly for movs, we cache the dst gpa if the check during movs decoding success. And use it directly during movs emulation. Tracked-On: #1207 Signed-off-by: Yin Fengwei <fengwei.yin@intel.com> Tested-by: Qi Yadong <yadog.qi@intel.com> Acked-by: Anthony Xu <anthony.xu@intel.com>
1 parent d84f7a4 commit d958d31

File tree

2 files changed

+68
-31
lines changed

2 files changed

+68
-31
lines changed

hypervisor/arch/x86/guest/instr_emul.c

Lines changed: 66 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -869,14 +869,14 @@ static int emulate_movx(struct vcpu *vcpu, struct instr_emul_vie *vie)
869869
*
870870
* It's only used by MOVS/STO
871871
*/
872-
static void get_gva_di_si_nocheck(struct vcpu *vcpu, uint8_t addrsize,
873-
enum cpu_reg_name seg, enum cpu_reg_name gpr, uint64_t *gva)
872+
static void get_gva_si_nocheck(struct vcpu *vcpu, uint8_t addrsize,
873+
enum cpu_reg_name seg, uint64_t *gva)
874874
{
875875
uint64_t val;
876876
struct seg_desc desc;
877877
enum vm_cpu_mode cpu_mode;
878878

879-
val = vm_get_register(vcpu, gpr);
879+
val = vm_get_register(vcpu, CPU_REG_RSI);
880880
vm_get_seg_desc(seg, &desc);
881881
cpu_mode = get_vcpu_mode(vcpu);
882882

@@ -888,29 +888,24 @@ static void get_gva_di_si_nocheck(struct vcpu *vcpu, uint8_t addrsize,
888888
/*
889889
* @pre only called during instruction decode phase
890890
*
891-
* @remark This function get gva from ES:DI and DS(other segment):SI. And
892-
* do check the failure condition and inject exception to guest accordingly.
891+
* @remark This function get gva from ES:DI. And do check the failure
892+
* condition and inject exception to guest accordingly.
893893
*
894894
* It's only used by MOVS/STO
895895
*/
896-
static int get_gva_di_si_check(struct vcpu *vcpu, uint8_t addrsize,
897-
uint32_t prot, enum cpu_reg_name seg, enum cpu_reg_name gpr,
898-
uint64_t *gva)
896+
static int get_gva_di_check(struct vcpu *vcpu, struct instr_emul_vie *vie,
897+
uint8_t addrsize, uint64_t *gva)
899898
{
900899
int ret;
901900
uint32_t err_code;
902901
struct seg_desc desc;
903902
enum vm_cpu_mode cpu_mode;
904903
uint64_t val, gpa;
905904

906-
val = vm_get_register(vcpu, gpr);
907-
vm_get_seg_desc(seg, &desc);
905+
val = vm_get_register(vcpu, CPU_REG_RDI);
906+
vm_get_seg_desc(CPU_REG_ES, &desc);
908907
cpu_mode = get_vcpu_mode(vcpu);
909908

910-
if (!is_desc_valid(&desc, prot)) {
911-
goto exception_inject;
912-
}
913-
914909
if (cpu_mode == CPU_MODE_64BIT) {
915910
if ((addrsize != 4U) && (addrsize != 8U)) {
916911
goto exception_inject;
@@ -919,46 +914,72 @@ static int get_gva_di_si_check(struct vcpu *vcpu, uint8_t addrsize,
919914
if ((addrsize != 2U) && (addrsize != 4U)) {
920915
goto exception_inject;
921916
}
917+
918+
if (!is_desc_valid(&desc, PROT_WRITE)) {
919+
goto exception_inject;
920+
}
922921
}
923922

924-
if (vie_calculate_gla(cpu_mode, seg, &desc, val, addrsize, gva) != 0) {
923+
if (vie_calculate_gla(cpu_mode, CPU_REG_ES, &desc, val, addrsize, gva)
924+
!= 0) {
925925
goto exception_inject;
926926
}
927927

928928
if (vie_canonical_check(cpu_mode, *gva) != 0) {
929929
goto exception_inject;
930930
}
931931

932-
err_code = (prot == PROT_WRITE) ? PAGE_FAULT_WR_FLAG : 0U;
933-
ret = gva2gpa(vcpu, (uint64_t)gva, &gpa, &err_code);
932+
err_code = PAGE_FAULT_WR_FLAG;
933+
ret = gva2gpa(vcpu, *gva, &gpa, &err_code);
934934
if (ret < 0) {
935935
if (ret == -EFAULT) {
936936
vcpu_inject_pf(vcpu, (uint64_t)gva, err_code);
937937
}
938938
return ret;
939939
}
940940

941+
/* If we are checking the dest operand for movs instruction,
942+
* we cache the gpa if check pass. It will be used during
943+
* movs instruction emulation.
944+
*/
945+
vie->dst_gpa = gpa;
946+
941947
return 0;
942948

943949
exception_inject:
944-
if (seg == CPU_REG_SS) {
945-
vcpu_inject_ss(vcpu);
946-
} else {
947-
vcpu_inject_gp(vcpu, 0U);
948-
}
950+
vcpu_inject_gp(vcpu, 0U);
949951
return -EFAULT;
950952
}
951953

954+
/* MOVs gets the operands from RSI and RDI. Both operands could be memory.
955+
* With VMX enabled, one of the operand triggers EPT voilation.
956+
*
957+
* If it's RSI access trigger EPT voilation, it's source operands and always
958+
* read operations. Not neccesary to check whether need to inject fault (done
959+
* by VMX already). We do need to check the RDI.
960+
*
961+
* If it's RDI access trigger EPT voilation, we need to check RDI because it's
962+
* always write operations and VMX doens't cover write access check.
963+
* Not neccesary to check RSI, because VMX cover it for us.
964+
*
965+
* In summary,
966+
* For MOVs instruction, we always check RDI during instruction decoding phase.
967+
* And access RSI without any check during instruction emulation phase.
968+
*/
952969
static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie)
953970
{
954-
uint64_t dstaddr, srcaddr;
971+
uint64_t src_gva, gpa, val;
972+
uint64_t *dst_hva, *src_hva;
955973
uint64_t rcx, rdi, rsi, rflags;
974+
uint32_t err_code;
975+
enum cpu_reg_name seg;
956976
int error, repeat;
957977
uint8_t opsize;
958-
enum cpu_reg_name seg;
978+
bool is_mmio_write;
959979

960980
opsize = (vie->opcode == 0xA4U) ? 1U : vie->opsize;
961981
error = 0;
982+
is_mmio_write = (vcpu->req.reqs.mmio.direction == REQUEST_WRITE);
962983

963984
/*
964985
* XXX although the MOVS instruction is only supposed to be used with
@@ -984,11 +1005,25 @@ static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie)
9841005

9851006
seg = (vie->seg_override != 0U) ? (vie->segment_register) : CPU_REG_DS;
9861007

987-
get_gva_di_si_nocheck(vcpu, vie->addrsize, seg, CPU_REG_RSI, &srcaddr);
988-
get_gva_di_si_nocheck(vcpu, vie->addrsize, CPU_REG_ES, CPU_REG_RDI,
989-
&dstaddr);
1008+
if (is_mmio_write) {
1009+
get_gva_si_nocheck(vcpu, vie->addrsize, seg, &src_gva);
9901010

991-
(void)memcpy_s((void *)dstaddr, 16U, (void *)srcaddr, opsize);
1011+
/* we are sure it will success */
1012+
(void)gva2gpa(vcpu, src_gva, &gpa, &err_code);
1013+
src_hva = gpa2hva(vcpu->vm, gpa);
1014+
1015+
val = *src_hva;
1016+
1017+
mmio_write(vcpu, val);
1018+
} else {
1019+
mmio_read(vcpu, &val);
1020+
1021+
/* The dest gpa is saved during dst check instruction
1022+
* decoding.
1023+
*/
1024+
dst_hva = gpa2hva(vcpu->vm, vie->dst_gpa);
1025+
memcpy_s(dst_hva, opsize, &val, opsize);
1026+
}
9921027

9931028
rsi = vm_get_register(vcpu, CPU_REG_RSI);
9941029
rdi = vm_get_register(vcpu, CPU_REG_RDI);
@@ -2128,14 +2163,14 @@ static int instr_check_di(struct vcpu *vcpu, struct instr_emul_ctxt *emul_ctxt)
21282163
int ret;
21292164
struct instr_emul_vie *vie = &emul_ctxt->vie;
21302165
uint64_t gva;
2131-
enum cpu_reg_name seg;
21322166

2133-
ret = get_gva_di_si_check(vcpu, vie->addrsize, PROT_WRITE,
2134-
CPU_REG_ES, CPU_REG_RDI, &gva);
2167+
ret = get_gva_di_check(vcpu, vie, vie->addrsize, &gva);
21352168

21362169
if (ret < 0) {
21372170
return -EFAULT;
21382171
}
2172+
2173+
return 0;
21392174
}
21402175

21412176
static int instr_check_gva(struct vcpu *vcpu, struct instr_emul_ctxt *emul_ctxt,

hypervisor/arch/x86/guest/instr_emul.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ struct instr_emul_vie {
128128

129129
uint8_t opcode;
130130
struct instr_emul_vie_op op; /* opcode description */
131+
132+
uint64_t dst_gpa; /* saved dst operand gpa. Only for movs */
131133
};
132134

133135
#define PSL_C 0x00000001U /* carry bit */

0 commit comments

Comments
 (0)