Re: [PATCH v3 03/17] virtio: queue_reset: struct virtio_config_ops add callbacks for queue_reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 8 Feb 2022 10:58:45 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
>
>  2022/2/7 pm 3:19, Xuan Zhuo :
> > On Mon, 7 Feb 2022 14:45:02 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> >>  2022/1/26 pm 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
>
>
> It looks to me we don't support this right now.

I am trying to implement this function based on virtio queue reset. It will be
added to the next version.

>
>
> >
> > 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.

OK.

Thanks.


>
> 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@xxxxxxxxxxxxxxxxx>
> >>> ---
> >>>    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. */
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux