|
From: | Jason Wang |
Subject: | Re: [PATCH 09/31] vhost-vdpa: Take into account SVQ in vhost_vdpa_set_vring_call |
Date: | Mon, 21 Feb 2022 15:39:37 +0800 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 |
在 2022/2/18 下午8:35, Eugenio Perez Martin 写道:
On Tue, Feb 8, 2022 at 4:23 AM Jason Wang <jasowang@redhat.com> wrote:在 2022/1/31 下午11:34, Eugenio Perez Martin 写道:On Sat, Jan 29, 2022 at 9:06 AM Jason Wang <jasowang@redhat.com> wrote:在 2022/1/22 上午4:27, Eugenio Pérez 写道:Signed-off-by: Eugenio Pérez <eperezma@redhat.com> --- hw/virtio/vhost-vdpa.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 18de14f0fb..029f98feee 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -687,13 +687,29 @@ static int vhost_vdpa_set_vring_kick(struct vhost_dev *dev, } } -static int vhost_vdpa_set_vring_call(struct vhost_dev *dev, - struct vhost_vring_file *file) +static int vhost_vdpa_set_vring_dev_call(struct vhost_dev *dev, + struct vhost_vring_file *file) { trace_vhost_vdpa_set_vring_call(dev, file->index, file->fd); return vhost_vdpa_call(dev, VHOST_SET_VRING_CALL, file); } +static int vhost_vdpa_set_vring_call(struct vhost_dev *dev, + struct vhost_vring_file *file) +{ + struct vhost_vdpa *v = dev->opaque; + + if (v->shadow_vqs_enabled) { + int vdpa_idx = vhost_vdpa_get_vq_index(dev, file->index); + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, vdpa_idx); + + vhost_svq_set_guest_call_notifier(svq, file->fd);Two questions here (had similar questions for vring kick): 1) Any reason that we setup the eventfd for vhost-vdpa in vhost_vdpa_svq_setup() not here?I'm not sure what you mean. The guest->SVQ call and kick fds are set here and at vhost_vdpa_set_vring_kick. The event notifier handler of the guest -> SVQ kick_fd is set at vhost_vdpa_set_vring_kick / vhost_svq_set_svq_kick_fd. The guest -> SVQ call fd has no event notifier handler since we don't poll it. On the other hand, the connection SVQ <-> device uses the same fds from the beginning to the end, and they will not change with, for example, call fd masking. That's why it's setup from vhost_vdpa_svq_setup. Delaying to vhost_vdpa_set_vring_call would make us add way more logic there.More logic in general shadow vq code but less codes for vhost-vdpa specific code I think. E.g for we can move the kick set logic from vhost_vdpa_svq_set_fds() to here.But they are different fds. vhost_vdpa_svq_set_fds sets the SVQ<->device. This function sets the SVQ->guest call file descriptor. To move the logic of vhost_vdpa_svq_set_fds here would imply either: a) Logic to know if we are receiving the first call fd or not.
Any reason for this? I guess you meant multiqueue. If yes, it should not be much difference since we have idx as the parameter.
That code is not in the series at the moment, because setting at vhost_vdpa_dev_start tells the difference for free. Is just adding code, not moving. b) Logic to set again *the same* file descriptor to device, with logic to tell if we have missed calls. That logic is not implemented for device->SVQ call file descriptor, because we are assuming it never changes from vhost_vdpa_svq_set_fds. So this is again adding code. At this moment, we have: vhost_vdpa_svq_set_fds: set SVQ<->device fds vhost_vdpa_set_vring_call: set guest<-SVQ call vhost_vdpa_set_vring_kick: set guest->SVQ kick. If I understood correctly, the alternative would be something like: vhost_vdpa_set_vring_call: set guest<-SVQ call if(!vq->call_set) { - set SVQ<-device call. - vq->call_set = true } vhost_vdpa_set_vring_kick: set guest<-SVQ call if(!vq->dev_kick_set) { - set guest->device kick. - vq->dev_kick_set = true } dev_reset / dev_stop: for vq in vqs: vq->dev_kick_set = vq->dev_call_set = false ... Or have I misunderstood something?
I wonder what happens if MSI-X is masking in guest. So if I understand correctly, we don't disable the eventfd from device? If yes, this seems suboptinal.
Thanks
Thanks!Thanks2) The call could be disabled by using -1 as the fd, I don't see any code to deal with that.Right, I didn't take that into account. vhost-kernel takes also -1 as kick_fd to unbind, so SVQ can be reworked to take that into account for sure. Thanks!Thanks+ return 0; + } else { + return vhost_vdpa_set_vring_dev_call(dev, file); + } +} + /** * Set shadow virtqueue descriptors to the device *
[Prev in Thread] | Current Thread | [Next in Thread] |