From 27a66acd0e3d2ec82da6017e5333c8a776cf898a Mon Sep 17 00:00:00 2001 From: Li Fei1 Date: Thu, 7 May 2020 13:55:39 +0800 Subject: [PATCH] hv: ptdev: refine look up MSI ptirq entry There's no need to look up MSI ptirq entry by virtual SID any more since the MSI ptirq entry would be removed before the device is assigned to a VM. Now the logic of MSI interrupt remap could simplify as: 1. Add the MSI interrupt remap first; 2. If step is already done, just do the remap part. Tracked-On: #4550 Signed-off-by: Li Fei1 Acked-by: Eddie Dong Reviewed-by: Grandhi, Sainath --- hypervisor/arch/x86/guest/assign.c | 69 +++++++--------------- hypervisor/dm/vpci/vmsi.c | 2 +- hypervisor/dm/vpci/vmsix.c | 2 +- hypervisor/include/arch/x86/guest/assign.h | 4 +- 4 files changed, 24 insertions(+), 53 deletions(-) diff --git a/hypervisor/arch/x86/guest/assign.c b/hypervisor/arch/x86/guest/assign.c index 7228f48a0..d3ce819c2 100644 --- a/hypervisor/arch/x86/guest/assign.c +++ b/hypervisor/arch/x86/guest/assign.c @@ -313,56 +313,36 @@ static struct ptirq_remapping_info *add_msix_remapping(struct acrn_vm *vm, entry = find_ptirq_entry(PTDEV_INTR_MSI, &phys_sid, NULL); if (entry == NULL) { - if (find_ptirq_entry(PTDEV_INTR_MSI, &virt_sid, vm) != NULL) { - pr_err("MSIX re-add vbdf%x", virt_bdf); - } else { - entry = ptirq_alloc_entry(vm, PTDEV_INTR_MSI); - if (entry != NULL) { - entry->phys_sid.value = phys_sid.value; - entry->virt_sid.value = virt_sid.value; - entry->release_cb = ptirq_free_irte; + entry = ptirq_alloc_entry(vm, PTDEV_INTR_MSI); + if (entry != NULL) { + entry->phys_sid.value = phys_sid.value; + entry->virt_sid.value = virt_sid.value; + entry->release_cb = ptirq_free_irte; - /* update msi source and active entry */ - if (ptirq_activate_entry(entry, IRQ_INVALID) < 0) { - ptirq_release_entry(entry); - entry = NULL; - } + /* update msi source and active entry */ + if (ptirq_activate_entry(entry, IRQ_INVALID) < 0) { + ptirq_release_entry(entry); + entry = NULL; } } - } else if (entry->vm != vm) { - if (is_sos_vm(entry->vm)) { - entry->vm = vm; - entry->virt_sid.msi_id.bdf = virt_bdf; - } else { - pr_err("MSIX pbdf%x idx=%d already in vm%d with vbdf%x, not able to add into vm%d with vbdf%x", - entry->phys_sid.msi_id.bdf, entry->phys_sid.msi_id.entry_nr, entry->vm->vm_id, - entry->virt_sid.msi_id.bdf, vm->vm_id, virt_bdf); - pr_err("msix entry pbdf%x idx%d already in vm%d", phys_bdf, entry_nr, entry->vm->vm_id); - entry = NULL; - } - } else { - /* The mapping has already been added to the VM. No action - * required. - */ + dev_dbg(DBG_LEVEL_IRQ, "VM%d MSIX add vector mapping vbdf%x:pbdf%x idx=%d", + vm->vm_id, virt_bdf, phys_bdf, entry_nr); } - dev_dbg(DBG_LEVEL_IRQ, "VM%d MSIX add vector mapping vbdf%x:pbdf%x idx=%d", - vm->vm_id, virt_bdf, phys_bdf, entry_nr); - return entry; } /* deactive & remove mapping entry of vbdf:entry_nr for vm */ static void -remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf, uint32_t entry_nr) +remove_msix_remapping(const struct acrn_vm *vm, uint16_t phys_bdf, uint32_t entry_nr) { struct ptirq_remapping_info *entry; - DEFINE_MSI_SID(virt_sid, virt_bdf, entry_nr); + DEFINE_MSI_SID(phys_sid, phys_bdf, entry_nr); struct intr_source intr_src; - entry = find_ptirq_entry(PTDEV_INTR_MSI, &virt_sid, vm); - if (entry != NULL) { + entry = find_ptirq_entry(PTDEV_INTR_MSI, &phys_sid, NULL); + if ((entry != NULL) && (entry->vm == vm)) { if (is_entry_active(entry)) { /*TODO: disable MSIX device when HV can in future */ ptirq_deactivate_entry(entry); @@ -372,10 +352,8 @@ remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf, uint32_t entr intr_src.src.msi.value = entry->phys_sid.msi_id.bdf; dmar_free_irte(&intr_src, (uint16_t)entry->allocated_pirq); - dev_dbg(DBG_LEVEL_IRQ, - "VM%d MSIX remove vector mapping vbdf-pbdf:0x%x-0x%x idx=%d", - entry->vm->vm_id, virt_bdf, - entry->phys_sid.msi_id.bdf, entry_nr); + dev_dbg(DBG_LEVEL_IRQ, "VM%d MSIX remove vector mapping vbdf-pbdf:0x%x-0x%x idx=%d", + vm->vm_id, entry->virt_sid.msi_id.bdf, phys_bdf, entry_nr); ptirq_release_entry(entry); } @@ -619,7 +597,6 @@ int32_t ptirq_prepare_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf, uint16_t uint16_t entry_nr, struct msi_info *info) { struct ptirq_remapping_info *entry; - DEFINE_MSI_SID(virt_sid, virt_bdf, entry_nr); int32_t ret = -ENODEV; union pci_bdf vbdf; @@ -628,13 +605,7 @@ int32_t ptirq_prepare_msix_remap(struct acrn_vm *vm, uint16_t virt_bdf, uint16_t * entry already be held by others, return error. */ spinlock_obtain(&ptdev_lock); - entry = find_ptirq_entry(PTDEV_INTR_MSI, &virt_sid, vm); - if (entry == NULL) { - entry = add_msix_remapping(vm, virt_bdf, phys_bdf, entry_nr); - if (entry == NULL) { - pr_err("dev-assign: msi entry exist in others"); - } - } + entry = add_msix_remapping(vm, virt_bdf, phys_bdf, entry_nr); spinlock_release(&ptdev_lock); if (entry != NULL) { @@ -846,14 +817,14 @@ void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, bo /* * @pre vm != NULL */ -void ptirq_remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf, +void ptirq_remove_msix_remapping(const struct acrn_vm *vm, uint16_t phys_bdf, uint32_t vector_count) { uint32_t i; for (i = 0U; i < vector_count; i++) { spinlock_obtain(&ptdev_lock); - remove_msix_remapping(vm, virt_bdf, i); + remove_msix_remapping(vm, phys_bdf, i); spinlock_release(&ptdev_lock); } } diff --git a/hypervisor/dm/vpci/vmsi.c b/hypervisor/dm/vpci/vmsi.c index 0f62ecee9..946fc6b51 100644 --- a/hypervisor/dm/vpci/vmsi.c +++ b/hypervisor/dm/vpci/vmsi.c @@ -128,7 +128,7 @@ void write_vmsi_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint void deinit_vmsi(struct pci_vdev *vdev) { if (has_msi_cap(vdev)) { - ptirq_remove_msix_remapping(vpci2vm(vdev->vpci), vdev->bdf.value, 1U); + ptirq_remove_msix_remapping(vpci2vm(vdev->vpci), vdev->pdev->bdf.value, 1U); (void)memset((void *)&vdev->msi, 0U, sizeof(struct pci_msi)); } } diff --git a/hypervisor/dm/vpci/vmsix.c b/hypervisor/dm/vpci/vmsix.c index 45eda4ed5..0d6cf42ed 100644 --- a/hypervisor/dm/vpci/vmsix.c +++ b/hypervisor/dm/vpci/vmsix.c @@ -264,7 +264,7 @@ void deinit_vmsix(struct pci_vdev *vdev) { if (has_msix_cap(vdev)) { if (vdev->msix.table_count != 0U) { - ptirq_remove_msix_remapping(vpci2vm(vdev->vpci), vdev->bdf.value, vdev->msix.table_count); + ptirq_remove_msix_remapping(vpci2vm(vdev->vpci), vdev->pdev->bdf.value, vdev->msix.table_count); (void)memset((void *)&vdev->msix, 0U, sizeof(struct pci_msix)); } } diff --git a/hypervisor/include/arch/x86/guest/assign.h b/hypervisor/include/arch/x86/guest/assign.h index c09f2ba94..0799537f2 100644 --- a/hypervisor/include/arch/x86/guest/assign.h +++ b/hypervisor/include/arch/x86/guest/assign.h @@ -130,7 +130,7 @@ void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, bo * Remove the mapping of given number of vectors of the given virtual BDF for the given vm. * * @param[in] vm pointer to acrn_vm - * @param[in] virt_bdf virtual bdf associated with the passthrough device + * @param[in] phys_bdf physical bdf associated with the passthrough device * @param[in] vector_count number of vectors * * @return None @@ -138,7 +138,7 @@ void ptirq_remove_intx_remapping(const struct acrn_vm *vm, uint32_t virt_gsi, bo * @pre vm != NULL * */ -void ptirq_remove_msix_remapping(const struct acrn_vm *vm, uint16_t virt_bdf, uint32_t vector_count); +void ptirq_remove_msix_remapping(const struct acrn_vm *vm, uint16_t phys_bdf, uint32_t vector_count); /** * @}