Thu, Jul 04, 2024 at 09:47:09AM CEST, m...@redhat.com wrote:
>On Thu, Jul 04, 2024 at 08:43:35AM +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <j...@nvidia.com>
>> 
>> Introduce a structure virtio_queue_info to carry name, callback and ctx
>> together. In order to allow config implementations to accept config op
>> with array of virtio_queue_info structures, introduce a new
>> find_vqs_info() op. Do the needed conversion in virtio_find_vqs_ctx().
>> Note that whole virtio_find_vqs_ctx() is going to be eventually removed
>> at the and of this patchset.
>> 
>> Signed-off-by: Jiri Pirko <j...@nvidia.com>
>> ---
>> v1->v2:
>> - fixed comments for struct virtqueue_info
>> - s/virtio_queue_info/virtqueue_info/
>> ---
>>  include/linux/virtio_config.h | 51 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 48 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
>> index 82a1d798b2f1..9b975c2830f6 100644
>> --- a/include/linux/virtio_config.h
>> +++ b/include/linux/virtio_config.h
>> @@ -18,6 +18,16 @@ struct virtio_shm_region {
>>  
>>  typedef void vq_callback_t(struct virtqueue *);
>>  
>> +struct virtqueue_info {
>> +    const char *name; /* Mainly for debugging, may be NULL for a virtqueue
>> +                       * unused by driver.
>> +                       */
>> +    vq_callback_t *callback; /* May be NULL for vq that does
>> +                              * not need a callback.
>> +                              */
>> +    bool ctx;
>
>
>       /*
>        * Always
>        * like this
>        * outside of netdev code
>        * and it comes before the code being documented
>        */

Ah, yeah.

I wonder howcome checkpatch does not warn.


>
>       /* Never
>        * like this
>        */
>
>
>Also "may be null" is a bit confusing - it has to be null
>for such a vq.
>
>Thus:
>
>       /*
>        * name: mainly for debugging, NULL for a virtqueue
>         * unused by the driver.
>        */

Okay.


>       const char *name;
>
>and similarly for the callback.

Yes.

Will send v3 later today if there are no more comments.

Thanks!

>
>
>
>> +};
>> +
>>  /**
>>   * struct virtio_config_ops - operations for configuring a virtio device
>>   * Note: Do not assume that a transport implements all of the operations
>> @@ -58,6 +68,12 @@ typedef void vq_callback_t(struct virtqueue *);
>>   *  names: array of virtqueue names (mainly for debugging)
>>   *          include a NULL entry for vqs unused by driver
>>   *  Returns 0 on success or error status
>> + * @find_vqs_info: find virtqueues and instantiate them.
>> + *  vdev: the virtio_device
>> + *  nvqs: the number of virtqueues to find
>> + *  vqs: on success, includes new virtqueues
>> + *  vqs_info: array of virtqueue info structures
>> + *  Returns 0 on success or error status
>>   * @del_vqs: free virtqueues found by find_vqs().
>>   * @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
>>   *      The function guarantees that all memory operations on the
>> @@ -109,6 +125,10 @@ struct virtio_config_ops {
>>                      struct virtqueue *vqs[], vq_callback_t *callbacks[],
>>                      const char * const names[], const bool *ctx,
>>                      struct irq_affinity *desc);
>> +    int (*find_vqs_info)(struct virtio_device *vdev, unsigned int nvqs,
>> +                         struct virtqueue *vqs[],
>> +                         struct virtqueue_info vqs_info[],
>> +                         struct irq_affinity *desc);
>>      void (*del_vqs)(struct virtio_device *);
>>      void (*synchronize_cbs)(struct virtio_device *);
>>      u64 (*get_features)(struct virtio_device *vdev);
>> @@ -117,7 +137,7 @@ struct virtio_config_ops {
>>      int (*set_vq_affinity)(struct virtqueue *vq,
>>                             const struct cpumask *cpu_mask);
>>      const struct cpumask *(*get_vq_affinity)(struct virtio_device *vdev,
>> -                    int index);
>> +                                             int index);
>>      bool (*get_shm_region)(struct virtio_device *vdev,
>>                             struct virtio_shm_region *region, u8 id);
>>      int (*disable_vq_and_reset)(struct virtqueue *vq);
>> @@ -210,14 +230,39 @@ static inline bool virtio_has_dma_quirk(const struct 
>> virtio_device *vdev)
>>      return !virtio_has_feature(vdev, VIRTIO_F_ACCESS_PLATFORM);
>>  }
>>  
>> +static inline
>> +int virtio_find_vqs_info(struct virtio_device *vdev, unsigned int nvqs,
>> +                     struct virtqueue *vqs[],
>> +                     struct virtqueue_info vqs_info[],
>> +                     struct irq_affinity *desc)
>> +{
>> +    return vdev->config->find_vqs_info(vdev, nvqs, vqs, vqs_info, desc);
>> +}
>> +
>>  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,
>>                      struct irq_affinity *desc)
>>  {
>> -    return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names, ctx,
>> -                                  desc);
>> +    struct virtqueue_info *vqs_info;
>> +    int err, i;
>> +
>> +    if (!vdev->config->find_vqs_info)
>> +            return vdev->config->find_vqs(vdev, nvqs, vqs, callbacks,
>> +                                          names, ctx, desc);
>> +
>> +    vqs_info = kmalloc_array(nvqs, sizeof(*vqs_info), GFP_KERNEL);
>> +    if (!vqs_info)
>> +            return -ENOMEM;
>> +    for (i = 0; i < nvqs; i++) {
>> +            vqs_info[i].name = names[i];
>> +            vqs_info[i].callback = callbacks[i];
>> +            vqs_info[i].ctx = ctx ? ctx[i] : false;
>> +    }
>> +    err = virtio_find_vqs_info(vdev, nvqs, vqs, vqs_info, desc);
>> +    kfree(vqs_info);
>> +    return err;
>>  }
>>  
>>  static inline
>> -- 
>> 2.45.2
>

Reply via email to