| Message ID | 20240424091533.86949-4-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | refactor the params of find_vqs() | expand |
On Wed, Apr 24, 2024 at 05:15:30PM +0800, Xuan Zhuo 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@linux.alibaba.com> > Acked-by: Johannes Berg <johannes@sipsolutions.net> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > Acked-by: Jason Wang <jasowang@redhat.com> > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > Acked-by: Halil Pasic <pasic@linux.ibm.com> > --- > 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 | 23 ++++--- > 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(+), 154 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 7f58634fcc41..bbde11287f8a 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 6cdd29952bc0..4d94d20b253a 100644 > --- a/drivers/s390/virtio/virtio_ccw.c > +++ b/drivers/s390/virtio/virtio_ccw.c > @@ -536,9 +536,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); > @@ -576,8 +575,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. */ > @@ -687,14 +689,12 @@ 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 int nvqs = cfg->nvqs; > dma64_t *indicatorp = NULL; > int ret, i; > struct ccw1 *ccw; > @@ -704,14 +704,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 c3c8dd282952..4ebb28b6b0ec 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); > @@ -411,7 +410,11 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > > /* Create the vring */ > vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev, > - true, true, ctx, notify, callback, name); > + true, true, > + cfg->ctx ? cfg->ctx[index] : false, > + notify, > + cfg->callbacks[index], > + cfg->names[index]); > if (!vq) { > err = -ENOMEM; > goto error_new_virtqueue; > @@ -484,15 +487,12 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > return ERR_PTR(err); > } > > -static int vm_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 vm_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > int irq = platform_get_irq(vm_dev->pdev, 0); > + struct virtqueue **vqs = cfg->vqs; > + unsigned int nvqs = cfg->nvqs; > int i, err; > > if (irq < 0) > @@ -507,13 +507,12 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > enable_irq_wake(irq); > > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > vm_del_vqs(vdev); > return -EINVAL; > } > > - vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i], > - ctx ? ctx[i] : false); > + vqs[i] = vm_setup_vq(vdev, i, cfg); > if (IS_ERR(vqs[i])) { > vm_del_vqs(vdev); > return PTR_ERR(vqs[i]); > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index eda71c6e87ee..cb2776e3d0e1 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -172,9 +172,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, > } > > static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int index, > - void (*callback)(struct virtqueue *vq), > - const char *name, > - bool ctx, > + struct virtio_vq_config *cfg, > u16 msix_vec) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > @@ -186,13 +184,12 @@ static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int in > if (!info) > return ERR_PTR(-ENOMEM); > > - vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, ctx, > - msix_vec); > + vq = vp_dev->setup_vq(vp_dev, info, index, cfg, msix_vec); > if (IS_ERR(vq)) > goto out_info; > > info->vq = vq; > - if (callback) { > + if (cfg->callbacks[index]) { > spin_lock_irqsave(&vp_dev->lock, flags); > list_add(&info->node, &vp_dev->virtqueues); > spin_unlock_irqrestore(&vp_dev->lock, flags); > @@ -284,15 +281,15 @@ void vp_del_vqs(struct virtio_device *vdev) > vp_dev->vqs = NULL; > } > > -static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > - const char * const names[], bool per_vq_vectors, > - const bool *ctx, > - struct irq_affinity *desc) > +static int vp_find_vqs_msix(struct virtio_device *vdev, > + struct virtio_vq_config *cfg, > + bool per_vq_vectors) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > u16 msix_vec; > int i, err, nvectors, allocated_vectors; > + struct virtqueue **vqs = cfg->vqs; > + unsigned int nvqs = cfg->nvqs; > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > if (!vp_dev->vqs) > @@ -302,7 +299,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > /* Best option: one for change interrupt, one per vq. */ > nvectors = 1; > for (i = 0; i < nvqs; ++i) > - if (callbacks[i]) > + if (cfg->callbacks[i]) > ++nvectors; > } else { > /* Second best: one for change, shared for all vqs. */ > @@ -310,27 +307,26 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > } > > err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors, > - per_vq_vectors ? desc : NULL); > + per_vq_vectors ? cfg->desc : NULL); > if (err) > goto error_find; > > vp_dev->per_vq_vectors = per_vq_vectors; > allocated_vectors = vp_dev->msix_used_vectors; > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > err = -EINVAL; > goto error_find; > } > > - if (!callbacks[i]) > + if (!cfg->callbacks[i]) > msix_vec = VIRTIO_MSI_NO_VECTOR; > else if (vp_dev->per_vq_vectors) > msix_vec = allocated_vectors++; > else > msix_vec = VP_MSIX_VQ_VECTOR; > - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > - ctx ? ctx[i] : false, > - msix_vec); > + > + vqs[i] = vp_setup_vq(vdev, i, cfg, msix_vec); > if (IS_ERR(vqs[i])) { > err = PTR_ERR(vqs[i]); > goto error_find; > @@ -343,7 +339,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > snprintf(vp_dev->msix_names[msix_vec], > sizeof *vp_dev->msix_names, > "%s-%s", > - dev_name(&vp_dev->vdev.dev), names[i]); > + dev_name(&vp_dev->vdev.dev), cfg->names[i]); > err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), > vring_interrupt, 0, > vp_dev->msix_names[msix_vec], > @@ -358,11 +354,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > return err; > } > > -static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > - const char * const names[], const bool *ctx) > +static int vp_find_vqs_intx(struct virtio_device *vdev, struct virtio_vq_config *cfg) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > + struct virtqueue **vqs = cfg->vqs; > + unsigned int nvqs = cfg->nvqs; > int i, err; > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > @@ -377,13 +373,11 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > vp_dev->intx_enabled = 1; > vp_dev->per_vq_vectors = false; > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > err = -EINVAL; > goto out_del_vqs; > } > - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > - ctx ? ctx[i] : false, > - VIRTIO_MSI_NO_VECTOR); > + vqs[i] = vp_setup_vq(vdev, i, cfg, VIRTIO_MSI_NO_VECTOR); > if (IS_ERR(vqs[i])) { > err = PTR_ERR(vqs[i]); > goto out_del_vqs; > @@ -397,26 +391,23 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > } > > /* the config->find_vqs() implementation */ > -int vp_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) > +int vp_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > { > int err; > > /* Try MSI-X with one vector per queue. */ > - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx, desc); > + err = vp_find_vqs_msix(vdev, cfg, true); > if (!err) > return 0; > /* Fallback: MSI-X with one vector for config, one shared for queues. */ > - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx, desc); > + err = vp_find_vqs_msix(vdev, cfg, false); > if (!err) > return 0; > /* Is there an interrupt? If not give up. */ > if (!(to_vp_device(vdev)->pci_dev->irq)) > return err; > /* Finally fall back to regular interrupts. */ > - return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx); > + return vp_find_vqs_intx(vdev, cfg); > } > > const char *vp_bus_name(struct virtio_device *vdev) > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h > index 7fef52bee455..5ba8b82fb765 100644 > --- a/drivers/virtio/virtio_pci_common.h > +++ b/drivers/virtio/virtio_pci_common.h > @@ -95,9 +95,7 @@ struct virtio_pci_device { > struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev, > struct virtio_pci_vq_info *info, > unsigned int idx, > - void (*callback)(struct virtqueue *vq), > - const char *name, > - bool ctx, > + struct virtio_vq_config *vq_cfg, > u16 msix_vec); > void (*del_vq)(struct virtio_pci_vq_info *info); > > @@ -126,10 +124,7 @@ bool vp_notify(struct virtqueue *vq); > /* the config->del_vqs() implementation */ > void vp_del_vqs(struct virtio_device *vdev); > /* the config->find_vqs() implementation */ > -int vp_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); > +int vp_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg); > const char *vp_bus_name(struct virtio_device *vdev); > > /* Setup the affinity for a virtqueue: > diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c > index d9cbb02b35a1..a8de653dd7a7 100644 > --- a/drivers/virtio/virtio_pci_legacy.c > +++ b/drivers/virtio/virtio_pci_legacy.c > @@ -110,9 +110,7 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector) > static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > struct virtio_pci_vq_info *info, > unsigned int index, > - void (*callback)(struct virtqueue *vq), > - const char *name, > - bool ctx, > + struct virtio_vq_config *cfg, > u16 msix_vec) > { > struct virtqueue *vq; > @@ -130,8 +128,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > /* create the vring */ > vq = vring_create_virtqueue(index, num, > VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev, > - true, false, ctx, > - vp_notify, callback, name); > + true, false, > + cfg->ctx ? cfg->ctx[index] : false, > + vp_notify, > + cfg->callbacks[index], > + cfg->names[index]); > if (!vq) > return ERR_PTR(-ENOMEM); > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > index f62b530aa3b5..bcb829ffec64 100644 > --- a/drivers/virtio/virtio_pci_modern.c > +++ b/drivers/virtio/virtio_pci_modern.c > @@ -530,9 +530,7 @@ static bool vp_notify_with_data(struct virtqueue *vq) > static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > struct virtio_pci_vq_info *info, > unsigned int index, > - void (*callback)(struct virtqueue *vq), > - const char *name, > - bool ctx, > + struct virtio_vq_config *cfg, > u16 msix_vec) > { > > @@ -563,8 +561,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > /* create the vring */ > vq = vring_create_virtqueue(index, num, > SMP_CACHE_BYTES, &vp_dev->vdev, > - true, true, ctx, > - notify, callback, name); > + true, true, > + cfg->ctx ? cfg->ctx[index] : false, > + notify, > + cfg->callbacks[index], > + cfg->names[index]); > if (!vq) > return ERR_PTR(-ENOMEM); > > @@ -593,15 +594,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > return ERR_PTR(err); > } > > -static int vp_modern_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 vp_modern_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > struct virtqueue *vq; > - int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc); > + int rc = vp_find_vqs(vdev, cfg); > > if (rc) > return rc; > @@ -739,10 +736,17 @@ static bool vp_get_shm_region(struct virtio_device *vdev, > static int vp_modern_create_avq(struct virtio_device *vdev) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > + vq_callback_t *callbacks[] = { NULL }; > + struct virtio_vq_config cfg = {}; > struct virtio_pci_admin_vq *avq; > struct virtqueue *vq; > + const char *names[1]; > u16 admin_q_num; > > + cfg.nvqs = 1; > + cfg.callbacks = callbacks; > + cfg.names = names; > + > if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)) > return 0; > init things where you declare them. Named initializers are a thing, too. > @@ -753,8 +757,10 @@ static int vp_modern_create_avq(struct virtio_device *vdev) > avq = &vp_dev->admin_vq; > avq->vq_index = vp_modern_avq_index(&vp_dev->mdev); > sprintf(avq->name, "avq.%u", avq->vq_index); > - vq = vp_dev->setup_vq(vp_dev, &vp_dev->admin_vq.info, avq->vq_index, NULL, > - avq->name, NULL, VIRTIO_MSI_NO_VECTOR); > + > + cfg.names[0] = avq->name; > + vq = vp_dev->setup_vq(vp_dev, &vp_dev->admin_vq.info, avq->vq_index, > + &cfg, VIRTIO_MSI_NO_VECTOR); > if (IS_ERR(vq)) { > dev_err(&vdev->dev, "failed to setup admin virtqueue, err=%ld", > PTR_ERR(vq)); > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > index e82cca24d6e6..6e7aafb42100 100644 > --- a/drivers/virtio/virtio_vdpa.c > +++ b/drivers/virtio/virtio_vdpa.c > @@ -142,8 +142,7 @@ static irqreturn_t virtio_vdpa_virtqueue_cb(void *private) > > static struct virtqueue * > virtio_vdpa_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_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev); > struct vdpa_device *vdpa = vd_get_vdpa(vdev); > @@ -203,8 +202,12 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > else > dma_dev = vdpa_get_dma_dev(vdpa); > vq = vring_create_virtqueue_dma(index, max_num, align, vdev, > - true, may_reduce_num, ctx, > - notify, callback, name, dma_dev); > + true, may_reduce_num, > + cfg->ctx ? cfg->ctx[index] : false, > + notify, > + cfg->callbacks[index], > + cfg->names[index], > + dma_dev); > if (!vq) { > err = -ENOMEM; > goto error_new_virtqueue; > @@ -213,7 +216,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > vq->num_max = max_num; > > /* Setup virtqueue callback */ > - cb.callback = callback ? virtio_vdpa_virtqueue_cb : NULL; > + cb.callback = cfg->callbacks[index] ? virtio_vdpa_virtqueue_cb : NULL; > cb.private = info; > cb.trigger = NULL; > ops->set_vq_cb(vdpa, index, &cb); > @@ -353,12 +356,8 @@ create_affinity_masks(unsigned int nvecs, struct irq_affinity *affd) > return masks; > } > > -static int virtio_vdpa_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 virtio_vdpa_find_vqs(struct virtio_device *vdev, > + struct virtio_vq_config *cfg) > { > struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev); > struct vdpa_device *vdpa = vd_get_vdpa(vdev); > @@ -366,24 +365,24 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > struct irq_affinity default_affd = { 0 }; > struct cpumask *masks; > struct vdpa_callback cb; > - bool has_affinity = desc && ops->set_vq_affinity; > + bool has_affinity = cfg->desc && ops->set_vq_affinity; > + struct virtqueue **vqs = cfg->vqs; > + unsigned int nvqs = cfg->nvqs; > int i, err; > > if (has_affinity) { > - masks = create_affinity_masks(nvqs, desc ? desc : &default_affd); > + masks = create_affinity_masks(nvqs, cfg->desc ? cfg->desc : &default_affd); > if (!masks) > return -ENOMEM; > } > > for (i = 0; i < nvqs; ++i) { > - if (!names[i]) { > + if (!cfg->names[i]) { > err = -EINVAL; > goto err_setup_vq; > } > > - vqs[i] = virtio_vdpa_setup_vq(vdev, i, > - callbacks[i], names[i], ctx ? > - ctx[i] : false); > + vqs[i] = virtio_vdpa_setup_vq(vdev, i, cfg); > if (IS_ERR(vqs[i])) { > err = PTR_ERR(vqs[i]); > goto err_setup_vq; > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > index 1c79cec258f4..370e79df50c4 100644 > --- a/include/linux/virtio_config.h > +++ b/include/linux/virtio_config.h > @@ -18,6 +18,29 @@ struct virtio_shm_region { > > typedef void vq_callback_t(struct virtqueue *); > > +/** > + * struct virtio_vq_config - configure for find_vqs() configure -> configuration > + * @nvqs: the number of virtqueues to find > + * @vqs: on success, includes new virtqueues > + * @callbacks: array of callbacks, for each virtqueue > + * include a NULL entry for vqs that do not need a callback > + * @names: array of virtqueue names (mainly for debugging) > + * MUST NOT be NULL > + * @ctx: (optional) array of context. must be a plural. E.g. of context pointers > If the value of the vq in the array > + * is true, the driver can pass ctx to virtio core when adding bufs to > + * virtqueue. > + * @desc: desc for interrupts does not really describe it. > + */ > +struct virtio_vq_config { > + unsigned int nvqs; > + > + struct virtqueue **vqs; > + vq_callback_t **callbacks; > + const char **names; > + const bool *ctx; > + struct irq_affinity *desc; > +}; > + > /** > * struct virtio_config_ops - operations for configuring a virtio device > * Note: Do not assume that a transport implements all of the operations > @@ -51,12 +74,7 @@ typedef void vq_callback_t(struct virtqueue *); > * parallel with being added/removed. > * @find_vqs: find virtqueues and instantiate them. > * vdev: the virtio_device > - * nvqs: the number of virtqueues to find > - * vqs: on success, includes new virtqueues > - * callbacks: array of callbacks, for each virtqueue > - * include a NULL entry for vqs that do not need a callback > - * names: array of virtqueue names (mainly for debugging) > - * MUST NOT be NULL > + * cfg: the config from the driver > * Returns 0 on success or error status > * @del_vqs: free virtqueues found by find_vqs(). > * @synchronize_cbs: synchronize with the virtqueue callbacks (optional) > @@ -96,6 +114,7 @@ typedef void vq_callback_t(struct virtqueue *); > * @create_avq: create admin virtqueue resource. > * @destroy_avq: destroy admin virtqueue resource. > */ > + > struct virtio_config_ops { > void (*get)(struct virtio_device *vdev, unsigned offset, > void *buf, unsigned len); > @@ -105,10 +124,7 @@ struct virtio_config_ops { > u8 (*get_status)(struct virtio_device *vdev); > void (*set_status)(struct virtio_device *vdev, u8 status); > void (*reset)(struct virtio_device *vdev); > - int (*find_vqs)(struct virtio_device *, unsigned nvqs, > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > - const char * const names[], const bool *ctx, > - struct irq_affinity *desc); > + int (*find_vqs)(struct virtio_device *vdev, struct virtio_vq_config *cfg); > void (*del_vqs)(struct virtio_device *); > void (*synchronize_cbs)(struct virtio_device *); > u64 (*get_features)(struct virtio_device *vdev); > @@ -217,8 +233,14 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > vq_callback_t *callbacks[] = { c }; > const char *names[] = { n }; > struct virtqueue *vq; > - int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names, NULL, > - NULL); > + struct virtio_vq_config cfg = {}; > + > + cfg.nvqs = 1; > + cfg.vqs = &vq; > + cfg.callbacks = callbacks; > + cfg.names = names; > + > + int err = vdev->config->find_vqs(vdev, &cfg); > if (err < 0) > return ERR_PTR(err); > return vq; > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > static inline > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > - const char * const names[], > - struct irq_affinity *desc) > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > + const char * const names[], > + struct irq_affinity *desc) > { > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > + struct virtio_vq_config cfg = {}; > + > + cfg.nvqs = nvqs; > + cfg.vqs = vqs; > + cfg.callbacks = callbacks; > + cfg.names = (const char **)names; Casting const away? Not safe. > + cfg.desc = desc; > + > + return vdev->config->find_vqs(vdev, &cfg); > } > > static inline > int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs, > struct virtqueue *vqs[], vq_callback_t *callbacks[], > - const char * const names[], const bool *ctx, > + const char *names[], const bool *ctx, > struct irq_affinity *desc) > { > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, > - desc); > + struct virtio_vq_config cfg = {}; > + > + cfg.nvqs = nvqs; > + cfg.vqs = vqs; > + cfg.callbacks = callbacks; > + cfg.names = names; > + cfg.ctx = ctx; > + cfg.desc = desc; > + The fields should be set up with named initializers, insidef {} > + return vdev->config->find_vqs(vdev, &cfg); > } > > /** > -- > 2.32.0.3.g01195cf9f
On Thu, 20 Jun 2024 03:56:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Apr 24, 2024 at 05:15:30PM +0800, Xuan Zhuo 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@linux.alibaba.com> > > Acked-by: Johannes Berg <johannes@sipsolutions.net> > > Reviewed-by: Ilpo J鋜vinen <ilpo.jarvinen@linux.intel.com> > > Acked-by: Jason Wang <jasowang@redhat.com> > > Acked-by: Eric Farman <farman@linux.ibm.com> # s390 > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > > --- > > 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 | 23 ++++--- > > 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(+), 154 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 7f58634fcc41..bbde11287f8a 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 6cdd29952bc0..4d94d20b253a 100644 > > --- a/drivers/s390/virtio/virtio_ccw.c > > +++ b/drivers/s390/virtio/virtio_ccw.c > > @@ -536,9 +536,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); > > @@ -576,8 +575,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. */ > > @@ -687,14 +689,12 @@ 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 int nvqs = cfg->nvqs; > > dma64_t *indicatorp = NULL; > > int ret, i; > > struct ccw1 *ccw; > > @@ -704,14 +704,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 c3c8dd282952..4ebb28b6b0ec 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); > > @@ -411,7 +410,11 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > > > > /* Create the vring */ > > vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev, > > - true, true, ctx, notify, callback, name); > > + true, true, > > + cfg->ctx ? cfg->ctx[index] : false, > > + notify, > > + cfg->callbacks[index], > > + cfg->names[index]); > > if (!vq) { > > err = -ENOMEM; > > goto error_new_virtqueue; > > @@ -484,15 +487,12 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in > > return ERR_PTR(err); > > } > > > > -static int vm_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 vm_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > > { > > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > > int irq = platform_get_irq(vm_dev->pdev, 0); > > + struct virtqueue **vqs = cfg->vqs; > > + unsigned int nvqs = cfg->nvqs; > > int i, err; > > > > if (irq < 0) > > @@ -507,13 +507,12 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > enable_irq_wake(irq); > > > > for (i = 0; i < nvqs; ++i) { > > - if (!names[i]) { > > + if (!cfg->names[i]) { > > vm_del_vqs(vdev); > > return -EINVAL; > > } > > > > - vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i], > > - ctx ? ctx[i] : false); > > + vqs[i] = vm_setup_vq(vdev, i, cfg); > > if (IS_ERR(vqs[i])) { > > vm_del_vqs(vdev); > > return PTR_ERR(vqs[i]); > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > > index eda71c6e87ee..cb2776e3d0e1 100644 > > --- a/drivers/virtio/virtio_pci_common.c > > +++ b/drivers/virtio/virtio_pci_common.c > > @@ -172,9 +172,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, > > } > > > > static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int index, > > - void (*callback)(struct virtqueue *vq), > > - const char *name, > > - bool ctx, > > + struct virtio_vq_config *cfg, > > u16 msix_vec) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > @@ -186,13 +184,12 @@ static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int in > > if (!info) > > return ERR_PTR(-ENOMEM); > > > > - vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, ctx, > > - msix_vec); > > + vq = vp_dev->setup_vq(vp_dev, info, index, cfg, msix_vec); > > if (IS_ERR(vq)) > > goto out_info; > > > > info->vq = vq; > > - if (callback) { > > + if (cfg->callbacks[index]) { > > spin_lock_irqsave(&vp_dev->lock, flags); > > list_add(&info->node, &vp_dev->virtqueues); > > spin_unlock_irqrestore(&vp_dev->lock, flags); > > @@ -284,15 +281,15 @@ void vp_del_vqs(struct virtio_device *vdev) > > vp_dev->vqs = NULL; > > } > > > > -static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > - const char * const names[], bool per_vq_vectors, > > - const bool *ctx, > > - struct irq_affinity *desc) > > +static int vp_find_vqs_msix(struct virtio_device *vdev, > > + struct virtio_vq_config *cfg, > > + bool per_vq_vectors) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > u16 msix_vec; > > int i, err, nvectors, allocated_vectors; > > + struct virtqueue **vqs = cfg->vqs; > > + unsigned int nvqs = cfg->nvqs; > > > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > > if (!vp_dev->vqs) > > @@ -302,7 +299,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > /* Best option: one for change interrupt, one per vq. */ > > nvectors = 1; > > for (i = 0; i < nvqs; ++i) > > - if (callbacks[i]) > > + if (cfg->callbacks[i]) > > ++nvectors; > > } else { > > /* Second best: one for change, shared for all vqs. */ > > @@ -310,27 +307,26 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > } > > > > err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors, > > - per_vq_vectors ? desc : NULL); > > + per_vq_vectors ? cfg->desc : NULL); > > if (err) > > goto error_find; > > > > vp_dev->per_vq_vectors = per_vq_vectors; > > allocated_vectors = vp_dev->msix_used_vectors; > > for (i = 0; i < nvqs; ++i) { > > - if (!names[i]) { > > + if (!cfg->names[i]) { > > err = -EINVAL; > > goto error_find; > > } > > > > - if (!callbacks[i]) > > + if (!cfg->callbacks[i]) > > msix_vec = VIRTIO_MSI_NO_VECTOR; > > else if (vp_dev->per_vq_vectors) > > msix_vec = allocated_vectors++; > > else > > msix_vec = VP_MSIX_VQ_VECTOR; > > - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > > - ctx ? ctx[i] : false, > > - msix_vec); > > + > > + vqs[i] = vp_setup_vq(vdev, i, cfg, msix_vec); > > if (IS_ERR(vqs[i])) { > > err = PTR_ERR(vqs[i]); > > goto error_find; > > @@ -343,7 +339,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > snprintf(vp_dev->msix_names[msix_vec], > > sizeof *vp_dev->msix_names, > > "%s-%s", > > - dev_name(&vp_dev->vdev.dev), names[i]); > > + dev_name(&vp_dev->vdev.dev), cfg->names[i]); > > err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), > > vring_interrupt, 0, > > vp_dev->msix_names[msix_vec], > > @@ -358,11 +354,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, > > return err; > > } > > > > -static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > - const char * const names[], const bool *ctx) > > +static int vp_find_vqs_intx(struct virtio_device *vdev, struct virtio_vq_config *cfg) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > + struct virtqueue **vqs = cfg->vqs; > > + unsigned int nvqs = cfg->nvqs; > > int i, err; > > > > vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); > > @@ -377,13 +373,11 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > > vp_dev->intx_enabled = 1; > > vp_dev->per_vq_vectors = false; > > for (i = 0; i < nvqs; ++i) { > > - if (!names[i]) { > > + if (!cfg->names[i]) { > > err = -EINVAL; > > goto out_del_vqs; > > } > > - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], > > - ctx ? ctx[i] : false, > > - VIRTIO_MSI_NO_VECTOR); > > + vqs[i] = vp_setup_vq(vdev, i, cfg, VIRTIO_MSI_NO_VECTOR); > > if (IS_ERR(vqs[i])) { > > err = PTR_ERR(vqs[i]); > > goto out_del_vqs; > > @@ -397,26 +391,23 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, > > } > > > > /* the config->find_vqs() implementation */ > > -int vp_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) > > +int vp_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > > { > > int err; > > > > /* Try MSI-X with one vector per queue. */ > > - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx, desc); > > + err = vp_find_vqs_msix(vdev, cfg, true); > > if (!err) > > return 0; > > /* Fallback: MSI-X with one vector for config, one shared for queues. */ > > - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx, desc); > > + err = vp_find_vqs_msix(vdev, cfg, false); > > if (!err) > > return 0; > > /* Is there an interrupt? If not give up. */ > > if (!(to_vp_device(vdev)->pci_dev->irq)) > > return err; > > /* Finally fall back to regular interrupts. */ > > - return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx); > > + return vp_find_vqs_intx(vdev, cfg); > > } > > > > const char *vp_bus_name(struct virtio_device *vdev) > > diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h > > index 7fef52bee455..5ba8b82fb765 100644 > > --- a/drivers/virtio/virtio_pci_common.h > > +++ b/drivers/virtio/virtio_pci_common.h > > @@ -95,9 +95,7 @@ struct virtio_pci_device { > > struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev, > > struct virtio_pci_vq_info *info, > > unsigned int idx, > > - void (*callback)(struct virtqueue *vq), > > - const char *name, > > - bool ctx, > > + struct virtio_vq_config *vq_cfg, > > u16 msix_vec); > > void (*del_vq)(struct virtio_pci_vq_info *info); > > > > @@ -126,10 +124,7 @@ bool vp_notify(struct virtqueue *vq); > > /* the config->del_vqs() implementation */ > > void vp_del_vqs(struct virtio_device *vdev); > > /* the config->find_vqs() implementation */ > > -int vp_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); > > +int vp_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg); > > const char *vp_bus_name(struct virtio_device *vdev); > > > > /* Setup the affinity for a virtqueue: > > diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c > > index d9cbb02b35a1..a8de653dd7a7 100644 > > --- a/drivers/virtio/virtio_pci_legacy.c > > +++ b/drivers/virtio/virtio_pci_legacy.c > > @@ -110,9 +110,7 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector) > > static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > > struct virtio_pci_vq_info *info, > > unsigned int index, > > - void (*callback)(struct virtqueue *vq), > > - const char *name, > > - bool ctx, > > + struct virtio_vq_config *cfg, > > u16 msix_vec) > > { > > struct virtqueue *vq; > > @@ -130,8 +128,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > > /* create the vring */ > > vq = vring_create_virtqueue(index, num, > > VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev, > > - true, false, ctx, > > - vp_notify, callback, name); > > + true, false, > > + cfg->ctx ? cfg->ctx[index] : false, > > + vp_notify, > > + cfg->callbacks[index], > > + cfg->names[index]); > > if (!vq) > > return ERR_PTR(-ENOMEM); > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > > index f62b530aa3b5..bcb829ffec64 100644 > > --- a/drivers/virtio/virtio_pci_modern.c > > +++ b/drivers/virtio/virtio_pci_modern.c > > @@ -530,9 +530,7 @@ static bool vp_notify_with_data(struct virtqueue *vq) > > static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > > struct virtio_pci_vq_info *info, > > unsigned int index, > > - void (*callback)(struct virtqueue *vq), > > - const char *name, > > - bool ctx, > > + struct virtio_vq_config *cfg, > > u16 msix_vec) > > { > > > > @@ -563,8 +561,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > > /* create the vring */ > > vq = vring_create_virtqueue(index, num, > > SMP_CACHE_BYTES, &vp_dev->vdev, > > - true, true, ctx, > > - notify, callback, name); > > + true, true, > > + cfg->ctx ? cfg->ctx[index] : false, > > + notify, > > + cfg->callbacks[index], > > + cfg->names[index]); > > if (!vq) > > return ERR_PTR(-ENOMEM); > > > > @@ -593,15 +594,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, > > return ERR_PTR(err); > > } > > > > -static int vp_modern_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 vp_modern_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > struct virtqueue *vq; > > - int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc); > > + int rc = vp_find_vqs(vdev, cfg); > > > > if (rc) > > return rc; > > @@ -739,10 +736,17 @@ static bool vp_get_shm_region(struct virtio_device *vdev, > > static int vp_modern_create_avq(struct virtio_device *vdev) > > { > > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > + vq_callback_t *callbacks[] = { NULL }; > > + struct virtio_vq_config cfg = {}; > > struct virtio_pci_admin_vq *avq; > > struct virtqueue *vq; > > + const char *names[1]; > > u16 admin_q_num; > > > > + cfg.nvqs = 1; > > + cfg.callbacks = callbacks; > > + cfg.names = names; > > + > > if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)) > > return 0; > > > > init things where you declare them. Named initializers are a thing, too. > > > > @@ -753,8 +757,10 @@ static int vp_modern_create_avq(struct virtio_device *vdev) > > avq = &vp_dev->admin_vq; > > avq->vq_index = vp_modern_avq_index(&vp_dev->mdev); > > sprintf(avq->name, "avq.%u", avq->vq_index); > > - vq = vp_dev->setup_vq(vp_dev, &vp_dev->admin_vq.info, avq->vq_index, NULL, > > - avq->name, NULL, VIRTIO_MSI_NO_VECTOR); > > + > > + cfg.names[0] = avq->name; > > + vq = vp_dev->setup_vq(vp_dev, &vp_dev->admin_vq.info, avq->vq_index, > > + &cfg, VIRTIO_MSI_NO_VECTOR); > > if (IS_ERR(vq)) { > > dev_err(&vdev->dev, "failed to setup admin virtqueue, err=%ld", > > PTR_ERR(vq)); > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > > index e82cca24d6e6..6e7aafb42100 100644 > > --- a/drivers/virtio/virtio_vdpa.c > > +++ b/drivers/virtio/virtio_vdpa.c > > @@ -142,8 +142,7 @@ static irqreturn_t virtio_vdpa_virtqueue_cb(void *private) > > > > static struct virtqueue * > > virtio_vdpa_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_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev); > > struct vdpa_device *vdpa = vd_get_vdpa(vdev); > > @@ -203,8 +202,12 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > > else > > dma_dev = vdpa_get_dma_dev(vdpa); > > vq = vring_create_virtqueue_dma(index, max_num, align, vdev, > > - true, may_reduce_num, ctx, > > - notify, callback, name, dma_dev); > > + true, may_reduce_num, > > + cfg->ctx ? cfg->ctx[index] : false, > > + notify, > > + cfg->callbacks[index], > > + cfg->names[index], > > + dma_dev); > > if (!vq) { > > err = -ENOMEM; > > goto error_new_virtqueue; > > @@ -213,7 +216,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, > > vq->num_max = max_num; > > > > /* Setup virtqueue callback */ > > - cb.callback = callback ? virtio_vdpa_virtqueue_cb : NULL; > > + cb.callback = cfg->callbacks[index] ? virtio_vdpa_virtqueue_cb : NULL; > > cb.private = info; > > cb.trigger = NULL; > > ops->set_vq_cb(vdpa, index, &cb); > > @@ -353,12 +356,8 @@ create_affinity_masks(unsigned int nvecs, struct irq_affinity *affd) > > return masks; > > } > > > > -static int virtio_vdpa_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 virtio_vdpa_find_vqs(struct virtio_device *vdev, > > + struct virtio_vq_config *cfg) > > { > > struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev); > > struct vdpa_device *vdpa = vd_get_vdpa(vdev); > > @@ -366,24 +365,24 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, > > struct irq_affinity default_affd = { 0 }; > > struct cpumask *masks; > > struct vdpa_callback cb; > > - bool has_affinity = desc && ops->set_vq_affinity; > > + bool has_affinity = cfg->desc && ops->set_vq_affinity; > > + struct virtqueue **vqs = cfg->vqs; > > + unsigned int nvqs = cfg->nvqs; > > int i, err; > > > > if (has_affinity) { > > - masks = create_affinity_masks(nvqs, desc ? desc : &default_affd); > > + masks = create_affinity_masks(nvqs, cfg->desc ? cfg->desc : &default_affd); > > if (!masks) > > return -ENOMEM; > > } > > > > for (i = 0; i < nvqs; ++i) { > > - if (!names[i]) { > > + if (!cfg->names[i]) { > > err = -EINVAL; > > goto err_setup_vq; > > } > > > > - vqs[i] = virtio_vdpa_setup_vq(vdev, i, > > - callbacks[i], names[i], ctx ? > > - ctx[i] : false); > > + vqs[i] = virtio_vdpa_setup_vq(vdev, i, cfg); > > if (IS_ERR(vqs[i])) { > > err = PTR_ERR(vqs[i]); > > goto err_setup_vq; > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > > index 1c79cec258f4..370e79df50c4 100644 > > --- a/include/linux/virtio_config.h > > +++ b/include/linux/virtio_config.h > > @@ -18,6 +18,29 @@ struct virtio_shm_region { > > > > typedef void vq_callback_t(struct virtqueue *); > > > > +/** > > + * struct virtio_vq_config - configure for find_vqs() > > configure -> configuration > > > > + * @nvqs: the number of virtqueues to find > > + * @vqs: on success, includes new virtqueues > > + * @callbacks: array of callbacks, for each virtqueue > > + * include a NULL entry for vqs that do not need a callback > > + * @names: array of virtqueue names (mainly for debugging) > > + * MUST NOT be NULL > > + * @ctx: (optional) array of context. > > must be a plural. E.g. > > of context pointers > > > If the value of the vq in the array > > + * is true, the driver can pass ctx to virtio core when adding bufs to > > + * virtqueue. > > + * @desc: desc for interrupts > > does not really describe it. > > > + */ > > +struct virtio_vq_config { > > + unsigned int nvqs; > > + > > + struct virtqueue **vqs; > > + vq_callback_t **callbacks; > > + const char **names; > > + const bool *ctx; > > + struct irq_affinity *desc; > > +}; > > + > > /** > > * struct virtio_config_ops - operations for configuring a virtio device > > * Note: Do not assume that a transport implements all of the operations > > @@ -51,12 +74,7 @@ typedef void vq_callback_t(struct virtqueue *); > > * parallel with being added/removed. > > * @find_vqs: find virtqueues and instantiate them. > > * vdev: the virtio_device > > - * nvqs: the number of virtqueues to find > > - * vqs: on success, includes new virtqueues > > - * callbacks: array of callbacks, for each virtqueue > > - * include a NULL entry for vqs that do not need a callback > > - * names: array of virtqueue names (mainly for debugging) > > - * MUST NOT be NULL > > + * cfg: the config from the driver > > * Returns 0 on success or error status > > * @del_vqs: free virtqueues found by find_vqs(). > > * @synchronize_cbs: synchronize with the virtqueue callbacks (optional) > > @@ -96,6 +114,7 @@ typedef void vq_callback_t(struct virtqueue *); > > * @create_avq: create admin virtqueue resource. > > * @destroy_avq: destroy admin virtqueue resource. > > */ > > + > > struct virtio_config_ops { > > void (*get)(struct virtio_device *vdev, unsigned offset, > > void *buf, unsigned len); > > @@ -105,10 +124,7 @@ struct virtio_config_ops { > > u8 (*get_status)(struct virtio_device *vdev); > > void (*set_status)(struct virtio_device *vdev, u8 status); > > void (*reset)(struct virtio_device *vdev); > > - int (*find_vqs)(struct virtio_device *, unsigned nvqs, > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > - const char * const names[], const bool *ctx, > > - struct irq_affinity *desc); > > + int (*find_vqs)(struct virtio_device *vdev, struct virtio_vq_config *cfg); > > void (*del_vqs)(struct virtio_device *); > > void (*synchronize_cbs)(struct virtio_device *); > > u64 (*get_features)(struct virtio_device *vdev); > > @@ -217,8 +233,14 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > vq_callback_t *callbacks[] = { c }; > > const char *names[] = { n }; > > struct virtqueue *vq; > > - int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names, NULL, > > - NULL); > > + struct virtio_vq_config cfg = {}; > > + > > + cfg.nvqs = 1; > > + cfg.vqs = &vq; > > + cfg.callbacks = callbacks; > > + cfg.names = names; > > + > > + int err = vdev->config->find_vqs(vdev, &cfg); > > if (err < 0) > > return ERR_PTR(err); > > return vq; > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > static inline > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > - const char * const names[], > > - struct irq_affinity *desc) > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > + const char * const names[], > > + struct irq_affinity *desc) > > { > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > + struct virtio_vq_config cfg = {}; > > + > > + cfg.nvqs = nvqs; > > + cfg.vqs = vqs; > > + cfg.callbacks = callbacks; > > + cfg.names = (const char **)names; > > > Casting const away? Not safe. 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". Other comments will be fix in next version. Thanks. > > > + cfg.desc = desc; > > + > > + return vdev->config->find_vqs(vdev, &cfg); > > } > > > > static inline > > int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs, > > struct virtqueue *vqs[], vq_callback_t *callbacks[], > > - const char * const names[], const bool *ctx, > > + const char *names[], const bool *ctx, > > struct irq_affinity *desc) > > { > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, > > - desc); > > + struct virtio_vq_config cfg = {}; > > + > > + cfg.nvqs = nvqs; > > + cfg.vqs = vqs; > > + cfg.callbacks = callbacks; > > + cfg.names = names; > > + cfg.ctx = ctx; > > + cfg.desc = desc; > > + > > The fields should be set up with named initializers, insidef {} > > > + return vdev->config->find_vqs(vdev, &cfg); > > } > > > > /** > > -- > > 2.32.0.3.g01195cf9f >
On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > static inline > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > - const char * const names[], > > > - struct irq_affinity *desc) > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > + const char * const names[], > > > + struct irq_affinity *desc) > > > { > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > + struct virtio_vq_config cfg = {}; > > > + > > > + cfg.nvqs = nvqs; > > > + cfg.vqs = vqs; > > > + cfg.callbacks = callbacks; > > > + cfg.names = (const char **)names; > > > > > > Casting const away? Not safe. > > > > 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". I'm not sure I understand which commit you mean, and this kind of change needs to be documented, but it does not matter. Don't cast away const.
On Thu, 20 Jun 2024 05:14:24 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > > > static inline > > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > - const char * const names[], > > > > - struct irq_affinity *desc) > > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > + const char * const names[], > > > > + struct irq_affinity *desc) > > > > { > > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > > + struct virtio_vq_config cfg = {}; > > > > + > > > > + cfg.nvqs = nvqs; > > > > + cfg.vqs = vqs; > > > > + cfg.callbacks = callbacks; > > > > + cfg.names = (const char **)names; > > > > > > > > > Casting const away? Not safe. > > > > > > > > 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". > > I'm not sure I understand which commit you mean, > and this kind of change needs to be documented, but it does not matter. > Don't cast away const. Do you mean change the virtio_find_vqs(), from const char * const names[] to const char *names[]. And update the caller? If we do not cast the const, we need to update all the caller to remove the const. Right? Thanks. > > -- > MST >
On Thu, Jun 20, 2024 at 05:20:49PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 05:14:24 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > > > > > static inline > > > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > - const char * const names[], > > > > > - struct irq_affinity *desc) > > > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > + const char * const names[], > > > > > + struct irq_affinity *desc) > > > > > { > > > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > > > + struct virtio_vq_config cfg = {}; > > > > > + > > > > > + cfg.nvqs = nvqs; > > > > > + cfg.vqs = vqs; > > > > > + cfg.callbacks = callbacks; > > > > > + cfg.names = (const char **)names; > > > > > > > > > > > > Casting const away? Not safe. > > > > > > > > > > > > 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". > > > > I'm not sure I understand which commit you mean, > > and this kind of change needs to be documented, but it does not matter. > > Don't cast away const. > > > Do you mean change the virtio_find_vqs(), from > const char * const names[] to const char *names[]. > > And update the caller? > > If we do not cast the const, we need to update all the caller to remove the > const. > > Right? > > Thanks. Just do not split the patchset at a boundary that makes you do that. If you are passing in an array from a const section then it has to be const and attempts to change it are a bad idea. > > > > -- > > MST > >
On Thu, 20 Jun 2024 06:15:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jun 20, 2024 at 05:20:49PM +0800, Xuan Zhuo wrote: > > On Thu, 20 Jun 2024 05:14:24 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > > > > > > > static inline > > > > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > - const char * const names[], > > > > > > - struct irq_affinity *desc) > > > > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > + const char * const names[], > > > > > > + struct irq_affinity *desc) > > > > > > { > > > > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > > > > + struct virtio_vq_config cfg = {}; > > > > > > + > > > > > > + cfg.nvqs = nvqs; > > > > > > + cfg.vqs = vqs; > > > > > > + cfg.callbacks = callbacks; > > > > > > + cfg.names = (const char **)names; > > > > > > > > > > > > > > > Casting const away? Not safe. > > > > > > > > > > > > > > > > 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". > > > > > > I'm not sure I understand which commit you mean, > > > and this kind of change needs to be documented, but it does not matter. > > > Don't cast away const. > > > > > > Do you mean change the virtio_find_vqs(), from > > const char * const names[] to const char *names[]. > > > > And update the caller? > > > > If we do not cast the const, we need to update all the caller to remove the > > const. > > > > Right? > > > > Thanks. > > > Just do not split the patchset at a boundary that makes you do that. > If you are passing in an array from a const section then it > has to be const and attempts to change it are a bad idea. Without this patch set: static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, unsigned index, vq_callback_t *callback, const char *name, bool ctx) { struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); struct platform_device *pdev = vu_dev->pdev; struct virtio_uml_vq_info *info; struct virtqueue *vq; int num = MAX_SUPPORTED_QUEUE_SIZE; int rc; info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) { rc = -ENOMEM; goto error_kzalloc; } -> snprintf(info->name, sizeof(info->name), "%s.%d-%s", pdev->name, pdev->id, name); vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true, ctx, vu_notify, callback, info->name); The name is changed by vu_setup_vq(). If we want to pass names to virtio ring, the names must not be "const char * const" And the admin queue of pci do the same thing. And I think you are right, we should not cast the const. So we have to remove the "const" from the source. And I checked the source code, if we remove the "const", I think that makes sense. Thanks. > > > > > > > > -- > > > MST > > > >
On Thu, Jun 20, 2024 at 06:43:30PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 06:15:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Thu, Jun 20, 2024 at 05:20:49PM +0800, Xuan Zhuo wrote: > > > On Thu, 20 Jun 2024 05:14:24 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > > > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > > > > > > > > > static inline > > > > > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > > - const char * const names[], > > > > > > > - struct irq_affinity *desc) > > > > > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > > + const char * const names[], > > > > > > > + struct irq_affinity *desc) > > > > > > > { > > > > > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > > > > > + struct virtio_vq_config cfg = {}; > > > > > > > + > > > > > > > + cfg.nvqs = nvqs; > > > > > > > + cfg.vqs = vqs; > > > > > > > + cfg.callbacks = callbacks; > > > > > > > + cfg.names = (const char **)names; > > > > > > > > > > > > > > > > > > Casting const away? Not safe. > > > > > > > > > > > > > > > > > > > > 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". > > > > > > > > I'm not sure I understand which commit you mean, > > > > and this kind of change needs to be documented, but it does not matter. > > > > Don't cast away const. > > > > > > > > > Do you mean change the virtio_find_vqs(), from > > > const char * const names[] to const char *names[]. > > > > > > And update the caller? > > > > > > If we do not cast the const, we need to update all the caller to remove the > > > const. > > > > > > Right? > > > > > > Thanks. > > > > > > Just do not split the patchset at a boundary that makes you do that. > > If you are passing in an array from a const section then it > > has to be const and attempts to change it are a bad idea. > > Without this patch set: > > static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, > unsigned index, vq_callback_t *callback, > const char *name, bool ctx) > { > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > struct platform_device *pdev = vu_dev->pdev; > struct virtio_uml_vq_info *info; > struct virtqueue *vq; > int num = MAX_SUPPORTED_QUEUE_SIZE; > int rc; > > info = kzalloc(sizeof(*info), GFP_KERNEL); > if (!info) { > rc = -ENOMEM; > goto error_kzalloc; > } > -> snprintf(info->name, sizeof(info->name), "%s.%d-%s", pdev->name, > pdev->id, name); > > vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true, > ctx, vu_notify, callback, info->name); > > > The name is changed by vu_setup_vq(). > If we want to pass names to > virtio ring, the names must not be "const char * const" > > And the admin queue of pci do the same thing. > > And I think you are right, we should not cast the const. > So we have to remove the "const" from the source. > And I checked the source code, if we remove the "const", I think > that makes sense. > > Thanks. /facepalm This is a different const. There should be no need to drop the annotation, core does not change these things and using const helps make sure that is the case. > > > > > > > > > > > > > > -- > > > > MST > > > > > >
On Thu, 20 Jun 2024 07:06:53 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jun 20, 2024 at 06:43:30PM +0800, Xuan Zhuo wrote: > > On Thu, 20 Jun 2024 06:15:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Jun 20, 2024 at 05:20:49PM +0800, Xuan Zhuo wrote: > > > > On Thu, 20 Jun 2024 05:14:24 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > > > > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > > > > > > > > > > > static inline > > > > > > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > > > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > > > - const char * const names[], > > > > > > > > - struct irq_affinity *desc) > > > > > > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > > > + const char * const names[], > > > > > > > > + struct irq_affinity *desc) > > > > > > > > { > > > > > > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > > > > > > + struct virtio_vq_config cfg = {}; > > > > > > > > + > > > > > > > > + cfg.nvqs = nvqs; > > > > > > > > + cfg.vqs = vqs; > > > > > > > > + cfg.callbacks = callbacks; > > > > > > > > + cfg.names = (const char **)names; > > > > > > > > > > > > > > > > > > > > > Casting const away? Not safe. > > > > > > > > > > > > > > > > > > > > > > > > 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". > > > > > > > > > > I'm not sure I understand which commit you mean, > > > > > and this kind of change needs to be documented, but it does not matter. > > > > > Don't cast away const. > > > > > > > > > > > > Do you mean change the virtio_find_vqs(), from > > > > const char * const names[] to const char *names[]. > > > > > > > > And update the caller? > > > > > > > > If we do not cast the const, we need to update all the caller to remove the > > > > const. > > > > > > > > Right? > > > > > > > > Thanks. > > > > > > > > > Just do not split the patchset at a boundary that makes you do that. > > > If you are passing in an array from a const section then it > > > has to be const and attempts to change it are a bad idea. > > > > Without this patch set: > > > > static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, > > unsigned index, vq_callback_t *callback, > > const char *name, bool ctx) > > { > > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > > struct platform_device *pdev = vu_dev->pdev; > > struct virtio_uml_vq_info *info; > > struct virtqueue *vq; > > int num = MAX_SUPPORTED_QUEUE_SIZE; > > int rc; > > > > info = kzalloc(sizeof(*info), GFP_KERNEL); > > if (!info) { > > rc = -ENOMEM; > > goto error_kzalloc; > > } > > -> snprintf(info->name, sizeof(info->name), "%s.%d-%s", pdev->name, > > pdev->id, name); > > > > vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true, > > ctx, vu_notify, callback, info->name); > > > > > > The name is changed by vu_setup_vq(). > > If we want to pass names to > > virtio ring, the names must not be "const char * const" > > > > And the admin queue of pci do the same thing. > > > > And I think you are right, we should not cast the const. > > So we have to remove the "const" from the source. > > And I checked the source code, if we remove the "const", I think > > that makes sense. > > > > Thanks. > > /facepalm > > This is a different const. > > > There should be no need to drop the annotation, core > does not change these things and using const helps make > sure that is the case. If you do not like this, the left only way is to allocate new memory to store the info, if the caller do not change. In the further, maybe the caller can use the follow struct directly. struct virtio_vq_config { vq_callback_t callback; const char *name; const bool ctx; }; For now, we can allocate memory to change the arrays (names, callbacks..) to the array of struct virtio_vq_config. And the find_vqs() accepts the array of struct virtio_vq_config. How about this? Thanks. > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > MST > > > > > > > > >
On Thu, Jun 20, 2024 at 07:12:48PM +0800, Xuan Zhuo wrote: > On Thu, 20 Jun 2024 07:06:53 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > On Thu, Jun 20, 2024 at 06:43:30PM +0800, Xuan Zhuo wrote: > > > On Thu, 20 Jun 2024 06:15:08 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > On Thu, Jun 20, 2024 at 05:20:49PM +0800, Xuan Zhuo wrote: > > > > > On Thu, 20 Jun 2024 05:14:24 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > On Thu, Jun 20, 2024 at 05:00:49PM +0800, Xuan Zhuo wrote: > > > > > > > > > @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, > > > > > > > > > > > > > > > > > > static inline > > > > > > > > > int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, > > > > > > > > > - struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > > > > - const char * const names[], > > > > > > > > > - struct irq_affinity *desc) > > > > > > > > > + struct virtqueue *vqs[], vq_callback_t *callbacks[], > > > > > > > > > + const char * const names[], > > > > > > > > > + struct irq_affinity *desc) > > > > > > > > > { > > > > > > > > > - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); > > > > > > > > > + struct virtio_vq_config cfg = {}; > > > > > > > > > + > > > > > > > > > + cfg.nvqs = nvqs; > > > > > > > > > + cfg.vqs = vqs; > > > > > > > > > + cfg.callbacks = callbacks; > > > > > > > > > + cfg.names = (const char **)names; > > > > > > > > > > > > > > > > > > > > > > > > Casting const away? Not safe. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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". > > > > > > > > > > > > I'm not sure I understand which commit you mean, > > > > > > and this kind of change needs to be documented, but it does not matter. > > > > > > Don't cast away const. > > > > > > > > > > > > > > > Do you mean change the virtio_find_vqs(), from > > > > > const char * const names[] to const char *names[]. > > > > > > > > > > And update the caller? > > > > > > > > > > If we do not cast the const, we need to update all the caller to remove the > > > > > const. > > > > > > > > > > Right? > > > > > > > > > > Thanks. > > > > > > > > > > > > Just do not split the patchset at a boundary that makes you do that. > > > > If you are passing in an array from a const section then it > > > > has to be const and attempts to change it are a bad idea. > > > > > > Without this patch set: > > > > > > static struct virtqueue *vu_setup_vq(struct virtio_device *vdev, > > > unsigned index, vq_callback_t *callback, > > > const char *name, bool ctx) > > > { > > > struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev); > > > struct platform_device *pdev = vu_dev->pdev; > > > struct virtio_uml_vq_info *info; > > > struct virtqueue *vq; > > > int num = MAX_SUPPORTED_QUEUE_SIZE; > > > int rc; > > > > > > info = kzalloc(sizeof(*info), GFP_KERNEL); > > > if (!info) { > > > rc = -ENOMEM; > > > goto error_kzalloc; > > > } > > > -> snprintf(info->name, sizeof(info->name), "%s.%d-%s", pdev->name, > > > pdev->id, name); > > > > > > vq = vring_create_virtqueue(index, num, PAGE_SIZE, vdev, true, true, > > > ctx, vu_notify, callback, info->name); > > > > > > > > > The name is changed by vu_setup_vq(). > > > If we want to pass names to > > > virtio ring, the names must not be "const char * const" > > > > > > And the admin queue of pci do the same thing. > > > > > > And I think you are right, we should not cast the const. > > > So we have to remove the "const" from the source. > > > And I checked the source code, if we remove the "const", I think > > > that makes sense. > > > > > > Thanks. > > > > /facepalm > > > > This is a different const. > > > > > > There should be no need to drop the annotation, core > > does not change these things and using const helps make > > sure that is the case. > > > If you do not like this, the left only way is to allocate new > memory to store the info, if the caller do not change. > > In the further, maybe the caller can use the follow struct directly. > > struct virtio_vq_config { > vq_callback_t callback; > const char *name; > const bool ctx; > }; > > For now, we can allocate memory to change the arrays (names, callbacks..) > to the array of struct virtio_vq_config. > > And the find_vqs() accepts the array of struct virtio_vq_config. > > How about this? > > Thanks. Basically I am not sure how bad a single big patch would be. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > MST > > > > > > > > > > > >
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 7f58634fcc41..bbde11287f8a 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 6cdd29952bc0..4d94d20b253a 100644 --- a/drivers/s390/virtio/virtio_ccw.c +++ b/drivers/s390/virtio/virtio_ccw.c @@ -536,9 +536,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); @@ -576,8 +575,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. */ @@ -687,14 +689,12 @@ 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 int nvqs = cfg->nvqs; dma64_t *indicatorp = NULL; int ret, i; struct ccw1 *ccw; @@ -704,14 +704,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 c3c8dd282952..4ebb28b6b0ec 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); @@ -411,7 +410,11 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in /* Create the vring */ vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev, - true, true, ctx, notify, callback, name); + true, true, + cfg->ctx ? cfg->ctx[index] : false, + notify, + cfg->callbacks[index], + cfg->names[index]); if (!vq) { err = -ENOMEM; goto error_new_virtqueue; @@ -484,15 +487,12 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in return ERR_PTR(err); } -static int vm_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 vm_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); int irq = platform_get_irq(vm_dev->pdev, 0); + struct virtqueue **vqs = cfg->vqs; + unsigned int nvqs = cfg->nvqs; int i, err; if (irq < 0) @@ -507,13 +507,12 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, enable_irq_wake(irq); for (i = 0; i < nvqs; ++i) { - if (!names[i]) { + if (!cfg->names[i]) { vm_del_vqs(vdev); return -EINVAL; } - vqs[i] = vm_setup_vq(vdev, i, callbacks[i], names[i], - ctx ? ctx[i] : false); + vqs[i] = vm_setup_vq(vdev, i, cfg); if (IS_ERR(vqs[i])) { vm_del_vqs(vdev); return PTR_ERR(vqs[i]); diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c index eda71c6e87ee..cb2776e3d0e1 100644 --- a/drivers/virtio/virtio_pci_common.c +++ b/drivers/virtio/virtio_pci_common.c @@ -172,9 +172,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors, } static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int index, - void (*callback)(struct virtqueue *vq), - const char *name, - bool ctx, + struct virtio_vq_config *cfg, u16 msix_vec) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); @@ -186,13 +184,12 @@ static struct virtqueue *vp_setup_vq(struct virtio_device *vdev, unsigned int in if (!info) return ERR_PTR(-ENOMEM); - vq = vp_dev->setup_vq(vp_dev, info, index, callback, name, ctx, - msix_vec); + vq = vp_dev->setup_vq(vp_dev, info, index, cfg, msix_vec); if (IS_ERR(vq)) goto out_info; info->vq = vq; - if (callback) { + if (cfg->callbacks[index]) { spin_lock_irqsave(&vp_dev->lock, flags); list_add(&info->node, &vp_dev->virtqueues); spin_unlock_irqrestore(&vp_dev->lock, flags); @@ -284,15 +281,15 @@ void vp_del_vqs(struct virtio_device *vdev) vp_dev->vqs = NULL; } -static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, - struct virtqueue *vqs[], vq_callback_t *callbacks[], - const char * const names[], bool per_vq_vectors, - const bool *ctx, - struct irq_affinity *desc) +static int vp_find_vqs_msix(struct virtio_device *vdev, + struct virtio_vq_config *cfg, + bool per_vq_vectors) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); u16 msix_vec; int i, err, nvectors, allocated_vectors; + struct virtqueue **vqs = cfg->vqs; + unsigned int nvqs = cfg->nvqs; vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); if (!vp_dev->vqs) @@ -302,7 +299,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, /* Best option: one for change interrupt, one per vq. */ nvectors = 1; for (i = 0; i < nvqs; ++i) - if (callbacks[i]) + if (cfg->callbacks[i]) ++nvectors; } else { /* Second best: one for change, shared for all vqs. */ @@ -310,27 +307,26 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, } err = vp_request_msix_vectors(vdev, nvectors, per_vq_vectors, - per_vq_vectors ? desc : NULL); + per_vq_vectors ? cfg->desc : NULL); if (err) goto error_find; vp_dev->per_vq_vectors = per_vq_vectors; allocated_vectors = vp_dev->msix_used_vectors; for (i = 0; i < nvqs; ++i) { - if (!names[i]) { + if (!cfg->names[i]) { err = -EINVAL; goto error_find; } - if (!callbacks[i]) + if (!cfg->callbacks[i]) msix_vec = VIRTIO_MSI_NO_VECTOR; else if (vp_dev->per_vq_vectors) msix_vec = allocated_vectors++; else msix_vec = VP_MSIX_VQ_VECTOR; - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], - ctx ? ctx[i] : false, - msix_vec); + + vqs[i] = vp_setup_vq(vdev, i, cfg, msix_vec); if (IS_ERR(vqs[i])) { err = PTR_ERR(vqs[i]); goto error_find; @@ -343,7 +339,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, snprintf(vp_dev->msix_names[msix_vec], sizeof *vp_dev->msix_names, "%s-%s", - dev_name(&vp_dev->vdev.dev), names[i]); + dev_name(&vp_dev->vdev.dev), cfg->names[i]); err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec), vring_interrupt, 0, vp_dev->msix_names[msix_vec], @@ -358,11 +354,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs, return err; } -static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, - struct virtqueue *vqs[], vq_callback_t *callbacks[], - const char * const names[], const bool *ctx) +static int vp_find_vqs_intx(struct virtio_device *vdev, struct virtio_vq_config *cfg) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); + struct virtqueue **vqs = cfg->vqs; + unsigned int nvqs = cfg->nvqs; int i, err; vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL); @@ -377,13 +373,11 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, vp_dev->intx_enabled = 1; vp_dev->per_vq_vectors = false; for (i = 0; i < nvqs; ++i) { - if (!names[i]) { + if (!cfg->names[i]) { err = -EINVAL; goto out_del_vqs; } - vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i], - ctx ? ctx[i] : false, - VIRTIO_MSI_NO_VECTOR); + vqs[i] = vp_setup_vq(vdev, i, cfg, VIRTIO_MSI_NO_VECTOR); if (IS_ERR(vqs[i])) { err = PTR_ERR(vqs[i]); goto out_del_vqs; @@ -397,26 +391,23 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs, } /* the config->find_vqs() implementation */ -int vp_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) +int vp_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) { int err; /* Try MSI-X with one vector per queue. */ - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true, ctx, desc); + err = vp_find_vqs_msix(vdev, cfg, true); if (!err) return 0; /* Fallback: MSI-X with one vector for config, one shared for queues. */ - err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, false, ctx, desc); + err = vp_find_vqs_msix(vdev, cfg, false); if (!err) return 0; /* Is there an interrupt? If not give up. */ if (!(to_vp_device(vdev)->pci_dev->irq)) return err; /* Finally fall back to regular interrupts. */ - return vp_find_vqs_intx(vdev, nvqs, vqs, callbacks, names, ctx); + return vp_find_vqs_intx(vdev, cfg); } const char *vp_bus_name(struct virtio_device *vdev) diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h index 7fef52bee455..5ba8b82fb765 100644 --- a/drivers/virtio/virtio_pci_common.h +++ b/drivers/virtio/virtio_pci_common.h @@ -95,9 +95,7 @@ struct virtio_pci_device { struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev, struct virtio_pci_vq_info *info, unsigned int idx, - void (*callback)(struct virtqueue *vq), - const char *name, - bool ctx, + struct virtio_vq_config *vq_cfg, u16 msix_vec); void (*del_vq)(struct virtio_pci_vq_info *info); @@ -126,10 +124,7 @@ bool vp_notify(struct virtqueue *vq); /* the config->del_vqs() implementation */ void vp_del_vqs(struct virtio_device *vdev); /* the config->find_vqs() implementation */ -int vp_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); +int vp_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg); const char *vp_bus_name(struct virtio_device *vdev); /* Setup the affinity for a virtqueue: diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c index d9cbb02b35a1..a8de653dd7a7 100644 --- a/drivers/virtio/virtio_pci_legacy.c +++ b/drivers/virtio/virtio_pci_legacy.c @@ -110,9 +110,7 @@ static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector) static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, struct virtio_pci_vq_info *info, unsigned int index, - void (*callback)(struct virtqueue *vq), - const char *name, - bool ctx, + struct virtio_vq_config *cfg, u16 msix_vec) { struct virtqueue *vq; @@ -130,8 +128,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, /* create the vring */ vq = vring_create_virtqueue(index, num, VIRTIO_PCI_VRING_ALIGN, &vp_dev->vdev, - true, false, ctx, - vp_notify, callback, name); + true, false, + cfg->ctx ? cfg->ctx[index] : false, + vp_notify, + cfg->callbacks[index], + cfg->names[index]); if (!vq) return ERR_PTR(-ENOMEM); diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index f62b530aa3b5..bcb829ffec64 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -530,9 +530,7 @@ static bool vp_notify_with_data(struct virtqueue *vq) static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, struct virtio_pci_vq_info *info, unsigned int index, - void (*callback)(struct virtqueue *vq), - const char *name, - bool ctx, + struct virtio_vq_config *cfg, u16 msix_vec) { @@ -563,8 +561,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, /* create the vring */ vq = vring_create_virtqueue(index, num, SMP_CACHE_BYTES, &vp_dev->vdev, - true, true, ctx, - notify, callback, name); + true, true, + cfg->ctx ? cfg->ctx[index] : false, + notify, + cfg->callbacks[index], + cfg->names[index]); if (!vq) return ERR_PTR(-ENOMEM); @@ -593,15 +594,11 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev, return ERR_PTR(err); } -static int vp_modern_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 vp_modern_find_vqs(struct virtio_device *vdev, struct virtio_vq_config *cfg) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); struct virtqueue *vq; - int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, desc); + int rc = vp_find_vqs(vdev, cfg); if (rc) return rc; @@ -739,10 +736,17 @@ static bool vp_get_shm_region(struct virtio_device *vdev, static int vp_modern_create_avq(struct virtio_device *vdev) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); + vq_callback_t *callbacks[] = { NULL }; + struct virtio_vq_config cfg = {}; struct virtio_pci_admin_vq *avq; struct virtqueue *vq; + const char *names[1]; u16 admin_q_num; + cfg.nvqs = 1; + cfg.callbacks = callbacks; + cfg.names = names; + if (!virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ)) return 0; @@ -753,8 +757,10 @@ static int vp_modern_create_avq(struct virtio_device *vdev) avq = &vp_dev->admin_vq; avq->vq_index = vp_modern_avq_index(&vp_dev->mdev); sprintf(avq->name, "avq.%u", avq->vq_index); - vq = vp_dev->setup_vq(vp_dev, &vp_dev->admin_vq.info, avq->vq_index, NULL, - avq->name, NULL, VIRTIO_MSI_NO_VECTOR); + + cfg.names[0] = avq->name; + vq = vp_dev->setup_vq(vp_dev, &vp_dev->admin_vq.info, avq->vq_index, + &cfg, VIRTIO_MSI_NO_VECTOR); if (IS_ERR(vq)) { dev_err(&vdev->dev, "failed to setup admin virtqueue, err=%ld", PTR_ERR(vq)); diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c index e82cca24d6e6..6e7aafb42100 100644 --- a/drivers/virtio/virtio_vdpa.c +++ b/drivers/virtio/virtio_vdpa.c @@ -142,8 +142,7 @@ static irqreturn_t virtio_vdpa_virtqueue_cb(void *private) static struct virtqueue * virtio_vdpa_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_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev); struct vdpa_device *vdpa = vd_get_vdpa(vdev); @@ -203,8 +202,12 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, else dma_dev = vdpa_get_dma_dev(vdpa); vq = vring_create_virtqueue_dma(index, max_num, align, vdev, - true, may_reduce_num, ctx, - notify, callback, name, dma_dev); + true, may_reduce_num, + cfg->ctx ? cfg->ctx[index] : false, + notify, + cfg->callbacks[index], + cfg->names[index], + dma_dev); if (!vq) { err = -ENOMEM; goto error_new_virtqueue; @@ -213,7 +216,7 @@ virtio_vdpa_setup_vq(struct virtio_device *vdev, unsigned int index, vq->num_max = max_num; /* Setup virtqueue callback */ - cb.callback = callback ? virtio_vdpa_virtqueue_cb : NULL; + cb.callback = cfg->callbacks[index] ? virtio_vdpa_virtqueue_cb : NULL; cb.private = info; cb.trigger = NULL; ops->set_vq_cb(vdpa, index, &cb); @@ -353,12 +356,8 @@ create_affinity_masks(unsigned int nvecs, struct irq_affinity *affd) return masks; } -static int virtio_vdpa_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 virtio_vdpa_find_vqs(struct virtio_device *vdev, + struct virtio_vq_config *cfg) { struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev); struct vdpa_device *vdpa = vd_get_vdpa(vdev); @@ -366,24 +365,24 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs, struct irq_affinity default_affd = { 0 }; struct cpumask *masks; struct vdpa_callback cb; - bool has_affinity = desc && ops->set_vq_affinity; + bool has_affinity = cfg->desc && ops->set_vq_affinity; + struct virtqueue **vqs = cfg->vqs; + unsigned int nvqs = cfg->nvqs; int i, err; if (has_affinity) { - masks = create_affinity_masks(nvqs, desc ? desc : &default_affd); + masks = create_affinity_masks(nvqs, cfg->desc ? cfg->desc : &default_affd); if (!masks) return -ENOMEM; } for (i = 0; i < nvqs; ++i) { - if (!names[i]) { + if (!cfg->names[i]) { err = -EINVAL; goto err_setup_vq; } - vqs[i] = virtio_vdpa_setup_vq(vdev, i, - callbacks[i], names[i], ctx ? - ctx[i] : false); + vqs[i] = virtio_vdpa_setup_vq(vdev, i, cfg); if (IS_ERR(vqs[i])) { err = PTR_ERR(vqs[i]); goto err_setup_vq; diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index 1c79cec258f4..370e79df50c4 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -18,6 +18,29 @@ struct virtio_shm_region { typedef void vq_callback_t(struct virtqueue *); +/** + * struct virtio_vq_config - configure for find_vqs() + * @nvqs: the number of virtqueues to find + * @vqs: on success, includes new virtqueues + * @callbacks: array of callbacks, for each virtqueue + * include a NULL entry for vqs that do not need a callback + * @names: array of virtqueue names (mainly for debugging) + * MUST NOT be NULL + * @ctx: (optional) array of context. If the value of the vq in the array + * is true, the driver can pass ctx to virtio core when adding bufs to + * virtqueue. + * @desc: desc for interrupts + */ +struct virtio_vq_config { + unsigned int nvqs; + + struct virtqueue **vqs; + vq_callback_t **callbacks; + const char **names; + const bool *ctx; + struct irq_affinity *desc; +}; + /** * struct virtio_config_ops - operations for configuring a virtio device * Note: Do not assume that a transport implements all of the operations @@ -51,12 +74,7 @@ typedef void vq_callback_t(struct virtqueue *); * parallel with being added/removed. * @find_vqs: find virtqueues and instantiate them. * vdev: the virtio_device - * nvqs: the number of virtqueues to find - * vqs: on success, includes new virtqueues - * callbacks: array of callbacks, for each virtqueue - * include a NULL entry for vqs that do not need a callback - * names: array of virtqueue names (mainly for debugging) - * MUST NOT be NULL + * cfg: the config from the driver * Returns 0 on success or error status * @del_vqs: free virtqueues found by find_vqs(). * @synchronize_cbs: synchronize with the virtqueue callbacks (optional) @@ -96,6 +114,7 @@ typedef void vq_callback_t(struct virtqueue *); * @create_avq: create admin virtqueue resource. * @destroy_avq: destroy admin virtqueue resource. */ + struct virtio_config_ops { void (*get)(struct virtio_device *vdev, unsigned offset, void *buf, unsigned len); @@ -105,10 +124,7 @@ struct virtio_config_ops { u8 (*get_status)(struct virtio_device *vdev); void (*set_status)(struct virtio_device *vdev, u8 status); void (*reset)(struct virtio_device *vdev); - int (*find_vqs)(struct virtio_device *, unsigned nvqs, - struct virtqueue *vqs[], vq_callback_t *callbacks[], - const char * const names[], const bool *ctx, - struct irq_affinity *desc); + int (*find_vqs)(struct virtio_device *vdev, struct virtio_vq_config *cfg); void (*del_vqs)(struct virtio_device *); void (*synchronize_cbs)(struct virtio_device *); u64 (*get_features)(struct virtio_device *vdev); @@ -217,8 +233,14 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, vq_callback_t *callbacks[] = { c }; const char *names[] = { n }; struct virtqueue *vq; - int err = vdev->config->find_vqs(vdev, 1, &vq, callbacks, names, NULL, - NULL); + struct virtio_vq_config cfg = {}; + + cfg.nvqs = 1; + cfg.vqs = &vq; + cfg.callbacks = callbacks; + cfg.names = names; + + int err = vdev->config->find_vqs(vdev, &cfg); if (err < 0) return ERR_PTR(err); return vq; @@ -226,21 +248,37 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, static inline int virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs, - struct virtqueue *vqs[], vq_callback_t *callbacks[], - const char * const names[], - struct irq_affinity *desc) + struct virtqueue *vqs[], vq_callback_t *callbacks[], + const char * const names[], + struct irq_affinity *desc) { - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, NULL, desc); + struct virtio_vq_config cfg = {}; + + cfg.nvqs = nvqs; + cfg.vqs = vqs; + cfg.callbacks = callbacks; + cfg.names = (const char **)names; + cfg.desc = desc; + + return vdev->config->find_vqs(vdev, &cfg); } static inline int virtio_find_vqs_ctx(struct virtio_device *vdev, unsigned nvqs, struct virtqueue *vqs[], vq_callback_t *callbacks[], - const char * const names[], const bool *ctx, + const char *names[], const bool *ctx, struct irq_affinity *desc) { - return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, ctx, - desc); + struct virtio_vq_config cfg = {}; + + cfg.nvqs = nvqs; + cfg.vqs = vqs; + cfg.callbacks = callbacks; + cfg.names = names; + cfg.ctx = ctx; + cfg.desc = desc; + + return vdev->config->find_vqs(vdev, &cfg); } /**