Skip to content

Conversation

otteryc
Copy link
Contributor

@otteryc otteryc commented Feb 1, 2025

According to Virtio Specification Version 1.3, Section 2.7.5 The Virtqueue Descriptor Table, the type of addr should be uint64_t instead of uint32_t.
This commit addresses the error in the previous implementation.

@jserv jserv requested a review from shengwen-tw February 1, 2025 15:15
virtio-blk.c Outdated
vq_desc[i].addr = desc->addr;
vq_desc[i].len = desc->len;
vq_desc[i].flags = desc->flags;
desc_idx = desc->next; /* vq_desc[desc_cnt].next */

This comment was marked as resolved.

virtio-rng.c Outdated
.flags = desc[3],
};
struct virtq_desc vq_desc =
*(struct virtq_desc *) &vrng->ram[queue->QueueDesc + buffer_idx * 4];

This comment was marked as resolved.

virtio-rng.c Outdated

/* Clear write flag */
desc[3] = 0;
vq_desc.flags = 0;

This comment was marked as resolved.

virtio.h Outdated
#undef _
};

struct virtq_desc {

This comment was marked as resolved.

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Avoid unnecessary capitalization. That is, use "Rectify length of Virtio descriptor address" instead of "Rectify Length of Virtio Descriptor Address."

@otteryc otteryc changed the title Rectify Length of Virtio Descriptor Address Rectify length of Virtio descriptor address Feb 2, 2025
According to Virtio Specification Version 1.3, Section 2.7.5 *The
Virtqueue Descriptor Table*, the type of 'addr' should be 'uint64_t'
instead of 'uint32_t'.
This commit addresses the error in the previous implementation.

Co-authored-by: Shengwen Cheng <[email protected]>
Copy link
Collaborator

@shengwen-tw shengwen-tw left a comment

Choose a reason for hiding this comment

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

Looks legitimate.

@jserv jserv merged commit b2e2de7 into sysprog21:master Feb 2, 2025
3 checks passed
@jserv
Copy link
Collaborator

jserv commented Feb 2, 2025

Thank @otteryc for contributing!

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.

3 participants