On Thu, 14 Mar 2024 11:12:24 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote: > On Tue, Mar 12, 2024 at 10:10 AM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > > > Now, we pass multi parameters to find_vqs. These parameters > > may work for transport or work for vring. > > > > And find_vqs has multi implements in many places: > > > > arch/um/drivers/virtio_uml.c > > drivers/platform/mellanox/mlxbf-tmfifo.c > > drivers/remoteproc/remoteproc_virtio.c > > drivers/s390/virtio/virtio_ccw.c > > drivers/virtio/virtio_mmio.c > > drivers/virtio/virtio_pci_legacy.c > > drivers/virtio/virtio_pci_modern.c > > drivers/virtio/virtio_vdpa.c > > > > Every time, we try to add a new parameter, that is difficult. > > We must change every find_vqs implement. > > > > One the other side, if we want to pass a parameter to vring, > > we must change the call path from transport to vring. > > Too many functions need to be changed. > > > > So it is time to refactor the find_vqs. We pass a structure > > cfg to find_vqs(), that will be passed to vring by transport. > > > > Because the vp_modern_create_avq() use the "const char *names[]", > > and the virtio_uml.c changes the name in the subsequent commit, so > > change the "names" inside the virtio_vq_config from "const char *const > > *names" to "const char **names". > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> > > Acked-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx> > > Reviewed-by: Ilpo J=E4rvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> > > The name seems broken here. Email APP bug. I will fix. > > [...] > > > > > typedef void vq_callback_t(struct virtqueue *); > > > > +/** > > + * struct virtio_vq_config - configure for find_vqs() > > + * @cfg_idx: Used by virtio core. The drivers should set this to 0. > > + * During the initialization of each vq(vring setup), we need to know which > > + * item in the array should be used at that time. But since the item in > > + * names can be null, which causes some item of array to be skipped, we > > + * cannot use vq.index as the current id. So add a cfg_idx to let vring > > + * know how to get the current configuration from the array when > > + * initializing vq. > > So this design is not good. If it is not something that the driver > needs to care about, the core needs to hide it from the API. The driver just ignore it. That will be beneficial to the virtio core. Otherwise, we must pass one more parameter everywhere. Thanks. > > Thanks >
