Date: Mon, 30 Jan 2017 10:06:36 -0800 From: John Baldwin <jhb@freebsd.org> To: Sean Bruno <sbruno@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Matthew Macy <mmacy@nextbsd.org> Subject: Re: svn commit: r312755 - head/sys/net Message-ID: <212260342.t54IaOFj3v@ralph.baldwin.cx> In-Reply-To: <aa86cad1-7a3c-e0d9-ee26-edde528c112d@freebsd.org> References: <201701251437.v0PEb5D7047773@repo.freebsd.org> <6817684.C985jk9qCN@ralph.baldwin.cx> <aa86cad1-7a3c-e0d9-ee26-edde528c112d@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, January 30, 2017 09:18:56 AM Sean Bruno wrote: > > On 01/27/17 12:28, John Baldwin wrote: > > On Wednesday, January 25, 2017 02:37:05 PM Sean Bruno wrote: > >> Author: sbruno > >> Date: Wed Jan 25 14:37:05 2017 > >> New Revision: 312755 > >> URL: https://svnweb.freebsd.org/changeset/base/312755 > >> > >> Log: > >> 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. > > > > Why is this writing directly to the MSIX registers at all? pci_alloc_msix() > > etc. handle those registers for all other drivers and proper suspend/resume > > depends on drivers using the existing PCI API for managing MSI and MSI-X. > > > > The comment above this code block explains what's up. Basically, > virtualized environments are sometimes "lazy" about correct register setup. Can you provide more details here? We already deploy workarounds for specific versions of Xen that emulate MSI-X incorrectly, though for Xen your change actually makes things worse (older versions of Xen don't "see" updates to the MSI-X table while MSI-X is enabled). However, you are still enabling MSI-X while there is uninitialized junk in the table meaning that if the device asserted an interrupt it could trigger a panic. If there are bugs with hypervisors, we need to work around them in the base PCI code as we have done with the Xen workarounds. They are not going to be e1000-specific (or even NIC-specific), so iflib is the wrong place to handle them. > If MSIX caps aren't set, try to enable them. If that fails, assume MSI. Enabling MSI-X with an uninitialized table is not safe. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?212260342.t54IaOFj3v>