Re: [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in the error path

[Jason Wang]

On Wed, Jun 22, 2022 at 12:13 AM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
>
> Call vringh_complete_iotlb() even when we encounter a serious error
> that prevents us from writing the status in the "in" header
> (e.g. the header length is incorrect, etc.).
>
> The guest is misbehaving, so maybe the ring is in a bad state, but
> let's avoid making things worse.
>
> Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>

Acked-by: Jason Wang <jasowang@xxxxxxxxxx>

> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index ac86478845b6..de9cd9843143 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -63,6 +63,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>  {
>         size_t pushed = 0, to_pull, to_push;
>         struct virtio_blk_outhdr hdr;
> +       bool handled = false;
>         ssize_t bytes;
>         loff_t offset;
>         u64 sector;
> @@ -78,12 +79,12 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>         if (vq->out_iov.used < 1 || vq->in_iov.used < 1) {
>                 dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n",
>                         vq->out_iov.used, vq->in_iov.used);
> -               return false;
> +               goto err;
>         }
>
>         if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) {
>                 dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n");
> -               return false;
> +               goto err;
>         }
>
>         /* The last byte is the status and we checked if the last iov has
> @@ -97,7 +98,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>                                       sizeof(hdr));
>         if (bytes != sizeof(hdr)) {
>                 dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n");
> -               return false;
> +               goto err;
>         }
>
>         to_pull -= bytes;
> @@ -182,16 +183,19 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>         /* Last byte is the status */
>         bytes = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, &status, 1);
>         if (bytes != 1)
> -               return false;
> +               goto err;
>
>         pushed += bytes;
>
>         /* Make sure data is wrote before advancing index */
>         smp_wmb();
>
> +       handled = true;
> +
> +err:
>         vringh_complete_iotlb(&vq->vring, vq->head, pushed);
>
> -       return true;
> +       return handled;
>  }
>
>  static void vdpasim_blk_work(struct work_struct *work)
> --
> 2.36.1
>