Re: [PATCH] vdpa: Fix memory listener deletions of iova tree

[Jason Wang]

On Mon, Jul 25, 2022 at 3:05 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Jul 22, 2022 at 4:26 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > vhost_vdpa_listener_region_del is always deleting the first iova entry
> > of the tree, since it's using the needle iova instead of the result's
> > one.
> >
> > This was detected using a vga virtual device in the VM using vdpa SVQ.
> > It makes some extra memory adding and deleting, so the wrong one was
> > mapped / unmapped. This was undetected before since all the memory was
> > mappend and unmapped totally without that device, but other conditions
> > could trigger it too:
> >
> > * mem_region was with .iova = 0, .translated_addr = (correct GPA).
> > * iova_tree_find_iova returned right result, but does not update
> >   mem_region.
> > * iova_tree_remove always removed region with .iova = 0. Right iova were
> >   sent to the device.
> > * Next map will fill the first region with .iova = 0, causing a mapping
> >   with the same iova and device complains, if the next action is a map.
> > * Next unmap will cause to try to unmap again iova = 0, causing the
> >   device to complain that no region was mapped at iova = 0.
> >
> > Fixes: 34e3c94edaef ("vdpa: Add custom IOTLB translations to SVQ")
> > Reported-by: Lei Yang <leiyang@redhat.com>
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>
> Acked-by: Jason Wang <jasowang@redhat.com>

I've queued this for hard-freeze.

Thanks

>
> > ---
> >  hw/virtio/vhost-vdpa.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 291cd19054..00e990ea40 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -290,7 +290,7 @@ static void 
> > vhost_vdpa_listener_region_del(MemoryListener *listener,
> >
> >          result = vhost_iova_tree_find_iova(v->iova_tree, &mem_region);
> >          iova = result->iova;
> > -        vhost_iova_tree_remove(v->iova_tree, &mem_region);
> > +        vhost_iova_tree_remove(v->iova_tree, result);
> >      }
> >      vhost_vdpa_iotlb_batch_begin_once(v);
> >      ret = vhost_vdpa_dma_unmap(v, iova, int128_get64(llsize));
> > --
> > 2.31.1
> >