[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] i386: pci-assign: Fix MSI-X table size
From: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH] i386: pci-assign: Fix MSI-X table size |
Date: |
Mon, 20 Jun 2016 11:51:05 -0600 |
On Mon, 20 Jun 2016 11:40:17 -0400
Ido Yariv <address@hidden> 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 <address@hidden>
> >
> > 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