Skip to content

Commit 42c4399

Browse files
ZideChen0wenlingz
ZideChen0
authored and
wenlingz
committed
hv: some coding refinement in hypercall.c
- since now we don't need to print error messages if copy_to/from_gpa() fails, then in many cases we can simplify the function return handling. In the following example, my fix could change the 'ret' value from the original '-1' to the actual errno returned from copy_to_gpa(). But this is valid. Ideally we may replace all '-1' with the actual errno. - if (copy_to_gpa() < 0) { - pr_err("error messages"); - ret = -1; - } else { - ret = 0; - } + ret = copy_to_gpa(); - in most cases, 'ret' is declared with a default value 0 or -1, then the redundant assignment statements can be removed. - replace white spaces with tabs. Tracked-On: #3854 Signed-off-by: Zide Chen <zide.chen@intel.com>
1 parent c9fa9c7 commit 42c4399

File tree

1 file changed

+29
-86
lines changed

1 file changed

+29
-86
lines changed

‎hypervisor/common/hypercall.c

Lines changed: 29 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ bool is_hypercall_from_ring0(void)
3030
cs_sel = exec_vmread16(VMX_GUEST_CS_SEL);
3131
/* cs_selector[1:0] is CPL */
3232
if ((cs_sel & 0x3U) == 0U) {
33-
ret = true;
33+
ret = true;
3434
} else {
3535
ret = false;
3636
}
@@ -90,15 +90,8 @@ int32_t hcall_get_api_version(struct acrn_vm *vm, uint64_t param)
9090

9191
version.major_version = HV_API_MAJOR_VERSION;
9292
version.minor_version = HV_API_MINOR_VERSION;
93-
int32_t ret;
94-
95-
if (copy_to_gpa(vm, &version, param, sizeof(version)) != 0) {
96-
ret = -1;
97-
} else {
98-
ret = 0;
99-
}
10093

101-
return ret;
94+
return copy_to_gpa(vm, &version, param, sizeof(version));
10295
}
10396

10497
/**
@@ -111,21 +104,17 @@ int32_t hcall_get_api_version(struct acrn_vm *vm, uint64_t param)
111104
* @param param GPA pointer to struct hc_platform_info.
112105
*
113106
* @pre Pointer vm shall point to SOS_VM
114-
* @return 0 on success, -1 in case of error.
107+
* @return 0 on success, non zero in case of error.
115108
*/
116109
int32_t hcall_get_platform_info(struct acrn_vm *vm, uint64_t param)
117110
{
118-
int32_t ret = 0;
119111
struct hc_platform_info platform_info;
120112

121113
platform_info.cpu_num = get_pcpu_nums();
122114
platform_info.max_vcpus_per_vm = MAX_VCPUS_PER_VM;
123115
platform_info.max_kata_containers = CONFIG_MAX_KATA_VM_NUM;
124-
if (copy_to_gpa(vm, &platform_info, param, sizeof(platform_info)) != 0) {
125-
ret = -1;
126-
}
127116

128-
return ret;
117+
return copy_to_gpa(vm, &platform_info, param, sizeof(platform_info));
129118
}
130119

131120
/**
@@ -163,27 +152,19 @@ int32_t hcall_create_vm(struct acrn_vm *vm, uint64_t param)
163152
if (((vm_config->guest_flags & GUEST_FLAG_LAPIC_PASSTHROUGH) != 0U)
164153
&& ((vm_config->guest_flags & GUEST_FLAG_RT) == 0U)) {
165154
pr_err("Wrong guest flags 0x%lx\n", vm_config->guest_flags);
166-
ret = -1;
167155
} else {
168-
ret = create_vm(vm_id, vm_config, &target_vm);
169-
if (ret != 0) {
156+
if (create_vm(vm_id, vm_config, &target_vm) != 0) {
170157
dev_dbg(DBG_LEVEL_HYCALL, "HCALL: Create VM failed");
171158
cv.vmid = ACRN_INVALID_VMID;
172-
ret = -1;
173159
} else {
174160
/* return a relative vm_id from SOS view */
175161
cv.vmid = vmid_2_rel_vmid(vm->vm_id, vm_id);
176162
cv.vcpu_num = vm_config->vcpu_num;
177-
ret = 0;
178163
}
179164

180-
if (copy_to_gpa(vm, &cv, param, sizeof(cv)) != 0) {
181-
ret = -1;
182-
}
165+
ret = copy_to_gpa(vm, &cv, param, sizeof(cv));
183166
}
184167
}
185-
} else {
186-
ret = -1;
187168
}
188169

189170
return ret;
@@ -281,7 +262,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
281262

282263
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
283264
/* TODO: check target_vm guest_flags */
284-
ret = reset_vm(target_vm);
265+
ret = reset_vm(target_vm);
285266
}
286267
return ret;
287268
}
@@ -357,7 +338,7 @@ int32_t hcall_set_irqline(const struct acrn_vm *vm, uint16_t vmid,
357338
*/
358339
irq_pic = (ops->gsi == 2U) ? 0U : ops->gsi;
359340
vpic_set_irqline(vm_pic(target_vm), irq_pic, ops->op);
360-
}
341+
}
361342

362343
/* handle IOAPIC irqline */
363344
vioapic_set_irqline_lock(target_vm, ops->gsi, ops->op);
@@ -411,7 +392,7 @@ static void inject_msi_lapic_pt(struct acrn_vm *vm, const struct acrn_msi_entry
411392
icr.bits.dest_field = dest;
412393
icr.bits.vector = vmsi_data.bits.vector;
413394
icr.bits.delivery_mode = vmsi_data.bits.delivery_mode;
414-
icr.bits.destination_mode = MSI_ADDR_DESTMODE_LOGICAL;
395+
icr.bits.destination_mode = MSI_ADDR_DESTMODE_LOGICAL;
415396

416397
msr_write(MSR_IA32_EXT_APIC_ICR, icr.value);
417398
dev_dbg(DBG_LEVEL_LAPICPT, "%s: icr.value 0x%016lx", __func__, icr.value);
@@ -439,8 +420,7 @@ int32_t hcall_inject_msi(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
439420
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
440421
struct acrn_msi_entry msi;
441422

442-
if (copy_from_gpa(vm, &msi, param, sizeof(msi)) != 0) {
443-
} else {
423+
if (copy_from_gpa(vm, &msi, param, sizeof(msi)) == 0) {
444424
/* For target cpu with lapic pt, send ipi instead of injection via vlapic */
445425
if (is_lapic_pt_configured(target_vm)) {
446426
enum vm_vlapic_state vlapic_state = check_vm_vlapic_state(target_vm);
@@ -496,8 +476,7 @@ int32_t hcall_set_ioreq_buffer(struct acrn_vm *vm, uint16_t vmid, uint64_t param
496476
if (is_created_vm(target_vm) && is_postlaunched_vm(target_vm)) {
497477
struct acrn_set_ioreq_buffer iobuf;
498478

499-
if (copy_from_gpa(vm, &iobuf, param, sizeof(iobuf)) != 0) {
500-
} else {
479+
if (copy_from_gpa(vm, &iobuf, param, sizeof(iobuf)) == 0) {
501480
dev_dbg(DBG_LEVEL_HYCALL, "[%d] SET BUFFER=0x%p",
502481
vmid, iobuf.req_buf);
503482

@@ -513,7 +492,7 @@ int32_t hcall_set_ioreq_buffer(struct acrn_vm *vm, uint16_t vmid, uint64_t param
513492
}
514493
ret = 0;
515494
}
516-
}
495+
}
517496
}
518497

519498
return ret;
@@ -560,7 +539,7 @@ int32_t hcall_notify_ioreq_finish(uint16_t vmid, uint16_t vcpu_id)
560539
*@pre Pointer vm shall point to SOS_VM
561540
*/
562541
static int32_t add_vm_memory_region(struct acrn_vm *vm, struct acrn_vm *target_vm,
563-
const struct vm_memory_region *region,uint64_t *pml4_page)
542+
const struct vm_memory_region *region,uint64_t *pml4_page)
564543
{
565544
int32_t ret;
566545
uint64_t prot;
@@ -758,8 +737,7 @@ int32_t hcall_write_protect_page(struct acrn_vm *vm, uint16_t vmid, uint64_t wp_
758737
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
759738
struct wp_data wp;
760739

761-
if (copy_from_gpa(vm, &wp, wp_gpa, sizeof(wp)) != 0) {
762-
} else {
740+
if (copy_from_gpa(vm, &wp, wp_gpa, sizeof(wp)) == 0) {
763741
ret = write_protect_page(target_vm, &wp);
764742
}
765743
} else {
@@ -795,9 +773,8 @@ int32_t hcall_gpa_to_hpa(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
795773
if (v_gpa2hpa.hpa == INVALID_HPA) {
796774
pr_err("%s,vm[%hu] gpa 0x%lx,GPA is unmapping.",
797775
__func__, target_vm->vm_id, v_gpa2hpa.gpa);
798-
} else if (copy_to_gpa(vm, &v_gpa2hpa, param, sizeof(v_gpa2hpa)) != 0) {
799776
} else {
800-
ret = 0;
777+
ret = copy_to_gpa(vm, &v_gpa2hpa, param, sizeof(v_gpa2hpa));
801778
}
802779
} else {
803780
pr_err("target_vm is invalid or HCALL gpa2hpa: Unable copy param from vm\n");
@@ -824,10 +801,9 @@ int32_t hcall_assign_pcidev(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
824801
struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
825802

826803
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
827-
if (copy_from_gpa(vm, &pcidev, param, sizeof(pcidev)) != 0) {
828-
} else {
804+
if (copy_from_gpa(vm, &pcidev, param, sizeof(pcidev)) == 0) {
829805
ret = vpci_assign_pcidev(target_vm, &pcidev);
830-
}
806+
}
831807
} else {
832808
pr_err("%s, vm[%d] is invalid\n", __func__, vm->vm_id);
833809
}
@@ -853,10 +829,9 @@ int32_t hcall_deassign_pcidev(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
853829
struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
854830

855831
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
856-
if (copy_from_gpa(vm, &pcidev, param, sizeof(pcidev)) != 0) {
857-
} else {
832+
if (copy_from_gpa(vm, &pcidev, param, sizeof(pcidev)) == 0) {
858833
ret = vpci_deassign_pcidev(target_vm, &pcidev);
859-
}
834+
}
860835
} else {
861836
pr_err("%s, vm[%d] is invalid\n", __func__, vm->vm_id);
862837
}
@@ -883,8 +858,7 @@ int32_t hcall_set_ptdev_intr_info(struct acrn_vm *vm, uint16_t vmid, uint64_t pa
883858
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
884859
struct hc_ptdev_irq irq;
885860

886-
if (copy_from_gpa(vm, &irq, param, sizeof(irq)) != 0) {
887-
} else {
861+
if (copy_from_gpa(vm, &irq, param, sizeof(irq)) == 0) {
888862
if (irq.type == IRQ_INTX) {
889863
struct pci_vdev *vdev;
890864
union pci_bdf bdf = {.value = irq.virt_bdf};
@@ -936,8 +910,7 @@ hcall_reset_ptdev_intr_info(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
936910
if (!is_poweroff_vm(target_vm) && is_postlaunched_vm(target_vm)) {
937911
struct hc_ptdev_irq irq;
938912

939-
if (copy_from_gpa(vm, &irq, param, sizeof(irq)) != 0) {
940-
} else {
913+
if (copy_from_gpa(vm, &irq, param, sizeof(irq)) == 0) {
941914
if (irq.type == IRQ_INTX) {
942915
struct pci_vdev *vdev;
943916
union pci_bdf bdf = {.value = irq.virt_bdf};
@@ -993,14 +966,8 @@ int32_t hcall_get_cpu_pm_state(struct acrn_vm *vm, uint64_t cmd, uint64_t param)
993966
if ((target_vm != NULL) && (!is_poweroff_vm(target_vm)) && (is_postlaunched_vm(target_vm))) {
994967
switch (cmd & PMCMD_TYPE_MASK) {
995968
case PMCMD_GET_PX_CNT: {
996-
997-
if (target_vm->pm.px_cnt == 0U) {
998-
ret = -1;
999-
} else if (copy_to_gpa(vm, &(target_vm->pm.px_cnt), param,
1000-
sizeof(target_vm->pm.px_cnt)) != 0) {
1001-
ret = -1;
1002-
} else {
1003-
ret = 0;
969+
if (target_vm->pm.px_cnt != 0U) {
970+
ret = copy_to_gpa(vm, &(target_vm->pm.px_cnt), param, sizeof(target_vm->pm.px_cnt));
1004971
}
1005972
break;
1006973
}
@@ -1013,35 +980,21 @@ int32_t hcall_get_cpu_pm_state(struct acrn_vm *vm, uint64_t cmd, uint64_t param)
1013980
* we need to check PMCMD_VCPUID_MASK in cmd.
1014981
*/
1015982
if (target_vm->pm.px_cnt == 0U) {
1016-
ret = -1;
1017983
break;
1018984
}
1019985

1020986
pn = (uint8_t)((cmd & PMCMD_STATE_NUM_MASK) >> PMCMD_STATE_NUM_SHIFT);
1021987
if (pn >= target_vm->pm.px_cnt) {
1022-
ret = -1;
1023988
break;
1024989
}
1025990

1026991
px_data = target_vm->pm.px_data + pn;
1027-
if (copy_to_gpa(vm, px_data, param,
1028-
sizeof(struct cpu_px_data)) != 0) {
1029-
ret = -1;
1030-
break;
1031-
}
1032-
1033-
ret = 0;
992+
ret = copy_to_gpa(vm, px_data, param, sizeof(struct cpu_px_data));
1034993
break;
1035994
}
1036995
case PMCMD_GET_CX_CNT: {
1037-
1038-
if (target_vm->pm.cx_cnt == 0U) {
1039-
ret = -1;
1040-
} else if (copy_to_gpa(vm, &(target_vm->pm.cx_cnt), param,
1041-
sizeof(target_vm->pm.cx_cnt)) != 0) {
1042-
ret = -1;
1043-
} else {
1044-
ret = 0;
996+
if (target_vm->pm.cx_cnt != 0U) {
997+
ret = copy_to_gpa(vm, &(target_vm->pm.cx_cnt), param, sizeof(target_vm->pm.cx_cnt));
1045998
}
1046999
break;
10471000
}
@@ -1050,32 +1003,22 @@ int32_t hcall_get_cpu_pm_state(struct acrn_vm *vm, uint64_t cmd, uint64_t param)
10501003
struct cpu_cx_data *cx_data;
10511004

10521005
if (target_vm->pm.cx_cnt == 0U) {
1053-
ret = -1;
10541006
break;
10551007
}
10561008

10571009
cx_idx = (uint8_t)
10581010
((cmd & PMCMD_STATE_NUM_MASK) >> PMCMD_STATE_NUM_SHIFT);
10591011
if ((cx_idx == 0U) || (cx_idx > target_vm->pm.cx_cnt)) {
1060-
ret = -1;
10611012
break;
10621013
}
10631014

10641015
cx_data = target_vm->pm.cx_data + cx_idx;
1065-
1066-
if (copy_to_gpa(vm, cx_data, param,
1067-
sizeof(struct cpu_cx_data)) != 0) {
1068-
ret = -1;
1069-
break;
1070-
}
1071-
1072-
ret = 0;
1016+
ret = copy_to_gpa(vm, cx_data, param, sizeof(struct cpu_cx_data));
10731017
break;
10741018
}
10751019
default:
1076-
ret = -1;
1020+
/* invalid command */
10771021
break;
1078-
10791022
}
10801023
}
10811024

@@ -1152,7 +1095,7 @@ int32_t hcall_set_callback_vector(const struct acrn_vm *vm, uint64_t param)
11521095

11531096
if (!is_sos_vm(vm)) {
11541097
pr_err("%s: Targeting to service vm", __func__);
1155-
ret = -EPERM;
1098+
ret = -EPERM;
11561099
} else if ((param > NR_MAX_VECTOR) || (param < VECTOR_DYNAMIC_START)) {
11571100
pr_err("%s: Invalid passed vector\n", __func__);
11581101
ret = -EINVAL;

0 commit comments

Comments
 (0)