|
Re: [PATCH 04/10] HV: remove few return statement in while loop of copy_gva function
Acked-by: Eddie Dong <eddie.dong@...>
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
Re: [PATCH 03/10] HV: add #ifdef for include header file
#ifdef
toggle quoted message
Show quoted text
-----Original Message-----MISRAC does not allow "_" for the 1st character of the name. struct e820_mem_params { |
|
|
|
Re: [PATCH 2/9] HV: define vm_pcpu_ids as inline array to simplify code
Acked-by: Eddie Dong <eddie.dong>
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
Re: [PATCH V2 2/4] hv: virq: refine vcpu_inject_hi_exception()
Li, Fei1
On Fri, Dec 21, 2018 at 01:24:35PM +0800, Dong, Eddie wrote:
Can we use GOTO?No. "One function only one exit and no goto" is high required. Thanks. -----Original Message----- |
|
|
|
Re: [PATCH 1/9] HV: remove unused enum vm_privilege_level
Acked-by: Eddie Dong <eddie.dong>
toggle quoted message
Show quoted text
Please add commit message -----Original Message----- |
|
|
|
Re: [PATCH V2 2/4] hv: virq: refine vcpu_inject_hi_exception()
Acked-by: Eddie Dong <eddie.dong@...>
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
Re: [PATCH V2 1/4] hv: virq: refine vcpu_inject_vlapic_int() has more than one exit point
Acked-by: Eddie Dong <eddie.dong@...>
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
Re: [PATCH V2 4/4] hv: virq: refine acrn_handle_pending_request() use goto instruction
Should this one be applied earlier than 3/4?
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
Re: [PATCH V2 2/4] hv: virq: refine vcpu_inject_hi_exception()
Can we use GOTO?
toggle quoted message
Show quoted text
-----Original Message----- |
|
|
|
[PATCH 10/10] HV: remove multiple exit/return in routines in the file of vlapic.c
YanLiang
From: Chaohong guo <chaohong.guo@...>
To meet MISRA, remove few return in routines of vlapic. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/vlapic.c | 229 +++++++++++++++-------------- 1 file changed, 122 insertions(+), 107 deletions(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index aa46b176..4f30e12e 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -111,17 +111,21 @@ static uint16_t vm_apicid2vcpu_id(struct acrn_vm *vm, uint8_t lapicid) { uint16_t i; struct acrn_vcpu *vcpu; + uint16_t cpuid = 0xFFFFU; foreach_vcpu(i, vm, vcpu) { const struct acrn_vlapic *vlapic = vcpu_vlapic(vcpu); if (vlapic_get_apicid(vlapic) == lapicid) { - return vcpu->vcpu_id; + cpuid = vcpu->vcpu_id; } } - pr_err("%s: bad lapicid %hhu", __func__, lapicid); + if (cpuid == 0xFFFFU) { + cpuid = phys_cpu_num; + pr_err("%s: bad lapicid %hhu", __func__, lapicid); + } - return phys_cpu_num; + return cpuid; } /* @@ -629,10 +633,12 @@ vlapic_get_lvtptr(struct acrn_vlapic *vlapic, uint32_t offset) { struct lapic_regs *lapic = &(vlapic->apic_page); uint32_t i; + uint32_t *lvt; switch (offset) { case APIC_OFFSET_CMCI_LVT: - return &lapic->lvt_cmci.v; + lvt = &lapic->lvt_cmci.v; + break; default: /* * The function caller could guarantee the pre condition. @@ -640,8 +646,10 @@ vlapic_get_lvtptr(struct acrn_vlapic *vlapic, uint32_t offset) * could be handled here. */ i = lvt_off_to_idx(offset); - return &(lapic->lvt[i].v); + lvt = &(lapic->lvt[i].v); + break; } + return lvt; } static inline uint32_t @@ -659,6 +667,7 @@ vlapic_lvt_write_handler(struct acrn_vlapic *vlapic, uint32_t offset) { uint32_t *lvtptr, mask, val, idx; struct lapic_regs *lapic; + int error = 0; lapic = &(vlapic->apic_page); lvtptr = vlapic_get_lvtptr(vlapic, offset); @@ -698,7 +707,7 @@ vlapic_lvt_write_handler(struct acrn_vlapic *vlapic, uint32_t offset) "vpic wire mode -> LAPIC"); } else { pr_err("WARNING:invalid vpic wire mode change"); - return; + error = 1; } /* unmask -> mask: only from the vlapic LINT0-ExtINT enabled */ } else if (((last & APIC_LVT_M) == 0U) && ((val & APIC_LVT_M) != 0U)) { @@ -716,9 +725,11 @@ vlapic_lvt_write_handler(struct acrn_vlapic *vlapic, uint32_t offset) /* No action required. */ } - *lvtptr = val; - idx = lvt_off_to_idx(offset); - atomic_store32(&vlapic->lvt_last[idx], val); + if (error == 0) { + *lvtptr = val; + idx = lvt_off_to_idx(offset); + atomic_store32(&vlapic->lvt_last[idx], val); + } } static void @@ -757,29 +768,28 @@ vlapic_fire_lvt(struct acrn_vlapic *vlapic, uint32_t lvt) uint32_t vec, mode; struct acrn_vcpu *vcpu = vlapic->vcpu; - if ((lvt & APIC_LVT_M) != 0U) { - return; - } + if ((lvt & APIC_LVT_M) == 0U) { - vec = lvt & APIC_LVT_VECTOR; - mode = lvt & APIC_LVT_DM; + vec = lvt & APIC_LVT_VECTOR; + mode = lvt & APIC_LVT_DM; - switch (mode) { - case APIC_LVT_DM_FIXED: - if (vlapic_set_intr_ready(vlapic, vec, false) != 0) { - vcpu_make_request(vcpu, ACRN_REQUEST_EVENT); + switch (mode) { + case APIC_LVT_DM_FIXED: + if (vlapic_set_intr_ready(vlapic, vec, false) != 0) { + vcpu_make_request(vcpu, ACRN_REQUEST_EVENT); + } + break; + case APIC_LVT_DM_NMI: + vcpu_inject_nmi(vcpu); + break; + case APIC_LVT_DM_EXTINT: + vcpu_inject_extint(vcpu); + break; + default: + /* Other modes ignored */ + pr_warn("func:%s other mode is not support\n",__func__); + break; } - break; - case APIC_LVT_DM_NMI: - vcpu_inject_nmi(vcpu); - break; - case APIC_LVT_DM_EXTINT: - vcpu_inject_extint(vcpu); - break; - default: - /* Other modes ignored */ - pr_warn("func:%s other mode is not support\n",__func__); - return; } return; } @@ -906,7 +916,7 @@ vlapic_process_eoi(struct acrn_vlapic *vlapic) /* hook to vIOAPIC */ vioapic_process_eoi(vlapic->vm, vector); } - return; + break; } } @@ -935,6 +945,7 @@ static int32_t vlapic_trigger_lvt(struct acrn_vlapic *vlapic, uint32_t vector) { uint32_t lvt; + uint32_t ret = 0; struct acrn_vcpu *vcpu = vlapic->vcpu; if (vlapic_enabled(vlapic) == false) { @@ -958,42 +969,46 @@ vlapic_trigger_lvt(struct acrn_vlapic *vlapic, uint32_t vector) */ break; } - return 0; - } - - switch (vector) { - case APIC_LVT_LINT0: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_LINT0_LVT); - break; - case APIC_LVT_LINT1: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_LINT1_LVT); - break; - case APIC_LVT_TIMER: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_TIMER_LVT); - lvt |= APIC_LVT_DM_FIXED; - break; - case APIC_LVT_ERROR: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_ERROR_LVT); - lvt |= APIC_LVT_DM_FIXED; - break; - case APIC_LVT_PMC: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_PERF_LVT); - break; - case APIC_LVT_THERMAL: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_THERM_LVT); - break; - case APIC_LVT_CMCI: - lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_CMCI_LVT); - break; - default: - return -EINVAL; - } - if (vector < 16U) { - vlapic_set_error(vlapic, APIC_ESR_RECEIVE_ILLEGAL_VECTOR); } else { - vlapic_fire_lvt(vlapic, lvt); + + switch (vector) { + case APIC_LVT_LINT0: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_LINT0_LVT); + break; + case APIC_LVT_LINT1: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_LINT1_LVT); + break; + case APIC_LVT_TIMER: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_TIMER_LVT); + lvt |= APIC_LVT_DM_FIXED; + break; + case APIC_LVT_ERROR: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_ERROR_LVT); + lvt |= APIC_LVT_DM_FIXED; + break; + case APIC_LVT_PMC: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_PERF_LVT); + break; + case APIC_LVT_THERMAL: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_THERM_LVT); + break; + case APIC_LVT_CMCI: + lvt = vlapic_get_lvt(vlapic, APIC_OFFSET_CMCI_LVT); + break; + default: + ret = -EINVAL; + break; + } + + if (ret == 0) { + if (vector < 16U) { + vlapic_set_error(vlapic, APIC_ESR_RECEIVE_ILLEGAL_VECTOR); + } else { + vlapic_fire_lvt(vlapic, lvt); + } + } } - return 0; + return ret; } /* @@ -1170,45 +1185,43 @@ vlapic_process_init_sipi(struct acrn_vcpu* target_vcpu, uint32_t mode, uint32_t icr_low, uint16_t vcpu_id) { if (mode == APIC_DELMODE_INIT) { - if ((icr_low & APIC_LEVEL_MASK) == APIC_LEVEL_DEASSERT) { - return; - } + if ((icr_low & APIC_LEVEL_MASK) != APIC_LEVEL_DEASSERT) { - dev_dbg(ACRN_DBG_LAPIC, + dev_dbg(ACRN_DBG_LAPIC, "Sending INIT from VCPU %hu to %hu", target_vcpu->vcpu_id, vcpu_id); - /* put target vcpu to INIT state and wait for SIPI */ - pause_vcpu(target_vcpu, VCPU_PAUSED); - reset_vcpu(target_vcpu); - /* new cpu model only need one SIPI to kick AP run, - * the second SIPI will be ignored as it move out of - * wait-for-SIPI state. - */ - target_vcpu->arch.nr_sipi = 1U; + /* put target vcpu to INIT state and wait for SIPI */ + pause_vcpu(target_vcpu, VCPU_PAUSED); + reset_vcpu(target_vcpu); + /* new cpu model only need one SIPI to kick AP run, + * the second SIPI will be ignored as it move out of + * wait-for-SIPI state. + */ + target_vcpu->arch.nr_sipi = 1U; + } } else if (mode == APIC_DELMODE_STARTUP) { /* Ignore SIPIs in any state other than wait-for-SIPI */ - if ((target_vcpu->state != VCPU_INIT) || - (target_vcpu->arch.nr_sipi == 0U)) { - return; - } + if ((target_vcpu->state == VCPU_INIT) && + (target_vcpu->arch.nr_sipi != 0U)) { - dev_dbg(ACRN_DBG_LAPIC, + dev_dbg(ACRN_DBG_LAPIC, "Sending SIPI from VCPU %hu to %hu with vector %u", target_vcpu->vcpu_id, vcpu_id, (icr_low & APIC_VECTOR_MASK)); - target_vcpu->arch.nr_sipi--; - if (target_vcpu->arch.nr_sipi > 0U) { - return; - } + target_vcpu->arch.nr_sipi--; + if (target_vcpu->arch.nr_sipi <= 0U) { - pr_err("Start Secondary VCPU%hu for VM[%d]...", - target_vcpu->vcpu_id, - target_vcpu->vm->vm_id); - set_ap_entry(target_vcpu, (icr_low & APIC_VECTOR_MASK) << 12U); - schedule_vcpu(target_vcpu); + pr_err("Start Secondary VCPU%hu for VM[%d]...", + target_vcpu->vcpu_id, + target_vcpu->vm->vm_id); + set_ap_entry(target_vcpu, (icr_low & APIC_VECTOR_MASK) << 12U); + schedule_vcpu(target_vcpu); + } + } } + return; } static int32_t @@ -1327,30 +1340,32 @@ vlapic_pending_intr(const struct acrn_vlapic *vlapic, uint32_t *vecptr) const struct lapic_regs *lapic = &(vlapic->apic_page); uint32_t i, vector, val, bitpos; const struct lapic_reg *irrptr; + int32_t ret = 0; if (is_apicv_intr_delivery_supported()) { - return apicv_pending_intr(vlapic); - } - - irrptr = &lapic->irr[0]; + ret = apicv_pending_intr(vlapic); + } else { - /* i ranges effectively from 7 to 0 */ - for (i = 8U; i > 0U; ) { - i--; - val = atomic_load32(&irrptr[i].v); - bitpos = (uint32_t)fls32(val); - if (bitpos != INVALID_BIT_INDEX) { - vector = (i * 32U) + bitpos; - if (prio(vector) > prio(lapic->ppr.v)) { - if (vecptr != NULL) { - *vecptr = vector; + irrptr = &lapic->irr[0]; + + /* i ranges effectively from 7 to 0 */ + for (i = 8U; i > 0U; ) { + i--; + val = atomic_load32(&irrptr[i].v); + bitpos = (uint32_t)fls32(val); + if (bitpos != INVALID_BIT_INDEX) { + vector = (i * 32U) + bitpos; + if (prio(vector) > prio(lapic->ppr.v)) { + if (vecptr != NULL) { + *vecptr = vector; + } + ret = 1; } - return 1; + break; } - break; } } - return 0; + return ret; } /** -- 2.19.0 |
|
|
|
[PATCH 09/10] HV: Remove goto statement in guest.c
YanLiang
From: Chaohong guo <chaohong.guo@...>
To meet MISRA, remove some goto statements in guest.c Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/guest.c | 162 ++++++++++++++---------------- 1 file changed, 75 insertions(+), 87 deletions(-) diff --git a/hypervisor/arch/x86/guest/guest.c b/hypervisor/arch/x86/guest/guest.c index 48c2fcfc..d5cd3d9e 100644 --- a/hypervisor/arch/x86/guest/guest.c +++ b/hypervisor/arch/x86/guest/guest.c @@ -75,85 +75,85 @@ static int32_t local_gva2gpa_common(struct acrn_vcpu *vcpu, const struct page_wa { uint32_t i; uint64_t index; - uint32_t shift; + uint32_t shift = 12U; void *base; - uint64_t entry; - uint64_t addr, page_size; + uint64_t entry = 0; + uint64_t addr; + uint64_t page_size = PAGE_SIZE_4K; int32_t ret = 0; int32_t fault = 0; bool is_user_mode_addr = true; bool is_page_rw_flags_on = true; if (pw_info->level < 1U) { - return -EINVAL; + fault = 1; + } else { + addr = pw_info->top_entry; + i = pw_info->level; + stac(); } - addr = pw_info->top_entry; - i = pw_info->level; - stac(); - while (i != 0U) { + while ((i != 0U) && (fault == 0)) { i--; addr = addr & IA32E_REF_MASK; base = gpa2hva(vcpu->vm, addr); - if (base == NULL) { - ret = -EFAULT; - goto out; - } + if (base != NULL) { + shift = (i * pw_info->width) + 12U; + index = (gva >> shift) & ((1UL << pw_info->width) - 1UL); + page_size = 1UL << shift; + + if (pw_info->width == 10U) { + uint32_t *base32 = (uint32_t *)base; + /* 32bit entry */ + entry = (uint64_t)(*(base32 + index)); + } else { + uint64_t *base64 = (uint64_t *)base; + entry = *(base64 + index); + } - shift = (i * pw_info->width) + 12U; - index = (gva >> shift) & ((1UL << pw_info->width) - 1UL); - page_size = 1UL << shift; + /* check if the entry present */ + if ((entry & PAGE_PRESENT) == 0U) { + fault = 1; + } else { - if (pw_info->width == 10U) { - uint32_t *base32 = (uint32_t *)base; - /* 32bit entry */ - entry = (uint64_t)(*(base32 + index)); + /* check for R/W */ + if ((entry & PAGE_RW) == 0U) { + if ((pw_info->is_write_access) && + (pw_info->is_user_mode_access || pw_info->wp)) { + /* Case1: Supermode and wp is 1 + * Case2: Usermode */ + fault = 1; + } + is_page_rw_flags_on = false; + } + } } else { - uint64_t *base64 = (uint64_t *)base; - entry = *(base64 + index); + fault = 1; } - /* check if the entry present */ - if ((entry & PAGE_PRESENT) == 0U) { - ret = -EFAULT; - goto out; - } + if (fault == 0) { + /* check for nx, since for 32-bit paing, the XD bit is + * reserved(0), use the same logic as PAE/4-level paging */ + if (pw_info->is_inst_fetch && pw_info->nxe && + ((entry & PAGE_NX) != 0U)) { + fault = 1; + } - /* check for R/W */ - if ((entry & PAGE_RW) == 0U) { - if (pw_info->is_write_access) { - /* Case1: Supermode and wp is 1 - * Case2: Usermode */ - if (pw_info->is_user_mode_access || - pw_info->wp) { + /* check for U/S */ + if ((entry & PAGE_USER) == 0U) { + is_user_mode_addr = false; + + if (pw_info->is_user_mode_access) { fault = 1; - goto out; } } - is_page_rw_flags_on = false; - } - /* check for nx, since for 32-bit paing, the XD bit is - * reserved(0), use the same logic as PAE/4-level paging */ - if (pw_info->is_inst_fetch && pw_info->nxe && - ((entry & PAGE_NX) != 0U)) { - fault = 1; - goto out; } - /* check for U/S */ - if ((entry & PAGE_USER) == 0U) { - is_user_mode_addr = false; - - if (pw_info->is_user_mode_access) { - fault = 1; - goto out; - } - } - - if (pw_info->pse && ((i > 0U) && ((entry & PAGE_PSE) != 0U))) { - break; + if ((fault == 0) && pw_info->pse && + ((i > 0U) && ((entry & PAGE_PSE) != 0U))) { + break; } addr = entry; } @@ -163,22 +163,19 @@ static int32_t local_gva2gpa_common(struct acrn_vcpu *vcpu, const struct page_wa * Also SMAP/SMEP only impact the supervisor-mode access. */ /* if smap is enabled and supervisor-mode access */ - if (pw_info->is_smap_on && !pw_info->is_user_mode_access && + if (!fault && pw_info->is_smap_on && !pw_info->is_user_mode_access && is_user_mode_addr) { bool rflags_ac = ((vcpu_get_rflags(vcpu) & RFLAGS_AC) != 0UL); /* read from user mode address, eflags.ac = 0 */ if (!pw_info->is_write_access && !rflags_ac) { fault = 1; - goto out; - } + } else if (pw_info->is_write_access) { + /* write to user mode address */ - /* write to user mode address */ - if (pw_info->is_write_access) { /* cr0.wp = 0, eflags.ac = 0 */ if (!pw_info->wp && !rflags_ac) { fault = 1; - goto out; } /* cr0.wp = 1, eflags.ac = 1, r/w flag is 0 @@ -186,30 +183,28 @@ static int32_t local_gva2gpa_common(struct acrn_vcpu *vcpu, const struct page_wa */ if (pw_info->wp && rflags_ac && !is_page_rw_flags_on) { fault = 1; - goto out; } /* cr0.wp = 1, eflags.ac = 0 */ if (pw_info->wp && !rflags_ac) { fault = 1; - goto out; } } } /* instruction fetch from user-mode address, smep on */ - if (pw_info->is_smep_on && !pw_info->is_user_mode_access && + if (!fault && pw_info->is_smep_on && !pw_info->is_user_mode_access && is_user_mode_addr && pw_info->is_inst_fetch) { fault = 1; - goto out; } - entry >>= shift; - /* shift left 12bit more and back to clear XD/Prot Key/Ignored bits */ - entry <<= (shift + 12U); - entry >>= 12U; - *gpa = entry | (gva & (page_size - 1UL)); -out: + if (fault == 0) { + entry >>= shift; + /* shift left 12bit more and back to clear XD/Prot Key/Ignored bits */ + entry <<= (shift + 12U); + entry >>= 12U; + *gpa = entry | (gva & (page_size - 1UL)); + } clac(); if (fault != 0) { @@ -226,28 +221,21 @@ static int32_t local_gva2gpa_pae(struct acrn_vcpu *vcpu, struct page_walk_info * uint64_t *base; uint64_t entry; uint64_t addr; - int32_t ret; + int32_t ret = -EFAULT; addr = pw_info->top_entry & 0xFFFFFFF0U; base = (uint64_t *)gpa2hva(vcpu->vm, addr); - if (base == NULL) { - ret = -EFAULT; - goto out; - } - - index = (gva >> 30U) & 0x3UL; - entry = base[index]; - - if ((entry & PAGE_PRESENT) == 0U) { - ret = -EFAULT; - goto out; + if (base != NULL) { + index = (gva >> 30U) & 0x3UL; + entry = base[index]; + + if (entry & PAGE_PRESENT) { + pw_info->level = 2U; + pw_info->top_entry = entry; + ret = local_gva2gpa_common(vcpu, pw_info, gva, gpa, err_code); + } } - pw_info->level = 2U; - pw_info->top_entry = entry; - ret = local_gva2gpa_common(vcpu, pw_info, gva, gpa, err_code); - -out: return ret; } -- 2.19.0 |
|
|
|
[PATCH 08/10] HV: trival changes to meet MISRA
YanLiang
From: Chaohong guo <chaohong.guo@...>
add @pre for routines and bracks for condition checking in vlapic.c Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/vlapic.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index f755d267..aa46b176 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -124,6 +124,9 @@ static uint16_t vm_apicid2vcpu_id(struct acrn_vm *vm, uint8_t lapicid) return phys_cpu_num; } +/* + * @pre vm != NULL + */ static uint64_t vm_active_cpus(const struct acrn_vm *vm) { @@ -138,6 +141,9 @@ vm_active_cpus(const struct acrn_vm *vm) return dmask; } +/* + * @pre vlapic != NULL + */ uint32_t vlapic_get_apicid(const struct acrn_vlapic *vlapic) { @@ -151,6 +157,9 @@ vlapic_get_apicid(const struct acrn_vlapic *vlapic) return apicid; } +/* + * @pre vlapic != NULL + */ static inline uint32_t vlapic_build_id(const struct acrn_vlapic *vlapic) { @@ -350,7 +359,7 @@ static uint32_t vlapic_get_ccr(const struct acrn_vlapic *vlapic) vtimer = &vlapic->vtimer; - if ((vtimer->tmicr != 0U) && !vlapic_lvtt_tsc_deadline(vlapic)) { + if ((vtimer->tmicr != 0U) && (!vlapic_lvtt_tsc_deadline(vlapic))) { uint64_t fire_tsc = vtimer->timer.fire_tsc; if (now < fire_tsc) { @@ -2055,6 +2064,9 @@ static void vlapic_timer_expired(void *data) } } +/* + * @pre vm != NULL + */ static inline bool is_x2apic_enabled(const struct acrn_vlapic *vlapic) { bool ret; -- 2.19.0 |
|
|
|
[PATCH 07/10] HV: check if the returned target_vcpu pointer is NULL
YanLiang
From: Chaohong guo <chaohong.guo@...>
vcpu_from_vid() might return NULL from code logic. MISRA complain about this. So, add a checking for the pointer, return ENODEV in case it is NULL. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/vlapic.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index f85dd9e0..f755d267 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -1270,6 +1270,10 @@ vlapic_icrlo_write_handler(struct acrn_vlapic *vlapic) for (vcpu_id = 0U; vcpu_id < vlapic->vm->hw.created_vcpus; vcpu_id++) { if ((dmask & (1UL << vcpu_id)) != 0UL) { target_vcpu = vcpu_from_vid(vlapic->vm, vcpu_id); + if (target_vcpu == NULL) { + return -ENODEV; + } + if (mode == APIC_DELMODE_FIXED) { vlapic_set_intr(target_vcpu, vec, LAPIC_TRIG_EDGE); -- 2.19.0 |
|
|
|
[PATCH 06/10] HV: add const qualifier for functions' argments in vlapic.c
YanLiang
From: Chaohong guo <chaohong.guo@...>
If a argment is not changed in function, MISRA recommends to add const qualifier for that argement. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/vlapic.c | 4 ++-- hypervisor/include/arch/x86/guest/vlapic.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index d4457015..f85dd9e0 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -113,7 +113,7 @@ static uint16_t vm_apicid2vcpu_id(struct acrn_vm *vm, uint8_t lapicid) struct acrn_vcpu *vcpu; foreach_vcpu(i, vm, vcpu) { - struct acrn_vlapic *vlapic = vcpu_vlapic(vcpu); + const struct acrn_vlapic *vlapic = vcpu_vlapic(vcpu); if (vlapic_get_apicid(vlapic) == lapicid) { return vcpu->vcpu_id; } @@ -139,7 +139,7 @@ vm_active_cpus(const struct acrn_vm *vm) } uint32_t -vlapic_get_apicid(struct acrn_vlapic *vlapic) +vlapic_get_apicid(const struct acrn_vlapic *vlapic) { uint32_t apicid; if (is_x2apic_enabled(vlapic)) { diff --git a/hypervisor/include/arch/x86/guest/vlapic.h b/hypervisor/include/arch/x86/guest/vlapic.h index 3a96ef07..fc929d96 100644 --- a/hypervisor/include/arch/x86/guest/vlapic.h +++ b/hypervisor/include/arch/x86/guest/vlapic.h @@ -264,7 +264,7 @@ void vlapic_set_tmr_one_vec(struct acrn_vlapic *vlapic, uint32_t delmode, uint32_t vector, bool level); void vlapic_apicv_batch_set_tmr(struct acrn_vlapic *vlapic); -uint32_t vlapic_get_apicid(struct acrn_vlapic *vlapic); +uint32_t vlapic_get_apicid(const struct acrn_vlapic *vlapic); int32_t vlapic_create(struct acrn_vcpu *vcpu); /* * @pre vcpu != NULL -- 2.19.0 |
|
|
|
[PATCH 05/10] HV: remove multiple return statement in get_vcpu_paging_mode() routine
YanLiang
From: Chaohong guo <chaohong.guo@...>
To meet MISRA, remove multiple return in get_vcpu_paging_mode() routine. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/guest.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/hypervisor/arch/x86/guest/guest.c b/hypervisor/arch/x86/guest/guest.c index 047f7d3d..48c2fcfc 100644 --- a/hypervisor/arch/x86/guest/guest.c +++ b/hypervisor/arch/x86/guest/guest.c @@ -47,23 +47,25 @@ uint64_t vcpumask2pcpumask(struct acrn_vm *vm, uint64_t vdmask) enum vm_paging_mode get_vcpu_paging_mode(struct acrn_vcpu *vcpu) { enum vm_cpu_mode cpu_mode; + enum vm_paging_mode ret; cpu_mode = get_vcpu_mode(vcpu); if (cpu_mode == CPU_MODE_REAL) { - return PAGING_MODE_0_LEVEL; - } - else if (cpu_mode == CPU_MODE_PROTECTED) { - if (is_pae(vcpu)) { - return PAGING_MODE_3_LEVEL; - } - else if (is_paging_enabled(vcpu)) { - return PAGING_MODE_2_LEVEL; - } - return PAGING_MODE_0_LEVEL; + ret = PAGING_MODE_0_LEVEL; + + } else if (cpu_mode == CPU_MODE_PROTECTED) { + if (is_pae(vcpu)) + ret = PAGING_MODE_3_LEVEL; + else if (is_paging_enabled(vcpu)) + ret = PAGING_MODE_2_LEVEL; + else + ret = PAGING_MODE_0_LEVEL; } else { /* compatibility or 64bit mode */ - return PAGING_MODE_4_LEVEL; + ret = PAGING_MODE_4_LEVEL; } + + return ret; } /* TODO: Add code to check for Revserved bits, SMAP and PKE when do translation -- 2.19.0 |
|
|
|
[PATCH 04/10] HV: remove few return statement in while loop of copy_gva function
YanLiang
From: Chaohong guo <chaohong.guo@...>
The coding style of multiple returns/exit in while loop is not MISRA compatible. Remove the returns in while loop. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/guest.c | 32 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/hypervisor/arch/x86/guest/guest.c b/hypervisor/arch/x86/guest/guest.c index 5c0fd7dc..047f7d3d 100644 --- a/hypervisor/arch/x86/guest/guest.c +++ b/hypervisor/arch/x86/guest/guest.c @@ -391,33 +391,29 @@ static inline int32_t copy_gva(struct acrn_vcpu *vcpu, void *h_ptr_arg, uint64_t { void *h_ptr = h_ptr_arg; uint64_t gpa = 0UL; - int32_t ret; + int32_t ret = 0; uint32_t len; uint64_t gva = gva_arg; uint32_t size = size_arg; - while (size > 0U) { + while ((size > 0U) && (ret == 0)) { ret = gva2gpa(vcpu, gva, &gpa, err_code); - if (ret < 0) { + if (ret >= 0) { + len = local_copy_gpa(vcpu->vm, h_ptr, gpa, size, PAGE_SIZE_4K, cp_from_vm); + if (len != 0U) { + gva += len; + h_ptr += len; + size -= len; + } else { + ret = -EINVAL; + } + } else { *fault_addr = gva; - pr_err("error[%d] in GVA2GPA, err_code=0x%x", - ret, *err_code); - return ret; + pr_err("error[%d] in GVA2GPA, err_code=0x%x", ret, *err_code); } - - len = local_copy_gpa(vcpu->vm, h_ptr, gpa, size, - PAGE_SIZE_4K, cp_from_vm); - - if (len == 0U) { - return -EINVAL; - } - - gva += len; - h_ptr += len; - size -= len; } - return 0; + return ret; } /* @pre Caller(Guest) should make sure gpa is continuous. -- 2.19.0 |
|
|
|
[PATCH 03/10] HV: add #ifdef for include header file
#ifdef
YanLiang
From: Chaohong guo <chaohong.guo@...>
e820.h file has no ifdef protection for repeated/recursive inclusion. Just add that. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/include/arch/x86/e820.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hypervisor/include/arch/x86/e820.h b/hypervisor/include/arch/x86/e820.h index d932a2ef..8acb9a0d 100644 --- a/hypervisor/include/arch/x86/e820.h +++ b/hypervisor/include/arch/x86/e820.h @@ -4,6 +4,8 @@ * SPDX-License-Identifier: BSD-3-Clause */ +#ifndef _E820_H_ +#define _E820_H_ struct e820_mem_params { uint64_t mem_bottom; uint64_t mem_top; @@ -43,3 +45,5 @@ const struct e820_mem_params *get_e820_mem_info(void); #define NUM_E820_ENTRIES 5U extern const struct e820_entry e820_default_entries[NUM_E820_ENTRIES]; #endif + +#endif /* _E820_H_ */ -- 2.19.0 |
|
|
|
[PATCH 02/10] HV: move global variable into the scope of calling function
YanLiang
From: Chaohong guo <chaohong.guo@...>
The static global variable apicv_apic_access_addr is used only by vlapic_apicv_get_apic_access_addr(), to remove the warning by MISRA, move it into function scope. Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/vlapic.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index 34461f58..d4457015 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -77,9 +77,6 @@ static inline void vlapic_dump_irr(__unused const struct acrn_vlapic *vlapic, __ static inline void vlapic_dump_isr(__unused const struct acrn_vlapic *vlapic, __unused const char *msg) {} #endif -/*APIC-v APIC-access address */ -static uint8_t apicv_apic_access_addr[PAGE_SIZE] __aligned(PAGE_SIZE); - static int32_t apicv_set_intr_ready(struct acrn_vlapic *vlapic, uint32_t vector); @@ -2347,6 +2344,9 @@ apicv_batch_set_tmr(const struct acrn_vlapic *vlapic) uint64_t vlapic_apicv_get_apic_access_addr(void) { + /*APIC-v APIC-access address */ + static uint8_t apicv_apic_access_addr[PAGE_SIZE] __aligned(PAGE_SIZE); + return hva2hpa(apicv_apic_access_addr); } -- 2.19.0 |
|
|
|
[PATCH 01/10] HV: APICBASE_RESERVED definition is not used by any code. Just remove it
YanLiang
From: Chaohong guo <chaohong.guo@...>
To meet MISRA standard, remove unused micro definition Signed-off-by: Chaohong guo <chaohong.guo@...> --- hypervisor/arch/x86/guest/vlapic.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index 8b9ce666..34461f58 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -45,7 +45,6 @@ static inline uint32_t prio(uint32_t x) #define VLAPIC_VERSION (16U) -#define APICBASE_RESERVED 0x000002ffU #define APICBASE_BSP 0x00000100UL #define APICBASE_X2APIC 0x00000400U #define APICBASE_ENABLED 0x00000800UL -- 2.19.0 |
|
|
|
[PATCH 9/9] HV: replace get_vm_from_vmid(0U) with get_sos_vm to remove the assumption that 1st sos's vmid is 0
Dongsheng Zhang
From: dongshen <dongsheng.x.zhang@...>
Tracked-On: #2143 Signed-off-by: dongshen <dongsheng.x.zhang@...> --- hypervisor/arch/x86/guest/vm.c | 45 ++++++++++++++++++++++++++++++++++ hypervisor/common/io_request.c | 6 ++--- hypervisor/debug/vuart.c | 2 +- hypervisor/dm/vpci/sharing_mode.c | 2 +- hypervisor/include/arch/x86/guest/vm.h | 4 +++ 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c index 824c4e2..e672755 100644 --- a/hypervisor/arch/x86/guest/vm.c +++ b/hypervisor/arch/x86/guest/vm.c @@ -430,3 +430,48 @@ bool is_sos(const struct acrn_vm *vm) { return vm->vm_desc->vm_type == VM_TYPE_SOS; } + +/* The return value is the last value returned by the callback. + * Its meaning is defined by the caller. + */ +int enum_vm(vm_enumeration_cb cb_func, void *cb_data) +{ + int ret; + uint16_t i; + bool processed; + + for (i = 0U; i < sizeof(vmid_bitmap) * 8U; i++) { + if (is_vm_valid(i) && (cb_func != NULL)) { + ret = cb_func(get_vm_from_vmid(i), cb_data, &processed); + if (processed) { + return ret; + } + } + } + + return -1; +} + +static int get_sos_vm_cb(struct acrn_vm *vm, void *cb_data, bool *processed) +{ + struct acrn_vm **vm_sos = (struct acrn_vm **)cb_data; + + *processed = false; + + if (is_sos(vm)) { + *vm_sos = vm; + *processed = true; + } + + return 0; +} + +/* TODO: add sos id as the parameter to support hybrid mode */ +struct acrn_vm *get_sos_vm(void) +{ + struct acrn_vm *vm_sos = NULL; + + (void)enum_vm(get_sos_vm_cb, (void*)&vm_sos); + + return vm_sos; +} diff --git a/hypervisor/common/io_request.c b/hypervisor/common/io_request.c index ce0b923..66a71b9 100644 --- a/hypervisor/common/io_request.c +++ b/hypervisor/common/io_request.c @@ -15,12 +15,12 @@ static void fire_vhm_interrupt(void) * use vLAPIC to inject vector to SOS vcpu 0 if vlapic is enabled * otherwise, send IPI hardcoded to BOOT_CPU_ID */ - struct acrn_vm *vm0; + struct acrn_vm *sos; struct acrn_vcpu *vcpu; - vm0 = get_vm_from_vmid(0U); + sos = get_sos_vm(); - vcpu = vcpu_from_vid(vm0, 0U); + vcpu = vcpu_from_vid(sos, 0U); vlapic_intr_edge(vcpu, acrn_vhm_vector); } diff --git a/hypervisor/debug/vuart.c b/hypervisor/debug/vuart.c index cd5c56f..dd95a59 100644 --- a/hypervisor/debug/vuart.c +++ b/hypervisor/debug/vuart.c @@ -377,7 +377,7 @@ struct acrn_vuart *vuart_console_active(void) vm = get_vm_from_vmid(vuart_vmid); #else - struct acrn_vm *vm = get_vm_from_vmid(0U); + struct acrn_vm *vm = get_sos_vm(); #endif if (vm != NULL) { diff --git a/hypervisor/dm/vpci/sharing_mode.c b/hypervisor/dm/vpci/sharing_mode.c index b41f073..049114d 100644 --- a/hypervisor/dm/vpci/sharing_mode.c +++ b/hypervisor/dm/vpci/sharing_mode.c @@ -228,7 +228,7 @@ void vpci_reset_ptdev_intr_info(const struct acrn_vm *target_vm, uint16_t vbdf, } else { /* Return this PCI device to SOS */ if (vdev->vpci->vm == target_vm) { - vm = get_vm_from_vmid(0U); + vm = get_sos_vm(); vdev->vpci = &vm->vpci; } } diff --git a/hypervisor/include/arch/x86/guest/vm.h b/hypervisor/include/arch/x86/guest/vm.h index 7e04c30..70fc900 100644 --- a/hypervisor/include/arch/x86/guest/vm.h +++ b/hypervisor/include/arch/x86/guest/vm.h @@ -235,4 +235,8 @@ struct acrn_vm *get_vm_from_vmid(uint16_t vm_id); void vrtc_init(struct acrn_vm *vm); #endif +typedef int (*vm_enumeration_cb)(struct acrn_vm *vm, void *cb_data, bool *processed); +int enum_vm(vm_enumeration_cb cb_func, void *cb_data); +struct acrn_vm *get_sos_vm(void); + #endif /* VM_H_ */ -- 2.7.4 |
|
|