From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bF3Lt-0003pw-JE for qemu-devel@nongnu.org; Mon, 20 Jun 2016 13:51:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bF3Ln-0005GF-NY for qemu-devel@nongnu.org; Mon, 20 Jun 2016 13:51:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36788) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bF3Ln-0005G4-Hp for qemu-devel@nongnu.org; Mon, 20 Jun 2016 13:51:07 -0400 Date: Mon, 20 Jun 2016 11:51:05 -0600 From: Alex Williamson Message-ID: <20160620115105.0f82afa7@ul30vt.home> In-Reply-To: <20160620154017.GA12442@WorkStation.localdomain> References: <1466282525-3018-1-git-send-email-ido@wizery.com> <87a99e71-d9b8-4c73-cced-6edf11a485a6@redhat.com> <20160620154017.GA12442@WorkStation.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] i386: pci-assign: Fix MSI-X table size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ido Yariv Cc: Paolo Bonzini , qemu-devel@nongnu.org, "Michael S. Tsirkin" , Richard Henderson , Eduardo Habkost On Mon, 20 Jun 2016 11:40:17 -0400 Ido Yariv wrote: > Hi Paolo, > > On Mon, Jun 20, 2016 at 04:54:17PM +0200, Paolo Bonzini wrote: > > On 18/06/2016 22:42, Ido Yariv wrote: > > > The current code creates a whole page mmio region for the MSI-X table > > > size. > > > > > > However, the page containing the MSI-X table may contain other registers > > > not related to MSI-X. Creating an mmio region for the whole page masks > > > such registers and may break drivers in the guest OS. > > > > > > Since maximal number of entries is known, use that instead to deduce the > > > table size when setting up the mmio region. > > > > > > Signed-off-by: Ido Yariv > > > > I can take this patch, but I'd like to warn you that pci-assign is > > deprecated (and replaced by VFIO). I seem to recall VFIO does this > > correctly, but it would be great if you could check that. > > Just gave it a quick shot and everything seems to be working fine with > VFIO, thanks! > > > Also, I would prefer the mmap/munmap to keep using MSIX_PAGE_SIZE, just > > to limit the number of things that could break. > > I believe there might be another issue with this. The number of entries > can be up to 2048, so the MSI-X table size can theoretically exceed one > page and take up to 8 pages. > > We can just modify MSIX_PAGE_SIZE to 0x8000, but wouldn't it be better > to just map exactly what we need? Or, let's just let pci-assign be broken and use vfio-pci :-D We should probably actually put an error_report() in pci-assign indicating that it's deprecated and subject to removal to ferret out the remaining users. Fixing anything in it sets the wrong precedent. Thanks, Alex