[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio: Add corresponding memory_listener_unregister to unre
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH] virtio: Add corresponding memory_listener_unregister to unrealize |
Date: |
Mon, 25 Jan 2021 17:55:35 +0100 |
On Mon, Jan 25, 2021 at 4:15 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/23 上午4:08, Eugenio Pérez wrote:
> > Cannot destroy address spaces of IOMMU-aware virtio devices without it,
> > since they can contain memory listeners.
>
>
> It's better to explain why the one in finalize doesn't work here.
>
Hi Jason! Good point. The other call is at virtio_device_instance_finalize.
Function virtio_device_instance_finalize is called after
address_space_destroy if we follow steps of buglink.
Address_space_destroy is called by
pci_qdev_unrealize/do_pci_unregister_device, after call to
virtio_device_unrealize. After that call,
virtio_device_instance_finalize is called through object_deinit,
freeing the bus.
Also, memory_listener_unregister can be called again because it checks
for listener->address_space != NULL at start, and sets it to NULL at
end.
In regular shutdown, nothing of this is called, so maybe we could
safely delete the call to memory_listener_unregister at
virtio_device_instance_finalize?
If not, should I send again the patch with a new commit message?
Thanks!
> Thanks
>
>
> >
> > Fixes: c611c76417f ("virtio: add MemoryListener to cache ring translations")
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1912846
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > hw/virtio/virtio.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index b308026596..67efd2c301 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -3680,6 +3680,7 @@ static void virtio_device_unrealize(DeviceState *dev)
> > VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(dev);
> >
> > + memory_listener_unregister(&vdev->listener);
> > virtio_bus_device_unplugged(vdev);
> >
> > if (vdc->unrealize != NULL) {
>