On Wed, Mar 27, 2024 at 5:57 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > Now, we pass multi parameters to find_vqs. These parameters > may work for transport or work for vring. > > And find_vqs has multi implements in many places: > > arch/um/drivers/virtio_uml.c > drivers/platform/mellanox/mlxbf-tmfifo.c > drivers/remoteproc/remoteproc_virtio.c > drivers/s390/virtio/virtio_ccw.c > drivers/virtio/virtio_mmio.c > drivers/virtio/virtio_pci_legacy.c > drivers/virtio/virtio_pci_modern.c > drivers/virtio/virtio_vdpa.c > > Every time, we try to add a new parameter, that is difficult. > We must change every find_vqs implement. > > One the other side, if we want to pass a parameter to vring, > we must change the call path from transport to vring. > Too many functions need to be changed. > > So it is time to refactor the find_vqs. We pass a structure > cfg to find_vqs(), that will be passed to vring by transport. > > Because the vp_modern_create_avq() use the "const char *names[]", > and the virtio_uml.c changes the name in the subsequent commit, so > change the "names" inside the virtio_vq_config from "const char *const > *names" to "const char **names". > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> > Acked-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> > --- > arch/um/drivers/virtio_uml.c | 22 +++---- > drivers/platform/mellanox/mlxbf-tmfifo.c | 13 ++-- > drivers/remoteproc/remoteproc_virtio.c | 25 ++++---- > drivers/s390/virtio/virtio_ccw.c | 28 ++++----- > drivers/virtio/virtio_mmio.c | 26 ++++---- > drivers/virtio/virtio_pci_common.c | 57 ++++++++---------- > drivers/virtio/virtio_pci_common.h | 9 +-- > drivers/virtio/virtio_pci_legacy.c | 11 ++-- > drivers/virtio/virtio_pci_modern.c | 32 ++++++---- > drivers/virtio/virtio_vdpa.c | 33 +++++----- > include/linux/virtio_config.h | 76 ++++++++++++++++++------ > 11 files changed, 175 insertions(+), 157 deletions(-) > > diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c > index 773f9fc4d582..adc619362cc0 100644 > --- a/arch/um/drivers/virtio_uml.c > +++ b/arch/um/drivers/virtio_uml.c > @@ -937,8 +937,8 @@ static int vu_setup_vq_call_fd(struct virtio_uml_device *vu_dev, > } > > static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, > - unsigned index, vq_callback_t *callback, > - const char *name, bool ctx) > + unsigned index, > + struct virtio_vq_config *cfg) > { > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > struct platform_device *pdev = vu_dev->pdev; > @@ -953,10 +953,12 @@ static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, > goto error_kzalloc; > } > snprintf(info->name, sizeof(info->name), "%s.%d-%s", pdev->name, > - pdev->id, name); > + pdev->id, cfg->names[index]); > > vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true, > - ctx, vu_notify, callback, info->name); > + cfg->ctx ? cfg->ctx[index] : false, > + vu_notify, > + cfg->callbacks[index], info->name); > if (!vq) { > rc = -ENOMEM; > goto error_create; > @@ -1013,12 +1015,11 @@ static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, > return ERR_PTR(rc); > } > > -static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > - const char * const names[], const bool *ctx, > - struct irq_affinity *desc) > +static int vu_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > { > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > + struct virtqueue **vqs = cfg->vqs; > + unsigned int nvqs = cfg->nvqs; > struct virtqueue *vq; > int i, rc; > > @@ -1031,13 +1032,12 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs, > return rc; > > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > rc = -EINVAL; > goto error_setup; > } > > - vqs[i] = vu_setup_vq(vdev, i, callbacks[i], names[i], > - ctx ? ctx[i] : false); > + vqs[i] = vu_setup_vq(vdev, i, cfg); > if (IS_ERR(vqs[i])) { > rc = PTR_ERR(vqs[i]); > goto error_setup; > diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c b/drivers/platform/mellanox/mlxbf-tmfifo.c > index b8d1e32e97eb..4252388f52a2 100644 > --- a/drivers/platform/mellanox/mlxbf-tmfifo.c > +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c > @@ -1056,15 +1056,12 @@ static void mlxbf_tmfifo_virtio_del_vqs(struct virtio_device *vdev) > > /* Create and initialize the virtual queues. */ > static int mlxbf_tmfifo_virtio_find_vqs(struct virtio_device *vdev, > - unsigned int nvqs, > - struct virtqueue *vqs[], > - vq_callback_t *callbacks[], > - const char * const names[], > - const bool *ctx, > - struct irq_affinity *desc) > + struct virtio_vq_config *cfg) > { > struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev); > + struct virtqueue **vqs = cfg->vqs; > struct mlxbf_tmfifo_vring *vring; > + unsigned int nvqs = cfg->nvqs; > struct virtqueue *vq; > int i, ret, size; > > @@ -1072,7 +1069,7 @@ static int mlxbf_tmfifo_virtio_find_vqs(struct virtio_device *vdev, > return -EINVAL; > > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > ret = -EINVAL; > goto error; > } > @@ -1084,7 +1081,7 @@ static int mlxbf_tmfifo_virtio_find_vqs(struct virtio_device *vdev, > vq = vring_new_virtqueue(i, vring->num, vring->align, vdev, > false, false, vring->va, > mlxbf_tmfifo_virtio_notify, > - callbacks[i], names[i]); > + cfg->callbacks[i], cfg->names[i]); > 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 8fb5118b6953..489fea1d41c0 100644 > --- a/drivers/remoteproc/remoteproc_virtio.c > +++ b/drivers/remoteproc/remoteproc_virtio.c > @@ -102,8 +102,7 @@ EXPORT_SYMBOL(rproc_vq_interrupt); > > static struct virtqueue *rp_find_vq(struct virtio_device *vdev, > unsigned int id, > - void (*callback)(struct virtqueue *vq), > - const char *name, bool ctx) > + struct virtio_vq_config *cfg) > { > struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); > struct rproc *rproc = vdev_to_rproc(vdev); > @@ -140,10 +139,12 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev, > * Create the new vq, and tell virtio we're not interested in > * the 'weak' smp barriers, since we're talking with a real device. > */ > - vq = vring_new_virtqueue(id, num, rvring->align, vdev, false, ctx, > - addr, rproc_virtio_notify, callback, name); > + vq = vring_new_virtqueue(id, num, rvring->align, vdev, false, > + cfg->ctx ? cfg->ctx[id] : false, > + addr, rproc_virtio_notify, cfg->callbacks[id], > + cfg->names[id]); > if (!vq) { > - dev_err(dev, "vring_new_virtqueue %s failed\n", name); > + dev_err(dev, "vring_new_virtqueue %s failed\n", cfg->names[id]); > rproc_free_vring(rvring); > return ERR_PTR(-ENOMEM); > } > @@ -177,23 +178,19 @@ static void rproc_virtio_del_vqs(struct virtio_device *vdev) > __rproc_virtio_del_vqs(vdev); > } > > -static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > - struct virtqueue *vqs[], > - vq_callback_t *callbacks[], > - const char * const names[], > - const bool * ctx, > - struct irq_affinity *desc) > +static int rproc_virtio_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > { > + struct virtqueue **vqs = cfg->vqs; > + unsigned int nvqs = cfg->nvqs; > int i, ret; > > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > ret = -EINVAL; > goto error; > } > > - vqs[i] = rp_find_vq(vdev, i, callbacks[i], names[i], > - ctx ? ctx[i] : false); > + vqs[i] = rp_find_vq(vdev, i, cfg); > if (IS_ERR(vqs[i])) { > ret = PTR_ERR(vqs[i]); > goto error; > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c > index 508154705554..3c78122f00f5 100644 > --- a/drivers/s390/virtio/virtio_ccw.c > +++ b/drivers/s390/virtio/virtio_ccw.c > @@ -499,9 +499,8 @@ static void virtio_ccw_del_vqs(struct virtio_device *vdev) > } > > static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev, > - int i, vq_callback_t *callback, > - const char *name, bool ctx, > - struct ccw1 *ccw) > + int i, struct ccw1 *ccw, > + struct virtio_vq_config *cfg) > { > struct virtio_ccw_device *vcdev = to_vc_device(vdev); > bool (*notify)(struct virtqueue *vq); > @@ -538,8 +537,11 @@ static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev, > } > may_reduce = vcdev->revision > 0; > vq = vring_create_virtqueue(i, info->num, KVM_VIRTIO_CCW_RING_ALIGN, > - vdev, true, may_reduce, ctx, > - notify, callback, name); > + vdev, true, may_reduce, > + cfg->ctx ? cfg->ctx[i] : false, > + notify, > + cfg->callbacks[i], > + cfg->names[i]); > > if (!vq) { > /* For now, we fail if we can't get the requested size. */ > @@ -650,15 +652,13 @@ static int virtio_ccw_register_adapter_ind(struct virtio_ccw_device *vcdev, > return ret; > } > > -static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, > - struct virtqueue *vqs[], > - vq_callback_t *callbacks[], > - const char * const names[], > - const bool *ctx, > - struct irq_affinity *desc) > +static int virtio_ccw_find_vqs(struct virtio_device *vdev, > + struct virtio_vq_config *cfg) > { > struct virtio_ccw_device *vcdev = to_vc_device(vdev); > + struct virtqueue **vqs = cfg->vqs; > unsigned long *indicatorp = NULL; > + unsigned int nvqs = cfg->nvqs; > int ret, i; > struct ccw1 *ccw; > > @@ -667,14 +667,12 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs, > return -ENOMEM; > > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > ret = -EINVAL; > goto out; > } > > - vqs[i] = virtio_ccw_setup_vq(vdev, i, callbacks[i], > - names[i], ctx ? ctx[i] : false, > - ccw); > + vqs[i] = virtio_ccw_setup_vq(vdev, i, ccw, cfg); > if (IS_ERR(vqs[i])) { > ret = PTR_ERR(vqs[i]); > vqs[i] = NULL; > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index 82ee4a288728..7f0fdc3f51cb 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -370,8 +370,7 @@ static void vm_synchronize_cbs(struct virtio_device *vdev) > } > > static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int index, > - void (*callback)(struct virtqueue *vq), > - const char *name, bool ctx) > + struct virtio_vq_config *cfg) > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > bool (*notify)(struct virtqueue *vq); > @@ -386,9 +385,6 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > else > notify = vm_notify; > > - if (!name) > - return NULL; Nit: This seems to belong to patch 2. Acked-by: Jason Wang <jasowang@xxxxxxxxxx> Thanks