question regarding QEMU adding overlapping memory regions to VFIO

question regarding QEMU adding overlapping memory regions to VFIO

[Thanos Makatos]

I've noticed that QEMU adds overlapping memory regions to VFIO, e.g.:

vfio_listener_region_add_ram region_add [ram] 0xc0000 - 0xc0fff [0x7f6702c00000]
vfio_listener_region_del region_del 0xc4000 - 0xdffff
vfio_listener_region_add_ram region_add [ram] 0xc1000 - 0xc3fff [0x7f66406c1000]
vfio_listener_region_del region_del 0xe0000 - 0xfffff
vfio_listener_region_add_ram region_add [ram] 0xc4000 - 0xdffff [0x7f6702c04000]
vfio_listener_region_add_ram region_add [ram] 0xc0000 - 0xc0fff [0x7f66406c0000]
2021-05-05T09:38:16.158864Z qemu-system-x86_64: vfio_dma_map(0x557b8fd281b0, 0xc0000, 0x1000, 0x7f66406c0000) = -22 (Resource temporarily unavailable)

Region 0xc0000 - 0xc0fff is added first and then region 0xc0000 - 0xc0fff is added again? Is this legitimate? What is the implication of this? Is the previous region replaced by the more recent one?

Re: question regarding QEMU adding overlapping memory regions to VFIO

[Alex Williamson]

On Fri, 7 May 2021 13:51:52 +0000
Thanos Makatos <thanos.makatos@nutanix.com> wrote:

> I've noticed that QEMU adds overlapping memory regions to VFIO, e.g.:
> 
> vfio_listener_region_add_ram region_add [ram] 0xc0000 - 0xc0fff [0x7f6702c00000]
> vfio_listener_region_del region_del 0xc4000 - 0xdffff
> vfio_listener_region_add_ram region_add [ram] 0xc1000 - 0xc3fff [0x7f66406c1000]
> vfio_listener_region_del region_del 0xe0000 - 0xfffff
> vfio_listener_region_add_ram region_add [ram] 0xc4000 - 0xdffff [0x7f6702c04000]
> vfio_listener_region_add_ram region_add [ram] 0xc0000 - 0xc0fff [0x7f66406c0000]
> 2021-05-05T09:38:16.158864Z qemu-system-x86_64: vfio_dma_map(0x557b8fd281b0, 0xc0000, 0x1000, 0x7f66406c0000) = -22 (Resource temporarily unavailable)
> 
> Region 0xc0000 - 0xc0fff is added first and then region 0xc0000 -
> 0xc0fff is added again? Is this legitimate? What is the implication
> of this? Is the previous region replaced by the more recent one?

This might be where the hack we have in hw/vfio/common.c:vfio_dma_map()
comes from:

    /*
     * Try the mapping, if it fails with EBUSY, unmap the region and try
     * again.  This shouldn't be necessary, but we sometimes see it in
     * the VGA ROM space.
     */
    if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map) == 0 ||
        (errno == EBUSY && vfio_dma_unmap(container, iova, size, NULL) == 0 &&
         ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map) == 0)) {
        return 0;
    }

Clearly that's only triggered with -EBUSY and you're getting -EINVAL,
did we unintentionally change the errno for this?  What's the host
kernel version?

It's my expectation that this really shouldn't happen, the above is a
lazy workaround, but a listener being told to map two different things
at the same address range without an unmap in between seems like it
should violate the MemoryListener protocol.  Thanks,

Alex

RE: question regarding QEMU adding overlapping memory regions to VFIO

[Thanos Makatos]

> -----Original Message-----
> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: 07 May 2021 16:42
> To: Thanos Makatos <thanos.makatos@nutanix.com>
> Cc: qemu-devel@nongnu.org; Raphael Norwitz
> <raphael.norwitz@nutanix.com>
> Subject: Re: question regarding QEMU adding overlapping memory regions
> to VFIO
> 
> On Fri, 7 May 2021 13:51:52 +0000
> Thanos Makatos <thanos.makatos@nutanix.com> wrote:
> 
> > I've noticed that QEMU adds overlapping memory regions to VFIO, e.g.:
> >
> > vfio_listener_region_add_ram region_add [ram] 0xc0000 - 0xc0fff
> > [0x7f6702c00000] vfio_listener_region_del region_del 0xc4000 - 0xdffff
> > vfio_listener_region_add_ram region_add [ram] 0xc1000 - 0xc3fff
> > [0x7f66406c1000] vfio_listener_region_del region_del 0xe0000 - 0xfffff
> > vfio_listener_region_add_ram region_add [ram] 0xc4000 - 0xdffff
> > [0x7f6702c04000] vfio_listener_region_add_ram region_add [ram] 0xc0000
> > - 0xc0fff [0x7f66406c0000] 2021-05-05T09:38:16.158864Z
> > qemu-system-x86_64: vfio_dma_map(0x557b8fd281b0, 0xc0000, 0x1000,
> > 0x7f66406c0000) = -22 (Resource temporarily unavailable)
> >
> > Region 0xc0000 - 0xc0fff is added first and then region 0xc0000 -
> > 0xc0fff is added again? Is this legitimate? What is the implication of
> > this? Is the previous region replaced by the more recent one?
> 
> This might be where the hack we have in
> hw/vfio/common.c:vfio_dma_map() comes from:
> 
>     /*
>      * Try the mapping, if it fails with EBUSY, unmap the region and try
>      * again.  This shouldn't be necessary, but we sometimes see it in
>      * the VGA ROM space.
>      */
>     if (ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map) == 0 ||
>         (errno == EBUSY && vfio_dma_unmap(container, iova, size, NULL) == 0
> &&
>          ioctl(container->fd, VFIO_IOMMU_MAP_DMA, &map) == 0)) {
>         return 0;
>     }
> 
> Clearly that's only triggered with -EBUSY and you're getting -EINVAL, did we
> unintentionally change the errno for this?  What's the host kernel version?
> 
> It's my expectation that this really shouldn't happen, the above is a lazy
> workaround, but a listener being told to map two different things at the
> same address range without an unmap in between seems like it should
> violate the MemoryListener protocol.  Thanks,
> 
> Alex

Sorry, I should have mentioned that this is seen on a version of QEMU where
we're modifying hw/vfio/* to send requests to a vfio-user server instead of the
kernel. I would expect this code to be the same in both cases but apparently it
isn't (or maybe we broke something). It's good to know what the expected
behavior is.

Thank you