Re: [Qemu-devel] [PULL 7/7] vfio: fix mapping of MSIX bar

[Alex Williamson]

On Sun, 2014-01-19 at 22:03 +0800, Kai Huang wrote:
> On Sat, Jan 18, 2014 at 3:25 AM, Alex Williamson
> <address@hidden> wrote:
> > From: Alexey Kardashevskiy <address@hidden>
> >
> > VFIO virtualizes MSIX table for the guest but not mapping the part of
> > a BAR which contains an MSIX table. Since vfio_mmap_bar() mmaps chunks
> > before and after the MSIX table, they have to be aligned to the host
> > page size which may be TARGET_PAGE_MASK (4K) or 64K in case of PPC64.
> >
> > This fixes boundaries calculations to use the real host page size.
> >
> > Without the patch, the chunk before MSIX table may overlap with the MSIX
> > table and mmap will fail in the host kernel. The result will be serious
> > slowdown as the whole BAR will be emulated by QEMU.
> >
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > Signed-off-by: Alex Williamson <address@hidden>
> > ---
> >  hw/misc/vfio.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> > index 432547c..8a1f1a1 100644
> > --- a/hw/misc/vfio.c
> > +++ b/hw/misc/vfio.c
> > @@ -2544,7 +2544,7 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
> >       * potentially insert a direct-mapped subregion before and after it.
> >       */
> >      if (vdev->msix && vdev->msix->table_bar == nr) {
> > -        size = vdev->msix->table_offset & TARGET_PAGE_MASK;
> > +        size = vdev->msix->table_offset & qemu_host_page_mask;
> >      }
> >
> >      strncat(name, " mmap", sizeof(name) - strlen(name) - 1);
> > @@ -2556,8 +2556,8 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
> >      if (vdev->msix && vdev->msix->table_bar == nr) {
> >          unsigned start;
> >
> > -        start = TARGET_PAGE_ALIGN(vdev->msix->table_offset +
> > -                                  (vdev->msix->entries * 
> > PCI_MSIX_ENTRY_SIZE));
> > +        start = HOST_PAGE_ALIGN(vdev->msix->table_offset +
> > +                                (vdev->msix->entries * 
> > PCI_MSIX_ENTRY_SIZE));
> >
> Hi Alex,
> 
> I am new to vfio and qemu, and have some questions. Does MSIX have one
> dedicated bar when qemu emulating the device? Looks your code maps
> both the content before and after the MSIX table? If MSIX has
> dedicated bar, I think we can just skip the MSIX bar, why do we need
> to map the context before and after the MSIX table?

vfio is used to pass through existing physical devices.  We don't get to
define the MSI-X layout of those devices.  Therefore we must be prepared
to handle any possible layout.  The BAR may be dedicated to the MSI-X
table or it may also include memory mapped register space for the
device.  Thanks,

Alex