Re: [Qemu-devel] [PATCH] i386: pci-assign: Fix MSI-X table size

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 20 Jun 2016 11:40:17 -0400
Ido Yariv <ido@wizery.com> 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 <ido@wizery.com>  
> > 
> > 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