https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216456
Bug ID: 216456
Summary: iflib error checking for MSIX
Product: Base System
Version: CURRENT
Hardware: Any
OS: Any
Status: New
Severity: Affects Some People
Priority: ---
Component: kern
Assignee: freebsd-net@FreeBSD.org
Reporter: b...@freebsd.org
CC: sbr...@freebsd.org
Structurally this seems entirely backwards that drivers do supply a
isc_msix_bar value without checking if MSIX is available. Writing code based
on assumptions is not stable but iflib should guard against this.
Add error checking to the pci_find_cap(, PCIY_MSIX,) call that is returns
success and a good value. Only then try to use it and set the MSIX_ENABLE bit.
With the current em(4) driver we have observed failures in this case in a
specific environment when pci_find_cap() would not return the assumed value,
which meant we ended up writing to PCI register 2 (PCI_DEVICE_ID) which is
read-only.
It seems that a lot more safeguarding and perhaps better structuring between
drivers and iflib is needed to avoid these kinds of errors.
This patch just adds the safeguard in a single place in iflib, em(4) should
ideally be improved to not signal an msix bar value anymore if MSIX is not
avail.
Index: head-r312664.svn/sys/net/iflib.c
===================================================================
--- head-r312664.svn/sys/net/iflib.c (revision 312664)
+++ head-r312664.svn/sys/net/iflib.c (working copy)
@@ -3710,6 +3710,10 @@ iflib_device_register(device_t dev, void *sc, if_s
if (sctx->isc_flags & IFLIB_SKIP_MSIX) {
msix = scctx->isc_vectors;
} else if (scctx->isc_msix_bar != 0)
+ /*
+ * The simple fact that isc_msix_bar is not 0 does not mean we
+ * we have a good value there that is known to work.
+ */
msix = iflib_msix_init(ctx);
else {
scctx->isc_vectors = 1;
@@ -4754,15 +4758,21 @@ iflib_msix_init(if_ctx_t ctx)
uint16_t pci_cmd_word;
int msix_ctrl, rid;
- rid = 0;
pci_cmd_word = pci_read_config(dev, PCIR_COMMAND, 2);
pci_cmd_word |= PCIM_CMD_BUSMASTEREN;
pci_write_config(dev, PCIR_COMMAND, pci_cmd_word, 2);
- pci_find_cap(dev, PCIY_MSIX, &rid);
- rid += PCIR_MSIX_CTRL;
- msix_ctrl = pci_read_config(dev, rid, 2);
- msix_ctrl |= PCIM_MSIXCTRL_MSIX_ENABLE;
- pci_write_config(dev, rid, msix_ctrl, 2);
+
+ rid = 0;
+ if (pci_find_cap(dev, PCIY_MSIX, &rid) == 0 && rid != 0) {
+ rid += PCIR_MSIX_CTRL;
+ msix_ctrl = pci_read_config(dev, rid, 2);
+ msix_ctrl |= PCIM_MSIXCTRL_MSIX_ENABLE;
+ pci_write_config(dev, rid, msix_ctrl, 2);
+ } else {
+ device_printf(dev, "PCIY_MSIX capability not found; "
+ "or rid %d == 0.\n", rid);
+ goto msi;
+ }
}
/*
--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
freebsd-net@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
