Skip to content

Commit 4adad73

Browse files
lifeixwenlingz
authored andcommitted
hv: mmio: refine mmio access handle lock granularity
Now only PCI MSI-X BAR access need dynamic register/unregister. Others don't need unregister once it's registered. So we don't need to lock the vm level emul_mmio_lock when we handle the MMIO access. Instead, we could use finer granularity lock in the handler to ptotest the shared resource. This patch fixed the dead lock issue when OVMF try to size the BAR size: Becasue OVMF use ECAM to access the PCI configuration space, it will first hold vm emul_mmio_lock, then calls vpci_handle_mmconfig_access. While this tries to size a BAR which is also a MSI-X Table BAR, it will call register_mmio_emulation_handler to register the MSI-X Table BAR MMIO access handler. This will causes the emul_mmio_lock dead lock. Tracked-On: #3475 Signed-off-by: Li Fei1 <fei1.li@intel.com>
1 parent fbe57d9 commit 4adad73

File tree

5 files changed

+25
-5
lines changed

5 files changed

+25
-5
lines changed

hypervisor/dm/io_req.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@ static int32_t
459459
hv_emulate_mmio(struct acrn_vcpu *vcpu, struct io_request *io_req)
460460
{
461461
int32_t status = -ENODEV;
462+
bool hold_lock = true;
462463
uint16_t idx;
463464
uint64_t address, size, base, end;
464465
struct mmio_request *mmio_req = &io_req->reqs.mmio;
@@ -484,6 +485,7 @@ hv_emulate_mmio(struct acrn_vcpu *vcpu, struct io_request *io_req)
484485
continue;
485486
} else {
486487
if ((address >= base) && ((address + size) <= end)) {
488+
hold_lock = mmio_handler->hold_lock;
487489
read_write = mmio_handler->read_write;
488490
handler_private_data = mmio_handler->handler_private_data;
489491
} else {
@@ -496,7 +498,16 @@ hv_emulate_mmio(struct acrn_vcpu *vcpu, struct io_request *io_req)
496498
}
497499

498500
if ((status == -ENODEV) && (read_write != NULL)) {
501+
/* This mmio_handler will never modify once register, so we don't
502+
* need to hold the lock when handling the MMIO access.
503+
*/
504+
if (!hold_lock) {
505+
spinlock_release(&vcpu->vm->emul_mmio_lock);
506+
}
499507
status = read_write(io_req, handler_private_data);
508+
if (!hold_lock) {
509+
spinlock_obtain(&vcpu->vm->emul_mmio_lock);
510+
}
500511
}
501512
spinlock_release(&vcpu->vm->emul_mmio_lock);
502513

@@ -669,7 +680,7 @@ static inline struct mem_io_node *find_free_mmio_node(struct acrn_vm *vm)
669680
*/
670681
void register_mmio_emulation_handler(struct acrn_vm *vm,
671682
hv_mem_io_handler_t read_write, uint64_t start,
672-
uint64_t end, void *handler_private_data)
683+
uint64_t end, void *handler_private_data, bool hold_lock)
673684
{
674685
struct mem_io_node *mmio_node;
675686

@@ -679,6 +690,7 @@ void register_mmio_emulation_handler(struct acrn_vm *vm,
679690
mmio_node = find_free_mmio_node(vm);
680691
if (mmio_node != NULL) {
681692
/* Fill in information for this node */
693+
mmio_node->hold_lock = hold_lock;
682694
mmio_node->read_write = read_write;
683695
mmio_node->handler_private_data = handler_private_data;
684696
mmio_node->range_start = start;

hypervisor/dm/vioapic.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ vioapic_init(struct acrn_vm *vm)
472472
vioapic_mmio_access_handler,
473473
(uint64_t)VIOAPIC_BASE,
474474
(uint64_t)VIOAPIC_BASE + VIOAPIC_SIZE,
475-
vm);
475+
vm, false);
476476
ept_del_mr(vm, (uint64_t *)vm->arch_vm.nworld_eptp,
477477
(uint64_t)VIOAPIC_BASE, (uint64_t)VIOAPIC_SIZE);
478478
vm->arch_vm.vioapic.ready = true;

hypervisor/dm/vpci/pci_pt.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ static void vdev_pt_map_mem_vbar(struct pci_vdev *vdev, uint32_t idx)
112112
addr_lo = round_page_down(addr_lo);
113113
addr_hi = round_page_up(addr_hi);
114114
register_mmio_emulation_handler(vm, vmsix_handle_table_mmio_access,
115-
addr_lo, addr_hi, vdev);
115+
addr_lo, addr_hi, vdev, true);
116116
ept_del_mr(vm, (uint64_t *)vm->arch_vm.nworld_eptp, addr_lo, addr_hi - addr_lo);
117117
msix->mmio_gpa = vbar->base;
118118
}

hypervisor/dm/vpci/vpci.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ void vpci_init(struct acrn_vm *vm)
249249
pci_mmcfg_base = (vm_config->load_order == SOS_VM) ? get_mmcfg_base() : 0xE0000000UL;
250250
vm->vpci.pci_mmcfg_base = pci_mmcfg_base;
251251
register_mmio_emulation_handler(vm, vpci_handle_mmconfig_access,
252-
pci_mmcfg_base, pci_mmcfg_base + PCI_MMCONFIG_SIZE, &vm->vpci);
252+
pci_mmcfg_base, pci_mmcfg_base + PCI_MMCONFIG_SIZE, &vm->vpci, false);
253253
}
254254

255255
/* Intercept and handle I/O ports CF8h */

hypervisor/include/dm/io_req.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,13 @@ typedef int32_t (*hv_mem_io_handler_t)(struct io_request *io_req, void *handler_
112112
* @brief Structure for MMIO handler node
113113
*/
114114
struct mem_io_node {
115+
116+
/**
117+
* @brief Whether the lock needs to hold when handle the MMIO access
118+
*/
119+
bool hold_lock;
120+
121+
115122
/**
116123
* @brief A pointer to the handler
117124
*
@@ -258,12 +265,13 @@ void register_pio_emulation_handler(struct acrn_vm *vm, uint32_t pio_idx,
258265
* @param start The base address of the range \p read_write can emulate
259266
* @param end The end of the range (exclusive) \p read_write can emulate
260267
* @param handler_private_data Handler-specific data which will be passed to \p read_write when called
268+
* @param hold_lock Whether hold the lock to handle the MMIO access
261269
*
262270
* @return None
263271
*/
264272
void register_mmio_emulation_handler(struct acrn_vm *vm,
265273
hv_mem_io_handler_t read_write, uint64_t start,
266-
uint64_t end, void *handler_private_data);
274+
uint64_t end, void *handler_private_data, bool hold_lock);
267275

268276
/**
269277
* @brief Unregister a MMIO handler

0 commit comments

Comments
 (0)