From 71b047cb61de364d0586f8f6763c8b4c4407b214 Mon Sep 17 00:00:00 2001 From: Shiqing Gao Date: Thu, 20 Sep 2018 16:52:48 +0800 Subject: [PATCH] hv: fix 'Switch case not terminated with break' MISRA-C requires that every switch case shall be terminated with break to avoid the unintentional fall through. The code will become redundant if we enforce this rule. So, we will keep the current implementation for the following two cases. 1. The fall through is intentional. 2. The function is returned in the switch case. If we decide to eliminate the mutiple returns in one function later, this case would be handled properly at that time. What this patch does: - add the mssing break for the default case - add the pre condition for some functions and remove the corresponding panic which will never happen since the function caller could guarantee the pre condition based on the code implementation v1 -> v2: * remove the redundant cases above default in 'vlapic_get_lvtptr' * add the similar pre condition for 'lvt_off_to_idx' as 'vlapic_get_lvtptr' since all the function callers could guarantee it * remove the assertion in 'lvt_off_to_idx' since the pre condition could guarantee that the assertion will never happen * add the similar pre condition for 'vpic_set_irqstate' as 'vioapic_set_irqstate' since all the function callers could guarantee it * remove the assertion in 'vpic_set_irqstate' since the pre condition could guarantee that the assertion will never happen Tracked-On: #861 Signed-off-by: Shiqing Gao Reviewed-by: Junjie Mao --- hypervisor/arch/x86/guest/vlapic.c | 49 +++++++++++++++++++----------- hypervisor/arch/x86/mtrr.c | 1 + hypervisor/dm/vioapic.c | 10 +++++- hypervisor/dm/vpic.c | 10 +++++- 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c index 7afbbe909..47bce5eda 100644 --- a/hypervisor/arch/x86/guest/vlapic.c +++ b/hypervisor/arch/x86/guest/vlapic.c @@ -518,10 +518,20 @@ vlapic_set_intr_ready(struct acrn_vlapic *vlapic, uint32_t vector, bool level) return 1; } +/** + * @pre offset value shall be one of the folllowing values: + * APIC_OFFSET_CMCI_LVT + * APIC_OFFSET_TIMER_LVT + * APIC_OFFSET_THERM_LVT + * APIC_OFFSET_PERF_LVT + * APIC_OFFSET_LINT0_LVT + * APIC_OFFSET_LINT1_LVT + * APIC_OFFSET_ERROR_LVT + */ static inline uint32_t lvt_off_to_idx(uint32_t offset) { - uint32_t index = ~0U; + uint32_t index; switch (offset) { case APIC_OFFSET_CMCI_LVT: @@ -543,24 +553,29 @@ lvt_off_to_idx(uint32_t offset) index = APIC_LVT_LINT1; break; case APIC_OFFSET_ERROR_LVT: - index = APIC_LVT_ERROR; - break; default: /* - * For the offset that is not handled (an invalid offset of - * Local Vector Table), its index is assigned to a default - * value, which indicates an invalid index. - * The index will be checked later to guarantee the validity. + * The function caller could guarantee the pre condition. + * So, all of the possible 'offset' other than + * APIC_OFFSET_ERROR_LVT has been handled in prior cases. */ + index = APIC_LVT_ERROR; break; } - ASSERT(index <= VLAPIC_MAXLVT_INDEX, - "%s: invalid lvt index %u for offset %#x", - __func__, index, offset); return index; } +/** + * @pre offset value shall be one of the folllowing values: + * APIC_OFFSET_CMCI_LVT + * APIC_OFFSET_TIMER_LVT + * APIC_OFFSET_THERM_LVT + * APIC_OFFSET_PERF_LVT + * APIC_OFFSET_LINT0_LVT + * APIC_OFFSET_LINT1_LVT + * APIC_OFFSET_ERROR_LVT + */ static inline uint32_t * vlapic_get_lvtptr(struct acrn_vlapic *vlapic, uint32_t offset) { @@ -570,16 +585,14 @@ vlapic_get_lvtptr(struct acrn_vlapic *vlapic, uint32_t offset) switch (offset) { case APIC_OFFSET_CMCI_LVT: return &lapic->lvt_cmci.v; - case APIC_OFFSET_TIMER_LVT: - case APIC_OFFSET_THERM_LVT: - case APIC_OFFSET_PERF_LVT: - case APIC_OFFSET_LINT0_LVT: - case APIC_OFFSET_LINT1_LVT: - case APIC_OFFSET_ERROR_LVT: + default: + /* + * The function caller could guarantee the pre condition. + * All the possible 'offset' other than APIC_OFFSET_CMCI_LVT + * could be handled here. + */ i = lvt_off_to_idx(offset); return &(lapic->lvt[i].v); - default: - panic("vlapic_get_lvt: invalid LVT\n"); } } diff --git a/hypervisor/arch/x86/mtrr.c b/hypervisor/arch/x86/mtrr.c index 631424cfa..7b9465200 100644 --- a/hypervisor/arch/x86/mtrr.c +++ b/hypervisor/arch/x86/mtrr.c @@ -145,6 +145,7 @@ static uint32_t update_ept(struct vm *vm, uint64_t start, case MTRR_MEM_TYPE_UC: default: attr = EPT_UNCACHED; + break; } ept_mr_modify(vm, (uint64_t *)vm->arch_vm.nworld_eptp, diff --git a/hypervisor/dm/vioapic.c b/hypervisor/dm/vioapic.c index 00888a006..f22714ec1 100644 --- a/hypervisor/dm/vioapic.c +++ b/hypervisor/dm/vioapic.c @@ -128,6 +128,10 @@ enum irqstate { /** * @pre irq < vioapic_pincount(vm) + * @pre irqstate value shall be one of the folllowing values: + * IRQSTATE_ASSERT + * IRQSTATE_DEASSERT + * IRQSTATE_PULSE */ static void vioapic_set_irqstate(struct vm *vm, uint32_t irq, enum irqstate irqstate) @@ -150,7 +154,11 @@ vioapic_set_irqstate(struct vm *vm, uint32_t irq, enum irqstate irqstate) vioapic_set_pinstate(vioapic, pin, false); break; default: - panic("vioapic_set_irqstate: invalid irqstate %d", irqstate); + /* + * The function caller could guarantee the pre condition. + * All the possible 'irqstate' has been handled in prior cases. + */ + break; } spinlock_release(&(vioapic->mtx)); } diff --git a/hypervisor/dm/vpic.c b/hypervisor/dm/vpic.c index 0c1f9da83..9eb5e8f88 100644 --- a/hypervisor/dm/vpic.c +++ b/hypervisor/dm/vpic.c @@ -453,6 +453,10 @@ static void vpic_set_pinstate(struct acrn_vpic *vpic, uint8_t pin, bool newstate /** * @pre irq < NR_VPIC_PINS_TOTAL + * @pre irqstate value shall be one of the folllowing values: + * IRQSTATE_ASSERT + * IRQSTATE_DEASSERT + * IRQSTATE_PULSE */ static void vpic_set_irqstate(struct vm *vm, uint32_t irq, enum irqstate irqstate) @@ -486,7 +490,11 @@ static void vpic_set_irqstate(struct vm *vm, uint32_t irq, vpic_set_pinstate(vpic, pin, false); break; default: - ASSERT(false, "vpic_set_irqstate: invalid irqstate"); + /* + * The function caller could guarantee the pre condition. + * All the possible 'irqstate' has been handled in prior cases. + */ + break; } spinlock_release(&(vpic->lock)); }