Skip to content

Commit c32d41a

Browse files
Shawnshhlijinxia
authored andcommitted
hv: irq: fix "Procedure has more than one exit point"
IEC 61508,ISO 26262 standards highly recommend single-exit rule. Reduce the count of the "return entries". Fix the violations which is comply with the cases list below: 1.Function has 2 return entries. 2.The first return entry is used to return the error code of checking variable whether is valid. Fix the violations in "if else" format. Tracked-On: #861 Signed-off-by: Huihuang Shi <huihuang.shi@intel.com> Acked-by: Eddie Dong <eddie.dong@intel.com>
1 parent 8dfb9bd commit c32d41a

File tree

2 files changed

+133
-137
lines changed

2 files changed

+133
-137
lines changed

hypervisor/arch/x86/ioapic.c

Lines changed: 43 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -176,22 +176,22 @@ create_rte_for_gsi_irq(uint32_t irq, uint32_t vr)
176176
union ioapic_rte rte;
177177

178178
if (irq < NR_LEGACY_IRQ) {
179-
return create_rte_for_legacy_irq(irq, vr);
179+
rte = create_rte_for_legacy_irq(irq, vr);
180+
} else {
181+
/* irq default masked, level trig */
182+
rte.full = IOAPIC_RTE_INTMSET;
183+
rte.full |= IOAPIC_RTE_TRGRLVL;
184+
rte.full |= DEFAULT_DEST_MODE;
185+
rte.full |= DEFAULT_DELIVERY_MODE;
186+
rte.full |= (IOAPIC_RTE_INTVEC & (uint64_t)vr);
187+
188+
/* Fixed to active high */
189+
rte.full |= IOAPIC_RTE_INTAHI;
190+
191+
/* Dest field */
192+
rte.full |= ((uint64_t)ALL_CPUS_MASK << IOAPIC_RTE_DEST_SHIFT);
180193
}
181194

182-
/* irq default masked, level trig */
183-
rte.full = IOAPIC_RTE_INTMSET;
184-
rte.full |= IOAPIC_RTE_TRGRLVL;
185-
rte.full |= DEFAULT_DEST_MODE;
186-
rte.full |= DEFAULT_DELIVERY_MODE;
187-
rte.full |= (IOAPIC_RTE_INTVEC & (uint64_t)vr);
188-
189-
/* Fixed to active high */
190-
rte.full |= IOAPIC_RTE_INTAHI;
191-
192-
/* Dest field */
193-
rte.full |= ((uint64_t)ALL_CPUS_MASK << IOAPIC_RTE_DEST_SHIFT);
194-
195195
return rte;
196196
}
197197

@@ -222,28 +222,24 @@ void ioapic_get_rte(uint32_t irq, union ioapic_rte *rte)
222222
{
223223
void *addr;
224224

225-
if (!irq_is_gsi(irq)) {
226-
return;
225+
if (irq_is_gsi(irq)) {
226+
addr = gsi_table[irq].addr;
227+
ioapic_get_rte_entry(addr, gsi_table[irq].pin, rte);
227228
}
228-
229-
addr = gsi_table[irq].addr;
230-
ioapic_get_rte_entry(addr, gsi_table[irq].pin, rte);
231229
}
232230

233231
void ioapic_set_rte(uint32_t irq, union ioapic_rte rte)
234232
{
235233
void *addr;
236234

237-
if (!irq_is_gsi(irq)) {
238-
return;
239-
}
240-
241-
addr = gsi_table[irq].addr;
242-
ioapic_set_rte_entry(addr, gsi_table[irq].pin, rte);
235+
if (irq_is_gsi(irq)) {
236+
addr = gsi_table[irq].addr;
237+
ioapic_set_rte_entry(addr, gsi_table[irq].pin, rte);
243238

244-
dev_dbg(ACRN_DBG_IRQ, "GSI: irq:%d pin:%hhu rte:%lx",
245-
irq, gsi_table[irq].pin,
246-
rte.full);
239+
dev_dbg(ACRN_DBG_IRQ, "GSI: irq:%d pin:%hhu rte:%lx",
240+
irq, gsi_table[irq].pin,
241+
rte.full);
242+
}
247243
}
248244

249245
bool irq_is_gsi(uint32_t irq)
@@ -253,11 +249,15 @@ bool irq_is_gsi(uint32_t irq)
253249

254250
uint8_t irq_to_pin(uint32_t irq)
255251
{
252+
uint8_t ret;
253+
256254
if (irq_is_gsi(irq)) {
257-
return gsi_table[irq].pin;
255+
ret = gsi_table[irq].pin;
258256
} else {
259-
return IOAPIC_INVALID_PIN;
257+
ret = IOAPIC_INVALID_PIN;
260258
}
259+
260+
return ret;
261261
}
262262

263263
uint32_t pin_to_irq(uint8_t pin)
@@ -279,22 +279,20 @@ irq_gsi_mask_unmask(uint32_t irq, bool mask)
279279
uint8_t pin;
280280
union ioapic_rte rte;
281281

282-
if (!irq_is_gsi(irq)) {
283-
return;
284-
}
285-
286-
addr = gsi_table[irq].addr;
287-
pin = gsi_table[irq].pin;
288-
289-
ioapic_get_rte_entry(addr, pin, &rte);
290-
if (mask) {
291-
rte.full |= IOAPIC_RTE_INTMSET;
292-
} else {
293-
rte.full &= ~IOAPIC_RTE_INTMASK;
282+
if (irq_is_gsi(irq)) {
283+
addr = gsi_table[irq].addr;
284+
pin = gsi_table[irq].pin;
285+
286+
ioapic_get_rte_entry(addr, pin, &rte);
287+
if (mask) {
288+
rte.full |= IOAPIC_RTE_INTMSET;
289+
} else {
290+
rte.full &= ~IOAPIC_RTE_INTMASK;
291+
}
292+
ioapic_set_rte_entry(addr, pin, rte);
293+
dev_dbg(ACRN_DBG_PTIRQ, "update: irq:%d pin:%hhu rte:%lx",
294+
irq, pin, rte.full);
294295
}
295-
ioapic_set_rte_entry(addr, pin, rte);
296-
dev_dbg(ACRN_DBG_PTIRQ, "update: irq:%d pin:%hhu rte:%lx",
297-
irq, pin, rte.full);
298296
}
299297

300298
void gsi_mask_irq(uint32_t irq)

hypervisor/arch/x86/irq.c

Lines changed: 90 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,28 @@ uint32_t alloc_irq_num(uint32_t req_irq)
3636
{
3737
uint32_t irq = req_irq;
3838
uint64_t rflags;
39+
uint32_t ret;
3940

4041
if ((irq >= NR_IRQS) && (irq != IRQ_INVALID)) {
4142
pr_err("[%s] invalid req_irq %u", __func__, req_irq);
42-
return IRQ_INVALID;
43-
}
44-
45-
spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags);
46-
if (irq == IRQ_INVALID) {
47-
/* if no valid irq num given, find a free one */
48-
irq = (uint32_t)ffz64_ex(irq_alloc_bitmap, NR_IRQS);
49-
}
50-
51-
if (irq >= NR_IRQS) {
52-
irq = IRQ_INVALID;
43+
ret = IRQ_INVALID;
5344
} else {
54-
bitmap_set_nolock((uint16_t)(irq & 0x3FU),
55-
irq_alloc_bitmap + (irq >> 6U));
45+
spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags);
46+
if (irq == IRQ_INVALID) {
47+
/* if no valid irq num given, find a free one */
48+
irq = (uint32_t)ffz64_ex(irq_alloc_bitmap, NR_IRQS);
49+
}
50+
51+
if (irq >= NR_IRQS) {
52+
irq = IRQ_INVALID;
53+
} else {
54+
bitmap_set_nolock((uint16_t)(irq & 0x3FU),
55+
irq_alloc_bitmap + (irq >> 6U));
56+
}
57+
spinlock_irqrestore_release(&irq_alloc_spinlock, rflags);
58+
ret = irq;
5659
}
57-
spinlock_irqrestore_release(&irq_alloc_spinlock, rflags);
58-
return irq;
60+
return ret;
5961
}
6062

6163
/*
@@ -66,15 +68,13 @@ void free_irq_num(uint32_t irq)
6668
{
6769
uint64_t rflags;
6870

69-
if (irq >= NR_IRQS) {
70-
return;
71-
}
72-
73-
if (irq_is_gsi(irq) == false) {
74-
spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags);
75-
(void)bitmap_test_and_clear_nolock((uint16_t)(irq & 0x3FU),
76-
irq_alloc_bitmap + (irq >> 6U));
77-
spinlock_irqrestore_release(&irq_alloc_spinlock, rflags);
71+
if (irq < NR_IRQS) {
72+
if (!irq_is_gsi(irq)) {
73+
spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags);
74+
(void)bitmap_test_and_clear_nolock((uint16_t)(irq & 0x3FU),
75+
irq_alloc_bitmap + (irq >> 6U));
76+
spinlock_irqrestore_release(&irq_alloc_spinlock, rflags);
77+
}
7878
}
7979
}
8080

@@ -89,43 +89,44 @@ uint32_t alloc_irq_vector(uint32_t irq)
8989
uint32_t vr;
9090
struct irq_desc *desc;
9191
uint64_t rflags;
92+
uint32_t ret;
9293

93-
if (irq >= NR_IRQS) {
94-
pr_err("invalid irq[%u] to alloc vector", irq);
95-
return VECTOR_INVALID;
96-
}
97-
98-
desc = &irq_desc_array[irq];
99-
100-
if (desc->vector != VECTOR_INVALID) {
101-
if (vector_to_irq[desc->vector] == irq) {
102-
/* statically binded */
103-
vr = desc->vector;
94+
if (irq < NR_IRQS) {
95+
desc = &irq_desc_array[irq];
96+
97+
if (desc->vector != VECTOR_INVALID) {
98+
if (vector_to_irq[desc->vector] == irq) {
99+
/* statically binded */
100+
vr = desc->vector;
101+
} else {
102+
pr_err("[%s] irq[%u]:vector[%u] mismatch",
103+
__func__, irq, desc->vector);
104+
vr = VECTOR_INVALID;
105+
}
104106
} else {
105-
pr_err("[%s] irq[%u]:vector[%u] mismatch",
106-
__func__, irq, desc->vector);
107-
vr = VECTOR_INVALID;
108-
}
109-
} else {
110-
/* alloc a vector between:
111-
* VECTOR_DYNAMIC_START ~ VECTOR_DYNAMC_END
112-
*/
113-
spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags);
114-
115-
for (vr = VECTOR_DYNAMIC_START;
116-
vr <= VECTOR_DYNAMIC_END; vr++) {
117-
if (vector_to_irq[vr] == IRQ_INVALID) {
118-
desc->vector = vr;
119-
vector_to_irq[vr] = irq;
120-
break;
107+
/* alloc a vector between:
108+
* VECTOR_DYNAMIC_START ~ VECTOR_DYNAMC_END
109+
*/
110+
spinlock_irqsave_obtain(&irq_alloc_spinlock, &rflags);
111+
112+
for (vr = VECTOR_DYNAMIC_START;
113+
vr <= VECTOR_DYNAMIC_END; vr++) {
114+
if (vector_to_irq[vr] == IRQ_INVALID) {
115+
desc->vector = vr;
116+
vector_to_irq[vr] = irq;
117+
break;
118+
}
121119
}
122-
}
123-
vr = (vr > VECTOR_DYNAMIC_END) ? VECTOR_INVALID : vr;
120+
vr = (vr > VECTOR_DYNAMIC_END) ? VECTOR_INVALID : vr;
124121

125-
spinlock_irqrestore_release(&irq_alloc_spinlock, rflags);
122+
spinlock_irqrestore_release(&irq_alloc_spinlock, rflags);
123+
}
124+
ret = vr;
125+
} else {
126+
pr_err("invalid irq[%u] to alloc vector", irq);
127+
ret = VECTOR_INVALID;
126128
}
127-
128-
return vr;
129+
return ret;
129130
}
130131

131132
/* free the vector allocated via alloc_irq_vector() */
@@ -226,50 +227,49 @@ void free_irq(uint32_t irq)
226227
uint64_t rflags;
227228
struct irq_desc *desc;
228229

229-
if (irq >= NR_IRQS) {
230-
return;
231-
}
232-
233-
desc = &irq_desc_array[irq];
234-
dev_dbg(ACRN_DBG_IRQ, "[%s] irq%d vr:0x%x",
235-
__func__, irq, irq_to_vector(irq));
230+
if (irq < NR_IRQS) {
231+
desc = &irq_desc_array[irq];
232+
dev_dbg(ACRN_DBG_IRQ, "[%s] irq%d vr:0x%x",
233+
__func__, irq, irq_to_vector(irq));
236234

237-
free_irq_vector(irq);
238-
free_irq_num(irq);
235+
free_irq_vector(irq);
236+
free_irq_num(irq);
239237

240-
spinlock_irqsave_obtain(&desc->lock, &rflags);
241-
desc->action = NULL;
242-
desc->priv_data = NULL;
243-
desc->flags = IRQF_NONE;
244-
spinlock_irqrestore_release(&desc->lock, rflags);
238+
spinlock_irqsave_obtain(&desc->lock, &rflags);
239+
desc->action = NULL;
240+
desc->priv_data = NULL;
241+
desc->flags = IRQF_NONE;
242+
spinlock_irqrestore_release(&desc->lock, rflags);
243+
}
245244
}
246245

247246
void set_irq_trigger_mode(uint32_t irq, bool is_level_triggered)
248247
{
249248
uint64_t rflags;
250249
struct irq_desc *desc;
251250

252-
if (irq >= NR_IRQS) {
253-
return;
254-
}
255-
256-
desc = &irq_desc_array[irq];
257-
spinlock_irqsave_obtain(&desc->lock, &rflags);
258-
if (is_level_triggered == true) {
259-
desc->flags |= IRQF_LEVEL;
260-
} else {
261-
desc->flags &= ~IRQF_LEVEL;
251+
if (irq < NR_IRQS) {
252+
desc = &irq_desc_array[irq];
253+
spinlock_irqsave_obtain(&desc->lock, &rflags);
254+
if (is_level_triggered == true) {
255+
desc->flags |= IRQF_LEVEL;
256+
} else {
257+
desc->flags &= ~IRQF_LEVEL;
258+
}
259+
spinlock_irqrestore_release(&desc->lock, rflags);
262260
}
263-
spinlock_irqrestore_release(&desc->lock, rflags);
264261
}
265262

266263
uint32_t irq_to_vector(uint32_t irq)
267264
{
265+
uint32_t ret;
268266
if (irq < NR_IRQS) {
269-
return irq_desc_array[irq].vector;
267+
ret = irq_desc_array[irq].vector;
270268
} else {
271-
return VECTOR_INVALID;
269+
ret = VECTOR_INVALID;
272270
}
271+
272+
return ret;
273273
}
274274

275275
static void handle_spurious_interrupt(uint32_t vector)
@@ -435,16 +435,14 @@ static void disable_pic_irqs(void)
435435

436436
void init_default_irqs(uint16_t cpu_id)
437437
{
438-
if (cpu_id != BOOT_CPU_ID) {
439-
return;
440-
}
438+
if (cpu_id == BOOT_CPU_ID) {
439+
init_irq_descs();
441440

442-
init_irq_descs();
443-
444-
/* we use ioapic only, disable legacy PIC */
445-
disable_pic_irqs();
446-
setup_ioapic_irqs();
447-
init_softirq();
441+
/* we use ioapic only, disable legacy PIC */
442+
disable_pic_irqs();
443+
setup_ioapic_irqs();
444+
init_softirq();
445+
}
448446
}
449447

450448
void interrupt_init(uint16_t pcpu_id)

0 commit comments

Comments
 (0)