On Thu, 28 Mar 2024 12:31:48 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> On Wed, Mar 27, 2024 at 5:58 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> >
> > Now, we pass multi parameters to vring_new_virtqueue. These parameters
> > may from transport or from driver.
> >
> > vring_new_virtqueue is called by many places.
> > Every time, we try to add a new parameter, that is difficult.
> >
> > If parameters from the driver, that should directly be passed to vring.
> > Then the vring can access the config from driver directly.
> >
> > If parameters from the transport, we squish the parameters to a
> > structure. That will be helpful to add new parameter.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > ---
> > drivers/platform/mellanox/mlxbf-tmfifo.c | 12 ++++---
> > drivers/remoteproc/remoteproc_virtio.c | 11 ++++---
> > drivers/virtio/virtio_ring.c | 29 +++++++++++-----
> > include/linux/virtio_ring.h | 42 +++++++++++++++++++-----
> > tools/virtio/virtio_test.c | 4 +--
> > tools/virtio/vringh_test.c | 28 ++++++++--------
> > 6 files changed, 84 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c b/drivers/platform/mellanox/mlxbf-tmfifo.c
> > index 4252388f52a2..d2e871fad8b4 100644
> > --- a/drivers/platform/mellanox/mlxbf-tmfifo.c
> > +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
> > @@ -1059,6 +1059,7 @@ static int mlxbf_tmfifo_virtio_find_vqs(struct virtio_device *vdev,
> > struct virtio_vq_config *cfg)
> > {
> > struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev);
> > + struct vq_transport_config tp_cfg = {};
> > struct virtqueue **vqs = cfg->vqs;
> > struct mlxbf_tmfifo_vring *vring;
> > unsigned int nvqs = cfg->nvqs;
> > @@ -1078,10 +1079,13 @@ static int mlxbf_tmfifo_virtio_find_vqs(struct virtio_device *vdev,
> > /* zero vring */
> > size = vring_size(vring->num, vring->align);
> > memset(vring->va, 0, size);
> > - vq = vring_new_virtqueue(i, vring->num, vring->align, vdev,
> > - false, false, vring->va,
> > - mlxbf_tmfifo_virtio_notify,
> > - cfg->callbacks[i], cfg->names[i]);
> > +
> > + tp_cfg.num = vring->num;
> > + tp_cfg.vring_align = vring->align;
> > + tp_cfg.weak_barriers = false;
> > + tp_cfg.notify = mlxbf_tmfifo_virtio_notify;
> > +
> > + vq = vring_new_virtqueue(vdev, i, vring->va, &tp_cfg, cfg);
> > if (!vq) {
> > dev_err(&vdev->dev, "vring_new_virtqueue failed\n");
> > ret = -ENOMEM;
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> > index 489fea1d41c0..2319c2007833 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -106,6 +106,7 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
> > {
> > struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> > struct rproc *rproc = vdev_to_rproc(vdev);
> > + struct vq_transport_config tp_cfg;
>
> Should we zero this structure?
YES.
Will fix in next version.
Thanks.
>
> Thanks
>
