Skip to content

Commit cb9866b

Browse files
fyin1wenlingz
authored andcommitted
softirq:spinlock: correct vioapic/vpic lock usage
Now, we are trying to move softirq from vcpu thread context to real softirq context (it was not real softirq context even it has softirq name), we need to make sure all the spinlock could be access from softirq handler to use spinlock_irqsave_obtain and spinlock_irqrestore_release. Tracked-On: #3387 Signed-off-by: Yin Fengwei <fengwei.yin@intel.com> Acked-by: Anthony Xu <anthony.xu@intel.com>
1 parent 87558b6 commit cb9866b

File tree

3 files changed

+38
-24
lines changed

3 files changed

+38
-24
lines changed

hypervisor/dm/vioapic.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,12 @@ vioapic_set_irqline_nolock(const struct acrn_vm *vm, uint32_t irqline, uint32_t
175175
void
176176
vioapic_set_irqline_lock(const struct acrn_vm *vm, uint32_t irqline, uint32_t operation)
177177
{
178+
uint64_t rflags;
178179
struct acrn_vioapic *vioapic = vm_ioapic(vm);
179180
if (vioapic->ready) {
180-
spinlock_obtain(&(vioapic->mtx));
181+
spinlock_irqsave_obtain(&(vioapic->mtx), &rflags);
181182
vioapic_set_irqline_nolock(vm, irqline, operation);
182-
spinlock_release(&(vioapic->mtx));
183+
spinlock_irqrestore_release(&(vioapic->mtx), rflags);
183184
}
184185
}
185186

@@ -245,8 +246,9 @@ static inline bool vioapic_need_intr(const struct acrn_vioapic *vioapic, uint16_
245246
}
246247

247248
/*
248-
* Due to the race between vcpus, ensure to do spinlock_obtain(&(vioapic->mtx))
249-
* & spinlock_release(&(vioapic->mtx)) by caller.
249+
* Due to the race between vcpus and vioapic->mtx could be accessed from softirq, ensure to do
250+
* spinlock_irqsave_obtain(&(vioapic->mtx), &rflags) & spinlock_irqrestore_release(&(vioapic->mtx), rflags)
251+
* by caller.
250252
*/
251253
static void vioapic_indirect_write(struct acrn_vioapic *vioapic, uint32_t addr, uint32_t data)
252254
{
@@ -354,10 +356,11 @@ vioapic_mmio_rw(struct acrn_vioapic *vioapic, uint64_t gpa,
354356
uint32_t *data, bool do_read)
355357
{
356358
uint32_t offset;
359+
uint64_t rflags;
357360

358361
offset = (uint32_t)(gpa - VIOAPIC_BASE);
359362

360-
spinlock_obtain(&(vioapic->mtx));
363+
spinlock_irqsave_obtain(&(vioapic->mtx), &rflags);
361364

362365
/* The IOAPIC specification allows 32-bit wide accesses to the
363366
* IOAPIC_REGSEL (offset 0) and IOAPIC_WINDOW (offset 16) registers.
@@ -386,7 +389,7 @@ vioapic_mmio_rw(struct acrn_vioapic *vioapic, uint64_t gpa,
386389
break;
387390
}
388391

389-
spinlock_release(&(vioapic->mtx));
392+
spinlock_irqrestore_release(&(vioapic->mtx), rflags);
390393
}
391394

392395
/*
@@ -399,6 +402,7 @@ vioapic_process_eoi(struct acrn_vm *vm, uint32_t vector)
399402
struct acrn_vioapic *vioapic;
400403
uint32_t pin, pincount = vioapic_pincount(vm);
401404
union ioapic_rte rte;
405+
uint64_t rflags;
402406

403407
if ((vector < VECTOR_DYNAMIC_START) || (vector > NR_MAX_VECTOR)) {
404408
pr_err("vioapic_process_eoi: invalid vector %u", vector);
@@ -422,7 +426,7 @@ vioapic_process_eoi(struct acrn_vm *vm, uint32_t vector)
422426
* XXX keep track of the pins associated with this vector instead
423427
* of iterating on every single pin each time.
424428
*/
425-
spinlock_obtain(&(vioapic->mtx));
429+
spinlock_irqsave_obtain(&(vioapic->mtx), &rflags);
426430
for (pin = 0U; pin < pincount; pin++) {
427431
rte = vioapic->rtbl[pin];
428432
if ((rte.bits.vector != vector) ||
@@ -437,7 +441,7 @@ vioapic_process_eoi(struct acrn_vm *vm, uint32_t vector)
437441
vioapic_generate_intr(vioapic, pin);
438442
}
439443
}
440-
spinlock_release(&(vioapic->mtx));
444+
spinlock_irqrestore_release(&(vioapic->mtx), rflags);
441445
}
442446

443447
void

hypervisor/dm/vpic.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <vm.h>
3131
#include <irq.h>
3232
#include <assign.h>
33+
#include <spinlock.h>
3334
#include <logmsg.h>
3435

3536
#define ACRN_DBG_PIC 6U
@@ -464,13 +465,14 @@ void vpic_set_irqline(struct acrn_vpic *vpic, uint32_t irqline, uint32_t operati
464465
{
465466
struct i8259_reg_state *i8259;
466467
uint32_t pin;
468+
uint64_t rflags;
467469

468470
if (irqline < NR_VPIC_PINS_TOTAL) {
469471
i8259 = &vpic->i8259[irqline >> 3U];
470472
pin = irqline;
471473

472474
if (i8259->ready) {
473-
spinlock_obtain(&(vpic->lock));
475+
spinlock_irqsave_obtain(&(vpic->lock), &rflags);
474476
switch (operation) {
475477
case GSI_SET_HIGH:
476478
vpic_set_pinstate(vpic, pin, 1U);
@@ -493,7 +495,7 @@ void vpic_set_irqline(struct acrn_vpic *vpic, uint32_t irqline, uint32_t operati
493495
break;
494496
}
495497
vpic_notify_intr(vpic);
496-
spinlock_release(&(vpic->lock));
498+
spinlock_irqrestore_release(&(vpic->lock), rflags);
497499
}
498500
}
499501
}

hypervisor/dm/vuart.c

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@
3737
#include <logmsg.h>
3838

3939
#define vuart_lock_init(vu) spinlock_init(&((vu)->lock))
40-
#define vuart_lock(vu) spinlock_obtain(&((vu)->lock))
41-
#define vuart_unlock(vu) spinlock_release(&((vu)->lock))
40+
#define vuart_lock(vu, flags) spinlock_irqsave_obtain(&((vu)->lock), &(flags))
41+
#define vuart_unlock(vu, flags) spinlock_irqrestore_release(&((vu)->lock), (flags))
4242

4343
static inline void fifo_reset(struct vuart_fifo *fifo)
4444
{
@@ -78,18 +78,21 @@ static inline uint32_t fifo_numchars(const struct vuart_fifo *fifo)
7878

7979
void vuart_putchar(struct acrn_vuart *vu, char ch)
8080
{
81-
vuart_lock(vu);
81+
uint64_t rflags;
82+
83+
vuart_lock(vu, rflags);
8284
fifo_putchar(&vu->rxfifo, ch);
83-
vuart_unlock(vu);
85+
vuart_unlock(vu, rflags);
8486
}
8587

8688
char vuart_getchar(struct acrn_vuart *vu)
8789
{
90+
uint64_t rflags;
8891
char c;
8992

90-
vuart_lock(vu);
93+
vuart_lock(vu, rflags);
9194
c = fifo_getchar(&vu->txfifo);
92-
vuart_unlock(vu);
95+
vuart_unlock(vu, rflags);
9396
return c;
9497
}
9598

@@ -177,12 +180,14 @@ void vuart_toggle_intr(const struct acrn_vuart *vu)
177180

178181
static void send_to_target(struct acrn_vuart *vu, uint8_t value_u8)
179182
{
180-
vuart_lock(vu);
183+
uint64_t rflags;
184+
185+
vuart_lock(vu, rflags);
181186
if (vu->active) {
182187
fifo_putchar(&vu->rxfifo, (char)value_u8);
183188
vuart_toggle_intr(vu);
184189
}
185-
vuart_unlock(vu);
190+
vuart_unlock(vu, rflags);
186191
}
187192

188193
static uint8_t get_modem_status(uint8_t mcr)
@@ -249,8 +254,9 @@ static uint8_t update_modem_status(uint8_t new_msr, uint8_t old_msr)
249254
static void write_reg(struct acrn_vuart *vu, uint16_t reg, uint8_t value_u8)
250255
{
251256
uint8_t msr;
257+
uint64_t rflags;
252258

253-
vuart_lock(vu);
259+
vuart_lock(vu, rflags);
254260
/*
255261
* Take care of the special case DLAB accesses first
256262
*/
@@ -330,7 +336,7 @@ static void write_reg(struct acrn_vuart *vu, uint16_t reg, uint8_t value_u8)
330336
}
331337
}
332338
vuart_toggle_intr(vu);
333-
vuart_unlock(vu);
339+
vuart_unlock(vu, rflags);
334340
}
335341

336342
static bool vuart_write(struct acrn_vm *vm, uint16_t offset_arg,
@@ -340,6 +346,7 @@ static bool vuart_write(struct acrn_vm *vm, uint16_t offset_arg,
340346
struct acrn_vuart *vu = find_vuart_by_port(vm, offset);
341347
uint8_t value_u8 = (uint8_t)value;
342348
struct acrn_vuart *target_vu = NULL;
349+
uint64_t rflags;
343350

344351
if (vu != NULL) {
345352
offset -= vu->port_base;
@@ -348,10 +355,10 @@ static bool vuart_write(struct acrn_vm *vm, uint16_t offset_arg,
348355
if (((vu->mcr & MCR_LOOPBACK) == 0U) &&
349356
(offset == UART16550_THR) && (target_vu != NULL)) {
350357
send_to_target(target_vu, value_u8);
351-
vuart_lock(vu);
358+
vuart_lock(vu, rflags);
352359
vu->thre_int_pending = true;
353360
vuart_toggle_intr(vu);
354-
vuart_unlock(vu);
361+
vuart_unlock(vu, rflags);
355362
} else {
356363
write_reg(vu, offset, value_u8);
357364
}
@@ -366,10 +373,11 @@ static bool vuart_read(struct acrn_vm *vm, struct acrn_vcpu *vcpu, uint16_t offs
366373
uint8_t iir, reg, intr_reason;
367374
struct acrn_vuart *vu = find_vuart_by_port(vm, offset);
368375
struct pio_request *pio_req = &vcpu->req.reqs.pio;
376+
uint64_t rflags;
369377

370378
if (vu != NULL) {
371379
offset -= vu->port_base;
372-
vuart_lock(vu);
380+
vuart_lock(vu, rflags);
373381
/*
374382
* Take care of the special case DLAB accesses first
375383
*/
@@ -438,7 +446,7 @@ static bool vuart_read(struct acrn_vm *vm, struct acrn_vcpu *vcpu, uint16_t offs
438446
}
439447
vuart_toggle_intr(vu);
440448
pio_req->value = (uint32_t)reg;
441-
vuart_unlock(vu);
449+
vuart_unlock(vu, rflags);
442450
}
443451

444452
return true;

0 commit comments

Comments
 (0)