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

[Jason Wang <jasowang@redhat.com>]

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 <maxime.coquelin@redhat.com>
>> ---
>> 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