| Message ID | 98cc06919b016e16b9abc606dd514ac2b4f85c06.1658141552.git.kangjie.xu@linux.alibaba.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support VIRTIO_F_RING_RESET for virtio-net and vhost-user in virtio pci | expand |
在 2022/7/18 19:17, Kangjie Xu 写道: > Introduce vhost_dev_virtqueue_restart(), which can restart the > virtqueue when the vhost has already started running. > > Meanwhile, vhost_dev_virtqueue_release(), which can ummap the > vrings and the desc of a specific vq of a device. > > Combining the two functions, we can reset a virtqueue with a > started vhost. > > Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > --- > hw/virtio/vhost.c | 29 +++++++++++++++++++++++++++++ > include/hw/virtio/vhost.h | 6 ++++++ > 2 files changed, 35 insertions(+) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index e467dfc7bc..d158d71866 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1904,3 +1904,32 @@ int vhost_net_set_backend(struct vhost_dev *hdev, > > return -ENOSYS; > } > + > +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, VirtIODevice *vdev, > + int vq_index) > +{ > + int idx = vq_index - hdev->vq_index; > + > + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); > + > + vhost_virtqueue_unmap(hdev, > + vdev, > + hdev->vqs + idx, > + hdev->vq_index + idx); > +} Anything wrong that makes you can't use vhost_virtqueue_stop() here? Thanks > + > +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice *vdev, > + int vq_index) > +{ > + int idx = vq_index - hdev->vq_index; > + int r = 0; > + > + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); > + > + r = vhost_virtqueue_start(hdev, > + vdev, > + hdev->vqs + idx, > + hdev->vq_index + idx); > + > + return r; > +} > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > index a346f23d13..7df7dbe24d 100644 > --- a/include/hw/virtio/vhost.h > +++ b/include/hw/virtio/vhost.h > @@ -277,6 +277,12 @@ bool vhost_has_free_slot(void); > int vhost_net_set_backend(struct vhost_dev *hdev, > struct vhost_vring_file *file); > > + > +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, VirtIODevice *vdev, > + int vq_index); > +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice *vdev, > + int vq_index); > + > int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write); > > void vhost_dev_reset_inflight(struct vhost_inflight *inflight);
在 2022/7/26 12:13, Jason Wang 写道: > > 在 2022/7/18 19:17, Kangjie Xu 写道: >> Introduce vhost_dev_virtqueue_restart(), which can restart the >> virtqueue when the vhost has already started running. >> >> Meanwhile, vhost_dev_virtqueue_release(), which can ummap the >> vrings and the desc of a specific vq of a device. >> >> Combining the two functions, we can reset a virtqueue with a >> started vhost. >> >> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> >> --- >> hw/virtio/vhost.c | 29 +++++++++++++++++++++++++++++ >> include/hw/virtio/vhost.h | 6 ++++++ >> 2 files changed, 35 insertions(+) >> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >> index e467dfc7bc..d158d71866 100644 >> --- a/hw/virtio/vhost.c >> +++ b/hw/virtio/vhost.c >> @@ -1904,3 +1904,32 @@ int vhost_net_set_backend(struct vhost_dev *hdev, >> return -ENOSYS; >> } >> + >> +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, >> VirtIODevice *vdev, >> + int vq_index) >> +{ >> + int idx = vq_index - hdev->vq_index; >> + >> + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); >> + >> + vhost_virtqueue_unmap(hdev, >> + vdev, >> + hdev->vqs + idx, >> + hdev->vq_index + idx); >> +} > > > Anything wrong that makes you can't use vhost_virtqueue_stop() here? > > Thanks > > Yeah, vhost_virtqueue_stop() will result in all the queues in the device being stopped. In the scenario of DPDK-OVS, vhost_virtqueue_stop will send VHOST_USER_GET_VRING_BASE to DPDK. This message is meant to destroy the queue. However, In DPDK, VHOST_USER_GET_VRING_BASE message will lead to vhost_destroy_device_notify(). This function will disable all the queues in the device. Thus, when restarting the queue, other queues except the restarted one are still disabled. On the other hand, I think it may be not necessary to destroy the queue. We can simply disable it. When restarting the queue, it will be intialized again. I'm sorry I forgot to cc in the last email, so I resend this email. Thanks >> + >> +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice >> *vdev, >> + int vq_index) >> +{ >> + int idx = vq_index - hdev->vq_index; >> + int r = 0; >> + >> + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); >> + >> + r = vhost_virtqueue_start(hdev, >> + vdev, >> + hdev->vqs + idx, >> + hdev->vq_index + idx); >> + >> + return r; >> +} >> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h >> index a346f23d13..7df7dbe24d 100644 >> --- a/include/hw/virtio/vhost.h >> +++ b/include/hw/virtio/vhost.h >> @@ -277,6 +277,12 @@ bool vhost_has_free_slot(void); >> int vhost_net_set_backend(struct vhost_dev *hdev, >> struct vhost_vring_file *file); >> + >> +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, >> VirtIODevice *vdev, >> + int vq_index); >> +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice >> *vdev, >> + int vq_index); >> + >> int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, >> int write); >> void vhost_dev_reset_inflight(struct vhost_inflight *inflight);
在 2022/7/27 12:47, Jason Wang 写道: > On Tue, Jul 26, 2022 at 1:13 PM Kangjie Xu <kangjie.xu@linux.alibaba.com> wrote: >> >> 在 2022/7/26 12:13, Jason Wang 写道: >>> 在 2022/7/18 19:17, Kangjie Xu 写道: >>>> Introduce vhost_dev_virtqueue_restart(), which can restart the >>>> virtqueue when the vhost has already started running. >>>> >>>> Meanwhile, vhost_dev_virtqueue_release(), which can ummap the >>>> vrings and the desc of a specific vq of a device. >>>> >>>> Combining the two functions, we can reset a virtqueue with a >>>> started vhost. >>>> >>>> Signed-off-by: Kangjie Xu <kangjie.xu@linux.alibaba.com> >>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> >>>> --- >>>> hw/virtio/vhost.c | 29 +++++++++++++++++++++++++++++ >>>> include/hw/virtio/vhost.h | 6 ++++++ >>>> 2 files changed, 35 insertions(+) >>>> >>>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >>>> index e467dfc7bc..d158d71866 100644 >>>> --- a/hw/virtio/vhost.c >>>> +++ b/hw/virtio/vhost.c >>>> @@ -1904,3 +1904,32 @@ int vhost_net_set_backend(struct vhost_dev *hdev, >>>> return -ENOSYS; >>>> } >>>> + >>>> +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, >>>> VirtIODevice *vdev, >>>> + int vq_index) >>>> +{ >>>> + int idx = vq_index - hdev->vq_index; >>>> + >>>> + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); >>>> + >>>> + vhost_virtqueue_unmap(hdev, >>>> + vdev, >>>> + hdev->vqs + idx, >>>> + hdev->vq_index + idx); >>>> +} >>> >>> Anything wrong that makes you can't use vhost_virtqueue_stop() here? >>> >>> Thanks >>> >> Yeah, vhost_virtqueue_stop() will result in all the queues in the device >> being stopped. > This sounds a defect of the backend. I think we'd better not > workaround it in the qemu. Yeah, I agree. >> In the scenario of DPDK-OVS, vhost_virtqueue_stop will send >> VHOST_USER_GET_VRING_BASE to DPDK. This message is meant to destroy the >> queue. > That's tricky. > >> However, In DPDK, VHOST_USER_GET_VRING_BASE message will lead to >> vhost_destroy_device_notify(). This function will disable all the queues >> in the device. > Ok. I wonder how the reset is implemented in DPDK? It looks to me a > new type of request is required. > Currently, DPDK does not support reset of queue level, but it supports device-level reset. VHOST_USER_RESET_DEVICE request exists in vhost protocol but it is not used in vhost-net module in QEMU. It seems like there is no way for the driver to directly reset the device in vhost user net (If we use ethtool -r <eth_name>, we will get operation not supported.) Actually we can plug the driver in and out to perform a device reset operation, but it will be applied to all the devices. I take it for example here. If you plug out the driver(modprobe -r virtio-net), vhost_dev_stop() will be called in qemu. DPDK will receive VHOST_USER_GET_VRING_BASE messages for each virtqueue. Then you can plug in the driver(modprobe virtio-net), vhost_dev_start() will be called in qemu. The process is same as when we boot the machine, DPDK will receive: VHOST_USER_SET_VRING_NUM, VHOST_USER_SET_VRING_BASE, VHOST_USER_SET_VRING_ADDR, VHOST_USER_SET_VRING_KICK, VHOST_USER_SET_VRING_CALL in order to start a virtqueue. Our vq reset implemenation does not need any change to the DPDK except adding the feature bit, since we reuse all the request types in vhost-user protocol. I agree that we should add a new request in vhost-user protocol to support vq reset, we'll implement it in the next version. >> Thus, when restarting the queue, other queues except the restarted one >> are still disabled. >> >> On the other hand, I think it may be not necessary to destroy the queue. >> We can simply disable it. When restarting the queue, it will be >> intialized again. > So for vhost-net kernel, we can simply set NULL as the backend for a > specific virtqueue. We just need to refactor the stop codes (which did > it at vhost device level instead of queue level currently). > > Thanks Yes, I think so. Thanks >> Thanks >> >>>> + >>>> +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice >>>> *vdev, >>>> + int vq_index) >>>> +{ >>>> + int idx = vq_index - hdev->vq_index; >>>> + int r = 0; >>>> + >>>> + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); >>>> + >>>> + r = vhost_virtqueue_start(hdev, >>>> + vdev, >>>> + hdev->vqs + idx, >>>> + hdev->vq_index + idx); >>>> + >>>> + return r; >>>> +} >>>> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h >>>> index a346f23d13..7df7dbe24d 100644 >>>> --- a/include/hw/virtio/vhost.h >>>> +++ b/include/hw/virtio/vhost.h >>>> @@ -277,6 +277,12 @@ bool vhost_has_free_slot(void); >>>> int vhost_net_set_backend(struct vhost_dev *hdev, >>>> struct vhost_vring_file *file); >>>> + >>>> +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, >>>> VirtIODevice *vdev, >>>> + int vq_index); >>>> +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice >>>> *vdev, >>>> + int vq_index); >>>> + >>>> int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, >>>> int write); >>>> void vhost_dev_reset_inflight(struct vhost_inflight *inflight);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index e467dfc7bc..d158d71866 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1904,3 +1904,32 @@ int vhost_net_set_backend(struct vhost_dev *hdev, return -ENOSYS; } + +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, VirtIODevice *vdev, + int vq_index) +{ + int idx = vq_index - hdev->vq_index; + + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); + + vhost_virtqueue_unmap(hdev, + vdev, + hdev->vqs + idx, + hdev->vq_index + idx); +} + +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice *vdev, + int vq_index) +{ + int idx = vq_index - hdev->vq_index; + int r = 0; + + idx = hdev->vhost_ops->vhost_get_vq_index(hdev, idx); + + r = vhost_virtqueue_start(hdev, + vdev, + hdev->vqs + idx, + hdev->vq_index + idx); + + return r; +} diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index a346f23d13..7df7dbe24d 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -277,6 +277,12 @@ bool vhost_has_free_slot(void); int vhost_net_set_backend(struct vhost_dev *hdev, struct vhost_vring_file *file); + +void vhost_dev_virtqueue_release(struct vhost_dev *hdev, VirtIODevice *vdev, + int vq_index); +int vhost_dev_virtqueue_restart(struct vhost_dev *hdev, VirtIODevice *vdev, + int vq_index); + int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write); void vhost_dev_reset_inflight(struct vhost_inflight *inflight);