On Thu, Feb 05, 2026 at 03:43:10PM -0800, Farhan Ali wrote:
> <...snip...>
>
> On 1/27/2026 8:15 AM, [email protected] wrote:
> > int virtio_pci_setup(VDev *vdev)
> > +{
> > + VRing *vr;
> > + int rc;
> > + uint8_t status;
> > + int i = 0;
> > +
> > + vdev->guessed_disk_nature = VIRTIO_GDN_NONE;
> > + vdev->cmd_vr_idx = 0;
> > +
> > + if (virtio_reset(vdev)) {
> > + return -EIO;
> > + }
> > +
> > + status = VPCI_S_ACKNOWLEDGE;
> > + if (virtio_pci_set_status(status)) {
> > + puts("Virtio-pci device Failed to ACKNOWLEDGE");
> > + return -EIO;
> > + }
> > +
> > + rc = virtio_pci_read_pci_cap_config();
> > + if (rc) {
> > + puts("Invalid virtio PCI capabilities");
> > + return -EIO;
> > + }
> > +
> > + vdev->guest_features[1] = VIRTIO_F_VERSION_1;
> > + if (virtio_pci_negotiate()) {
> > + panic("Virtio feature negotation failed!");
> > + }
> > +
> > + switch (vdev->dev_type) {
> > + case VIRTIO_ID_BLOCK:
> > + vdev->nr_vqs = 1;
> > + vdev->cmd_vr_idx = 0;
> > + vdev->config.blk.blk_size = 0;
> > + virtio_pci_get_blk_config();
> > + break;
> > + default:
> > + puts("Unsupported virtio device");
> > + return -ENODEV;
> > + }
> > +
> > + status |= VPCI_S_DRIVER;
> > + rc = virtio_pci_set_status(status);
> > + if (rc) {
> > + puts("Set status failed");
> > + return -EIO;
> > + }
> > +
> > + /* Configure virt-queues for pci */
> > + for (i = 0; i < vdev->nr_vqs; i++) {
> > + VqInfo info = {
> > + .queue = (unsigned long long) virtio_get_ring_area(i),
> > + .align = KVM_S390_VIRTIO_RING_ALIGN,
> > + .index = i,
> > + .num = 0,
> > + };
> > +
> > + vr = &vdev->vrings[i];
> > +
> > + if (vpci_read_flex(VPCI_C_COMMON_NUMQ, c_cap.bar, &info.num, 2)) {
> > + return -EIO;
> > + }
> > +
> > + vring_init(vr, &info);
> > +
> > + if (vpci_set_selected_vq(vr->id)) {
> > + puts("Failed to set selected virt-queue");
> > + return -EIO;
> > + }
> > +
> > + if (vpci_set_queue_size(VIRTIO_RING_SIZE)) {
> > + puts("Failed to set virt-queue size");
> > + return -EIO;
> > + }
> > +
> > + rc = set_pci_vq_addr(VPCI_C_OFFSET_Q_DESCLO, vr->desc);
> > + rc |= set_pci_vq_addr(VPCI_C_OFFSET_Q_AVAILLO, vr->avail);
> > + rc |= set_pci_vq_addr(VPCI_C_OFFSET_Q_USEDLO, vr->used);
> > + if (rc) {
> > + puts("Failed to configure virt-queue address");
> > + return -EIO;
> > + }
> > +
> > + if (vpci_set_queue_enable(true)) {
> > + puts("Failed to set virt-queue enabled");
> > + return -EIO;
> > + }
> > + }
> > +
> > + status |= VPCI_S_FEATURES_OK | VPCI_S_DRIVER_OK;
> > + return virtio_pci_set_status(status);
>
> Should we also enable Bus master bit as part of the device setup? QEMU
> though has a workaround where if we have setup the ACKNOWLEDGE and DRIVER
> status flags set then it would enable the Bus master by default since older
> Linux virtio-pci drivers didn't do it
> (https://github.com/qemu/qemu/blob/cd5a79dc98e3087e7658e643bdbbb0baec77ac8a/hw/virtio/virtio-pci.c#L480-L487).
> But latest virtio-pci drivers seems to be doing the right thing, so not sure
> how long this workaround would be supported :) .
>
Please do. linux 2.6.x is ancient and there's no guarantee we will keep
supporting this indefinitely.
> > +}
> > +
>
> <..snip..>
>
>
> > diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h
> > index 96c17ac3c7..883b00e0c6 100644
> > --- a/pc-bios/s390-ccw/virtio-pci.h
> > +++ b/pc-bios/s390-ccw/virtio-pci.h
> > @@ -71,6 +71,8 @@ typedef struct VirtioPciCap VirtioPciCap;
> > void virtio_pci_id2type(VDev *vdev, uint16_t device_id);
> > int virtio_pci_reset(VDev *vdev);
> > long virtio_pci_notify(int vq_id);
> > +int virtio_pci_setup(VDev *vdev);
> > +int virtio_pci_setup_device(void);
> > int vpci_read_flex(uint64_t offset, uint8_t pcias, void *buf, int len);
> > int vpci_read_bswap64(uint64_t offset, uint8_t pcias, uint64_t *buf);
> > diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
> > index 956b34ff33..f65571a920 100644
> > --- a/pc-bios/s390-ccw/virtio.c
> > +++ b/pc-bios/s390-ccw/virtio.c
> > @@ -17,6 +17,7 @@
> > #include "virtio.h"
> > #include "virtio-scsi.h"
> > #include "virtio-ccw.h"
> > +#include "virtio-pci.h"
> > #include "bswap.h"
> > #include "helper.h"
> > #include "s390-time.h"
> > @@ -112,6 +113,8 @@ bool vring_notify(VRing *vr)
> > case S390_IPL_TYPE_CCW:
> > vr->cookie = virtio_ccw_notify(vdev.schid, vr->id, vr->cookie);
> > break;
> > + case S390_IPL_TYPE_PCI:
> > + vr->cookie = virtio_pci_notify(vr->id);
> > default:
> > return 1;
> > }
> > @@ -119,8 +122,43 @@ bool vring_notify(VRing *vr)
> > return vr->cookie >= 0;
> > }
> > +/*
> > + * Get endienness of the IPL type
> > + * Return true for s390x native big-endian
> > + */
> > +bool be_ipl(void)
> > +{
> > + switch (virtio_get_device()->ipl_type) {
> > + case S390_IPL_TYPE_QEMU_SCSI:
> > + case S390_IPL_TYPE_CCW:
> > + return true;
> > + case S390_IPL_TYPE_PCI:
> > + return false;
> > + default:
> > + return true;
> > + }
> > +}
> > +
> > +/*
> > + * Format the virtio ring descriptor endianness
> > + * Return the available index increment in the appropriate endianness
> > + */
> > +static uint16_t vr_fmt_descriptor(VRingDesc *desc)
> > +{
> > + if (!be_ipl()) {
> > + desc->addr = bswap64(desc->addr);
> > + desc->len = bswap32(desc->len);
> > + desc->flags = bswap16(desc->flags);
> > + desc->next = bswap16(desc->next);
> > + }
> > +
> > + return be_ipl() ? 1 : bswap16(1);
> > +}
> > +
> > void vring_send_buf(VRing *vr, void *p, int len, int flags)
> > {
> > + uint16_t increment;
> > +
> > /* For follow-up chains we need to keep the first entry point */
> > if (!(flags & VRING_HIDDEN_IS_CHAIN)) {
> > vr->avail->ring[vr->avail->idx % vr->num] = vr->next_idx;
> > @@ -131,11 +169,13 @@ void vring_send_buf(VRing *vr, void *p, int len, int
> > flags)
> > vr->desc[vr->next_idx].flags = flags & ~VRING_HIDDEN_IS_CHAIN;
> > vr->desc[vr->next_idx].next = vr->next_idx;
> > vr->desc[vr->next_idx].next++;
> > +
> > + increment = vr_fmt_descriptor(&vr->desc[vr->next_idx]);
>
> If we are only returning 1 from the function vr_fmt_descriptor(), why not
> just set increment=1 or remove it completely and keep vr->avail->idx++? That
> way we don't need to do any endian conversion?
>
> Thanks
>
> Farhan
>
>
> > vr->next_idx++;
> > /* Chains only have a single ID */
> > if (!(flags & VRING_DESC_F_NEXT)) {
> > - vr->avail->idx++;
> > + vr->avail->idx += increment;
> > }
> > }
