Skip to content

Commit

Permalink
Rectify Length of Virtio Descriptor Address
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
otteryc and shengwen-tw committed Feb 1, 2025
1 parent d61c9f6 commit da5f66f
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 39 deletions.
16 changes: 9 additions & 7 deletions virtio-blk.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ static void virtio_blk_update_status(virtio_blk_state_t *vblk, uint32_t status)

static void virtio_blk_write_handler(virtio_blk_state_t *vblk,
uint64_t sector,
uint32_t desc_addr,
uint64_t desc_addr,
uint32_t len)
{
void *dest = (void *) ((uintptr_t) vblk->disk + sector * DISK_BLK_SIZE);
Expand All @@ -112,7 +112,7 @@ static void virtio_blk_write_handler(virtio_blk_state_t *vblk,

static void virtio_blk_read_handler(virtio_blk_state_t *vblk,
uint64_t sector,
uint32_t desc_addr,
uint64_t desc_addr,
uint32_t len)
{
void *dest = (void *) ((uintptr_t) vblk->ram + desc_addr);
Expand Down Expand Up @@ -141,13 +141,15 @@ static int virtio_blk_desc_handler(virtio_blk_state_t *vblk,
/* Collect the descriptors */
for (int i = 0; i < 3; i++) {
/* The size of the `struct virtq_desc` is 4 words */
const uint32_t *desc = &vblk->ram[queue->QueueDesc + desc_idx * 4];
const struct virtq_desc *desc =
(struct virtq_desc *) &vblk->ram[queue->QueueDesc + desc_idx * 4];


/* Retrieve the fields of current descriptor */
vq_desc[i].addr = desc[0];
vq_desc[i].len = desc[2];
vq_desc[i].flags = desc[3];
desc_idx = desc[3] >> 16; /* vq_desc[desc_cnt].next */
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 */
}

/* The next flag for the first and second descriptors should be set,
Expand Down
49 changes: 25 additions & 24 deletions virtio-net.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,35 +180,36 @@ static ssize_t handle_write(netdev_t *netdev,
/* Require existing 'desc_idx' to use as iteration variable, and input
* 'buffer_idx'.
*/
#define VNET_ITERATE_BUFFER(checked, body) \
desc_idx = buffer_idx; \
while (1) { \
if (checked && desc_idx >= queue->QueueNum) \
return virtio_net_set_fail(vnet); \
const uint32_t *desc = &ram[queue->QueueDesc + desc_idx * 4]; \
uint16_t desc_flags = desc[3]; \
body if (!(desc_flags & VIRTIO_DESC_F_NEXT)) break; \
desc_idx = desc[3] >> 16; \
#define VNET_ITERATE_BUFFER(checked, body) \
desc_idx = buffer_idx; \
while (1) { \
if (checked && desc_idx >= queue->QueueNum) \
return virtio_net_set_fail(vnet); \
const struct virtq_desc *desc = \
(struct virtq_desc *) &ram[queue->QueueDesc + desc_idx * 4]; \
uint16_t desc_flags = desc->flags; \
body if (!(desc_flags & VIRTIO_DESC_F_NEXT)) break; \
desc_idx = desc->next; \
}

/* Input: 'buffer_idx'.
* Output: 'buffer_niovs' and 'buffer_iovs'
*/
#define VNET_BUFFER_TO_IOV(expect_readable) \
uint16_t desc_idx; \
/* do a first pass to validate flags and count buffers */ \
size_t buffer_niovs = 0; \
VNET_ITERATE_BUFFER( \
true, if ((!!(desc_flags & VIRTIO_DESC_F_WRITE)) != \
(expect_readable)) return virtio_net_set_fail(vnet); \
buffer_niovs++;) \
/* convert to iov */ \
struct iovec buffer_iovs[buffer_niovs]; \
buffer_niovs = 0; \
VNET_ITERATE_BUFFER( \
false, uint32_t desc_addr = desc[0]; uint32_t desc_len = desc[2]; \
buffer_iovs[buffer_niovs].iov_base = \
(void *) ((uintptr_t) ram + desc_addr); \
#define VNET_BUFFER_TO_IOV(expect_readable) \
uint16_t desc_idx; \
/* do a first pass to validate flags and count buffers */ \
size_t buffer_niovs = 0; \
VNET_ITERATE_BUFFER( \
true, if ((!!(desc_flags & VIRTIO_DESC_F_WRITE)) != \
(expect_readable)) return virtio_net_set_fail(vnet); \
buffer_niovs++;) \
/* convert to iov */ \
struct iovec buffer_iovs[buffer_niovs]; \
buffer_niovs = 0; \
VNET_ITERATE_BUFFER( \
false, uint64_t desc_addr = desc->addr; uint32_t desc_len = desc->len; \
buffer_iovs[buffer_niovs].iov_base = \
(void *) ((uintptr_t) ram + desc_addr); \
buffer_iovs[buffer_niovs].iov_len = desc_len; buffer_niovs++;)

#define VNET_GENERATE_QUEUE_HANDLER(NAME_SUFFIX, VERB, QUEUE_IDX, READ) \
Expand Down
10 changes: 3 additions & 7 deletions virtio-rng.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,16 @@ static void virtio_queue_notify_handler(virtio_rng_state_t *vrng,
VRNG_QUEUE.last_avail++;

/* Read descriptor */
uint32_t *desc = &vrng->ram[queue->QueueDesc + buffer_idx * 4];
struct virtq_desc vq_desc = {
.addr = desc[0],
.len = desc[2],
.flags = desc[3],
};
struct virtq_desc vq_desc =
*(struct virtq_desc *) &vrng->ram[queue->QueueDesc + buffer_idx * 4];

/* Write entropy buffer */
void *entropy_buf =
(void *) ((uintptr_t) vrng->ram + (uintptr_t) vq_desc.addr);
ssize_t total = read(rng_fd, entropy_buf, vq_desc.len);

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

/* Get virtq_used.idx (le16) */
uint16_t used = ram[queue->QueueUsed] >> 16;
Expand Down
2 changes: 1 addition & 1 deletion virtio.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ enum {
};

struct virtq_desc {
uint32_t addr;
uint64_t addr;
uint32_t len;
uint16_t flags;
uint16_t next;
Expand Down

0 comments on commit da5f66f

Please sign in to comment.