On 04/03/2026 15.29, Jared Rossi wrote:
On 3/4/26 4:53 AM, Thomas Huth wrote:
On 04/03/2026 03.59, [email protected] wrote:
From: Jared Rossi <[email protected]>
Add little-endian virt-queue configuration and support for virtio-blk-pci
IPL
devices.
Signed-off-by: Jared Rossi <[email protected]>
---
...
+static int virtio_pci_get_blk_config(void)
+{
+ VirtioBlkConfig *cfg = &virtio_get_device()->config.blk;
+ int rc = vpci_read_flex(d_cap.off, d_cap.bar, cfg,
sizeof(VirtioBlkConfig));
+
+ /* single byte fields are not touched */
+ cfg->capacity = bswap64(cfg->capacity);
+ cfg->size_max = bswap32(cfg->size_max);
+ cfg->seg_max = bswap32(cfg->seg_max);
+
+ cfg->geometry.cylinders = bswap16(cfg->geometry.cylinders);
+
+ cfg->blk_size = bswap32(cfg->blk_size);
+ cfg->min_io_size = bswap16(cfg->min_io_size);
+ cfg->opt_io_size = bswap32(cfg->opt_io_size);
So it looks like you read a bunch of optional config fields here ...
+ return rc;
+}
...
+int virtio_pci_setup(VDev *vdev)
+{
+ VRing *vr;
+ int rc;
+ uint8_t status;
+ uint16_t vq_size;
+ int i = 0;
+
+ vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
+ vdev->cmd_vr_idx = 0;
+
+ if (virtio_pci_read_pci_cap_config()) {
+ puts("Invalid virtio PCI capabilities");
+ return -EIO;
+ }
+
+ if (enable_pci_bus_master()) {
+ return -EIO;
+ }
+
+ if (virtio_reset(vdev)) {
+ return -EIO;
+ }
+
+ status = VIRTIO_CONFIG_S_ACKNOWLEDGE;
+ if (virtio_pci_set_status(status)) {
+ puts("Virtio-pci device Failed to ACKNOWLEDGE");
+ return -EIO;
+ }
... so I think you should enable the corresponding feature bits in vdev-
>guest_features[0] here? QEMU might be very forgiving and provide you with
the fields anyway, but let's better play safe. Something like:
vdev->guest_features[0] = VIRTIO_BLK_F_SIZE_MAX |
VIRTIO_BLK_F_SEG_MAX |
VIRTIO_BLK_F_GEOMETRY |
VIRTIO_BLK_F_BLK_SIZE;
?
VIRTIO_BLK_F_GEOMETRY and VIRTIO_BLK_F_BLK_SIZE are already set during the
virtio-blk setup. I actually don't think the other two fields are used, I
jut swapped any fields larger than 1 byte. I suppose the feature bits
should be enabled though... otherwise we could just just not touch the
unused fields at all?
Ah, right, I missed the initialization in virtio_blk_setup_device(), so we
should be fine here, indeed!
+ vdev->guest_features[1] = VIRTIO_F_VERSION_1;
+ if (virtio_pci_negotiate()) {
+ panic("Virtio feature negotation failed!");
+ }
Maybe double-check whether VIRTIO_F_VERSION_1 has really been negotiated?
Otherwise, what happens if a user runs QEMU with "-device virtio-blk-
pci,disable-modern=on" ?
I haven't tried running it with "disable-modern=on" (I will test that next)
but the config is big endian if I don't negotiate that feature bit, and
little endian if I do, which I think is the expected behavior when
VIRTIO_F_VERSION_1 is set.
Just for my understanding, do you see something that makes you suspect the
negotiation isn't actually happening? I will try running with "disable-
modern=on" and let you know the results.
No, I think it's fine for the default case. I'm just wondering what happens
when someone uses "disable-modern=on" ... I guess the code will currently
behave in weird ways since the endianness is wrong ... thus I thought it
might be better to check VIRTIO_F_VERSION_1 again and emit a proper error
message in this case?
Thomas
