Hi Jason.
On Thu, Jan 28, 2021 at 3:32 AM Jason Wang <jasow...@redhat.com> wrote:
>
>
> On 2021/1/28 上午4:44, Eugenio Pérez wrote:
> > Not registering this can lead to vhost_backend_handle_iotlb_msg and
> > vhost_device_iotlb_miss if backend issue a miss after qemu vhost device
> > stop.
> >
> > This causes a try to access dev->vdev->dma_as with vdev == NULL.
>
>
> Hi Eugenio:
>
> What condition can we get this? Did you mean we receive IOTLB_MISS
> before vhost_dev_start()?
>
In the VM reboot case, yes, between vhost_dev_stop() and
vhost_dev_start(). But I can reproduce the bug in VM shutdown too,
with no vhost_dev_start expected.
I didn't try to send IOTLB_MISS before starting vhost_dev, but this
patch should solve that problem too.
I think we can get this with whatever malicious/buggy vhost-user
backend sending unexpected iotlb misses, but I didn't test so deeply.
> If yes, it looks to me a bug somewhere else.
Where should I look for it?
Thanks!
>
> Thanks
>
>
> >
> > Reproduced rebooting a guest with testpmd in txonly forward mode.
> > #0 0x0000559ffff94394 in vhost_device_iotlb_miss (
> > dev=dev@entry=0x55a0012f6680, iova=10245279744, write=1)
> > at ../hw/virtio/vhost.c:1013
> > #1 0x0000559ffff9ac31 in vhost_backend_handle_iotlb_msg (
> > imsg=0x7ffddcfd32c0, dev=0x55a0012f6680)
> > at ../hw/virtio/vhost-backend.c:411
> > #2 vhost_backend_handle_iotlb_msg (dev=dev@entry=0x55a0012f6680,
> > imsg=imsg@entry=0x7ffddcfd32c0)
> > at ../hw/virtio/vhost-backend.c:404
> > #3 0x0000559fffeded7b in slave_read (opaque=0x55a0012f6680)
> > at ../hw/virtio/vhost-user.c:1464
> > #4 0x000055a0000c541b in aio_dispatch_handler (
> > ctx=ctx@entry=0x55a0010a2120, node=0x55a0012d9e00)
> > at ../util/aio-posix.c:329
> >
> > Fixes: 6dcdd06e3b ("spec/vhost-user spec: Add IOMMU support")
> > Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> > ---
> > hw/virtio/vhost-user.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 2fdd5daf74..a49b2229fb 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -238,6 +238,7 @@ struct vhost_user {
> > /* Shared between vhost devs of the same virtio device */
> > VhostUserState *user;
> > int slave_fd;
> > + bool iotlb_enabled;
> > NotifierWithReturn postcopy_notifier;
> > struct PostCopyFD postcopy_fd;
> > uint64_t postcopy_client_bases[VHOST_USER_MAX_RAM_SLOTS];
> > @@ -1461,7 +1462,11 @@ static void slave_read(void *opaque)
> >
> > switch (hdr.request) {
> > case VHOST_USER_SLAVE_IOTLB_MSG:
> > - ret = vhost_backend_handle_iotlb_msg(dev, &payload.iotlb);
> > + if (likely(u->iotlb_enabled)) {
> > + ret = vhost_backend_handle_iotlb_msg(dev, &payload.iotlb);
> > + } else {
> > + ret = -EFAULT;
> > + }
> > break;
> > case VHOST_USER_SLAVE_CONFIG_CHANGE_MSG :
> > ret = vhost_user_slave_handle_config_change(dev);
> > @@ -2044,7 +2049,8 @@ static int vhost_user_send_device_iotlb_msg(struct
> > vhost_dev *dev,
> >
> > static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int
> > enabled)
> > {
> > - /* No-op as the receive channel is not dedicated to IOTLB messages. */
> > + struct vhost_user *u = dev->opaque;
> > + u->iotlb_enabled = enabled;
> > }
> >
> > static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config,
>
