virtio/net: refactor

build.zig

@@ -14,6 +14,7 @@ const src = [_][]const u8{
     "src/virtio/console.c",
     "src/virtio/net.c",
     "src/virtio/sound.c",
+    "src/virtio/util.c",
 };
 
 const src_aarch64 = [_][]const u8{

include/libvmm/virtio/mmio.h

@@ -48,12 +48,6 @@
 #define VIRTIO_DEVICE_ID_CONSOLE      3
 #define VIRTIO_DEVICE_ID_SOUND        25
 
-/* The maximum size (number of elements) of a virtqueue. It is set
- * to 128 because I copied it from the camkes virtio device. If you find
- * out that the virtqueue gets full easily, increase the number.
- */
-#define QUEUE_SIZE 128
-
 /* handler of a virtqueue */
 // @ivanv: we can pack/bitfield this struct
 typedef struct virtio_queue_handler {

include/libvmm/virtio/pci.h

@@ -144,7 +144,6 @@
 #define VIRTIO_PCI_CONSOLE_DEV_ID       0x1003
 #define VIRTIO_PCI_NOTIF_OFF_MULTIPLIER 0x2
 #define VIRTIO_PCI_QUEUE_NUM_MAX        0x2
-#define VIRTIO_PCI_QUEUE_SIZE           0x100
 
 // Type 0 headers for endpoints
 struct pci_config_space {

include/libvmm/virtio/util.h

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2026, UNSW
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <microkit.h>
+#include <string.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+/* Given a descriptor head, walk the descriptor chain and compute the sum of the length of all the desciptor chain's buffers */
+uint64_t virtio_desc_chain_payload_len(virtio_queue_handler_t *vq_handler, uint16_t desc_head);
+
+/* Given a scatter gather list of a virtio request in the form of a descriptor head, copy a chunk of data from the list,
+ * starting from `read_off` into `data`. For example, if you want the body of a virtio block read or write request,
+ * `read_off` should be `sizeof(struct virtio_blk_outhdr)`.
+ * Return true if the operation completed successfully, false if the amount of data overflows the
+ * scatter gather list. */
+bool virtio_read_data_from_desc_chain(virtio_queue_handler_t *vq_handler, uint16_t desc_head, uint64_t bytes_to_read,
+                                      uint64_t read_off, char *data);
+
+/* Same idea as above but we are now writing to the scatter gather buffers in guest RAM */
+bool virtio_write_data_to_desc_chain(virtio_queue_handler_t *vq_handler, uint16_t desc_head, uint64_t bytes_to_write,
+                                     uint64_t write_off, char *data);
+
+/* Peek an available descriptor head from a virtq */
+bool virtio_virtq_peek_avail(virtio_queue_handler_t *vq_handler, uint16_t *ret);
+
+/* Pop an available descriptor head from a virtq */
+bool virtio_virtq_pop_avail(virtio_queue_handler_t *vq_handler, uint16_t *ret);
+
+/* Given a descriptor head and the number of bytes that were written to it by the VMM, place it
+ * in the used queue */
+void virtio_virtq_add_used(virtio_queue_handler_t *vq_handler, uint16_t desc_head, uint32_t bytes_written);

include/libvmm/virtio/virtio.h

@@ -9,6 +9,12 @@
 #include <libvmm/virtio/mmio.h>
 #include <libvmm/virtio/pci.h>
 
+/* The maximum size (number of elements) of a virtqueue. It is set
+* to 128 because I copied it from the camkes virtio device. If you find
+* out that the virtqueue gets full easily, increase the number.
+*/
+#define VIRTIO_QUEUE_SIZE 128
+
 /*
  * All terminology used and functionality of the virtIO device implementation
  * adheres with the following specification:

src/virtio/console.c

@@ -10,6 +10,7 @@
 #include <libvmm/virtio/config.h>
 #include <libvmm/virtio/mmio.h>
 #include <libvmm/virtio/console.h>
+#include <libvmm/virtio/util.h>
 #include <sddf/serial/queue.h>
 
 /* Uncomment this to enable debug logging */
@@ -62,7 +63,8 @@ static bool virtio_console_get_device_features(struct virtio_device *dev, uint32
         *features = BIT_HIGH(VIRTIO_F_VERSION_1);
         break;
     default:
-        LOG_CONSOLE_ERR("driver sets DeviceFeaturesSel to 0x%x, which doesn't make sense\n", dev->regs.DeviceFeaturesSel);
+        LOG_CONSOLE_ERR("driver sets DeviceFeaturesSel to 0x%x, which doesn't make sense\n",
+                        dev->regs.DeviceFeaturesSel);
         return false;
     }
 
@@ -87,7 +89,8 @@ static bool virtio_console_set_driver_features(struct virtio_device *dev, uint32
         success = (features == BIT_HIGH(VIRTIO_F_VERSION_1));
         break;
     default:
-        LOG_CONSOLE_ERR("driver sets DriverFeaturesSel to 0x%x, which doesn't make sense\n", dev->regs.DriverFeaturesSel);
+        LOG_CONSOLE_ERR("driver sets DriverFeaturesSel to 0x%x, which doesn't make sense\n",
+                        dev->regs.DriverFeaturesSel);
         return false;
     }
 
@@ -123,40 +126,65 @@ static bool virtio_console_handle_tx(struct virtio_device *dev)
     /* Transmit all available descriptors possible */
     LOG_CONSOLE("processing available buffers from index [0x%lx..0x%lx)\n", vq->last_idx, vq->virtq.avail->idx);
     bool transferred = false;
-    while (vq->last_idx != vq->virtq.avail->idx && !serial_queue_full(console->txq, console->txq->queue->head)) {
-        uint16_t desc_idx = vq->virtq.avail->ring[vq->last_idx % vq->virtq.num];
-        struct virtq_desc desc;
-        /* Traverse chained descriptors */
-        do {
-            desc = vq->virtq.desc[desc_idx];
-            // @ivanv: to the debug logging, we should actually print out the buffer contents
-            LOG_CONSOLE("processing descriptor (0x%lx) with buffer [0x%lx..0x%lx)\n", desc_idx, desc.addr, desc.addr + desc.len);
-
-            uint32_t bytes_remain = desc.len;
-            /* Copy all contiguous data */
-            while (bytes_remain > 0 && !serial_queue_full(console->txq, console->txq->queue->head)) {
-                uint32_t free = serial_queue_contiguous_free(console->txq);
-                uint32_t to_transfer = (bytes_remain < free) ? bytes_remain : free;
-                if (to_transfer) {
-                    transferred = true;
-                }
-
-                memcpy(console->txq->data_region + (console->txq->queue->tail % console->txq->capacity),
-                       (char *)(desc.addr + (desc.len - bytes_remain)), to_transfer);
-
-                serial_update_shared_tail(console->txq, console->txq->queue->tail + to_transfer);
-                bytes_remain -= to_transfer;
-            }
-
-            desc_idx = desc.next;
-
-        } while (desc.flags & VIRTQ_DESC_F_NEXT && !serial_queue_full(console->txq, console->txq->queue->head));
-
-        struct virtq_used_elem used_elem = {vq->virtq.avail->ring[vq->last_idx % vq->virtq.num], 0};
-        vq->virtq.used->ring[vq->virtq.used->idx % vq->virtq.num] = used_elem;
-        vq->virtq.used->idx++;
-
-        vq->last_idx++;
+    uint16_t desc_head;
+    while (virtio_virtq_peek_avail(vq, &desc_head) && !serial_queue_full(console->txq, console->txq->queue->head)) {
+        uint64_t payload_len = virtio_desc_chain_payload_len(vq, desc_head);
+
+        if (payload_len > console->txq->capacity) {
+            // @billn, fix properly by partial TX, bookkeep, then continue once serial virt notifies?
+            LOG_CONSOLE_ERR(
+                "payload length 0x%lx bytes of desciptor %u is larger than serial TX queue capacity of 0x%lx bytes\n",
+                payload_len, desc_head, console->txq->capacity);
+            assert(false);
+        }
+
+        uint32_t serial_txq_free_len = serial_queue_free(console->txq);
+
+        if (payload_len > serial_txq_free_len) {
+            /* Can't consume this descriptor in full for now, just wait for serial TXQ to drain. */
+            LOG_CONSOLE("payload length 0x%lx bytes of desciptor %u is larger than serial TX queue contiguous free "
+                        "length of 0x%lx bytes, won't consume for now.\n",
+                        payload_len, desc_head, serial_txq_free_len);
+            serial_request_consumer_signal(console->txq);
+            break;
+        }
+        /* Ok there is enough space in TX queue for this descriptor */
+
+        uint32_t serial_txq_contiguous_free_len = serial_queue_contiguous_free(console->txq);
+        bool copy_twice = false;
+        if (payload_len > serial_txq_contiguous_free_len) {
+            /* Handle case where the free space wraps around */
+            copy_twice = true;
+        }
+
+        uint32_t bytes_copied = 0;
+
+        /* Copy data until no more to copy or until the queue wraps around */
+        char *serial_txq_dest = (char *)(console->txq->data_region
+                                         + (console->txq->queue->tail % console->txq->capacity));
+        uint32_t copy_len = MIN(payload_len, serial_txq_contiguous_free_len);
+        assert(virtio_read_data_from_desc_chain(vq, desc_head, copy_len, bytes_copied, serial_txq_dest));
+        bytes_copied += copy_len;
+        serial_update_shared_tail(console->txq, console->txq->queue->tail + copy_len);
+
+        if (copy_twice) {
+            /* Need to copy more data after the queue wraps around */
+            serial_txq_dest = (char *)(console->txq->data_region
+                                       + (console->txq->queue->tail % console->txq->capacity));
+            copy_len = payload_len - bytes_copied;
+            assert(copy_len <= serial_queue_contiguous_free(console->txq));
+            assert(virtio_read_data_from_desc_chain(vq, desc_head, copy_len, bytes_copied, serial_txq_dest));
+            bytes_copied += copy_len;
+            serial_update_shared_tail(console->txq, console->txq->queue->tail + copy_len);
+        }
+
+        assert(bytes_copied == payload_len);
+
+        LOG_CONSOLE("processed descriptor %u with content: %s\n", desc_head, serial_txq_dest);
+
+        virtio_virtq_add_used(vq, desc_head, 0);
+        virtio_virtq_pop_avail(vq, &desc_head);
+        transferred = true;
     }
 
     /* While unlikely, it is possible that we could not consume any of the
@@ -179,6 +207,9 @@ bool virtio_console_handle_rx(struct virtio_console_device *console)
     LOG_CONSOLE("operation: handle rx\n");
     assert(console->virtio_device.num_vqs > RX_QUEUE);
 
+    /* See if we have more room in TX queue to send more data. */
+    virtio_console_handle_tx(&(console->virtio_device));
+
     /* Used to know whether to set the IRQ status. */
     bool transferred = false;
 
@@ -194,24 +225,20 @@ bool virtio_console_handle_rx(struct virtio_console_device *console)
     }
 
     LOG_CONSOLE("processing available buffers from index [0x%lx..0x%lx)\n", vq->last_idx, vq->virtq.avail->idx);
-    while (vq->last_idx != vq->virtq.avail->idx && !serial_queue_empty(console->rxq, console->rxq->queue->head)) {
+    uint16_t desc_head;
+    while (!serial_queue_empty(console->rxq, console->rxq->queue->head) && virtio_virtq_pop_avail(vq, &desc_head)) {
         transferred = true;
-
-        uint16_t desc_head = vq->virtq.avail->ring[vq->last_idx % vq->virtq.num];
         struct virtq_desc desc = vq->virtq.desc[desc_head];
-        LOG_CONSOLE("processing descriptor (0x%lx) with buffer [0x%lx..0x%lx)\n", desc_head, desc.addr, desc.addr + desc.len);
+        LOG_CONSOLE("processing descriptor (0x%lx) with buffer [0x%lx..0x%lx)\n", desc_head, desc.addr,
+                    desc.addr + desc.len);
         uint32_t bytes_written = 0;
         char c;
         while (bytes_written < desc.len && !serial_dequeue(console->rxq, &c)) {
-            *(char *)(desc.addr + bytes_written) = c;
+            assert(virtio_write_data_to_desc_chain(vq, desc_head, 1, bytes_written, &c));
             bytes_written++;
         }
 
-        struct virtq_used_elem used_elem = {desc_head, bytes_written};
-        vq->virtq.used->ring[vq->virtq.used->idx % vq->virtq.num] = used_elem;
-        vq->virtq.used->idx++;
-
-        vq->last_idx++;
+        virtio_virtq_add_used(vq, desc_head, bytes_written);
     }
 
     /* While unlikely, it is possible that we could not consume any of the

src/virtio/mmio.c

@@ -106,7 +106,7 @@ static bool handle_virtio_mmio_reg_read(virtio_device_t *dev, size_t vcpu_id, si
         success = dev->funs->get_device_features(dev, &reg);
         break;
     case REG_RANGE(REG_VIRTIO_MMIO_QUEUE_NUM_MAX, REG_VIRTIO_MMIO_QUEUE_NUM):
-        reg = QUEUE_SIZE;
+        reg = VIRTIO_QUEUE_SIZE;
         break;
     case REG_RANGE(REG_VIRTIO_MMIO_QUEUE_READY, REG_VIRTIO_MMIO_QUEUE_NOTIFY):
         if (dev->regs.QueueSel < dev->num_vqs) {