Virtualization - Mar 2022 - [PATCH v2 07/14] vhost: Shadow virtqueue buffers forwarding

? 2022/3/3 ??2:23, Eugenio Perez Martin ??:
>>> +
>>> +static bool vhost_svq_add_split(VhostShadowVirtqueue *svq,
>>> +                                VirtQueueElement *elem,
>>> +                                unsigned *head)
>>> +{
>>> +    unsigned avail_idx;
>>> +    vring_avail_t *avail = svq->vring.avail;
>>> +
>>> +    *head = svq->free_head;
>>> +
>>> +    /* We need some descriptors here */
>>> +    if (unlikely(!elem->out_num && !elem->in_num)) {
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +            "Guest provided element with no
descriptors");
>>> +        return false;
>>> +    }
>>> +
>>> +    vhost_vring_write_descs(svq, elem->out_sg,
elem->out_num,
>>> +                            elem->in_num > 0, false);
>>> +    vhost_vring_write_descs(svq, elem->in_sg, elem->in_num,
false, true);
>> I wonder instead of passing in/out separately and using the hint like
>> more_descs, is it better to simply pass the elem to
>> vhost_vrign_write_descs() then we know which one is the last that
>> doesn't depend on more_descs.
>>
> I'm not sure I follow this.
>
> The purpose of vhost_vring_write_descs is to abstract the writing of a
> batch of descriptors, its chaining, etc. It accepts the write
> parameter just for the write flag. If we make elem as a parameter, we
> would need to duplicate that for loop for read and for write
> descriptors, isn't it?
>
> To duplicate the for loop is the way it is done in the kernel, but I
> actually think the kernel could benefit from abstracting both in the
> same function too. Please let me know if you think otherwise or I've
> missed your point.

Ok, so it's just a suggestion and we can do optimization on top for sure.

Thanks

>