Xuan Zhuo
2022-Feb-07 07:19 UTC
[PATCH v3 03/17] virtio: queue_reset: struct virtio_config_ops add callbacks for queue_reset
On Mon, 7 Feb 2022 14:45:02 +0800, Jason Wang <jasowang at redhat.com> wrote:> > ? 2022/1/26 ??3:35, Xuan Zhuo ??: > > Performing reset on a queue is divided into two steps: > > > > 1. reset_vq: reset one vq > > 2. enable_reset_vq: re-enable the reset queue > > > > In the first step, these tasks will be completed: > > 1. notify the hardware queue to reset > > 2. recycle the buffer from vq > > 3. release the ring of the vq > > > > The second step is similar to find vqs, > > > Not sure, since find_vqs will usually try to allocate interrupts. > >Yes.> > passing parameters callback and > > name, etc. Based on the original vq, the ring is re-allocated and > > configured to the backend. > > > I wonder whether we really have such requirement. > > For example, do we really have a use case that may change: > > vq callback, ctx, ring_num or even re-create the virtqueue?1. virtqueue is not recreated 2. ring_num can be used to modify ring num by ethtool -G There is really no scene to modify vq callback, ctx, name. Do you mean we still use the old one instead of adding these parameters? Thanks.> > Thanks > > > > > > So add two callbacks reset_vq, enable_reset_vq to struct > > virtio_config_ops. > > > > Add a structure for passing parameters. This will facilitate subsequent > > expansion of the parameters of enable reset vq. > > There is currently only one default extended parameter ring_num. > > > > Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > > --- > > include/linux/virtio_config.h | 43 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 42 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > > index 4d107ad31149..51dd8461d1b6 100644 > > --- a/include/linux/virtio_config.h > > +++ b/include/linux/virtio_config.h > > @@ -16,6 +16,44 @@ struct virtio_shm_region { > > u64 len; > > }; > > > > +typedef void vq_callback_t(struct virtqueue *); > > + > > +/* virtio_reset_vq: specify parameters for queue_reset > > + * > > + * vdev: the device > > + * queue_index: the queue index > > + * > > + * free_unused_cb: callback to free unused bufs > > + * data: used by free_unused_cb > > + * > > + * callback: callback for the virtqueue, NULL for vq that do not need a > > + * callback > > + * name: virtqueue names (mainly for debugging), NULL for vq unused by > > + * driver > > + * ctx: ctx > > + * > > + * ring_num: specify ring num for the vq to be re-enabled. 0 means use the > > + * default value. MUST be a power of 2. > > + */ > > +struct virtio_reset_vq; > > +typedef void vq_reset_callback_t(struct virtio_reset_vq *param, void *buf); > > +struct virtio_reset_vq { > > + struct virtio_device *vdev; > > + u16 queue_index; > > + > > + /* reset vq param */ > > + vq_reset_callback_t *free_unused_cb; > > + void *data; > > + > > + /* enable reset vq param */ > > + vq_callback_t *callback; > > + const char *name; > > + const bool *ctx; > > + > > + /* ext enable reset vq param */ > > + u16 ring_num; > > +}; > > + > > /** > > * virtio_config_ops - operations for configuring a virtio device > > * Note: Do not assume that a transport implements all of the operations > > @@ -74,8 +112,9 @@ struct virtio_shm_region { > > * @set_vq_affinity: set the affinity for a virtqueue (optional). > > * @get_vq_affinity: get the affinity for a virtqueue (optional). > > * @get_shm_region: get a shared memory region based on the index. > > + * @reset_vq: reset a queue individually > > + * @enable_reset_vq: enable a reset queue > > */ > > -typedef void vq_callback_t(struct virtqueue *); > > struct virtio_config_ops { > > void (*enable_cbs)(struct virtio_device *vdev); > > void (*get)(struct virtio_device *vdev, unsigned offset, > > @@ -100,6 +139,8 @@ struct virtio_config_ops { > > int index); > > bool (*get_shm_region)(struct virtio_device *vdev, > > struct virtio_shm_region *region, u8 id); > > + int (*reset_vq)(struct virtio_reset_vq *param); > > + struct virtqueue *(*enable_reset_vq)(struct virtio_reset_vq *param); > > }; > > > > /* If driver didn't advertise the feature, it will never appear. */ >
Jason Wang
2022-Feb-08 02:58 UTC
[PATCH v3 03/17] virtio: queue_reset: struct virtio_config_ops add callbacks for queue_reset
? 2022/2/7 ??3:19, Xuan Zhuo ??:> On Mon, 7 Feb 2022 14:45:02 +0800, Jason Wang <jasowang at redhat.com> wrote: >> ? 2022/1/26 ??3:35, Xuan Zhuo ??: >>> Performing reset on a queue is divided into two steps: >>> >>> 1. reset_vq: reset one vq >>> 2. enable_reset_vq: re-enable the reset queue >>> >>> In the first step, these tasks will be completed: >>> 1. notify the hardware queue to reset >>> 2. recycle the buffer from vq >>> 3. release the ring of the vq >>> >>> The second step is similar to find vqs, >> >> Not sure, since find_vqs will usually try to allocate interrupts. >> >> > Yes. > > >>> passing parameters callback and >>> name, etc. Based on the original vq, the ring is re-allocated and >>> configured to the backend. >> >> I wonder whether we really have such requirement. >> >> For example, do we really have a use case that may change: >> >> vq callback, ctx, ring_num or even re-create the virtqueue? > 1. virtqueue is not recreated > 2. ring_num can be used to modify ring num by ethtool -GIt looks to me we don't support this right now.> > There is really no scene to modify vq callback, ctx, name. > > Do you mean we still use the old one instead of adding these parameters?Yes, I think for driver we need to implement the function that is needed for the first user (e.g AF_XDP). If there's no use case, we can leave those extension for the future. Thanks> > Thanks. > >> Thanks >> >> >>> So add two callbacks reset_vq, enable_reset_vq to struct >>> virtio_config_ops. >>> >>> Add a structure for passing parameters. This will facilitate subsequent >>> expansion of the parameters of enable reset vq. >>> There is currently only one default extended parameter ring_num. >>> >>> Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com> >>> --- >>> include/linux/virtio_config.h | 43 ++++++++++++++++++++++++++++++++++- >>> 1 file changed, 42 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h >>> index 4d107ad31149..51dd8461d1b6 100644 >>> --- a/include/linux/virtio_config.h >>> +++ b/include/linux/virtio_config.h >>> @@ -16,6 +16,44 @@ struct virtio_shm_region { >>> u64 len; >>> }; >>> >>> +typedef void vq_callback_t(struct virtqueue *); >>> + >>> +/* virtio_reset_vq: specify parameters for queue_reset >>> + * >>> + * vdev: the device >>> + * queue_index: the queue index >>> + * >>> + * free_unused_cb: callback to free unused bufs >>> + * data: used by free_unused_cb >>> + * >>> + * callback: callback for the virtqueue, NULL for vq that do not need a >>> + * callback >>> + * name: virtqueue names (mainly for debugging), NULL for vq unused by >>> + * driver >>> + * ctx: ctx >>> + * >>> + * ring_num: specify ring num for the vq to be re-enabled. 0 means use the >>> + * default value. MUST be a power of 2. >>> + */ >>> +struct virtio_reset_vq; >>> +typedef void vq_reset_callback_t(struct virtio_reset_vq *param, void *buf); >>> +struct virtio_reset_vq { >>> + struct virtio_device *vdev; >>> + u16 queue_index; >>> + >>> + /* reset vq param */ >>> + vq_reset_callback_t *free_unused_cb; >>> + void *data; >>> + >>> + /* enable reset vq param */ >>> + vq_callback_t *callback; >>> + const char *name; >>> + const bool *ctx; >>> + >>> + /* ext enable reset vq param */ >>> + u16 ring_num; >>> +}; >>> + >>> /** >>> * virtio_config_ops - operations for configuring a virtio device >>> * Note: Do not assume that a transport implements all of the operations >>> @@ -74,8 +112,9 @@ struct virtio_shm_region { >>> * @set_vq_affinity: set the affinity for a virtqueue (optional). >>> * @get_vq_affinity: get the affinity for a virtqueue (optional). >>> * @get_shm_region: get a shared memory region based on the index. >>> + * @reset_vq: reset a queue individually >>> + * @enable_reset_vq: enable a reset queue >>> */ >>> -typedef void vq_callback_t(struct virtqueue *); >>> struct virtio_config_ops { >>> void (*enable_cbs)(struct virtio_device *vdev); >>> void (*get)(struct virtio_device *vdev, unsigned offset, >>> @@ -100,6 +139,8 @@ struct virtio_config_ops { >>> int index); >>> bool (*get_shm_region)(struct virtio_device *vdev, >>> struct virtio_shm_region *region, u8 id); >>> + int (*reset_vq)(struct virtio_reset_vq *param); >>> + struct virtqueue *(*enable_reset_vq)(struct virtio_reset_vq *param); >>> }; >>> >>> /* If driver didn't advertise the feature, it will never appear. */