Re: [PATCH] vhost-user: Check for iotlb callback in iotlb_miss

[Jason Wang]

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()?

If yes, it looks to me a bug somewhere else.

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 <eperezma@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,