Assorted fixes to MSI-X/MSI/INTx setup in iflib(9):

- In iflib_msix_init(), VMMs with broken MSI-X activation are trying
  to be worked around by manually enabling PCIM_MSIXCTRL_MSIX_ENABLE
  before calling pci_alloc_msix(9). Apart from constituting a layering
  violation, this has the problem of leaving PCIM_MSIXCTRL_MSIX_ENABLE
  enabled when falling back to MSI or INTx when e. g. MSI-X is black-
  listed and initially also when disabled via hw.pci.enable_msix. The
  later in turn was incorrectly worked around in r325166.
  Since r310806, pci(4) itself has code to deal with broken MSI-X
  handling of VMMs, so all of these workarounds in iflib(9) can go,
  fixing non-working interrupts when falling back to MSI/INTx. In
  any case, possibly further adjustments to broken MSI-X activation
  of VMMs like enabling r310806 by default in VM environments need to
  be placed into pci(4), not iflib(9). [1]
- Also remove the pci_enable_busmaster(9) call from iflib_msix_init(),
  which is already more properly invoked from iflib_device_attach().
- When falling back to MSI/INTx, release the MSI-X BAR resource again.
- When falling back to INTx, ensure scctx->isc_vectors is set to 1 and
  not to something higher from a device with more than one MSI message
  supported.
- Make the nearby ring_state(s) stuff (static) const.

Discussed with:	jhb at BSDCan 2018 [1]
Reviewed by:	imp, jhb
Differential Revision:	https://reviews.freebsd.org/D15729
This commit is contained in:
Marius Strobl 2018-06-17 20:33:02 +00:00
parent eca1e88c0b
commit e4defe55a8

View File

@ -5879,48 +5879,10 @@ iflib_msix_init(if_ctx_t ctx)
bar = ctx->ifc_softc_ctx.isc_msix_bar; bar = ctx->ifc_softc_ctx.isc_msix_bar;
admincnt = sctx->isc_admin_intrcnt; admincnt = sctx->isc_admin_intrcnt;
/* Override by global tuneable */
{
int i;
size_t len = sizeof(i);
err = kernel_sysctlbyname(curthread, "hw.pci.enable_msix", &i, &len, NULL, 0, NULL, 0);
if (err == 0) {
if (i == 0)
goto msi;
}
else {
device_printf(dev, "unable to read hw.pci.enable_msix.");
}
}
/* Override by tuneable */ /* Override by tuneable */
if (scctx->isc_disable_msix) if (scctx->isc_disable_msix)
goto msi; goto msi;
/*
** When used in a virtualized environment
** PCI BUSMASTER capability may not be set
** so explicity set it here and rewrite
** the ENABLE in the MSIX control register
** at this point to cause the host to
** successfully initialize us.
*/
{
int msix_ctrl, rid;
pci_enable_busmaster(dev);
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;
}
}
/* /*
* bar == -1 => "trust me I know what I'm doing" * bar == -1 => "trust me I know what I'm doing"
* Some drivers are for hardware that is so shoddily * Some drivers are for hardware that is so shoddily
@ -6007,6 +5969,9 @@ iflib_msix_init(if_ctx_t ctx)
return (vectors); return (vectors);
} else { } else {
device_printf(dev, "failed to allocate %d msix vectors, err: %d - using MSI\n", vectors, err); device_printf(dev, "failed to allocate %d msix vectors, err: %d - using MSI\n", vectors, err);
bus_release_resource(dev, SYS_RES_MEMORY, bar,
ctx->ifc_msix_mem);
ctx->ifc_msix_mem = NULL;
} }
msi: msi:
vectors = pci_msi_count(dev); vectors = pci_msi_count(dev);
@ -6017,6 +5982,7 @@ iflib_msix_init(if_ctx_t ctx)
device_printf(dev,"Using an MSI interrupt\n"); device_printf(dev,"Using an MSI interrupt\n");
scctx->isc_intr = IFLIB_INTR_MSI; scctx->isc_intr = IFLIB_INTR_MSI;
} else { } else {
scctx->isc_vectors = 1;
device_printf(dev,"Using a Legacy interrupt\n"); device_printf(dev,"Using a Legacy interrupt\n");
scctx->isc_intr = IFLIB_INTR_LEGACY; scctx->isc_intr = IFLIB_INTR_LEGACY;
} }
@ -6024,7 +5990,7 @@ iflib_msix_init(if_ctx_t ctx)
return (vectors); return (vectors);
} }
char * ring_states[] = { "IDLE", "BUSY", "STALLED", "ABDICATED" }; static const char *ring_states[] = { "IDLE", "BUSY", "STALLED", "ABDICATED" };
static int static int
mp_ring_state_handler(SYSCTL_HANDLER_ARGS) mp_ring_state_handler(SYSCTL_HANDLER_ARGS)
@ -6032,7 +5998,7 @@ mp_ring_state_handler(SYSCTL_HANDLER_ARGS)
int rc; int rc;
uint16_t *state = ((uint16_t *)oidp->oid_arg1); uint16_t *state = ((uint16_t *)oidp->oid_arg1);
struct sbuf *sb; struct sbuf *sb;
char *ring_state = "UNKNOWN"; const char *ring_state = "UNKNOWN";
/* XXX needed ? */ /* XXX needed ? */
rc = sysctl_wire_old_buffer(req, 0); rc = sysctl_wire_old_buffer(req, 0);