Skip to content

Commit 59c0f35

Browse files
KaigeFulijinxia
KaigeFu
authored and
lijinxia
committed
HV: instr_emul: Make vm_set/get_register as non-failed function
Originally, vm_set/get_register return -EINVAL when "vcpu == NULL" or reg is invalid. But, we don't check the return value actually and there is no chance we get an null-vcpu and invalid reg in current implementation. This patch add pre-assumptions about valid parameters before the function and make them as non-failed functions. - static uint64_t vm_get_register(struct vcpu *vcpu, enum cpu_reg_name reg) - static void vm_set_register(struct vcpu *vcpu, enum cpu_reg_name reg, uint64_t val) Signed-off-by: Kaige Fu <kaige.fu@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
1 parent b6b7e75 commit 59c0f35

File tree

1 file changed

+63
-110
lines changed

1 file changed

+63
-110
lines changed

‎hypervisor/arch/x86/guest/instr_emul.c

Lines changed: 63 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -306,75 +306,61 @@ static uint32_t get_vmcs_field(enum cpu_reg_name ident)
306306
return VMX_GUEST_PDPTE2_FULL;
307307
case CPU_REG_PDPTE3:
308308
return VMX_GUEST_PDPTE3_FULL;
309-
default:
309+
default: /* Never get here */
310310
return VMX_INVALID_VMCS_FIELD;
311311
}
312312
}
313313

314-
static int vm_get_register(struct vcpu *vcpu, enum cpu_reg_name reg,
315-
uint64_t *retval)
314+
/**
315+
* @pre vcpu != NULL
316+
* @pre ((reg <= CPU_REG_LAST) && (reg >= CPU_REG_FIRST))
317+
* @pre ((reg != CPU_REG_CR2) && (reg != CPU_REG_IDTR) && (reg != CPU_REG_GDTR))
318+
*/
319+
static uint64_t vm_get_register(struct vcpu *vcpu, enum cpu_reg_name reg)
316320
{
317-
if (vcpu == NULL) {
318-
return -EINVAL;
319-
}
320-
321-
if ((reg > CPU_REG_LAST) || (reg < CPU_REG_FIRST)) {
322-
return -EINVAL;
323-
}
324-
321+
uint64_t reg_val;
322+
325323
if ((reg >= CPU_REG_GENERAL_FIRST) && (reg <= CPU_REG_GENERAL_LAST)) {
326-
*retval = vcpu_get_gpreg(vcpu, reg);
324+
reg_val = vcpu_get_gpreg(vcpu, reg);
327325
} else if ((reg >= CPU_REG_NONGENERAL_FIRST) &&
328326
(reg <= CPU_REG_NONGENERAL_LAST)) {
329327
uint32_t field = get_vmcs_field(reg);
330328

331-
if (field != VMX_INVALID_VMCS_FIELD) {
332-
if (reg <= CPU_REG_NATURAL_LAST) {
333-
*retval = exec_vmread(field);
334-
} else if (reg <= CPU_REG_64BIT_LAST) {
335-
*retval = exec_vmread64(field);
336-
} else {
337-
*retval = (uint64_t)exec_vmread16(field);
338-
}
329+
if (reg <= CPU_REG_NATURAL_LAST) {
330+
reg_val = exec_vmread(field);
331+
} else if (reg <= CPU_REG_64BIT_LAST) {
332+
reg_val = exec_vmread64(field);
339333
} else {
340-
return -EINVAL;
334+
reg_val = (uint64_t)exec_vmread16(field);
341335
}
342336
}
343337

344-
return 0;
338+
return reg_val;
345339
}
346340

347-
static int vm_set_register(struct vcpu *vcpu, enum cpu_reg_name reg,
341+
/**
342+
* @pre vcpu != NULL
343+
* @pre ((reg <= CPU_REG_LAST) && (reg >= CPU_REG_FIRST))
344+
* @pre ((reg != CPU_REG_CR2) && (reg != CPU_REG_IDTR) && (reg != CPU_REG_GDTR))
345+
*/
346+
static void vm_set_register(struct vcpu *vcpu, enum cpu_reg_name reg,
348347
uint64_t val)
349348
{
350-
if (vcpu == NULL) {
351-
return -EINVAL;
352-
}
353-
354-
if ((reg > CPU_REG_LAST) || (reg < CPU_REG_FIRST)) {
355-
return -EINVAL;
356-
}
357349

358350
if ((reg >= CPU_REG_GENERAL_FIRST) && (reg <= CPU_REG_GENERAL_LAST)) {
359351
vcpu_set_gpreg(vcpu, reg, val);
360352
} else if ((reg >= CPU_REG_NONGENERAL_FIRST) &&
361353
(reg <= CPU_REG_NONGENERAL_LAST)) {
362354
uint32_t field = get_vmcs_field(reg);
363355

364-
if (field != VMX_INVALID_VMCS_FIELD) {
365-
if (reg <= CPU_REG_NATURAL_LAST) {
366-
exec_vmwrite(field, val);
367-
} else if (reg <= CPU_REG_64BIT_LAST) {
368-
exec_vmwrite64(field, val);
369-
} else {
370-
exec_vmwrite16(field, (uint16_t)val);
371-
}
356+
if (reg <= CPU_REG_NATURAL_LAST) {
357+
exec_vmwrite(field, val);
358+
} else if (reg <= CPU_REG_64BIT_LAST) {
359+
exec_vmwrite64(field, val);
372360
} else {
373-
return -EINVAL;
361+
exec_vmwrite16(field, (uint16_t)val);
374362
}
375363
}
376-
377-
return 0;
378364
}
379365

380366
/**
@@ -615,11 +601,11 @@ static int vie_read_bytereg(struct vcpu *vcpu, struct instr_emul_vie *vie,
615601
uint8_t *rval)
616602
{
617603
uint64_t val;
618-
int error, lhbr;
604+
int error = 0, lhbr;
619605
enum cpu_reg_name reg;
620606

621607
vie_calc_bytereg(vie, &reg, &lhbr);
622-
error = vm_get_register(vcpu, reg, &val);
608+
val = vm_get_register(vcpu, reg);
623609

624610
/*
625611
* To obtain the value of a legacy high byte register shift the
@@ -637,11 +623,11 @@ static int vie_write_bytereg(struct vcpu *vcpu, struct instr_emul_vie *vie,
637623
uint8_t byte)
638624
{
639625
uint64_t origval, val, mask;
640-
int error, lhbr;
626+
int error = 0, lhbr;
641627
enum cpu_reg_name reg;
642628

643629
vie_calc_bytereg(vie, &reg, &lhbr);
644-
error = vm_get_register(vcpu, reg, &origval);
630+
origval = vm_get_register(vcpu, reg);
645631
if (error == 0) {
646632
val = byte;
647633
mask = 0xffU;
@@ -654,25 +640,22 @@ static int vie_write_bytereg(struct vcpu *vcpu, struct instr_emul_vie *vie,
654640
mask <<= 8;
655641
}
656642
val |= origval & ~mask;
657-
error = vm_set_register(vcpu, reg, val);
643+
vm_set_register(vcpu, reg, val);
658644
}
659645
return error;
660646
}
661647

662648
static int vie_update_register(struct vcpu *vcpu, enum cpu_reg_name reg,
663649
uint64_t val_arg, uint8_t size)
664650
{
665-
int error;
651+
int error = 0;
666652
uint64_t origval;
667653
uint64_t val = val_arg;
668654

669655
switch (size) {
670656
case 1U:
671657
case 2U:
672-
error = vm_get_register(vcpu, reg, &origval);
673-
if (error != 0) {
674-
return error;
675-
}
658+
origval = vm_get_register(vcpu, reg);
676659
val &= size2mask[size];
677660
val |= origval & ~size2mask[size];
678661
break;
@@ -685,7 +668,7 @@ static int vie_update_register(struct vcpu *vcpu, enum cpu_reg_name reg,
685668
return -EINVAL;
686669
}
687670

688-
error = vm_set_register(vcpu, reg, val);
671+
vm_set_register(vcpu, reg, val);
689672

690673
return error;
691674
}
@@ -694,14 +677,11 @@ static int vie_update_register(struct vcpu *vcpu, enum cpu_reg_name reg,
694677

695678
static int vie_update_rflags(struct vcpu *vcpu, uint64_t rflags2, uint64_t psl)
696679
{
697-
int error;
680+
int error = 0;
698681
uint8_t size;
699682
uint64_t rflags;
700683

701-
error = vm_get_register(vcpu, CPU_REG_RFLAGS, &rflags);
702-
if (error != 0) {
703-
return error;
704-
}
684+
rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
705685

706686
rflags &= ~RFLAGS_STATUS_BITS;
707687
rflags |= rflags2 & psl;
@@ -778,11 +758,9 @@ static int emulate_mov(struct vcpu *vcpu, struct instr_emul_vie *vie)
778758
*/
779759

780760
reg = vie->reg;
781-
error = vm_get_register(vcpu, reg, &val);
782-
if (error == 0) {
783-
val &= size2mask[size];
784-
error = mmio_write(vcpu, val);
785-
}
761+
val = vm_get_register(vcpu, reg);
762+
val &= size2mask[size];
763+
error = mmio_write(vcpu, val);
786764
break;
787765
case 0x8AU:
788766
/*
@@ -831,12 +809,9 @@ static int emulate_mov(struct vcpu *vcpu, struct instr_emul_vie *vie)
831809
* A3: mov moffs32, EAX
832810
* REX.W + A3: mov moffs64, RAX
833811
*/
834-
error = vm_get_register(vcpu, CPU_REG_RAX,
835-
&val);
836-
if (error == 0) {
837-
val &= size2mask[size];
838-
error = mmio_write(vcpu, val);
839-
}
812+
val = vm_get_register(vcpu, CPU_REG_RAX);
813+
val &= size2mask[size];
814+
error = mmio_write(vcpu, val);
840815
break;
841816
case 0xC6U:
842817
/*
@@ -962,19 +937,12 @@ static int get_gla(struct vcpu *vcpu, __unused struct instr_emul_vie *vie,
962937
{
963938
struct seg_desc desc;
964939
uint64_t cr0, val, rflags;
965-
int error;
966940

967-
error = vm_get_register(vcpu, CPU_REG_CR0, &cr0);
968-
error |= vm_get_register(vcpu, CPU_REG_RFLAGS, &rflags);
969-
error |= vm_get_register(vcpu, gpr, &val);
941+
cr0 = vm_get_register(vcpu, CPU_REG_CR0);
942+
rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
943+
val = vm_get_register(vcpu, gpr);
970944
vm_get_seg_desc(seg, &desc);
971945

972-
if (error) {
973-
pr_err("%s: error(%d) happens when getting cr0/rflags/segment"
974-
"desc/gpr", __func__, error);
975-
return -1;
976-
}
977-
978946
if (vie_calculate_gla(paging->cpu_mode, seg, &desc, val, opsize,
979947
addrsize, prot, gla) != 0) {
980948
if (seg == CPU_REG_SS) {
@@ -1034,7 +1002,7 @@ static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie,
10341002
repeat = vie->repz_present | vie->repnz_present;
10351003

10361004
if (repeat != 0) {
1037-
error = vm_get_register(vcpu, CPU_REG_RCX, &rcx);
1005+
rcx = vm_get_register(vcpu, CPU_REG_RCX);
10381006

10391007
/*
10401008
* The count register is %rcx, %ecx or %cx depending on the
@@ -1062,9 +1030,9 @@ static int emulate_movs(struct vcpu *vcpu, struct instr_emul_vie *vie,
10621030

10631031
(void)memcpy_s((void *)dstaddr, 16U, (void *)srcaddr, opsize);
10641032

1065-
error = vm_get_register(vcpu, CPU_REG_RSI, &rsi);
1066-
error = vm_get_register(vcpu, CPU_REG_RDI, &rdi);
1067-
error = vm_get_register(vcpu, CPU_REG_RFLAGS, &rflags);
1033+
rsi = vm_get_register(vcpu, CPU_REG_RSI);
1034+
rdi = vm_get_register(vcpu, CPU_REG_RDI);
1035+
rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
10681036

10691037
if ((rflags & PSL_D) != 0U) {
10701038
rsi -= opsize;
@@ -1107,7 +1075,7 @@ static int emulate_stos(struct vcpu *vcpu, struct instr_emul_vie *vie)
11071075
repeat = vie->repz_present | vie->repnz_present;
11081076

11091077
if (repeat != 0) {
1110-
error = vm_get_register(vcpu, CPU_REG_RCX, &rcx);
1078+
rcx = vm_get_register(vcpu, CPU_REG_RCX);
11111079

11121080
/*
11131081
* The count register is %rcx, %ecx or %cx depending on the
@@ -1118,15 +1086,15 @@ static int emulate_stos(struct vcpu *vcpu, struct instr_emul_vie *vie)
11181086
}
11191087
}
11201088

1121-
error = vm_get_register(vcpu, CPU_REG_RAX, &val);
1089+
val = vm_get_register(vcpu, CPU_REG_RAX);
11221090

11231091
error = mmio_write(vcpu, val);
11241092
if (error != 0) {
11251093
return error;
11261094
}
11271095

1128-
error = vm_get_register(vcpu, CPU_REG_RDI, &rdi);
1129-
error = vm_get_register(vcpu, CPU_REG_RFLAGS, &rflags);
1096+
rdi = vm_get_register(vcpu, CPU_REG_RDI);
1097+
rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
11301098

11311099
if ((rflags & PSL_D) != 0U) {
11321100
rdi -= opsize;
@@ -1183,10 +1151,7 @@ static int emulate_test(struct vcpu *vcpu, struct instr_emul_vie *vie)
11831151

11841152
/* get the first operand */
11851153
reg = vie->reg;
1186-
error = vm_get_register(vcpu, reg, &val1);
1187-
if (error != 0) {
1188-
break;
1189-
}
1154+
val1 = vm_get_register(vcpu, reg);
11901155

11911156
/* get the second operand */
11921157
error = mmio_read(vcpu, &val2);
@@ -1239,10 +1204,7 @@ static int emulate_and(struct vcpu *vcpu, struct instr_emul_vie *vie)
12391204

12401205
/* get the first operand */
12411206
reg = vie->reg;
1242-
error = vm_get_register(vcpu, reg, &val1);
1243-
if (error != 0) {
1244-
break;
1245-
}
1207+
val1 = vm_get_register(vcpu, reg);
12461208

12471209
/* get the second operand */
12481210
error = mmio_read(vcpu, &val2);
@@ -1361,10 +1323,7 @@ static int emulate_or(struct vcpu *vcpu, struct instr_emul_vie *vie)
13611323

13621324
/* get the second operand */
13631325
reg = vie->reg;
1364-
error = vm_get_register(vcpu, reg, &val2);
1365-
if (error != 0) {
1366-
break;
1367-
}
1326+
val2 = vm_get_register(vcpu, reg);
13681327

13691328
/* perform the operation and write the result */
13701329
result = val1 | val2;
@@ -1419,10 +1378,7 @@ static int emulate_cmp(struct vcpu *vcpu, struct instr_emul_vie *vie)
14191378

14201379
/* Get the register operand */
14211380
reg = vie->reg;
1422-
error = vm_get_register(vcpu, reg, &regop);
1423-
if (error != 0) {
1424-
return error;
1425-
}
1381+
regop = vm_get_register(vcpu, reg);
14261382

14271383
/* Get the memory operand */
14281384
error = mmio_read(vcpu, &memop);
@@ -1508,10 +1464,7 @@ static int emulate_sub(struct vcpu *vcpu, struct instr_emul_vie *vie)
15081464

15091465
/* get the first operand */
15101466
reg = vie->reg;
1511-
error = vm_get_register(vcpu, reg, &val1);
1512-
if (error != 0) {
1513-
break;
1514-
}
1467+
val1 = vm_get_register(vcpu, reg);
15151468

15161469
/* get the second operand */
15171470
error = mmio_read(vcpu, &val2);
@@ -1582,9 +1535,9 @@ static int emulate_stack_op(struct vcpu *vcpu, struct instr_emul_vie *vie,
15821535
}
15831536
}
15841537

1585-
error = vm_get_register(vcpu, CPU_REG_CR0, &cr0);
1586-
error = vm_get_register(vcpu, CPU_REG_RFLAGS, &rflags);
1587-
error = vm_get_register(vcpu, CPU_REG_RSP, &rsp);
1538+
cr0 = vm_get_register(vcpu, CPU_REG_CR0);
1539+
rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
1540+
rsp = vm_get_register(vcpu, CPU_REG_RSP);
15881541

15891542
if (pushop != 0) {
15901543
rsp -= size;
@@ -1711,7 +1664,7 @@ static int emulate_bittest(struct vcpu *vcpu, struct instr_emul_vie *vie)
17111664
return -EINVAL;
17121665
}
17131666

1714-
error = vm_get_register(vcpu, CPU_REG_RFLAGS, &rflags);
1667+
rflags = vm_get_register(vcpu, CPU_REG_RFLAGS);
17151668

17161669
error = mmio_read(vcpu, &val);
17171670
if (error != 0) {

0 commit comments

Comments
 (0)