On Tue, 2026-03-03 at 21:59 -0500, [email protected] wrote: > From: Jared Rossi <[email protected]> > > Define common functionality for interacting with virtio-pci devices. > > Signed-off-by: Jared Rossi <[email protected]> > --- > pc-bios/s390-ccw/Makefile | 2 +- > pc-bios/s390-ccw/virtio-pci.c | 167 ++++++++++++++++++++++++++++++++++ > pc-bios/s390-ccw/virtio-pci.h | 80 ++++++++++++++++ > pc-bios/s390-ccw/virtio.h | 5 + > 4 files changed, 253 insertions(+), 1 deletion(-) > create mode 100644 pc-bios/s390-ccw/virtio-pci.c > create mode 100644 pc-bios/s390-ccw/virtio-pci.h >
...snip... (Only comment I had in here was the s/0x1402/0x1042/ that Thomas pointed out.) > diff --git a/pc-bios/s390-ccw/virtio-pci.h b/pc-bios/s390-ccw/virtio-pci.h > new file mode 100644 > index 0000000000..54c524f698 > --- /dev/null > +++ b/pc-bios/s390-ccw/virtio-pci.h > @@ -0,0 +1,80 @@ > +/* > + * Definitions for virtio-pci > + * > + * Copyright 2025 IBM Corp. > + * Author(s): Jared Rossi <[email protected]> > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > +#ifndef VIRTIO_PCI_H > +#define VIRTIO_PCI_H > + > +/* Common configuration */ > +#define VPCI_CAP_COMMON_CFG 1 > +/* Notifications */ > +#define VPCI_CAP_NOTIFY_CFG 2 > +/* ISR access */ > +#define VPCI_CAP_ISR_CFG 3 > +/* Device specific configuration */ > +#define VPCI_CAP_DEVICE_CFG 4 > +/* PCI configuration access */ > +#define VPCI_CAP_PCI_CFG 5 > +/* Additional shared memory capability */ > +#define VPCI_CAP_SHARED_MEMORY_CFG 8 > +/* PCI vendor data configuration */ > +#define VPCI_CAP_VENDOR_CFG 9 > + > +/* Offsets within capability header */ > +#define VPCI_CAP_VNDR 0 > +#define VPCI_CAP_NEXT 1 > +#define VPCI_CAP_LEN 2 > +#define VPCI_CAP_CFG_TYPE 3 > +#define VPCI_CAP_BAR 4 > +#define VPCI_CAP_OFFSET 8 > +#define VPCI_CAP_LENGTH 12 > + > +#define VPCI_N_CAP_MULT 16 /* Notify multiplier, VPCI_CAP_NOTIFY_CFG only */ > + > +/* Common Area Offsets for virtio-pci queue */ > +#define VPCI_C_OFFSET_DFSELECT 0 > +#define VPCI_C_OFFSET_DF 4 > +#define VPCI_C_OFFSET_GFSELECT 8 > +#define VPCI_C_OFFSET_GF 12 > +#define VPCI_C_COMMON_NUMQ 18 > +#define VPCI_C_OFFSET_STATUS 20 > +#define VPCI_C_OFFSET_Q_SELECT 22 > +#define VPCI_C_OFFSET_Q_SIZE 24 > +#define VPCI_C_OFFSET_Q_ENABLE 28 > +#define VPCI_C_OFFSET_Q_NOFF 30 > +#define VPCI_C_OFFSET_Q_DESCLO 32 > +#define VPCI_C_OFFSET_Q_DESCHI 36 > +#define VPCI_C_OFFSET_Q_AVAILLO 40 > +#define VPCI_C_OFFSET_Q_AVAILHI 44 > +#define VPCI_C_OFFSET_Q_USEDLO 48 > +#define VPCI_C_OFFSET_Q_USEDHI 52 > + > +#define VIRTIO_F_VERSION_1 1 /* Feature bit 32 */ > + > +struct VirtioPciCap { > + uint8_t bar; /* Which PCIAS it's in */ > + uint32_t off; /* Offset within bar */ > +}; > +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 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); > +int vpci_read_bswap32(uint64_t offset, uint8_t pcias, uint32_t *buf); > +int vpci_read_bswap16(uint64_t offset, uint8_t pcias, uint16_t *buf); > +int vpci_read_byte(uint64_t offset, uint8_t pcias, uint8_t *buf); > + > +int vpci_bswap64_write(uint64_t offset, uint8_t pcias, uint64_t data); > +int vpci_bswap32_write(uint64_t offset, uint8_t pcias, uint32_t data); > +int vpci_bswap16_write(uint64_t offset, uint8_t pcias, uint16_t data); > +int vpci_write_byte(uint64_t offset, uint8_t pcias, uint8_t data); I know it's pedantic, but since the reads are vpci_read_bXXXX while the writes are vcpi_bXXXX_write (except the single byte case) could we make them consistent, please? (Soft preference for vpci_read/write_bXXXX, but I don't much care.) > + > +#endif > diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h > index c3cb5a6ee3..0e7dbdb64c 100644 > --- a/pc-bios/s390-ccw/virtio.h > +++ b/pc-bios/s390-ccw/virtio.h > @@ -18,6 +18,10 @@ > #define VIRTIO_CONFIG_S_DRIVER 2 > /* Driver has used its parts of the config, and is happy */ > #define VIRTIO_CONFIG_S_DRIVER_OK 4 > +/* Feature negotiation complete */ > +#define VIRTIO_CONFIG_S_FEATURES_OK 8 > +/* Clear status byte */ > +#define VIRTIO_CONFIG_S_RESET 0x00 Heh, in v3 I wondered if you could add the new status bit(s) here. Buuuut... :) Virtio section 2.1 defines "NEEDS_RESET" as 64 (0x40). Of course it says that the status field starts as zero and is reinitialized to zero BY a reset, which is what you end up doing. Its placement here between FEATURES_OK and FAILED with their spec-accurate definitions, but with a different definition had me puzzled. Maybe move it to the top of this list, and rename it to ..._S_INIT or ..._S_ZERO or something? (Or don't define it and just set it to zero in the code.) > /* We've given up on this device. */ > #define VIRTIO_CONFIG_S_FAILED 0x80 > > @@ -255,6 +259,7 @@ struct VDev { > uint8_t scsi_dev_heads; > bool scsi_device_selected; > ScsiDevice selected_scsi_device; > + uint32_t pci_fh; > uint32_t max_transfer; > uint32_t guest_features[2]; > };
