diff options
author | Thanos Makatos <thanos.makatos@nutanix.com> | 2022-02-04 13:51:59 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-04 13:51:59 +0000 |
commit | 2d1d87016133b6c2f38e4f6a5fca6be5b820653c (patch) | |
tree | f316e6a045a8c08d9d5ab31f29e96b7034d73ea7 | |
parent | 314ed91fd991756bef2c87ecdf7feff4016c1d40 (diff) | |
download | libvfio-user-2d1d87016133b6c2f38e4f6a5fca6be5b820653c.zip libvfio-user-2d1d87016133b6c2f38e4f6a5fca6be5b820653c.tar.gz libvfio-user-2d1d87016133b6c2f38e4f6a5fca6be5b820653c.tar.bz2 |
ignore writes to RO MSI-X registers (#642)
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>
-rw-r--r-- | include/pci_caps/msix.h | 19 | ||||
-rw-r--r-- | lib/pci_caps.c | 51 | ||||
-rw-r--r-- | test/py/libvfio_user.py | 9 | ||||
-rw-r--r-- | test/py/test_pci_caps.py | 67 |
4 files changed, 104 insertions, 42 deletions
diff --git a/include/pci_caps/msix.h b/include/pci_caps/msix.h index 49e0765..36a7de6 100644 --- a/include/pci_caps/msix.h +++ b/include/pci_caps/msix.h @@ -39,23 +39,26 @@ extern "C" { #endif +/* Message Control for MSI-X */ struct mxc { - uint16_t ts:11; - uint16_t reserved:3; - uint16_t fm:1; - uint16_t mxe:1; + uint16_t ts:11; /* RO */ + uint16_t reserved:3; /* read must return 0, write has no effect */ + uint16_t fm:1; /* RW */ + uint16_t mxe:1; /* RW */ } __attribute__ ((packed)); _Static_assert(sizeof(struct mxc) == PCI_MSIX_FLAGS, "bad MXC size"); +/* Table Offset / Table BIR for MSI-X */ struct mtab { - uint32_t tbir:3; - uint32_t to:29; + uint32_t tbir:3; /* RO */ + uint32_t to:29; /* RO */ } __attribute__ ((packed)); _Static_assert(sizeof(struct mtab) == PCI_MSIX_TABLE, "bad MTAB size"); +/* PBA Offset / PBA BIR for MSI-X */ struct mpba { - uint32_t pbir:3; - uint32_t pbao:29; + uint32_t pbir:3; /* RO */ + uint32_t pbao:29; /* RO */ } __attribute__ ((packed)); _Static_assert(sizeof(struct mtab) == PCI_MSIX_PBA - PCI_MSIX_TABLE, "bad MPBA size"); diff --git a/lib/pci_caps.c b/lib/pci_caps.c index cd91914..e9dc5ac 100644 --- a/lib/pci_caps.c +++ b/lib/pci_caps.c @@ -167,49 +167,36 @@ cap_write_pm(vfu_ctx_t *vfu_ctx, struct pci_cap *cap, char * buf, } static ssize_t -handle_mxc_write(vfu_ctx_t *vfu_ctx, struct msixcap *msix, - const struct mxc *const mxc) +cap_write_msix(vfu_ctx_t *vfu_ctx, struct pci_cap *cap, char *buf, + size_t count, loff_t offset) { - assert(msix != NULL); - assert(mxc != NULL); + struct msixcap *msix = cap_data(vfu_ctx, cap); + struct msixcap new_msix = *msix; - if (mxc->mxe != msix->mxc.mxe) { - vfu_log(vfu_ctx, LOG_DEBUG, "%s MSI-X", - mxc->mxe ? "enable" : "disable"); - msix->mxc.mxe = mxc->mxe; - } + memcpy((char *)&new_msix + offset - cap->off, buf, count); + + /* + * Same as doing &= (PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE), but + * prefer to log what's changing. + */ - if (mxc->fm != msix->mxc.fm) { - if (mxc->fm) { + if (msix->mxc.fm != new_msix.mxc.fm) { + if (new_msix.mxc.fm) { vfu_log(vfu_ctx, LOG_DEBUG, "all MSI-X vectors masked"); } else { vfu_log(vfu_ctx, LOG_DEBUG, "vector's mask bit determines whether vector is masked"); } - msix->mxc.fm = mxc->fm; + msix->mxc.fm = new_msix.mxc.fm; } - return sizeof(struct mxc); -} - -static ssize_t -cap_write_msix(vfu_ctx_t *vfu_ctx, struct pci_cap *cap, char *buf, - size_t count, loff_t offset) -{ - struct msixcap *msix = cap_data(vfu_ctx, cap); - - if (count == sizeof(struct mxc)) { - switch (offset - cap->off) { - case offsetof(struct msixcap, mxc): - return handle_mxc_write(vfu_ctx, msix, (struct mxc *)buf); - default: - vfu_log(vfu_ctx, LOG_ERR, - "invalid MSI-X write offset %ld", offset); - return ERROR_INT(EINVAL); - } + if (msix->mxc.mxe != new_msix.mxc.mxe) { + vfu_log(vfu_ctx, LOG_DEBUG, "%s MSI-X", + msix->mxc.mxe ? "enable" : "disable"); + msix->mxc.mxe = new_msix.mxc.mxe; } - vfu_log(vfu_ctx, LOG_ERR, "invalid MSI-X write size %lu", count); - return ERROR_INT(EINVAL); + + return count; } static int diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 4b4a96a..6d71efd 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -63,6 +63,7 @@ PCI_CAP_LIST_NEXT = 1 PCI_CAP_ID_PM = 0x1 PCI_CAP_ID_VNDR = 0x9 +PCI_CAP_ID_MSIX = 0x11 PCI_CAP_ID_EXP = 0x10 PCI_EXP_DEVCTL2 = 40 @@ -75,6 +76,14 @@ PCI_EXT_CAP_DSN_SIZEOF = 12 PCI_EXT_CAP_VNDR_HDR_SIZEOF = 8 +# MSI-X registers +PCI_MSIX_FLAGS = 2 # Message Control +PCI_MSIX_TABLE = 4 # Table offset +PCI_MSIX_FLAGS_MASKALL = 0x4000 # Mask all vectors for this function +PCI_MSIX_FLAGS_ENABLE = 0x8000 # MSI-X enable +PCI_CAP_MSIX_SIZEOF = 12 # size of MSIX registers + + # from linux/vfio.h VFIO_DEVICE_FLAGS_RESET = (1 << 0) diff --git a/test/py/test_pci_caps.py b/test/py/test_pci_caps.py index effd6d3..b83c06c 100644 --- a/test/py/test_pci_caps.py +++ b/test/py/test_pci_caps.py @@ -349,9 +349,72 @@ def test_pci_cap_write_px(mock_quiesce, mock_reset): expect=errno.EINVAL) +def to_bytes_le(n, length=1): + return n.to_bytes(length, 'little') + + def test_pci_cap_write_msix(): - # FIXME - pass + setup_pci_dev(realize=True) + sock = connect_client(ctx) + + flags = PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE + pos = vfu_pci_add_capability(ctx, pos=0, flags=0, + data=struct.pack("ccHII", + to_byte(PCI_CAP_ID_MSIX), + b'\0', 0, 0, 0)) + assert pos == cap_offsets[0] + + offset = vfu_pci_find_capability(ctx, False, PCI_CAP_ID_MSIX) + + # write exactly to Message Control: mask all vectors and enable MSI-X + data = b'\xff\xff' + write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset + PCI_MSIX_FLAGS, + count=len(data), data=data) + data = b'\xff\xff' + to_bytes_le(flags, 2) + payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + count=len(data)) + expected = to_bytes_le(PCI_CAP_ID_MSIX) + b'\x00' + \ + to_bytes_le(PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE, 2) + assert expected == payload + + # reset + data = b'\x00\x00' + write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset + PCI_MSIX_FLAGS, + count=len(data), data=data) + expected = to_bytes_le(PCI_CAP_ID_MSIX) + b'\x00\x00' + payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + count=len(expected)) + assert expected == payload + + # write 2 bytes to Message Control + 1: mask all vectors and enable MSI-X + # This looks bizarre, but some versions of QEMU do this. + data = to_bytes_le(flags >> 8) + b'\xff' + write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset + PCI_MSIX_FLAGS + 1, + count=len(data), data=data) + # read back entire MSI-X + expected = to_bytes_le(PCI_CAP_ID_MSIX) + b'\x00' + \ + to_bytes_le(flags, 2) + b'\x00\x00\x00\x00' + b'\x00\x00\x00\x00' + payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + count=PCI_CAP_MSIX_SIZEOF) + assert expected == payload + + # reset with MSI-X enabled + data = to_bytes_le(PCI_MSIX_FLAGS_ENABLE, 2) + write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset + PCI_MSIX_FLAGS, + count=len(data), data=data) + + # write 1 byte past Message Control, MSI-X should still be enabled + data = b'\x00' + write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset + PCI_MSIX_TABLE, + count=len(data), data=data) + payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset + PCI_MSIX_FLAGS, count=2) + assert payload == to_bytes_le(PCI_MSIX_FLAGS_ENABLE, 2) def test_pci_cap_write_pxdc2(): |