On 10/22/25 12:40 PM, Zhuoying Cai wrote:
[snip...]

+/*
+ * Read PCI configuration space to find the offset of the Common, Device, and
+ * Notification memory regions within the modern memory space.
+ * Returns 0 if success, 1 if a capability could not be located, or a
+ * negative RC if the configuration read failed.
+ */
+static int virtio_pci_read_pci_cap_config(VDev *vdev)
+{
+    uint8_t pos;
+    uint64_t data;
+
+    /* Common cabilities */
+    pos = find_cap_pos(vdev->pci_fh, VIRTIO_PCI_CAP_COMMON_CFG);
+    if (!pos) {
+        puts("Failed to locate PCI common configuration");
+        return 1;
+    }
+    if (pci_read(vdev->pci_fh, pos + VIRTIO_PCI_CAP_OFFSET, 15, &data, 4)) {
+        return -EIO;
+    }
+    common_offset = bswap32(data);
+
+    /* Device cabilities */
+    pos = find_cap_pos(vdev->pci_fh, VIRTIO_PCI_CAP_DEVICE_CFG);
+    if (!pos) {
+        puts("Failed to locate PCI device configuration");
+        return 1;
+    }
+    if (pci_read(vdev->pci_fh, pos + VIRTIO_PCI_CAP_OFFSET, 15, &data, 4)) {
+        return -EIO;
+    }
+    device_offset = bswap32(data);
+
+    /* Notification cabilities */
+    pos = find_cap_pos(vdev->pci_fh, VIRTIO_PCI_CAP_NOTIFY_CFG);
+    if (!pos) {
+        puts("Failed to locate PCI notification configuration");
+        return 1;
+    }
+    if (pci_read(vdev->pci_fh, pos + VIRTIO_PCI_CAP_OFFSET, 15, &data, 4)) {
+        return -EIO;
+    }
+    notify_offset = bswap32(data);
+
+    if (pci_read(vdev->pci_fh, pos + VIRTIO_PCI_NOTIFY_CAP_MULT, 15, &data, 
4)) {
+        return -EIO;
+    }
Could you please explain why we use
pci_read(vdev->pci_fh, pos + VIRTIO_PCI_NOTIFY_CAP_MULT, 15, &data, 4)
instead of
pci_read(vdev->pci_fh, notify_offset + VIRTIO_PCI_NOTIFY_CAP_MULT, 4,
&data, 4) here?
The notification capability in particular has an extra field at the end, which is what we are looking for.  We are still in PCI configuration space (BAR 15), we just want to read some different bytes in the notification capability configuration.
+    notify_mult = bswap32(data);
+
+    if (pci_read(vdev->pci_fh, device_offset + VPCI_C_OFFSET_Q_NOFF, 4, &data, 
2)) {
+        return -EIO;
+    }
Should queue_notify_off be read using pci_read(vdev->pci_fh,
common_offset + VPCI_C_OFFSET_Q_NOFF, 4, &data, 2)?
(Virtio specs v-1.3 section 4.1.4.3 Common configuration structure layout)
I think you are correct.  This is a mistake.  The queue notification offset is in the common configuration, not the device configuration.  I suspect it didn't cause a problem here because virtio-blk uses only one queue.


+long virtio_pci_notify(uint32_t fhandle, int vq_id)
+{
+    uint64_t notice = 1;
+    uint32_t offset = notify_offset + vq_id * q_notify_offset;
Shoud the offset be calculated as notify_offset + q_notify_offset *
notify_mult?
(Virtio specs v-1.3 section 4.1.4.4 Notification structure layout)
I'll double check this, but you are probably right.  As with the previous instance it didn't cause a problem due to there only being one queue for virtio-blk, so the offset is always just the base notify_offset and, luckily, the incorrect parts get multiplied by zero.


+
+    return pci_write(fhandle, offset, notice, 4);
Please correct me if I'm wrong - instead of always writing notice = 1,
should we write vq_id to vq_index of the Queue Notify address.
(Virtio specs v-1.3 section 4.1.5.2 Available Buffer Notifications)
I'll double and triple check the queue offset/notification calculations since it seems there are multiple errors on my part and I just got lucky that virtio-blk wasn't affected.  Thanks for catching these mistakes.

Regards,
Jared Rossi


Reply via email to