Re: [Qemu-devel] [PATCH v2 3/6] vhost: extend ring information update for IOTLB to all rings

[Jason Wang]

On 2017年05月31日 02:12, Michael S. Tsirkin wrote:
> On Fri, May 26, 2017 at 04:28:55PM +0200, Maxime Coquelin wrote:
> > Vhost-kernel backend need
> needs
>
> > to receive IOTLB entry for used ring
> > information early, which is done by triggering a miss event on
> > its address.
> >
> > This patch extends this behaviour to all rings information, to be
> > compatible with vhost-user backend design.
> Why does vhost-user need it though?
>
> > Signed-off-by: Maxime Coquelin <address@hidden>
> > ---
> > v2:
> >   - Revert back to existing behaviour, i.e. only send IOTLB updates
> > at ring enablement time, not at ring address setting time (mst).
> >   - Extend IOTLB misses to all ring addresses, not only used ring.
> >
> >   hw/virtio/vhost.c | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 6eddb09..7867034 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -1552,11 +1552,15 @@ int vhost_dev_start(struct vhost_dev *hdev, 
> > VirtIODevice *vdev)
> >       if (vhost_dev_has_iommu(hdev)) {
> >           hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true);
> >   
> > -        /* Update used ring information for IOTLB to work correctly,
> > -         * vhost-kernel code requires for this.*/
> > +        /*
> > +         * Update rings information for IOTLB to work correctly,
> > +         * vhost-kernel and vhost-user codes require for this.
> Better just say "Update ring info for vhost iotlb."
>
> The rest isn't really informative.
>
>
>
> > +         */
> >           for (i = 0; i < hdev->nvqs; ++i) {
> >               struct vhost_virtqueue *vq = hdev->vqs + i;
> >               vhost_device_iotlb_miss(hdev, vq->used_phys, true);
> > +            vhost_device_iotlb_miss(hdev, vq->desc_phys, true);
> > +            vhost_device_iotlb_miss(hdev, vq->avail_phys, true);
> So I don't remember why does vhost in kernel want miss on used
> at start time.
>
> Jason, could you comment on this please?

In vhost_vq_init_access() we try to update used flags and fetch 
last_used_idx, this requires we cache its translation in advance.

We don't support IOTLB miss on control path (ioctl) now, so I choose to 
update IOTLB.

Thanks

> >           }
> >       }
> >       return 0;
> > --
> > 2.9.4