From c05d9f80863509116592a2d25bae71e4e25fd182 Mon Sep 17 00:00:00 2001 From: Li Fei1 Date: Thu, 28 Nov 2019 19:21:14 +0800 Subject: [PATCH] hv: vmsix: refine vmsix remap Do vMSI-X remap only when Mask Bit in Vector Control Register for MSI-X Table Entry is unmask. The previous implementation also has two issues: 1. It only check whether Message Control Register for MSI-X has been modified when guest writes MSI-X CFG space at Message Control Register offset. 2. It doesn't really disable MSI-X when guest wants to disable MSI-X. Tracked-On: #3475 Signed-off-by: Li Fei1 --- hypervisor/dm/vpci/vmsix.c | 211 ++++++++++--------------------------- 1 file changed, 58 insertions(+), 153 deletions(-) diff --git a/hypervisor/dm/vpci/vmsix.c b/hypervisor/dm/vpci/vmsix.c index ab0847967..4be338311 100644 --- a/hypervisor/dm/vpci/vmsix.c +++ b/hypervisor/dm/vpci/vmsix.c @@ -46,121 +46,68 @@ static inline bool msixtable_access(const struct pci_vdev *vdev, uint32_t offset return in_range(offset, vdev->msix.table_offset, vdev->msix.table_count * MSIX_TABLE_ENTRY_SIZE); } +/** + * @pre vdev != NULL + */ +static inline struct msix_table_entry *get_msix_table_entry(const struct pci_vdev *vdev, uint32_t index) +{ + void *hva = hpa2hva(vdev->msix.mmio_hpa + vdev->msix.table_offset); + return ((struct msix_table_entry *)hva + index); +} + +/** + * @pre vdev != NULL + */ +static void mask_one_msix_vector(const struct pci_vdev *vdev, uint32_t index) +{ + uint32_t vector_control; + struct msix_table_entry *pentry = get_msix_table_entry(vdev, index); + + stac(); + vector_control = pentry->vector_control | PCIM_MSIX_VCTRL_MASK; + mmio_write32(vector_control, (void *)&(pentry->vector_control)); + clac(); +} + /** * @pre vdev != NULL * @pre vdev->vpci != NULL * @pre vdev->vpci->vm != NULL * @pre vdev->pdev != NULL */ -static int32_t remap_vmsix_entry(const struct pci_vdev *vdev, uint32_t index, bool enable) +static void remap_one_vmsix_entry(const struct pci_vdev *vdev, uint32_t index) { + const struct msix_table_entry *ventry; struct msix_table_entry *pentry; - struct ptirq_msi_info info; - void *hva; + struct ptirq_msi_info info = {}; int32_t ret; - info.vmsi_addr.full = vdev->msix.table_entries[index].addr; - info.vmsi_data.full = (enable) ? vdev->msix.table_entries[index].data : 0U; + mask_one_msix_vector(vdev, index); + ventry = &vdev->msix.table_entries[index]; + if ((ventry->vector_control & PCIM_MSIX_VCTRL_MASK) == 0U) { + info.vmsi_addr.full = vdev->msix.table_entries[index].addr; + info.vmsi_data.full = vdev->msix.table_entries[index].data; - ret = ptirq_prepare_msix_remap(vdev->vpci->vm, vdev->bdf.value, vdev->pdev->bdf.value, (uint16_t)index, &info); - if (ret == 0) { - /* Write the table entry to the physical structure */ - hva = hpa2hva(vdev->msix.mmio_hpa + vdev->msix.table_offset); - pentry = (struct msix_table_entry *)hva + index; + ret = ptirq_prepare_msix_remap(vdev->vpci->vm, vdev->bdf.value, vdev->pdev->bdf.value, (uint16_t)index, &info); + if (ret == 0) { + /* Write the table entry to the physical structure */ + pentry = get_msix_table_entry(vdev, index); - /* - * PCI 3.0 Spec allows writing to Message Address and Message Upper Address - * fields with a single QWORD write, but some hardware can accept 32 bits - * write only - */ - stac(); - mmio_write32((uint32_t)(info.pmsi_addr.full), (void *)&(pentry->addr)); - mmio_write32((uint32_t)(info.pmsi_addr.full >> 32U), (void *)((char *)&(pentry->addr) + 4U)); + /* + * PCI 3.0 Spec allows writing to Message Address and Message Upper Address + * fields with a single QWORD write, but some hardware can accept 32 bits + * write only + */ + stac(); + mmio_write32((uint32_t)(info.pmsi_addr.full), (void *)&(pentry->addr)); + mmio_write32((uint32_t)(info.pmsi_addr.full >> 32U), (void *)((char *)&(pentry->addr) + 4U)); - mmio_write32(info.pmsi_data.full, (void *)&(pentry->data)); - mmio_write32(vdev->msix.table_entries[index].vector_control, (void *)&(pentry->vector_control)); - clac(); - } - - return ret; -} - -/** - * @pre vdev != NULL - * @pre vdev->pdev != NULL - */ -static inline void enable_disable_msix(const struct pci_vdev *vdev, bool enable) -{ - uint32_t msgctrl; - - msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U); - if (enable) { - msgctrl |= PCIM_MSIXCTRL_MSIX_ENABLE; - } else { - msgctrl &= ~PCIM_MSIXCTRL_MSIX_ENABLE; - } - pci_pdev_write_cfg(vdev->pdev->bdf, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U, msgctrl); -} - -/** - * Do MSI-X remap for all MSI-X table entries in the target device - * @pre vdev != NULL - * @pre vdev->pdev != NULL - */ -static int32_t remap_vmsix(const struct pci_vdev *vdev, bool enable) -{ - uint32_t index; - int32_t ret = 0; - - /* disable MSI-X during configuration */ - enable_disable_msix(vdev, false); - - for (index = 0U; index < vdev->msix.table_count; index++) { - ret = remap_vmsix_entry(vdev, index, enable); - if (ret != 0) { - break; + mmio_write32(info.pmsi_data.full, (void *)&(pentry->data)); + mmio_write32(vdev->msix.table_entries[index].vector_control, (void *)&(pentry->vector_control)); + clac(); } } - /* If MSI Enable is being set, make sure INTxDIS bit is set */ - if (ret == 0) { - if (enable) { - enable_disable_pci_intx(vdev->pdev->bdf, false); - } - enable_disable_msix(vdev, enable); - } - - return ret; -} - -/** - * Do MSI-X remap for one MSI-X table entry only - * @pre vdev != NULL - * @pre vdev->pdev != NULL - */ -static int32_t remap_one_vmsx_entry(const struct pci_vdev *vdev, uint32_t index, bool enable) -{ - uint32_t msgctrl; - int32_t ret; - - /* disable MSI-X during configuration */ - enable_disable_msix(vdev, false); - - ret = remap_vmsix_entry(vdev, index, enable); - if (ret == 0) { - /* If MSI Enable is being set, make sure INTxDIS bit is set */ - if (enable) { - enable_disable_pci_intx(vdev->pdev->bdf, false); - } - - /* Restore MSI-X Enable bit */ - msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U); - if ((msgctrl & PCIM_MSIXCTRL_MSIX_ENABLE) == PCIM_MSIXCTRL_MSIX_ENABLE) { - pci_pdev_write_cfg(vdev->pdev->bdf, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U, msgctrl); - } - } - - return ret; } /** @@ -180,26 +127,19 @@ void vmsix_read_cfg(const struct pci_vdev *vdev, uint32_t offset, uint32_t bytes */ void vmsix_write_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uint32_t val) { - uint32_t msgctrl; - - msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U); + uint32_t old_msgctrl, msgctrl; + old_msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U); /* Write to vdev */ pci_vdev_write_cfg(vdev, offset, bytes, val); + msgctrl = pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U); - /* Writing Message Control field? */ - if ((offset - vdev->msix.capoff) == PCIR_MSIX_CTRL) { - if (((msgctrl ^ val) & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) { - if ((val & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) { - (void)remap_vmsix(vdev, true); - } else { - (void)remap_vmsix(vdev, false); - } - } - - if (((msgctrl ^ val) & PCIM_MSIXCTRL_FUNCTION_MASK) != 0U) { - pci_pdev_write_cfg(vdev->pdev->bdf, offset, 2U, val); + if (((old_msgctrl ^ msgctrl) & (PCIM_MSIXCTRL_MSIX_ENABLE | PCIM_MSIXCTRL_FUNCTION_MASK)) != 0U) { + /* If MSI Enable is being set, make sure INTxDIS bit is set */ + if ((msgctrl & PCIM_MSIXCTRL_MSIX_ENABLE) != 0U) { + enable_disable_pci_intx(vdev->pdev->bdf, false); } + pci_pdev_write_cfg(vdev->pdev->bdf, offset, 2U, msgctrl); } } @@ -207,12 +147,10 @@ void vmsix_write_cfg(struct pci_vdev *vdev, uint32_t offset, uint32_t bytes, uin * @pre vdev != NULL * @pre mmio != NULL */ -static void rw_vmsix_table(const struct pci_vdev *vdev, struct mmio_request *mmio, uint32_t offset) +static void rw_vmsix_table(struct pci_vdev *vdev, struct mmio_request *mmio, uint32_t offset) { - const struct msix_table_entry *entry; - uint32_t vector_control, entry_offset, table_offset, index; - bool message_changed = false; - bool unmasked; + struct msix_table_entry *entry; + uint32_t entry_offset, table_offset, index; /* Find out which entry it's accessing */ table_offset = offset - vdev->msix.table_offset; @@ -228,43 +166,10 @@ static void rw_vmsix_table(const struct pci_vdev *vdev, struct mmio_request *mmi } else { /* Only DWORD and QWORD are permitted */ if ((mmio->size == 4U) || (mmio->size == 8U)) { - /* Save for comparison */ - vector_control = entry->vector_control; - - /* - * Writing different value to Message Data/Addr? - * PCI Spec: Software is permitted to fill in MSI-X Table entry DWORD fields - * individually with DWORD writes, or software in certain cases is permitted - * to fill in appropriate pairs of DWORDs with a single QWORD write - */ - if (entry_offset < offsetof(struct msix_table_entry, data)) { - uint64_t qword_mask = ~0UL; - - if (mmio->size == 4U) { - qword_mask = (entry_offset == 0U) ? - 0x00000000FFFFFFFFUL : 0xFFFFFFFF00000000UL; - } - message_changed = ((entry->addr & qword_mask) != (mmio->value & qword_mask)); - } else { - if (entry_offset == offsetof(struct msix_table_entry, data)) { - message_changed = (entry->data != (uint32_t)mmio->value); - } - } - /* Write to pci_vdev */ (void)memcpy_s((void *)entry + entry_offset, (size_t)mmio->size, &mmio->value, (size_t)mmio->size); - - /* If MSI-X hasn't been enabled, do nothing */ - if ((pci_vdev_read_cfg(vdev, vdev->msix.capoff + PCIR_MSIX_CTRL, 2U) - & PCIM_MSIXCTRL_MSIX_ENABLE) == PCIM_MSIXCTRL_MSIX_ENABLE) { - - if ((((entry->vector_control ^ vector_control) & PCIM_MSIX_VCTRL_MASK) != 0U) - || message_changed) { - unmasked = ((entry->vector_control & PCIM_MSIX_VCTRL_MASK) == 0U); - (void)remap_one_vmsx_entry(vdev, index, unmasked); - } - } + remap_one_vmsix_entry(vdev, index); } else { pr_err("%s, Only DWORD and QWORD are permitted", __func__); }