Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

virtqueue: add more virtqueue apis for the virtio device side #631

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CV-Bowen
Copy link
Contributor

@CV-Bowen CV-Bowen commented Nov 4, 2024

In virtio device side, we always need to get the next avaiable buffer based on current buffer index. So add this API for convinience use.

For example, virtio blk driver origanize the buffer:

+----------+
| Reqeust  | (Flags: Read | Next)
+----------+
| Buffer   | (Flags: Read/Write | Next)
+----------+
| Response | (Flags: Write)
+----------+

For the virtio blk device size, we need get the Buffer and Response buffer based on the Request buffer index.

@CV-Bowen CV-Bowen force-pushed the virtqueue_next_buffer branch 3 times, most recently from 7740f3e to 78f2a0f Compare November 4, 2024 02:43
@arnopo arnopo requested review from edmooring, arnopo and tnmysh November 5, 2024 14:07
@arnopo
Copy link
Collaborator

arnopo commented Nov 19, 2024

@CV-Bowen : We usually wait that the CI is pass before having a look at a PR. So for the next time please ensure that CI has passed.

That said, an API that allows getting several buffers would seem more generic and flexible than one that gets the next buffer. What about:

int virtqueue_get_available_buffer(struct virtqueue *vq, struct vq_buff *buff, unsigned int needed)
struct vq_buf would contain the address, the size and the index of the needed buffers

@CV-Bowen CV-Bowen force-pushed the virtqueue_next_buffer branch from 78f2a0f to cdb4e01 Compare November 21, 2024 03:24
@CV-Bowen
Copy link
Contributor Author

@arnopo Yes, this is what we want to do and I have updated this PR to add a new API: virtqueue_get_available_buffers() to get all the buffers. Please take a look again.

In virtio device side, we always need to get the next avaiable
buffer based on current buffer index. So add these two APIs for
convinience use.

For example, virtio blk driver origanize the buffer:
+----------+
| Reqeust  | (Flags: Read | Next)
+----------+
| Buffer   | (Flags: Read/Write | Next)
+----------+
| Response | (Flags: Write)
+----------+

For the virtio blk device size, we need get the Buffer and Response buffer
based on the Request buffer index.

So add virtqueue_get_next_avail_buffer() and
virtqueue_get_available_buffers() api.

Signed-off-by: Bowen Wang <[email protected]>
Signed-off-by: Yongrong Wang <[email protected]>
@CV-Bowen CV-Bowen force-pushed the virtqueue_next_buffer branch from cdb4e01 to 76bc7f2 Compare November 21, 2024 06:31
@CV-Bowen CV-Bowen changed the title virtqueue: add virtqueue_get_next_avail_buffer() API virtqueue: add more virtqueue apis for the virtio device side Nov 21, 2024
@@ -231,6 +231,63 @@ void *virtqueue_get_available_buffer(struct virtqueue *vq, uint16_t *avail_idx,
return buffer;
}

void *virtqueue_get_next_avail_buffer(struct virtqueue *vq, uint16_t idx,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you still need this one? virtqueue_get_available_buffers should replace this function

}

int virtqueue_get_available_buffers(struct virtqueue *vq,
struct virtqueue_buf *vb, int vbsize,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as mentioned in my previous comment you should return a struct vq_buf that contain the address, the size and the index of the needed buffers instead of just returning the buffer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants