From owner-svn-src-all@freebsd.org Mon Jan 30 18:06:45 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 818CDCC7D27; Mon, 30 Jan 2017 18:06:45 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 60E0F130C; Mon, 30 Jan 2017 18:06:45 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 8FDBA10A791; Mon, 30 Jan 2017 13:06:43 -0500 (EST) From: John Baldwin To: Sean Bruno Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Matthew Macy Subject: Re: svn commit: r312755 - head/sys/net Date: Mon, 30 Jan 2017 10:06:36 -0800 Message-ID: <212260342.t54IaOFj3v@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-STABLE; KDE/4.14.10; amd64; ; ) In-Reply-To: References: <201701251437.v0PEb5D7047773@repo.freebsd.org> <6817684.C985jk9qCN@ralph.baldwin.cx> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Mon, 30 Jan 2017 13:06:43 -0500 (EST) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Jan 2017 18:06:45 -0000 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