MSI-X vector tables hold 64-bit entries. So far, we used mmio_write64 to set them.
This conforms the PCI specification: "For all accesses to MSI-X Table and MSI-X PBA fields, software must use aligned full DWORD or aligned full QWORD transactions; otherwise the result is undefined" (PCI Local Bus Specification Rev 3.0, chapter 8.1.2). Nevertheless, some vendors don't support 64-bit writes, e.g., Broadcom ethernet cards (BCM5720). mmio_write64 stalls, and the transfer won't happen. Replace mmio_write64 with a wrapper mmio_write64_split that substitutes the 64-bit write with two 32-bit write operations. This accessor first writes the upper 32 bits and then the lower 32 bits. Credits go to Jan, the root cause of this bug was found in a private off-list discussion. Signed-off-by: Ralf Ramsauer <[email protected]> --- Only tested on x86, untested on ARM. I lack PCI devices with MSI-X suppport. hypervisor/arch/arm-common/pci.c | 4 ++-- hypervisor/arch/x86/pci.c | 8 ++++---- hypervisor/include/jailhouse/mmio.h | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/hypervisor/arch/arm-common/pci.c b/hypervisor/arch/arm-common/pci.c index 60ffabb8..4f35175f 100644 --- a/hypervisor/arch/arm-common/pci.c +++ b/hypervisor/arch/arm-common/pci.c @@ -58,8 +58,8 @@ int arch_pci_update_msi(struct pci_device *device, int arch_pci_update_msix_vector(struct pci_device *device, unsigned int index) { /* NOTE: See arch_pci_update_msi. */ - mmio_write64(&device->msix_table[index].address, - device->msix_vectors[index].address); + mmio_write64_split(&device->msix_table[index].address, + device->msix_vectors[index].address); mmio_write32(&device->msix_table[index].data, device->msix_vectors[index].data); return 0; diff --git a/hypervisor/arch/x86/pci.c b/hypervisor/arch/x86/pci.c index 492e9859..560c5697 100644 --- a/hypervisor/arch/x86/pci.c +++ b/hypervisor/arch/x86/pci.c @@ -393,8 +393,8 @@ int arch_pci_update_msix_vector(struct pci_device *device, unsigned int index) irq_msg); // HACK for QEMU if (result == -ENOSYS) { - mmio_write64(&device->msix_table[index].address, - device->msix_vectors[index].address); + mmio_write64_split(&device->msix_table[index].address, + device->msix_vectors[index].address); mmio_write32(&device->msix_table[index].data, device->msix_vectors[index].data); return 0; @@ -402,8 +402,8 @@ int arch_pci_update_msix_vector(struct pci_device *device, unsigned int index) if (result < 0) return result; - mmio_write64(&device->msix_table[index].address, - pci_get_x86_msi_remap_address(result)); + mmio_write64_split(&device->msix_table[index].address, + pci_get_x86_msi_remap_address(result)); mmio_write32(&device->msix_table[index].data, 0); return 0; diff --git a/hypervisor/include/jailhouse/mmio.h b/hypervisor/include/jailhouse/mmio.h index 61b4647e..567901d0 100644 --- a/hypervisor/include/jailhouse/mmio.h +++ b/hypervisor/include/jailhouse/mmio.h @@ -73,6 +73,21 @@ DEFINE_MMIO_WRITE(32) DEFINE_MMIO_WRITE(64) /** @} */ +/** + * Write a 64-bit value to a memory-mapper I/O register. Perform two 32-bit + * write operations instead of one 64-bit write operation. + * + * @param address Virtual address of the register. + * @param value Value to write. + * @{ + */ +static inline void mmio_write64_split(void *address, u64 value) +{ + mmio_write32(address + sizeof(u32), (u32)(value >> (sizeof(u32) * 8))); + mmio_write32(address, (u32)value); +} +/** @} */ + /** * Read value from 32 or 64-bit MMIO register field. * @param address Virtual address of the register. -- 2.22.0 -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/jailhouse-dev/20190619165224.15322-1-ralf.ramsauer%40oth-regensburg.de. For more options, visit https://groups.google.com/d/optout.
