From 91e0612e88bd9bc704cdd379403ace9055c3dfa9 Mon Sep 17 00:00:00 2001 From: Jiaqing Zhao Date: Fri, 31 May 2024 07:22:53 +0000 Subject: [PATCH] hv: dm: refine create/destroy functions The create function of hv-emulated device must check the return value of vpci_init_vdev() as it returns NULL pointer on failure, and that function should be called atomically. Also, the destory function should deinit the vpci devices created to prevent resource leak. Tracked-On: #8590 Signed-off-by: Jiaqing Zhao Reviewed-by: Junjie Mao --- hypervisor/dm/vpci/ivshmem.c | 22 ++++++++++++++++------ hypervisor/dm/vpci/vmcs9900.c | 16 +++++++++++++--- hypervisor/dm/vpci/vroot_port.c | 10 +++++++++- hypervisor/include/dm/vpci.h | 1 + 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/hypervisor/dm/vpci/ivshmem.c b/hypervisor/dm/vpci/ivshmem.c index 129e791d8..22db5ae60 100644 --- a/hypervisor/dm/vpci/ivshmem.c +++ b/hypervisor/dm/vpci/ivshmem.c @@ -384,6 +384,7 @@ int32_t create_ivshmem_vdev(struct acrn_vm *vm, struct acrn_vdev *dev) uint32_t i; struct acrn_vm_config *vm_config = get_vm_config(vm->vm_id); struct acrn_vm_pci_dev_config *dev_config = NULL; + struct pci_vdev *vdev = NULL; int32_t ret = -EINVAL; for (i = 0U; i < vm_config->pci_dev_num; i++) { @@ -397,27 +398,36 @@ int32_t create_ivshmem_vdev(struct acrn_vm *vm, struct acrn_vdev *dev) dev_config->vbar_base[IVSHMEM_MSIX_BAR] = (uint64_t) dev->io_addr[IVSHMEM_MSIX_BAR]; dev_config->vbar_base[IVSHMEM_SHM_BAR] = (uint64_t) dev->io_addr[IVSHMEM_SHM_BAR]; dev_config->vbar_base[IVSHMEM_SHM_BAR] |= ((uint64_t) dev->io_addr[IVSHMEM_SHM_BAR + 1U]) << 32U; - (void) vpci_init_vdev(&vm->vpci, dev_config, NULL); + vdev = vpci_init_vdev(&vm->vpci, dev_config, NULL); spinlock_release(&vm->vpci.lock); - ret = 0; - } else { - pr_warn("%s, failed to create ivshmem device %x:%x.%x\n", __func__, - dev->slot >> 8U, (dev->slot >> 3U) & 0x1fU, dev->slot & 0x7U); - } + if (vdev != NULL) { + ret = 0; + } + } break; } } + + if (ret != 0) { + pr_warn("%s, failed to create ivshmem device %x:%x.%x\n", __func__, + dev->slot >> 8U, (dev->slot >> 3U) & 0x1fU, dev->slot & 0x7U); + } return ret; } int32_t destroy_ivshmem_vdev(struct pci_vdev *vdev) { uint32_t i; + struct acrn_vpci *vpci = vdev->vpci; for (i = 0U; i < vdev->nr_bars; i++) { vpci_update_one_vbar(vdev, i, 0U, NULL, ivshmem_vbar_unmap); } + spinlock_obtain(&vpci->lock); + vpci_deinit_vdev(vdev); + spinlock_release(&vpci->lock); + return 0; } diff --git a/hypervisor/dm/vpci/vmcs9900.c b/hypervisor/dm/vpci/vmcs9900.c index 09e23cdfa..900f73039 100644 --- a/hypervisor/dm/vpci/vmcs9900.c +++ b/hypervisor/dm/vpci/vmcs9900.c @@ -166,6 +166,7 @@ const struct pci_vdev_ops vmcs9900_ops = { int32_t create_vmcs9900_vdev(struct acrn_vm *vm, struct acrn_vdev *dev) { uint16_t i; + struct pci_vdev *vdev; struct acrn_vm_config *vm_config = get_vm_config(vm->vm_id); struct acrn_vm_pci_dev_config *dev_config = NULL; int32_t ret = -EINVAL; @@ -177,14 +178,18 @@ int32_t create_vmcs9900_vdev(struct acrn_vm *vm, struct acrn_vdev *dev) dev_config->vbdf.value = (uint16_t) dev->slot; dev_config->vbar_base[0] = (uint64_t) dev->io_addr[0]; dev_config->vbar_base[1] = (uint64_t) dev->io_addr[1]; - (void) vpci_init_vdev(&vm->vpci, dev_config, NULL); - ret = 0; + spinlock_obtain(&vm->vpci.lock); + vdev = vpci_init_vdev(&vm->vpci, dev_config, NULL); + spinlock_release(&vm->vpci.lock); + if (vdev != NULL) { + ret = 0; + } break; } } if (ret != 0) { - pr_err("Unsupport: create VM%d vuart_idx=%d", vm->vm_id, vuart_idx); + pr_err("Failed: create VM%d vuart_idx=%d", vm->vm_id, vuart_idx); } return ret; @@ -193,6 +198,7 @@ int32_t create_vmcs9900_vdev(struct acrn_vm *vm, struct acrn_vdev *dev) int32_t destroy_vmcs9900_vdev(struct pci_vdev *vdev) { uint32_t i; + struct acrn_vpci *vpci = vdev->vpci; for (i = 0U; i < vdev->nr_bars; i++) { vpci_update_one_vbar(vdev, i, 0U, NULL, unmap_vmcs9900_vbar); @@ -200,5 +206,9 @@ int32_t destroy_vmcs9900_vdev(struct pci_vdev *vdev) deinit_pci_vuart(vdev); + spinlock_obtain(&vpci->lock); + vpci_deinit_vdev(vdev); + spinlock_release(&vpci->lock); + return 0; } diff --git a/hypervisor/dm/vpci/vroot_port.c b/hypervisor/dm/vpci/vroot_port.c index 1b34d8d43..5e7f21c81 100644 --- a/hypervisor/dm/vpci/vroot_port.c +++ b/hypervisor/dm/vpci/vroot_port.c @@ -140,7 +140,9 @@ int32_t create_vrp(struct acrn_vm *vm, struct acrn_vdev *dev) dev_config->vrp_max_payload = vrp_config->max_payload; dev_config->vdev_ops = &vrp_ops; + spinlock_obtain(&vm->vpci.lock); vdev = vpci_init_vdev(&vm->vpci, dev_config, NULL); + spinlock_release(&vm->vpci.lock); if (vdev == NULL) { pr_err("%s: failed to create virtual root port\n", __func__); ret = -EFAULT; @@ -156,8 +158,14 @@ int32_t create_vrp(struct acrn_vm *vm, struct acrn_vdev *dev) return ret; } -int32_t destroy_vrp(__unused struct pci_vdev *vdev) +int32_t destroy_vrp(struct pci_vdev *vdev) { + struct acrn_vpci *vpci = vdev->vpci; + + spinlock_obtain(&vpci->lock); + vpci_deinit_vdev(vdev); + spinlock_release(&vpci->lock); + return 0; } diff --git a/hypervisor/include/dm/vpci.h b/hypervisor/include/dm/vpci.h index 52b2b8ee5..073b99bcd 100644 --- a/hypervisor/include/dm/vpci.h +++ b/hypervisor/include/dm/vpci.h @@ -194,6 +194,7 @@ struct acrn_pcidev; int32_t vpci_assign_pcidev(struct acrn_vm *tgt_vm, struct acrn_pcidev *pcidev); int32_t vpci_deassign_pcidev(struct acrn_vm *tgt_vm, struct acrn_pcidev *pcidev); struct pci_vdev *vpci_init_vdev(struct acrn_vpci *vpci, struct acrn_vm_pci_dev_config *dev_config, struct pci_vdev *parent_pf_vdev); +void vpci_deinit_vdev(struct pci_vdev *vdev); static inline bool is_pci_io_bar(struct pci_vbar *vbar) {