From 277c7e1fe91bc204a207a39b93e0d04426249a27 Mon Sep 17 00:00:00 2001 From: Yu-Cheng Chen Date: Sat, 1 Feb 2025 21:02:42 +0800 Subject: [PATCH] Rectify length of Virtio descriptor address 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 --- virtio-blk.c | 16 +++++++++------- virtio-net.c | 49 +++++++++++++++++++++++++------------------------ virtio-rng.c | 14 +++++--------- virtio.h | 6 +++--- 4 files changed, 42 insertions(+), 43 deletions(-) diff --git a/virtio-blk.c b/virtio-blk.c index 63ecf1d..c64cda7 100644 --- a/virtio-blk.c +++ b/virtio-blk.c @@ -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); @@ -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); @@ -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; } /* The next flag for the first and second descriptors should be set, diff --git a/virtio-net.c b/virtio-net.c index e8b0f6e..4aa4542 100644 --- a/virtio-net.c +++ b/virtio-net.c @@ -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) \ diff --git a/virtio-rng.c b/virtio-rng.c index 21a62ae..2876fe0 100644 --- a/virtio-rng.c +++ b/virtio-rng.c @@ -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); + (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; diff --git a/virtio.h b/virtio.h index b893ef5..c142da5 100644 --- a/virtio.h +++ b/virtio.h @@ -57,9 +57,9 @@ enum { #undef _ }; -struct virtq_desc { - uint32_t addr; +PACKED(struct virtq_desc { + uint64_t addr; uint32_t len; uint16_t flags; uint16_t next; -}; +});