Hi,
This patch series adds virtio 1.0 support to the virtio blk and scsi drivers in seabios. With this series applied seabios happily boots in virtio 1.0 mode from both transitional and modern devices.
Tested with Fedora 22 guest, booting from virtio-scsi cdrom (live iso), virtio-scsi disk and virtio-blk disk.
The patches are also available in the git repository at: git://git.kraxel.org/seabios virtio
v2 changes: * rename vp_modern_{read_write} to vp_{read,write} * switch legacy virtio code to vp_{read,write} too. * make vp_read return the values.
Gerd Hoffmann (22): pci: allow to loop over capabilities virtio: run drivers in 32bit mode virtio: add struct vp_device virtio: pass struct pci_device to vp_init_simple virtio: add version 1.0 structs and #defines virtio: add version 0.9.5 struct virtio: find version 1.0 virtio capabilities virtio: create vp_cap struct for legacy bar virtio: add version 0.9.5 struct [fixup] virtio: add read/write functions and macros virtio: make features 64bit, support version 1.0 features virtio: add version 1.0 support to vp_{get,set}_status virtio: add version 1.0 support to vp_get_isr virtio: add version 1.0 support to vp_reset virtio: add version 1.0 support to vp_notify virtio: remove unused vp_del_vq virtio: add version 1.0 support to vp_find_vq virtio-scsi: fix initialization for version 1.0 virtio-blk: fix initialization for version 1.0 virtio: use version 1.0 if available (flip the big switch) virtio: also probe version 1.0 pci ids virtio: legacy cleanup
src/block.c | 8 +- src/fw/pciinit.c | 4 +- src/hw/pci.c | 11 ++- src/hw/pci.h | 2 +- src/hw/pci_ids.h | 8 +- src/hw/virtio-blk.c | 104 +++++++++++++++------ src/hw/virtio-pci.c | 228 +++++++++++++++++++++++++++++++++++++++++----- src/hw/virtio-pci.h | 252 ++++++++++++++++++++++++++++++++++++--------------- src/hw/virtio-ring.c | 4 +- src/hw/virtio-ring.h | 9 +- src/hw/virtio-scsi.c | 60 ++++++++---- 11 files changed, 535 insertions(+), 155 deletions(-)
Add a parameter to pci_find_capability, to specify the start point. This allows to find multiple capabilities of the same type, by calling pci_find_capability again with the offset of the last capability found.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/fw/pciinit.c | 4 ++-- src/hw/pci.c | 11 ++++++++--- src/hw/pci.h | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index ac39d23..45870f2 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -642,7 +642,7 @@ pci_region_create_entry(struct pci_bus *bus, struct pci_device *dev,
static int pci_bus_hotplug_support(struct pci_bus *bus) { - u8 pcie_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_EXP); + u8 pcie_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_EXP, 0); u8 shpc_cap;
if (pcie_cap) { @@ -666,7 +666,7 @@ static int pci_bus_hotplug_support(struct pci_bus *bus) return downstream_port && slot_implemented; }
- shpc_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_SHPC); + shpc_cap = pci_find_capability(bus->bus_dev, PCI_CAP_ID_SHPC, 0); return !!shpc_cap; }
diff --git a/src/hw/pci.c b/src/hw/pci.c index 0379b55..a241d06 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -221,16 +221,21 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg) return NULL; }
-u8 pci_find_capability(struct pci_device *pci, u8 cap_id) +u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap) { int i; - u8 cap; u16 status = pci_config_readw(pci->bdf, PCI_STATUS);
if (!(status & PCI_STATUS_CAP_LIST)) return 0;
- cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST); + if (cap == 0) { + /* find first */ + cap = pci_config_readb(pci->bdf, PCI_CAPABILITY_LIST); + } else { + /* find next */ + cap = pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_NEXT); + } for (i = 0; cap && i <= 0xff; i++) { if (pci_config_readb(pci->bdf, cap + PCI_CAP_LIST_ID) == cap_id) return cap; diff --git a/src/hw/pci.h b/src/hw/pci.h index 0aaa84c..fc5e7b9 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -123,7 +123,7 @@ int pci_init_device(const struct pci_device_id *ids , struct pci_device *pci, void *arg); struct pci_device *pci_find_init_device(const struct pci_device_id *ids , void *arg); -u8 pci_find_capability(struct pci_device *pci, u8 cap_id); +u8 pci_find_capability(struct pci_device *pci, u8 cap_id, u8 cap); int pci_bridge_has_region(struct pci_device *pci, enum pci_region_type region_type); void pci_reboot(void);
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/block.c | 8 +++++--- src/hw/virtio-blk.c | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/block.c b/src/block.c index 3f7ecb1..a9b9851 100644 --- a/src/block.c +++ b/src/block.c @@ -503,8 +503,10 @@ process_op(struct disk_op_s *op) case DTYPE_CDEMU: ret = process_cdemu_op(op); break; - case DTYPE_VIRTIO_BLK: - ret = process_virtio_blk_op(op); + case DTYPE_VIRTIO_BLK: ; + extern void _cfunc32flat_process_virtio_blk_op(void); + ret = call32(_cfunc32flat_process_virtio_blk_op + , (u32)MAKE_FLATPTR(GET_SEG(SS), op), DISK_RET_EPARAM); break; case DTYPE_AHCI: ; extern void _cfunc32flat_process_ahci_op(void); @@ -526,7 +528,6 @@ process_op(struct disk_op_s *op) break; case DTYPE_USB: case DTYPE_UAS: - case DTYPE_VIRTIO_SCSI: case DTYPE_LSI_SCSI: case DTYPE_ESP_SCSI: case DTYPE_MEGASAS: @@ -534,6 +535,7 @@ process_op(struct disk_op_s *op) break; case DTYPE_USB_32: case DTYPE_UAS_32: + case DTYPE_VIRTIO_SCSI: case DTYPE_PVSCSI: ; extern void _cfunc32flat_scsi_process_op(void); ret = call32(_cfunc32flat_scsi_process_op diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index e2dbd3c..15ac171 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -77,7 +77,7 @@ virtio_blk_op(struct disk_op_s *op, int write) return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK; }
-int +int VISIBLE32FLAT process_virtio_blk_op(struct disk_op_s *op) { if (! CONFIG_VIRTIO_BLK)
For virtio 1.0 support we will need more state than just the (legacy mode) ioaddr for each virtio-pci device. Prepare for that by adding a new struct for it. For now it carries the ioaddr only.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-blk.c | 20 ++++++++++---------- src/hw/virtio-pci.c | 15 +++++++++------ src/hw/virtio-pci.h | 46 +++++++++++++++++++++++++++------------------- src/hw/virtio-ring.c | 4 ++-- src/hw/virtio-ring.h | 3 ++- src/hw/virtio-scsi.c | 32 +++++++++++++++++--------------- 6 files changed, 67 insertions(+), 53 deletions(-)
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 15ac171..13cf09a 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -25,7 +25,7 @@ struct virtiodrive_s { struct drive_s drive; struct vring_virtqueue *vq; - u16 ioaddr; + struct vp_device *vp; };
static int @@ -60,7 +60,7 @@ virtio_blk_op(struct disk_op_s *op, int write) vring_add_buf(vq, sg, 2, 1, 0, 0); else vring_add_buf(vq, sg, 1, 2, 0, 0); - vring_kick(GET_GLOBALFLAT(vdrive_gf->ioaddr), vq, 1); + vring_kick(GET_GLOBALFLAT(vdrive_gf->vp), vq, 1);
/* Wait for reply */ while (!vring_more_used(vq)) @@ -72,7 +72,7 @@ virtio_blk_op(struct disk_op_s *op, int write) /* Clear interrupt status register. Avoid leaving interrupts stuck if * VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised. */ - vp_get_isr(GET_GLOBALFLAT(vdrive_gf->ioaddr)); + vp_get_isr(GET_GLOBALFLAT(vdrive_gf->vp));
return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK; } @@ -113,18 +113,17 @@ init_virtio_blk(struct pci_device *pci) vdrive->drive.type = DTYPE_VIRTIO_BLK; vdrive->drive.cntl_id = bdf;
- u16 ioaddr = vp_init_simple(bdf); - vdrive->ioaddr = ioaddr; - if (vp_find_vq(ioaddr, 0, &vdrive->vq) < 0 ) { + vdrive->vp = vp_init_simple(bdf); + if (vp_find_vq(vdrive->vp, 0, &vdrive->vq) < 0 ) { dprintf(1, "fail to find vq for virtio-blk %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); goto fail; }
struct virtio_blk_config cfg; - vp_get(ioaddr, 0, &cfg, sizeof(cfg)); + vp_get(vdrive->vp, 0, &cfg, sizeof(cfg));
- u32 f = vp_get_features(ioaddr); + u32 f = vp_get_features(vdrive->vp); vdrive->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ? cfg.blk_size : DISK_SECTOR_SIZE;
@@ -148,12 +147,13 @@ init_virtio_blk(struct pci_device *pci)
boot_add_hd(&vdrive->drive, desc, bootprio_find_pci_device(pci));
- vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE | + vp_set_status(vdrive->vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK); return;
fail: - vp_reset(ioaddr); + vp_reset(vdrive->vp); + free(vdrive->vp); free(vdrive->vq); free(vdrive); } diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index b9b3ab1..3cd478d 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -24,9 +24,10 @@ #include "virtio-pci.h" #include "virtio-ring.h"
-int vp_find_vq(unsigned int ioaddr, int queue_index, +int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { + int ioaddr = GET_LOWFLAT(vp->ioaddr); u16 num;
ASSERT32FLAT(); @@ -84,14 +85,16 @@ fail: return -1; }
-u16 vp_init_simple(u16 bdf) +struct vp_device *vp_init_simple(u16 bdf) { - u16 ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) & + struct vp_device *vp = malloc_high(sizeof(*vp)); + + vp->ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK;
- vp_reset(ioaddr); + vp_reset(vp); pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); - vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE | + vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER ); - return ioaddr; + return vp; } diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index bc04b03..47bef3d 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -2,6 +2,7 @@ #define _VIRTIO_PCI_H
#include "x86.h" // inl +#include "biosvar.h" // GET_LOWFLAT
/* A 32-bit r/o bitmask of the features supported by the host */ #define VIRTIO_PCI_HOST_FEATURES 0 @@ -39,19 +40,24 @@ /* Virtio ABI version, this must match exactly */ #define VIRTIO_PCI_ABI_VERSION 0
-static inline u32 vp_get_features(unsigned int ioaddr) +struct vp_device { + unsigned int ioaddr; +}; + +static inline u32 vp_get_features(struct vp_device *vp) { - return inl(ioaddr + VIRTIO_PCI_HOST_FEATURES); + return inl(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_HOST_FEATURES); }
-static inline void vp_set_features(unsigned int ioaddr, u32 features) +static inline void vp_set_features(struct vp_device *vp, u32 features) { - outl(features, ioaddr + VIRTIO_PCI_GUEST_FEATURES); + outl(features, GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_GUEST_FEATURES); }
-static inline void vp_get(unsigned int ioaddr, unsigned offset, +static inline void vp_get(struct vp_device *vp, unsigned offset, void *buf, unsigned len) { + int ioaddr = GET_LOWFLAT(vp->ioaddr); u8 *ptr = buf; unsigned i;
@@ -59,47 +65,49 @@ static inline void vp_get(unsigned int ioaddr, unsigned offset, ptr[i] = inb(ioaddr + VIRTIO_PCI_CONFIG + offset + i); }
-static inline u8 vp_get_status(unsigned int ioaddr) +static inline u8 vp_get_status(struct vp_device *vp) { - return inb(ioaddr + VIRTIO_PCI_STATUS); + return inb(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_STATUS); }
-static inline void vp_set_status(unsigned int ioaddr, u8 status) +static inline void vp_set_status(struct vp_device *vp, u8 status) { if (status == 0) /* reset */ return; - outb(status, ioaddr + VIRTIO_PCI_STATUS); + outb(status, GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_STATUS); }
-static inline u8 vp_get_isr(unsigned int ioaddr) +static inline u8 vp_get_isr(struct vp_device *vp) { - return inb(ioaddr + VIRTIO_PCI_ISR); + return inb(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_ISR); }
-static inline void vp_reset(unsigned int ioaddr) +static inline void vp_reset(struct vp_device *vp) { + int ioaddr = GET_LOWFLAT(vp->ioaddr); + outb(0, ioaddr + VIRTIO_PCI_STATUS); (void)inb(ioaddr + VIRTIO_PCI_ISR); }
-static inline void vp_notify(unsigned int ioaddr, int queue_index) +static inline void vp_notify(struct vp_device *vp, int queue_index) { - outw(queue_index, ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); + outw(queue_index, GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_QUEUE_NOTIFY); }
-static inline void vp_del_vq(unsigned int ioaddr, int queue_index) +static inline void vp_del_vq(struct vp_device *vp, int queue_index) { + int ioaddr = GET_LOWFLAT(vp->ioaddr); + /* select the queue */ - outw(queue_index, ioaddr + VIRTIO_PCI_QUEUE_SEL);
/* deactivate the queue */ - outl(0, ioaddr + VIRTIO_PCI_QUEUE_PFN); }
struct vring_virtqueue; -u16 vp_init_simple(u16 bdf); -int vp_find_vq(unsigned int ioaddr, int queue_index, +struct vp_device *vp_init_simple(u16 bdf); +int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq); #endif /* _VIRTIO_PCI_H_ */ diff --git a/src/hw/virtio-ring.c b/src/hw/virtio-ring.c index 97e0b34..5c6a32e 100644 --- a/src/hw/virtio-ring.c +++ b/src/hw/virtio-ring.c @@ -136,7 +136,7 @@ void vring_add_buf(struct vring_virtqueue *vq, SET_LOWFLAT(avail->ring[av], head); }
-void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added) +void vring_kick(struct vp_device *vp, struct vring_virtqueue *vq, int num_added) { struct vring *vr = &vq->vring; struct vring_avail *avail = GET_LOWFLAT(vr->avail); @@ -145,5 +145,5 @@ void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added) smp_wmb(); SET_LOWFLAT(avail->idx, GET_LOWFLAT(avail->idx) + num_added);
- vp_notify(ioaddr, GET_LOWFLAT(vq->queue_index)); + vp_notify(vp, GET_LOWFLAT(vq->queue_index)); } diff --git a/src/hw/virtio-ring.h b/src/hw/virtio-ring.h index b7a7aaf..fe5133b 100644 --- a/src/hw/virtio-ring.h +++ b/src/hw/virtio-ring.h @@ -120,12 +120,13 @@ static inline void vring_init(struct vring *vr, vr->desc[i].next = 0; }
+struct vp_device; int vring_more_used(struct vring_virtqueue *vq); void vring_detach(struct vring_virtqueue *vq, unsigned int head); int vring_get_buf(struct vring_virtqueue *vq, unsigned int *len); void vring_add_buf(struct vring_virtqueue *vq, struct vring_list list[], unsigned int out, unsigned int in, int index, int num_added); -void vring_kick(unsigned int ioaddr, struct vring_virtqueue *vq, int num_added); +void vring_kick(struct vp_device *vp, struct vring_virtqueue *vq, int num_added);
#endif /* _VIRTIO_RING_H_ */ diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c index 8f96687..25d2db7 100644 --- a/src/hw/virtio-scsi.c +++ b/src/hw/virtio-scsi.c @@ -27,14 +27,15 @@ struct virtio_lun_s { struct drive_s drive; struct pci_device *pci; struct vring_virtqueue *vq; - u16 ioaddr; + struct vp_device *vp; u16 target; u16 lun; };
static int -virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, - void *cdbcmd, u16 target, u16 lun, u16 blocksize) +virtio_scsi_cmd(struct vp_device *vp, struct vring_virtqueue *vq, + struct disk_op_s *op, void *cdbcmd, u16 target, u16 lun, + u16 blocksize) { struct virtio_scsi_req_cmd req; struct virtio_scsi_resp_cmd resp; @@ -66,7 +67,7 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op,
/* Add to virtqueue and kick host */ vring_add_buf(vq, sg, out_num, in_num, 0, 0); - vring_kick(ioaddr, vq, 1); + vring_kick(vp, vq, 1);
/* Wait for reply */ while (!vring_more_used(vq)) @@ -78,7 +79,7 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op, /* Clear interrupt status register. Avoid leaving interrupts stuck if * VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised. */ - vp_get_isr(ioaddr); + vp_get_isr(vp);
if (resp.response == VIRTIO_SCSI_S_OK && resp.status == 0) { return DISK_RET_SUCCESS; @@ -92,7 +93,7 @@ virtio_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) struct virtio_lun_s *vlun_gf = container_of(op->drive_gf, struct virtio_lun_s, drive);
- return virtio_scsi_cmd(GET_GLOBALFLAT(vlun_gf->ioaddr), + return virtio_scsi_cmd(GET_GLOBALFLAT(vlun_gf->vp), GET_GLOBALFLAT(vlun_gf->vq), op, cdbcmd, GET_GLOBALFLAT(vlun_gf->target), GET_GLOBALFLAT(vlun_gf->lun), @@ -100,7 +101,7 @@ virtio_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) }
static int -virtio_scsi_add_lun(struct pci_device *pci, u16 ioaddr, +virtio_scsi_add_lun(struct pci_device *pci, struct vp_device *vp, struct vring_virtqueue *vq, u16 target, u16 lun) { struct virtio_lun_s *vlun = malloc_fseg(sizeof(*vlun)); @@ -112,7 +113,7 @@ virtio_scsi_add_lun(struct pci_device *pci, u16 ioaddr, vlun->drive.type = DTYPE_VIRTIO_SCSI; vlun->drive.cntl_id = pci->bdf; vlun->pci = pci; - vlun->ioaddr = ioaddr; + vlun->vp = vp; vlun->vq = vq; vlun->target = target; vlun->lun = lun; @@ -129,11 +130,11 @@ fail: }
static int -virtio_scsi_scan_target(struct pci_device *pci, u16 ioaddr, +virtio_scsi_scan_target(struct pci_device *pci, struct vp_device *vp, struct vring_virtqueue *vq, u16 target) { /* TODO: send REPORT LUNS. For now, only LUN 0 is recognized. */ - int ret = virtio_scsi_add_lun(pci, ioaddr, vq, target, 0); + int ret = virtio_scsi_add_lun(pci, vp, vq, target, 0); return ret < 0 ? 0 : 1; }
@@ -144,19 +145,19 @@ init_virtio_scsi(struct pci_device *pci) dprintf(1, "found virtio-scsi at %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); struct vring_virtqueue *vq = NULL; - u16 ioaddr = vp_init_simple(bdf); - if (vp_find_vq(ioaddr, 2, &vq) < 0 ) { + struct vp_device *vp = vp_init_simple(bdf); + if (vp_find_vq(vp, 2, &vq) < 0 ) { dprintf(1, "fail to find vq for virtio-scsi %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); goto fail; }
- vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE | + vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK);
int i, tot; for (tot = 0, i = 0; i < 256; i++) - tot += virtio_scsi_scan_target(pci, ioaddr, vq, i); + tot += virtio_scsi_scan_target(pci, vp, vq, i);
if (!tot) goto fail; @@ -164,7 +165,8 @@ init_virtio_scsi(struct pci_device *pci) return;
fail: - vp_reset(ioaddr); + vp_reset(vp); + free(vp); free(vq); }
... instead of the bdf only.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-blk.c | 2 +- src/hw/virtio-pci.c | 6 +++--- src/hw/virtio-pci.h | 3 ++- src/hw/virtio-scsi.c | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 13cf09a..4f39aa2 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -113,7 +113,7 @@ init_virtio_blk(struct pci_device *pci) vdrive->drive.type = DTYPE_VIRTIO_BLK; vdrive->drive.cntl_id = bdf;
- vdrive->vp = vp_init_simple(bdf); + vdrive->vp = vp_init_simple(pci); if (vp_find_vq(vdrive->vp, 0, &vdrive->vq) < 0 ) { dprintf(1, "fail to find vq for virtio-blk %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 3cd478d..4971021 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -85,15 +85,15 @@ fail: return -1; }
-struct vp_device *vp_init_simple(u16 bdf) +struct vp_device *vp_init_simple(struct pci_device *pci) { struct vp_device *vp = malloc_high(sizeof(*vp));
- vp->ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) & + vp->ioaddr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK;
vp_reset(vp); - pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); + pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER ); return vp; diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 47bef3d..3cf0327 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -106,8 +106,9 @@ static inline void vp_del_vq(struct vp_device *vp, int queue_index) outl(0, ioaddr + VIRTIO_PCI_QUEUE_PFN); }
+struct pci_device; struct vring_virtqueue; -struct vp_device *vp_init_simple(u16 bdf); +struct vp_device *vp_init_simple(struct pci_device *pci); int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq); #endif /* _VIRTIO_PCI_H_ */ diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c index 25d2db7..b208d49 100644 --- a/src/hw/virtio-scsi.c +++ b/src/hw/virtio-scsi.c @@ -145,7 +145,7 @@ init_virtio_scsi(struct pci_device *pci) dprintf(1, "found virtio-scsi at %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); struct vring_virtqueue *vq = NULL; - struct vp_device *vp = vp_init_simple(bdf); + struct vp_device *vp = vp_init_simple(pci); if (vp_find_vq(vp, 2, &vq) < 0 ) { dprintf(1, "fail to find vq for virtio-scsi %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf));
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.h | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/virtio-ring.h | 5 +++++ 2 files changed, 66 insertions(+)
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 3cf0327..18beb83 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -40,6 +40,67 @@ /* Virtio ABI version, this must match exactly */ #define VIRTIO_PCI_ABI_VERSION 0
+/* --- virtio 1.0 (modern) structs ---------------------------------- */ + +/* Common configuration */ +#define VIRTIO_PCI_CAP_COMMON_CFG 1 +/* Notifications */ +#define VIRTIO_PCI_CAP_NOTIFY_CFG 2 +/* ISR access */ +#define VIRTIO_PCI_CAP_ISR_CFG 3 +/* Device specific configuration */ +#define VIRTIO_PCI_CAP_DEVICE_CFG 4 +/* PCI configuration access */ +#define VIRTIO_PCI_CAP_PCI_CFG 5 + +/* This is the PCI capability header: */ +struct virtio_pci_cap { + u8 cap_vndr; /* Generic PCI field: PCI_CAP_ID_VNDR */ + u8 cap_next; /* Generic PCI field: next ptr. */ + u8 cap_len; /* Generic PCI field: capability length */ + u8 cfg_type; /* Identifies the structure. */ + u8 bar; /* Where to find it. */ + u8 padding[3]; /* Pad to full dword. */ + u32 offset; /* Offset within bar. */ + u32 length; /* Length of the structure, in bytes. */ +}; + +struct virtio_pci_notify_cap { + struct virtio_pci_cap cap; + u32 notify_off_multiplier; /* Multiplier for queue_notify_off. */ +}; + +typedef struct virtio_pci_common_cfg { + /* About the whole device. */ + u32 device_feature_select; /* read-write */ + u32 device_feature; /* read-only */ + u32 guest_feature_select; /* read-write */ + u32 guest_feature; /* read-write */ + u16 msix_config; /* read-write */ + u16 num_queues; /* read-only */ + u8 device_status; /* read-write */ + u8 config_generation; /* read-only */ + + /* About a specific virtqueue. */ + u16 queue_select; /* read-write */ + u16 queue_size; /* read-write, power of 2. */ + u16 queue_msix_vector; /* read-write */ + u16 queue_enable; /* read-write */ + u16 queue_notify_off; /* read-only */ + u32 queue_desc_lo; /* read-write */ + u32 queue_desc_hi; /* read-write */ + u32 queue_avail_lo; /* read-write */ + u32 queue_avail_hi; /* read-write */ + u32 queue_used_lo; /* read-write */ + u32 queue_used_hi; /* read-write */ +} virtio_pci_common_cfg; + +typedef struct virtio_pci_isr { + u8 isr; +} virtio_pci_isr; + +/* --- driver structs ----------------------------------------------- */ + struct vp_device { unsigned int ioaddr; }; diff --git a/src/hw/virtio-ring.h b/src/hw/virtio-ring.h index fe5133b..553a508 100644 --- a/src/hw/virtio-ring.h +++ b/src/hw/virtio-ring.h @@ -20,9 +20,14 @@ #define VIRTIO_CONFIG_S_DRIVER 2 /* Driver has used its parts of the config, and is happy */ #define VIRTIO_CONFIG_S_DRIVER_OK 4 +/* Driver has finished configuring features */ +#define VIRTIO_CONFIG_S_FEATURES_OK 8 /* We've given up on this device. */ #define VIRTIO_CONFIG_S_FAILED 0x80
+/* v1.0 compliant. */ +#define VIRTIO_F_VERSION_1 32 + #define MAX_QUEUE_NUM (128)
#define VRING_DESC_F_NEXT 1
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 18beb83..bc2eb05 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -40,6 +40,20 @@ /* Virtio ABI version, this must match exactly */ #define VIRTIO_PCI_ABI_VERSION 0
+/* --- virtio 0.9.5 (legacy) struct --------------------------------- */ + +struct virtio_pci_legacy { + u32 host_features; + u32 guest_features; + u32 queue_pfn; + u16 queue_num; + u16 queue_sel; + u16 queue_notify; + u8 status; + u8 isr; + u8 device[]; +}; + /* --- virtio 1.0 (modern) structs ---------------------------------- */
/* Common configuration */
virtio 1.0 specifies the location of the various virtio regions using pci capabilities. Look them up and store the results.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/virtio-pci.h | 8 ++++++++ 2 files changed, 63 insertions(+)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 4971021..0acf65f 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -88,6 +88,61 @@ fail: struct vp_device *vp_init_simple(struct pci_device *pci) { struct vp_device *vp = malloc_high(sizeof(*vp)); + u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0); + struct vp_cap *vp_cap; + u32 addr, offset; + u8 type; + + memset(vp, 0, sizeof(*vp)); + while (cap != 0) { + type = pci_config_readb(pci->bdf, cap + + offsetof(struct virtio_pci_cap, cfg_type)); + switch (type) { + case VIRTIO_PCI_CAP_COMMON_CFG: + vp_cap = &vp->common; + break; + case VIRTIO_PCI_CAP_NOTIFY_CFG: + vp_cap = &vp->notify; + break; + case VIRTIO_PCI_CAP_ISR_CFG: + vp_cap = &vp->isr; + break; + case VIRTIO_PCI_CAP_DEVICE_CFG: + vp_cap = &vp->device; + break; + default: + vp_cap = NULL; + break; + } + if (vp_cap) { + vp_cap->cap = cap; + vp_cap->bar = pci_config_readb(pci->bdf, cap + + offsetof(struct virtio_pci_cap, bar)); + offset = pci_config_readl(pci->bdf, cap + + offsetof(struct virtio_pci_cap, offset)); + addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0 + 4 * vp_cap->bar); + if (addr & PCI_BASE_ADDRESS_SPACE_IO) { + vp_cap->is_io = 1; + addr &= PCI_BASE_ADDRESS_IO_MASK; + } else { + vp_cap->is_io = 0; + addr &= PCI_BASE_ADDRESS_MEM_MASK; + } + vp_cap->addr = addr + offset; + dprintf(3, "pci dev %x:%x virtio cap at 0x%x type %d " + "bar %d at 0x%08x off +0x%04x [%s]\n", + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf), + vp_cap->cap, type, vp_cap->bar, addr, offset, + vp_cap->is_io ? "io" : "mmio"); + } + + cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, cap); + } + + if (vp->common.cap && vp->notify.cap && vp->isr.cap && vp->device.cap) { + dprintf(1, "pci dev %x:%x supports virtio 1.0\n", + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf)); + }
vp->ioaddr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK; diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index bc2eb05..0f57ca8 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -115,8 +115,16 @@ typedef struct virtio_pci_isr {
/* --- driver structs ----------------------------------------------- */
+struct vp_cap { + u32 addr; + u8 cap; + u8 bar; + u8 is_io; +}; + struct vp_device { unsigned int ioaddr; + struct vp_cap common, notify, isr, device; };
static inline u32 vp_get_features(struct vp_device *vp)
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 5 ++++- src/hw/virtio-pci.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 0acf65f..3badb15 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -144,8 +144,11 @@ struct vp_device *vp_init_simple(struct pci_device *pci) pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf)); }
- vp->ioaddr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & + vp->legacy.bar = 0; + vp->legacy.addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK; + vp->legacy.is_io = 1; + vp->ioaddr = vp->legacy.addr; /* temporary */
vp_reset(vp); pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 0f57ca8..29cd969 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -124,7 +124,7 @@ struct vp_cap {
struct vp_device { unsigned int ioaddr; - struct vp_cap common, notify, isr, device; + struct vp_cap common, notify, isr, device, legacy; };
static inline u32 vp_get_features(struct vp_device *vp)
--- src/hw/virtio-pci.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 29cd969..a04d38b 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -42,7 +42,7 @@
/* --- virtio 0.9.5 (legacy) struct --------------------------------- */
-struct virtio_pci_legacy { +typedef struct virtio_pci_legacy { u32 host_features; u32 guest_features; u32 queue_pfn; @@ -52,7 +52,7 @@ struct virtio_pci_legacy { u8 status; u8 isr; u8 device[]; -}; +} virtio_pci_legacy;
/* --- virtio 1.0 (modern) structs ---------------------------------- */
Add macros to read/write virtio registers.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index a04d38b..a092e86 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -127,6 +127,92 @@ struct vp_device { struct vp_cap common, notify, isr, device, legacy; };
+static inline u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) +{ + u32 addr = cap->addr + offset; + u64 var; + + if (cap->is_io) { + switch (size) { + case 8: + var = inl(addr); + var |= (u64)inl(addr+4) << 32; + break; + case 4: + var = inl(addr); + break; + case 2: + var = inw(addr); + break; + case 1: + var = inb(addr); + break; + default: + var = 0; + } + } else { + switch (size) { + case 8: + var = readl((void*)addr); + var |= (u64)readl((void*)(addr+4)) << 32; + break; + case 4: + var = readl((void*)addr); + break; + case 2: + var = readw((void*)addr); + break; + case 1: + var = readb((void*)addr); + break; + default: + var = 0; + } + } + dprintf(9, "vp read %x (%d) -> 0x%llx\n", addr, size, var); + return var; +} + +static inline void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) +{ + u32 addr = cap->addr + offset; + + dprintf(9, "vp write %x (%d) <- 0x%llx\n", addr, size, var); + if (cap->is_io) { + switch (size) { + case 4: + outl(var, addr); + break; + case 2: + outw(var, addr); + break; + case 1: + outb(var, addr); + break; + } + } else { + switch (size) { + case 4: + writel((void*)addr, var); + break; + case 2: + writew((void*)addr, var); + break; + case 1: + writeb((void*)addr, var); + break; + } + } +} + +#define vp_read(_cap, _struct, _field) \ + _vp_read(_cap, offsetof(_struct, _field), \ + sizeof(((_struct *)0)->_field)) + +#define vp_write(_cap, _struct, _field, _var) \ + _vp_write(_cap, offsetof(_struct, _field), \ + sizeof(((_struct *)0)->_field), _var) + static inline u32 vp_get_features(struct vp_device *vp) { return inl(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_HOST_FEATURES);
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-blk.c | 2 +- src/hw/virtio-pci.c | 33 +++++++++++++++++++++++++++++++++ src/hw/virtio-pci.h | 12 +++--------- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 4f39aa2..9e56d42 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -123,7 +123,7 @@ init_virtio_blk(struct pci_device *pci) struct virtio_blk_config cfg; vp_get(vdrive->vp, 0, &cfg, sizeof(cfg));
- u32 f = vp_get_features(vdrive->vp); + u64 f = vp_get_features(vdrive->vp); vdrive->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ? cfg.blk_size : DISK_SECTOR_SIZE;
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 3badb15..80b6fcc 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -24,6 +24,39 @@ #include "virtio-pci.h" #include "virtio-ring.h"
+u64 vp_get_features(struct vp_device *vp) +{ + u32 f0, f1; + + if (vp->use_modern) { + vp_write(&vp->common, virtio_pci_common_cfg, device_feature_select, 0); + f0 = vp_read(&vp->common, virtio_pci_common_cfg, device_feature); + vp_write(&vp->common, virtio_pci_common_cfg, device_feature_select, 1); + f1 = vp_read(&vp->common, virtio_pci_common_cfg, device_feature); + } else { + f0 = vp_read(&vp->legacy, virtio_pci_legacy, host_features); + f1 = 0; + } + return ((u64)f1 << 32) | f0; +} + +void vp_set_features(struct vp_device *vp, u64 features) +{ + u32 f0, f1; + + f0 = features; + f1 = features >> 32; + + if (vp->use_modern) { + vp_write(&vp->common, virtio_pci_common_cfg, guest_feature_select, 0); + vp_write(&vp->common, virtio_pci_common_cfg, guest_feature, f0); + vp_write(&vp->common, virtio_pci_common_cfg, guest_feature_select, 1); + vp_write(&vp->common, virtio_pci_common_cfg, guest_feature, f1); + } else { + vp_write(&vp->legacy, virtio_pci_legacy, guest_features, f0); + } +} + int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index a092e86..abb603f 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -125,6 +125,7 @@ struct vp_cap { struct vp_device { unsigned int ioaddr; struct vp_cap common, notify, isr, device, legacy; + u8 use_modern; };
static inline u64 _vp_read(struct vp_cap *cap, u32 offset, u8 size) @@ -213,15 +214,8 @@ static inline void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) _vp_write(_cap, offsetof(_struct, _field), \ sizeof(((_struct *)0)->_field), _var)
-static inline u32 vp_get_features(struct vp_device *vp) -{ - return inl(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_HOST_FEATURES); -} - -static inline void vp_set_features(struct vp_device *vp, u32 features) -{ - outl(features, GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_GUEST_FEATURES); -} +u64 vp_get_features(struct vp_device *vp); +void vp_set_features(struct vp_device *vp, u64 features);
static inline void vp_get(struct vp_device *vp, unsigned offset, void *buf, unsigned len)
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 20 ++++++++++++++++++++ src/hw/virtio-pci.h | 13 ++----------- 2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 80b6fcc..29d0295 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -57,6 +57,26 @@ void vp_set_features(struct vp_device *vp, u64 features) } }
+u8 vp_get_status(struct vp_device *vp) +{ + if (vp->use_modern) { + return vp_read(&vp->common, virtio_pci_common_cfg, device_status); + } else { + return vp_read(&vp->legacy, virtio_pci_legacy, status); + } +} + +void vp_set_status(struct vp_device *vp, u8 status) +{ + if (status == 0) /* reset */ + return; + if (vp->use_modern) { + vp_write(&vp->common, virtio_pci_common_cfg, device_status, status); + } else { + vp_write(&vp->legacy, virtio_pci_legacy, status, status); + } +} + int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index abb603f..f4e0c7d 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -228,17 +228,8 @@ static inline void vp_get(struct vp_device *vp, unsigned offset, ptr[i] = inb(ioaddr + VIRTIO_PCI_CONFIG + offset + i); }
-static inline u8 vp_get_status(struct vp_device *vp) -{ - return inb(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_STATUS); -} - -static inline void vp_set_status(struct vp_device *vp, u8 status) -{ - if (status == 0) /* reset */ - return; - outb(status, GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_STATUS); -} +u8 vp_get_status(struct vp_device *vp); +void vp_set_status(struct vp_device *vp, u8 status);
static inline u8 vp_get_isr(struct vp_device *vp) {
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 9 +++++++++ src/hw/virtio-pci.h | 6 +----- 2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 29d0295..b66326a 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -77,6 +77,15 @@ void vp_set_status(struct vp_device *vp, u8 status) } }
+u8 vp_get_isr(struct vp_device *vp) +{ + if (vp->use_modern) { + return vp_read(&vp->isr, virtio_pci_isr, isr); + } else { + return vp_read(&vp->legacy, virtio_pci_legacy, isr); + } +} + int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index f4e0c7d..470a53f 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -230,11 +230,7 @@ static inline void vp_get(struct vp_device *vp, unsigned offset,
u8 vp_get_status(struct vp_device *vp); void vp_set_status(struct vp_device *vp, u8 status); - -static inline u8 vp_get_isr(struct vp_device *vp) -{ - return inb(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_ISR); -} +u8 vp_get_isr(struct vp_device *vp);
static inline void vp_reset(struct vp_device *vp) {
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 11 +++++++++++ src/hw/virtio-pci.h | 9 +-------- 2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index b66326a..5a599c3 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -86,6 +86,17 @@ u8 vp_get_isr(struct vp_device *vp) } }
+void vp_reset(struct vp_device *vp) +{ + if (vp->use_modern) { + vp_write(&vp->common, virtio_pci_common_cfg, device_status, 0); + vp_read(&vp->isr, virtio_pci_isr, isr); + } else { + vp_write(&vp->legacy, virtio_pci_legacy, status, 0); + vp_read(&vp->legacy, virtio_pci_legacy, isr); + } +} + int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 470a53f..1e2ac4f 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -231,14 +231,7 @@ static inline void vp_get(struct vp_device *vp, unsigned offset, u8 vp_get_status(struct vp_device *vp); void vp_set_status(struct vp_device *vp, u8 status); u8 vp_get_isr(struct vp_device *vp); - -static inline void vp_reset(struct vp_device *vp) -{ - int ioaddr = GET_LOWFLAT(vp->ioaddr); - - outb(0, ioaddr + VIRTIO_PCI_STATUS); - (void)inb(ioaddr + VIRTIO_PCI_ISR); -} +void vp_reset(struct vp_device *vp);
static inline void vp_notify(struct vp_device *vp, int queue_index) {
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 22 +++++++++++++++++++++- src/hw/virtio-pci.h | 7 ++----- src/hw/virtio-ring.c | 2 +- src/hw/virtio-ring.h | 1 + 4 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 5a599c3..3f530e5 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -97,6 +97,24 @@ void vp_reset(struct vp_device *vp) } }
+void vp_notify(struct vp_device *vp, struct vring_virtqueue *vq) +{ + if (vp->use_modern) { + u32 addr = vp->notify.addr + + vq->queue_notify_off * + vp->notify_off_multiplier; + if (vp->notify.is_io) { + outw(vq->queue_index, addr); + } else { + writew((void*)addr, vq->queue_index); + } + dprintf(9, "vp notify %x (%d) -- 0x%x\n", + addr, 2, vq->queue_index); + } else { + vp_write(&vp->legacy, virtio_pci_legacy, queue_notify, vq->queue_index); + } +} + int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { @@ -163,7 +181,7 @@ struct vp_device *vp_init_simple(struct pci_device *pci) struct vp_device *vp = malloc_high(sizeof(*vp)); u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0); struct vp_cap *vp_cap; - u32 addr, offset; + u32 addr, offset, mul; u8 type;
memset(vp, 0, sizeof(*vp)); @@ -176,6 +194,8 @@ struct vp_device *vp_init_simple(struct pci_device *pci) break; case VIRTIO_PCI_CAP_NOTIFY_CFG: vp_cap = &vp->notify; + mul = offsetof(struct virtio_pci_notify_cap, notify_off_multiplier); + vp->notify_off_multiplier = pci_config_readl(pci->bdf, cap + mul); break; case VIRTIO_PCI_CAP_ISR_CFG: vp_cap = &vp->isr; diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 1e2ac4f..7e7a377 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -125,6 +125,7 @@ struct vp_cap { struct vp_device { unsigned int ioaddr; struct vp_cap common, notify, isr, device, legacy; + u32 notify_off_multiplier; u8 use_modern; };
@@ -233,11 +234,6 @@ void vp_set_status(struct vp_device *vp, u8 status); u8 vp_get_isr(struct vp_device *vp); void vp_reset(struct vp_device *vp);
-static inline void vp_notify(struct vp_device *vp, int queue_index) -{ - outw(queue_index, GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_QUEUE_NOTIFY); -} - static inline void vp_del_vq(struct vp_device *vp, int queue_index) { int ioaddr = GET_LOWFLAT(vp->ioaddr); @@ -252,6 +248,7 @@ static inline void vp_del_vq(struct vp_device *vp, int queue_index) struct pci_device; struct vring_virtqueue; struct vp_device *vp_init_simple(struct pci_device *pci); +void vp_notify(struct vp_device *vp, struct vring_virtqueue *vq); int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq); #endif /* _VIRTIO_PCI_H_ */ diff --git a/src/hw/virtio-ring.c b/src/hw/virtio-ring.c index 5c6a32e..6c86c38 100644 --- a/src/hw/virtio-ring.c +++ b/src/hw/virtio-ring.c @@ -145,5 +145,5 @@ void vring_kick(struct vp_device *vp, struct vring_virtqueue *vq, int num_added) smp_wmb(); SET_LOWFLAT(avail->idx, GET_LOWFLAT(avail->idx) + num_added);
- vp_notify(vp, GET_LOWFLAT(vq->queue_index)); + vp_notify(vp, vq); } diff --git a/src/hw/virtio-ring.h b/src/hw/virtio-ring.h index 553a508..7df9004 100644 --- a/src/hw/virtio-ring.h +++ b/src/hw/virtio-ring.h @@ -88,6 +88,7 @@ struct vring_virtqueue { u16 vdata[MAX_QUEUE_NUM]; /* PCI */ int queue_index; + int queue_notify_off; };
struct vring_list {
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.h | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 7e7a377..2790a48 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -234,17 +234,6 @@ void vp_set_status(struct vp_device *vp, u8 status); u8 vp_get_isr(struct vp_device *vp); void vp_reset(struct vp_device *vp);
-static inline void vp_del_vq(struct vp_device *vp, int queue_index) -{ - int ioaddr = GET_LOWFLAT(vp->ioaddr); - - /* select the queue */ - outw(queue_index, ioaddr + VIRTIO_PCI_QUEUE_SEL); - - /* deactivate the queue */ - outl(0, ioaddr + VIRTIO_PCI_QUEUE_PFN); -} - struct pci_device; struct vring_virtqueue; struct vp_device *vp_init_simple(struct pci_device *pci);
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 58 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 15 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 3f530e5..f8f9c4e 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -118,7 +118,6 @@ void vp_notify(struct vp_device *vp, struct vring_virtqueue *vq) int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) { - int ioaddr = GET_LOWFLAT(vp->ioaddr); u16 num;
ASSERT32FLAT(); @@ -129,34 +128,49 @@ int vp_find_vq(struct vp_device *vp, int queue_index, } memset(vq, 0, sizeof(*vq));
+ /* select the queue */ - - outw(queue_index, ioaddr + VIRTIO_PCI_QUEUE_SEL); + if (vp->use_modern) { + vp_write(&vp->common, virtio_pci_common_cfg, queue_select, queue_index); + } else { + vp_write(&vp->legacy, virtio_pci_legacy, queue_sel, queue_index); + }
/* check if the queue is available */ - - num = inw(ioaddr + VIRTIO_PCI_QUEUE_NUM); + if (vp->use_modern) { + num = vp_read(&vp->common, virtio_pci_common_cfg, queue_size); + if (num > MAX_QUEUE_NUM) { + vp_write(&vp->common, virtio_pci_common_cfg, queue_size, + MAX_QUEUE_NUM); + num = vp_read(&vp->common, virtio_pci_common_cfg, queue_size); + } + } else { + num = vp_read(&vp->legacy, virtio_pci_legacy, queue_num); + } if (!num) { dprintf(1, "ERROR: queue size is 0\n"); goto fail; } - if (num > MAX_QUEUE_NUM) { dprintf(1, "ERROR: queue size %d > %d\n", num, MAX_QUEUE_NUM); goto fail; }
/* check if the queue is already active */ - - if (inl(ioaddr + VIRTIO_PCI_QUEUE_PFN)) { - dprintf(1, "ERROR: queue already active\n"); - goto fail; + if (vp->use_modern) { + if (vp_read(&vp->common, virtio_pci_common_cfg, queue_enable)) { + dprintf(1, "ERROR: queue already active\n"); + goto fail; + } + } else { + if (vp_read(&vp->legacy, virtio_pci_legacy, queue_pfn)) { + dprintf(1, "ERROR: queue already active\n"); + goto fail; + } } - vq->queue_index = queue_index;
/* initialize the queue */ - struct vring * vr = &vq->vring; vring_init(vr, num, (unsigned char*)&vq->queue);
@@ -165,9 +179,23 @@ int vp_find_vq(struct vp_device *vp, int queue_index, * NOTE: vr->desc is initialized by vring_init() */
- outl((unsigned long)virt_to_phys(vr->desc) >> PAGE_SHIFT, - ioaddr + VIRTIO_PCI_QUEUE_PFN); - + if (vp->use_modern) { + vp_write(&vp->common, virtio_pci_common_cfg, queue_desc_lo, + (unsigned long)virt_to_phys(vr->desc)); + vp_write(&vp->common, virtio_pci_common_cfg, queue_desc_hi, 0); + vp_write(&vp->common, virtio_pci_common_cfg, queue_avail_lo, + (unsigned long)virt_to_phys(vr->avail)); + vp_write(&vp->common, virtio_pci_common_cfg, queue_avail_hi, 0); + vp_write(&vp->common, virtio_pci_common_cfg, queue_used_lo, + (unsigned long)virt_to_phys(vr->used)); + vp_write(&vp->common, virtio_pci_common_cfg, queue_used_hi, 0); + vp_write(&vp->common, virtio_pci_common_cfg, queue_enable, 1); + vq->queue_notify_off = vp_read(&vp->common, virtio_pci_common_cfg, + queue_notify_off); + } else { + vp_write(&vp->legacy, virtio_pci_legacy, queue_pfn, + (unsigned long)virt_to_phys(vr->desc) >> PAGE_SHIFT); + } return num;
fail:
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-scsi.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c index b208d49..3ee0f49 100644 --- a/src/hw/virtio-scsi.c +++ b/src/hw/virtio-scsi.c @@ -146,14 +146,35 @@ init_virtio_scsi(struct pci_device *pci) pci_bdf_to_dev(bdf)); struct vring_virtqueue *vq = NULL; struct vp_device *vp = vp_init_simple(pci); + u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER; + + if (vp->use_modern) { + u64 features = vp_get_features(vp); + u64 version1 = 1ull << VIRTIO_F_VERSION_1; + if (!(features & version1)) { + dprintf(1, "modern device without virtio_1 feature bit: %x:%x\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); + goto fail; + } + + vp_set_features(vp, version1); + status |= VIRTIO_CONFIG_S_FEATURES_OK; + vp_set_status(vp, status); + if (!(vp_get_status(vp) & VIRTIO_CONFIG_S_FEATURES_OK)) { + dprintf(1, "device didn't accept features: %x:%x\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); + goto fail; + } + } + if (vp_find_vq(vp, 2, &vq) < 0 ) { dprintf(1, "fail to find vq for virtio-scsi %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); goto fail; }
- vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | - VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK); + status |= VIRTIO_CONFIG_S_DRIVER_OK; + vp_set_status(vp, status);
int i, tot; for (tot = 0, i = 0; i < 256; i++)
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-blk.c | 85 +++++++++++++++++++++++++++++++++++++++++------------ src/hw/virtio-pci.h | 13 ++++---- 2 files changed, 72 insertions(+), 26 deletions(-)
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 9e56d42..0a4a677 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -102,6 +102,7 @@ static void init_virtio_blk(struct pci_device *pci) { u16 bdf = pci->bdf; + u8 status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER; dprintf(1, "found virtio-blk at %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); struct virtiodrive_s *vdrive = malloc_fseg(sizeof(*vdrive)); @@ -113,42 +114,88 @@ init_virtio_blk(struct pci_device *pci) vdrive->drive.type = DTYPE_VIRTIO_BLK; vdrive->drive.cntl_id = bdf;
- vdrive->vp = vp_init_simple(pci); + struct vp_device *vp = vdrive->vp = vp_init_simple(pci); if (vp_find_vq(vdrive->vp, 0, &vdrive->vq) < 0 ) { dprintf(1, "fail to find vq for virtio-blk %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); goto fail; }
- struct virtio_blk_config cfg; - vp_get(vdrive->vp, 0, &cfg, sizeof(cfg)); + if (vp->use_modern) { + u64 features = vp_get_features(vp); + u64 version1 = 1ull << VIRTIO_F_VERSION_1; + u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE; + if (!(features & version1)) { + dprintf(1, "modern device without virtio_1 feature bit: %x:%x\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); + goto fail; + }
- u64 f = vp_get_features(vdrive->vp); - vdrive->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ? - cfg.blk_size : DISK_SECTOR_SIZE; + features = features & (version1 | blk_size); + vp_set_features(vp, features); + status |= VIRTIO_CONFIG_S_FEATURES_OK; + vp_set_status(vp, status); + if (!(vp_get_status(vp) & VIRTIO_CONFIG_S_FEATURES_OK)) { + dprintf(1, "device didn't accept features: %x:%x\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); + goto fail; + }
- vdrive->drive.sectors = cfg.capacity; - dprintf(3, "virtio-blk %x:%x blksize=%d sectors=%u\n", - pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), - vdrive->drive.blksize, (u32)vdrive->drive.sectors); + vdrive->drive.sectors = + vp_read(&vp->device, struct virtio_blk_config, capacity); + if (features & blk_size) { + vdrive->drive.blksize = + vp_read(&vp->device, struct virtio_blk_config, blk_size); + } else { + vdrive->drive.blksize = DISK_SECTOR_SIZE; + } + if (vdrive->drive.blksize != DISK_SECTOR_SIZE) { + dprintf(1, "virtio-blk %x:%x block size %d is unsupported\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), + vdrive->drive.blksize); + goto fail; + } + dprintf(3, "virtio-blk %x:%x blksize=%d sectors=%u\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), + vdrive->drive.blksize, (u32)vdrive->drive.sectors); + + vdrive->drive.pchs.cylinder = + vp_read(&vp->device, struct virtio_blk_config, cylinders); + vdrive->drive.pchs.head = + vp_read(&vp->device, struct virtio_blk_config, heads); + vdrive->drive.pchs.sector = + vp_read(&vp->device, struct virtio_blk_config, sectors); + } else { + struct virtio_blk_config cfg; + vp_get_legacy(vdrive->vp, 0, &cfg, sizeof(cfg));
- if (vdrive->drive.blksize != DISK_SECTOR_SIZE) { - dprintf(1, "virtio-blk %x:%x block size %d is unsupported\n", + u64 f = vp_get_features(vdrive->vp); + vdrive->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ? + cfg.blk_size : DISK_SECTOR_SIZE; + + vdrive->drive.sectors = cfg.capacity; + dprintf(3, "virtio-blk %x:%x blksize=%d sectors=%u\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), - vdrive->drive.blksize); - goto fail; + vdrive->drive.blksize, (u32)vdrive->drive.sectors); + + if (vdrive->drive.blksize != DISK_SECTOR_SIZE) { + dprintf(1, "virtio-blk %x:%x block size %d is unsupported\n", + pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf), + vdrive->drive.blksize); + goto fail; + } + vdrive->drive.pchs.cylinder = cfg.cylinders; + vdrive->drive.pchs.head = cfg.heads; + vdrive->drive.pchs.sector = cfg.sectors; }
- vdrive->drive.pchs.cylinder = cfg.cylinders; - vdrive->drive.pchs.head = cfg.heads; - vdrive->drive.pchs.sector = cfg.sectors; char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%x:%x", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf));
boot_add_hd(&vdrive->drive, desc, bootprio_find_pci_device(pci));
- vp_set_status(vdrive->vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | - VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK); + status |= VIRTIO_CONFIG_S_DRIVER_OK; + vp_set_status(vdrive->vp, status); return;
fail: diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index 2790a48..f1d5e3e 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -218,15 +218,14 @@ static inline void _vp_write(struct vp_cap *cap, u32 offset, u8 size, u64 var) u64 vp_get_features(struct vp_device *vp); void vp_set_features(struct vp_device *vp, u64 features);
-static inline void vp_get(struct vp_device *vp, unsigned offset, - void *buf, unsigned len) +static inline void vp_get_legacy(struct vp_device *vp, unsigned offset, + void *buf, unsigned len) { - int ioaddr = GET_LOWFLAT(vp->ioaddr); - u8 *ptr = buf; - unsigned i; + u8 *ptr = buf; + unsigned i;
- for (i = 0; i < len; i++) - ptr[i] = inb(ioaddr + VIRTIO_PCI_CONFIG + offset + i); + for (i = 0; i < len; i++) + ptr[i] = inb(vp->ioaddr + VIRTIO_PCI_CONFIG + offset + i); }
u8 vp_get_status(struct vp_device *vp);
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index f8f9c4e..1186603 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -261,16 +261,19 @@ struct vp_device *vp_init_simple(struct pci_device *pci) }
if (vp->common.cap && vp->notify.cap && vp->isr.cap && vp->device.cap) { - dprintf(1, "pci dev %x:%x supports virtio 1.0\n", + dprintf(1, "pci dev %x:%x using modern (1.0) virtio mode\n", pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf)); + vp->use_modern = 1; + } else { + dprintf(1, "pci dev %x:%x using legacy (0.9.5) virtio mode\n", + pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf)); + vp->legacy.bar = 0; + vp->legacy.addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & + PCI_BASE_ADDRESS_IO_MASK; + vp->legacy.is_io = 1; + vp->ioaddr = vp->legacy.addr; /* temporary */ }
- vp->legacy.bar = 0; - vp->legacy.addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & - PCI_BASE_ADDRESS_IO_MASK; - vp->legacy.is_io = 1; - vp->ioaddr = vp->legacy.addr; /* temporary */ - vp_reset(vp); pci_config_maskw(pci->bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER); vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE |
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/pci_ids.h | 8 ++++++-- src/hw/virtio-blk.c | 5 +++-- src/hw/virtio-scsi.c | 5 +++-- 3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/hw/pci_ids.h b/src/hw/pci_ids.h index 1cd4f72..cdf9b3c 100644 --- a/src/hw/pci_ids.h +++ b/src/hw/pci_ids.h @@ -2616,8 +2616,12 @@ #define PCI_DEVICE_ID_RME_DIGI32_8 0x9898
#define PCI_VENDOR_ID_REDHAT_QUMRANET 0x1af4 -#define PCI_DEVICE_ID_VIRTIO_BLK 0x1001 -#define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004 +/* virtio 0.9.5 ids (legacy/transitional devices) */ +#define PCI_DEVICE_ID_VIRTIO_BLK_09 0x1001 +#define PCI_DEVICE_ID_VIRTIO_SCSI_09 0x1004 +/* virtio 1.0 ids (modern devices) */ +#define PCI_DEVICE_ID_VIRTIO_BLK_10 0x1042 +#define PCI_DEVICE_ID_VIRTIO_SCSI_10 0x1048
#define PCI_VENDOR_ID_VMWARE 0x15ad #define PCI_DEVICE_ID_VMWARE_PVSCSI 0x07C0 diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 0a4a677..a037cbe 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -216,8 +216,9 @@ virtio_blk_setup(void)
struct pci_device *pci; foreachpci(pci) { - if (pci->vendor != PCI_VENDOR_ID_REDHAT_QUMRANET - || pci->device != PCI_DEVICE_ID_VIRTIO_BLK) + if (pci->vendor != PCI_VENDOR_ID_REDHAT_QUMRANET || + (pci->device != PCI_DEVICE_ID_VIRTIO_BLK_09 && + pci->device != PCI_DEVICE_ID_VIRTIO_BLK_10)) continue; init_virtio_blk(pci); } diff --git a/src/hw/virtio-scsi.c b/src/hw/virtio-scsi.c index 3ee0f49..946fe5f 100644 --- a/src/hw/virtio-scsi.c +++ b/src/hw/virtio-scsi.c @@ -202,8 +202,9 @@ virtio_scsi_setup(void)
struct pci_device *pci; foreachpci(pci) { - if (pci->vendor != PCI_VENDOR_ID_REDHAT_QUMRANET - || pci->device != PCI_DEVICE_ID_VIRTIO_SCSI) + if (pci->vendor != PCI_VENDOR_ID_REDHAT_QUMRANET || + (pci->device != PCI_DEVICE_ID_VIRTIO_SCSI_09 && + pci->device != PCI_DEVICE_ID_VIRTIO_SCSI_10)) continue; init_virtio_scsi(pci); }
Now that all code is switched over to use vp_read/write we can drop the ioaddr field from vp_device and the offset #defines.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com --- src/hw/virtio-pci.c | 1 - src/hw/virtio-pci.h | 33 +-------------------------------- 2 files changed, 1 insertion(+), 33 deletions(-)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 1186603..a53db7b 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -271,7 +271,6 @@ struct vp_device *vp_init_simple(struct pci_device *pci) vp->legacy.addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK; vp->legacy.is_io = 1; - vp->ioaddr = vp->legacy.addr; /* temporary */ }
vp_reset(vp); diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index f1d5e3e..e017ce8 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -4,39 +4,9 @@ #include "x86.h" // inl #include "biosvar.h" // GET_LOWFLAT
-/* A 32-bit r/o bitmask of the features supported by the host */ -#define VIRTIO_PCI_HOST_FEATURES 0 - -/* A 32-bit r/w bitmask of features activated by the guest */ -#define VIRTIO_PCI_GUEST_FEATURES 4 - -/* A 32-bit r/w PFN for the currently selected queue */ -#define VIRTIO_PCI_QUEUE_PFN 8 - -/* A 16-bit r/o queue size for the currently selected queue */ -#define VIRTIO_PCI_QUEUE_NUM 12 - -/* A 16-bit r/w queue selector */ -#define VIRTIO_PCI_QUEUE_SEL 14 - -/* A 16-bit r/w queue notifier */ -#define VIRTIO_PCI_QUEUE_NOTIFY 16 - -/* An 8-bit device status register. */ -#define VIRTIO_PCI_STATUS 18 - -/* An 8-bit r/o interrupt status register. Reading the value will return the - * current contents of the ISR and will also clear it. This is effectively - * a read-and-acknowledge. */ -#define VIRTIO_PCI_ISR 19 - /* The bit of the ISR which indicates a device configuration change. */ #define VIRTIO_PCI_ISR_CONFIG 0x2
-/* The remaining space is defined by each driver as the per-driver - * configuration space */ -#define VIRTIO_PCI_CONFIG 20 - /* Virtio ABI version, this must match exactly */ #define VIRTIO_PCI_ABI_VERSION 0
@@ -123,7 +93,6 @@ struct vp_cap { };
struct vp_device { - unsigned int ioaddr; struct vp_cap common, notify, isr, device, legacy; u32 notify_off_multiplier; u8 use_modern; @@ -225,7 +194,7 @@ static inline void vp_get_legacy(struct vp_device *vp, unsigned offset, unsigned i;
for (i = 0; i < len; i++) - ptr[i] = inb(vp->ioaddr + VIRTIO_PCI_CONFIG + offset + i); + ptr[i] = vp_read(&vp->legacy, virtio_pci_legacy, device[i]); }
u8 vp_get_status(struct vp_device *vp);
On Tue, Jun 30, 2015 at 10:38:54AM +0200, Gerd Hoffmann wrote:
For virtio 1.0 support we will need more state than just the (legacy mode) ioaddr for each virtio-pci device. Prepare for that by adding a new struct for it. For now it carries the ioaddr only.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/hw/virtio-blk.c | 20 ++++++++++---------- src/hw/virtio-pci.c | 15 +++++++++------ src/hw/virtio-pci.h | 46 +++++++++++++++++++++++++++------------------- src/hw/virtio-ring.c | 4 ++-- src/hw/virtio-ring.h | 3 ++- src/hw/virtio-scsi.c | 32 +++++++++++++++++--------------- 6 files changed, 67 insertions(+), 53 deletions(-)
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 15ac171..13cf09a 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -25,7 +25,7 @@ struct virtiodrive_s { struct drive_s drive; struct vring_virtqueue *vq;
- u16 ioaddr;
- struct vp_device *vp;
};
static int @@ -60,7 +60,7 @@ virtio_blk_op(struct disk_op_s *op, int write) vring_add_buf(vq, sg, 2, 1, 0, 0); else vring_add_buf(vq, sg, 1, 2, 0, 0);
- vring_kick(GET_GLOBALFLAT(vdrive_gf->ioaddr), vq, 1);
vring_kick(GET_GLOBALFLAT(vdrive_gf->vp), vq, 1);
/* Wait for reply */ while (!vring_more_used(vq))
@@ -72,7 +72,7 @@ virtio_blk_op(struct disk_op_s *op, int write) /* Clear interrupt status register. Avoid leaving interrupts stuck if * VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised. */
- vp_get_isr(GET_GLOBALFLAT(vdrive_gf->ioaddr));
vp_get_isr(GET_GLOBALFLAT(vdrive_gf->vp));
return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK;
} @@ -113,18 +113,17 @@ init_virtio_blk(struct pci_device *pci) vdrive->drive.type = DTYPE_VIRTIO_BLK; vdrive->drive.cntl_id = bdf;
- u16 ioaddr = vp_init_simple(bdf);
- vdrive->ioaddr = ioaddr;
- if (vp_find_vq(ioaddr, 0, &vdrive->vq) < 0 ) {
vdrive->vp = vp_init_simple(bdf);
if (vp_find_vq(vdrive->vp, 0, &vdrive->vq) < 0 ) { dprintf(1, "fail to find vq for virtio-blk %x:%x\n", pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf)); goto fail; }
struct virtio_blk_config cfg;
- vp_get(ioaddr, 0, &cfg, sizeof(cfg));
- vp_get(vdrive->vp, 0, &cfg, sizeof(cfg));
- u32 f = vp_get_features(ioaddr);
- u32 f = vp_get_features(vdrive->vp); vdrive->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ? cfg.blk_size : DISK_SECTOR_SIZE;
@@ -148,12 +147,13 @@ init_virtio_blk(struct pci_device *pci)
boot_add_hd(&vdrive->drive, desc, bootprio_find_pci_device(pci));
- vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
- vp_set_status(vdrive->vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK); return;
fail:
- vp_reset(ioaddr);
- vp_reset(vdrive->vp);
- free(vdrive->vp); free(vdrive->vq); free(vdrive);
} diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index b9b3ab1..3cd478d 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -24,9 +24,10 @@ #include "virtio-pci.h" #include "virtio-ring.h"
-int vp_find_vq(unsigned int ioaddr, int queue_index, +int vp_find_vq(struct vp_device *vp, int queue_index, struct vring_virtqueue **p_vq) {
int ioaddr = GET_LOWFLAT(vp->ioaddr); u16 num;
ASSERT32FLAT();
@@ -84,14 +85,16 @@ fail: return -1; }
-u16 vp_init_simple(u16 bdf) +struct vp_device *vp_init_simple(u16 bdf) {
- u16 ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) &
- struct vp_device *vp = malloc_high(sizeof(*vp));
- vp->ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK;
The malloc_high() call can fail - so the code needs to check if it returns NULL. (It really can fail in practice and if code writes to NULL it will corrupt the 16bit isr table, which is painful to debug.)
Why not pass in a vp_device* to vp_init_simple() and have vp_init_simple() fill it. Then init_virtio_scsi() can stack allocate a temporary vp_device and virtio_scsi_add_lun() can memcpy it to virtio_lun_s.
- vp_reset(ioaddr);
- vp_reset(vp); pci_config_maskw(bdf, PCI_COMMAND, 0, PCI_COMMAND_MASTER);
- vp_set_status(ioaddr, VIRTIO_CONFIG_S_ACKNOWLEDGE |
- vp_set_status(vp, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER );
- return ioaddr;
- return vp;
} diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index bc04b03..47bef3d 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -2,6 +2,7 @@ #define _VIRTIO_PCI_H
#include "x86.h" // inl +#include "biosvar.h" // GET_LOWFLAT
/* A 32-bit r/o bitmask of the features supported by the host */ #define VIRTIO_PCI_HOST_FEATURES 0 @@ -39,19 +40,24 @@ /* Virtio ABI version, this must match exactly */ #define VIRTIO_PCI_ABI_VERSION 0
-static inline u32 vp_get_features(unsigned int ioaddr) +struct vp_device {
- unsigned int ioaddr;
+};
+static inline u32 vp_get_features(struct vp_device *vp) {
- return inl(ioaddr + VIRTIO_PCI_HOST_FEATURES);
- return inl(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_HOST_FEATURES);
}
These GET_LOWFLAT() calls are confusing (as they are only valid for memory allocated with malloc_low() ). Granted, they are no-ops in 32bit mode, but they are still confusing.
The rest of the series looks good to me. (Two other minor points would be that patch 9 should be squashed into patch 6, and I have some comments on patch 2.)
Thanks. -Kevin
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
I think this patch is missing a "if (!MODESEGMENT)" check in src/hw/blockcmd.c - otherwise the virtio scsi code will still be included in the 16bit section.
Also, this patch (or a follow up patch) should update the Makefile to not compile the virtio code in 16bit mode.
-Kevin
On Di, 2015-06-30 at 10:36 -0400, Kevin O'Connor wrote:
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
I think this patch is missing a "if (!MODESEGMENT)" check in src/hw/blockcmd.c - otherwise the virtio scsi code will still be included in the 16bit section.
Also, this patch (or a follow up patch) should update the Makefile to not compile the virtio code in 16bit mode.
Both fixed.
cheers, Gerd
-u16 vp_init_simple(u16 bdf) +struct vp_device *vp_init_simple(u16 bdf) {
- u16 ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) &
- struct vp_device *vp = malloc_high(sizeof(*vp));
- vp->ioaddr = pci_config_readl(bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK;
The malloc_high() call can fail - so the code needs to check if it returns NULL. (It really can fail in practice and if code writes to NULL it will corrupt the 16bit isr table, which is painful to debug.)
Why not pass in a vp_device* to vp_init_simple() and have vp_init_simple() fill it. Then init_virtio_scsi() can stack allocate a temporary vp_device and virtio_scsi_add_lun() can memcpy it to virtio_lun_s.
Moved allocation to callers now. virtio-blk embeds the struct, so there is no extra malloc needed. virtio-scsi continues to allocate it separately and stick a pointer to each device, which makes sense IMHO considering that the struct will grow with the following patches.
+static inline u32 vp_get_features(struct vp_device *vp) {
- return inl(ioaddr + VIRTIO_PCI_HOST_FEATURES);
- return inl(GET_LOWFLAT(vp->ioaddr) + VIRTIO_PCI_HOST_FEATURES);
}
These GET_LOWFLAT() calls are confusing (as they are only valid for memory allocated with malloc_low() ). Granted, they are no-ops in 32bit mode, but they are still confusing.
Leftover oddity from the initial revision of the patch, which was ordered before the 32bit switchover patch. The following patches which switch over all the vp_*() functions to use vp_{read,write) clean that up.
cheers, Gerd
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Is there an advantage to running them in a 16 bit mode?
If yes - it's actually possible to access virtio in 16 bit mode because all virtio devices have a special capability which maps memory into pci config space. So you can poke memory (slowly) through the cf8/cfc registers.
It's missing in QEMU but that's a clear spec violation: spec says it's mandatory.
I'll code it up and send an RFC shortly.
src/block.c | 8 +++++--- src/hw/virtio-blk.c | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/block.c b/src/block.c index 3f7ecb1..a9b9851 100644 --- a/src/block.c +++ b/src/block.c @@ -503,8 +503,10 @@ process_op(struct disk_op_s *op) case DTYPE_CDEMU: ret = process_cdemu_op(op); break;
- case DTYPE_VIRTIO_BLK:
ret = process_virtio_blk_op(op);
- case DTYPE_VIRTIO_BLK: ;
extern void _cfunc32flat_process_virtio_blk_op(void);ret = call32(_cfunc32flat_process_virtio_blk_op, (u32)MAKE_FLATPTR(GET_SEG(SS), op), DISK_RET_EPARAM); break;@@ -526,7 +528,6 @@ process_op(struct disk_op_s *op) break; case DTYPE_USB: case DTYPE_UAS:
- case DTYPE_VIRTIO_SCSI: case DTYPE_LSI_SCSI: case DTYPE_ESP_SCSI: case DTYPE_MEGASAS:
@@ -534,6 +535,7 @@ process_op(struct disk_op_s *op) break; case DTYPE_USB_32: case DTYPE_UAS_32:
- case DTYPE_VIRTIO_SCSI: case DTYPE_PVSCSI: ; extern void _cfunc32flat_scsi_process_op(void); ret = call32(_cfunc32flat_scsi_process_op
diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index e2dbd3c..15ac171 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -77,7 +77,7 @@ virtio_blk_op(struct disk_op_s *op, int write) return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK; }
-int +int VISIBLE32FLAT process_virtio_blk_op(struct disk_op_s *op) { if (! CONFIG_VIRTIO_BLK) -- 1.8.3.1
On Tue, Jun 30, 2015 at 10:38:58AM +0200, Gerd Hoffmann wrote:
virtio 1.0 specifies the location of the various virtio regions using pci capabilities. Look them up and store the results.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
src/hw/virtio-pci.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++ src/hw/virtio-pci.h | 8 ++++++++ 2 files changed, 63 insertions(+)
diff --git a/src/hw/virtio-pci.c b/src/hw/virtio-pci.c index 4971021..0acf65f 100644 --- a/src/hw/virtio-pci.c +++ b/src/hw/virtio-pci.c @@ -88,6 +88,61 @@ fail: struct vp_device *vp_init_simple(struct pci_device *pci) { struct vp_device *vp = malloc_high(sizeof(*vp));
u8 cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, 0);
struct vp_cap *vp_cap;
u32 addr, offset;
u8 type;
memset(vp, 0, sizeof(*vp));
while (cap != 0) {
type = pci_config_readb(pci->bdf, cap +offsetof(struct virtio_pci_cap, cfg_type));switch (type) {case VIRTIO_PCI_CAP_COMMON_CFG:vp_cap = &vp->common;break;case VIRTIO_PCI_CAP_NOTIFY_CFG:vp_cap = &vp->notify;break;case VIRTIO_PCI_CAP_ISR_CFG:vp_cap = &vp->isr;break;case VIRTIO_PCI_CAP_DEVICE_CFG:vp_cap = &vp->device;break;default:vp_cap = NULL;break;}if (vp_cap) {vp_cap->cap = cap;vp_cap->bar = pci_config_readb(pci->bdf, cap +offsetof(struct virtio_pci_cap, bar));offset = pci_config_readl(pci->bdf, cap +offsetof(struct virtio_pci_cap, offset));addr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0 + 4 * vp_cap->bar);if (addr & PCI_BASE_ADDRESS_SPACE_IO) {vp_cap->is_io = 1;addr &= PCI_BASE_ADDRESS_IO_MASK;} else {vp_cap->is_io = 0;addr &= PCI_BASE_ADDRESS_MEM_MASK;}vp_cap->addr = addr + offset;dprintf(3, "pci dev %x:%x virtio cap at 0x%x type %d ""bar %d at 0x%08x off +0x%04x [%s]\n",pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf),vp_cap->cap, type, vp_cap->bar, addr, offset,vp_cap->is_io ? "io" : "mmio");}cap = pci_find_capability(pci, PCI_CAP_ID_VNDR, cap);}
if (vp->common.cap && vp->notify.cap && vp->isr.cap && vp->device.cap) {
dprintf(1, "pci dev %x:%x supports virtio 1.0\n",pci_bdf_to_bus(pci->bdf), pci_bdf_to_dev(pci->bdf));}
vp->ioaddr = pci_config_readl(pci->bdf, PCI_BASE_ADDRESS_0) & PCI_BASE_ADDRESS_IO_MASK;
Hmm this seems to violate this rule in the spec:
The driver SHOULD use the first instance of each virtio structure type they can support.
"can support" here means that bios was able to allocate it during enumeration.
For example there could be both IO and memory, in this order you need to check that IO/memory got enabled (in theory, also that they are within parent bridge's windows - used by some guests, but seabios doesn't disable memmory/io in this strange way).
diff --git a/src/hw/virtio-pci.h b/src/hw/virtio-pci.h index bc2eb05..0f57ca8 100644 --- a/src/hw/virtio-pci.h +++ b/src/hw/virtio-pci.h @@ -115,8 +115,16 @@ typedef struct virtio_pci_isr {
/* --- driver structs ----------------------------------------------- */
+struct vp_cap {
- u32 addr;
- u8 cap;
- u8 bar;
- u8 is_io;
+};
struct vp_device { unsigned int ioaddr;
- struct vp_cap common, notify, isr, device;
};
static inline u32 vp_get_features(struct vp_device *vp)
1.8.3.1
On Tue, Jun 30, 2015 at 10:38:51AM +0200, Gerd Hoffmann wrote:
Hi,
This patch series adds virtio 1.0 support to the virtio blk and scsi drivers in seabios. With this series applied seabios happily boots in virtio 1.0 mode from both transitional and modern devices.
Tested with Fedora 22 guest, booting from virtio-scsi cdrom (live iso), virtio-scsi disk and virtio-blk disk.
The patches are also available in the git repository at: git://git.kraxel.org/seabios virtio
I went over this and this looks very good to me. Thanks for working on this! I found a couple of issues, and responded to the specific patches.
v2 changes:
- rename vp_modern_{read_write} to vp_{read,write}
- switch legacy virtio code to vp_{read,write} too.
- make vp_read return the values.
Gerd Hoffmann (22): pci: allow to loop over capabilities virtio: run drivers in 32bit mode virtio: add struct vp_device virtio: pass struct pci_device to vp_init_simple virtio: add version 1.0 structs and #defines virtio: add version 0.9.5 struct virtio: find version 1.0 virtio capabilities virtio: create vp_cap struct for legacy bar virtio: add version 0.9.5 struct [fixup] virtio: add read/write functions and macros virtio: make features 64bit, support version 1.0 features virtio: add version 1.0 support to vp_{get,set}_status virtio: add version 1.0 support to vp_get_isr virtio: add version 1.0 support to vp_reset virtio: add version 1.0 support to vp_notify virtio: remove unused vp_del_vq virtio: add version 1.0 support to vp_find_vq virtio-scsi: fix initialization for version 1.0 virtio-blk: fix initialization for version 1.0 virtio: use version 1.0 if available (flip the big switch) virtio: also probe version 1.0 pci ids virtio: legacy cleanup
src/block.c | 8 +- src/fw/pciinit.c | 4 +- src/hw/pci.c | 11 ++- src/hw/pci.h | 2 +- src/hw/pci_ids.h | 8 +- src/hw/virtio-blk.c | 104 +++++++++++++++------ src/hw/virtio-pci.c | 228 +++++++++++++++++++++++++++++++++++++++++----- src/hw/virtio-pci.h | 252 ++++++++++++++++++++++++++++++++++++--------------- src/hw/virtio-ring.c | 4 +- src/hw/virtio-ring.h | 9 +- src/hw/virtio-scsi.c | 60 ++++++++---- 11 files changed, 535 insertions(+), 155 deletions(-)
-- 1.8.3.1
Hi,
Hmm this seems to violate this rule in the spec:
The driver SHOULD use the first instance of each virtio structure type they can support.
"can support" here means that bios was able to allocate it during enumeration.
For example there could be both IO and memory, in this order you need to check that IO/memory got enabled (in theory, also that they are within parent bridge's windows - used by some guests, but seabios doesn't disable memmory/io in this strange way).
Yes, seabios always allocates both mem and io. So this incremental fix ...
@@ -234,7 +234,7 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) vp_cap = NULL; break; } - if (vp_cap) { + if (vp_cap && !vp_cap->cap) { vp_cap->cap = cap; vp_cap->bar = pci_config_readb(pci->bdf, cap + offsetof(struct virtio_pci_cap, bar));
... makes seabios use the first not the last and should do the trick, right?
cheers, Gerd
On Wed, Jul 01, 2015 at 02:24:02PM +0200, Gerd Hoffmann wrote:
Hi,
Hmm this seems to violate this rule in the spec:
The driver SHOULD use the first instance of each virtio structure type they can support.
"can support" here means that bios was able to allocate it during enumeration.
For example there could be both IO and memory, in this order you need to check that IO/memory got enabled (in theory, also that they are within parent bridge's windows - used by some guests, but seabios doesn't disable memmory/io in this strange way).
Yes, seabios always allocates both mem and io.
What if it can't? E.g. too many devices.
So this incremental fix ...
@@ -234,7 +234,7 @@ void vp_init_simple(struct vp_device *vp, struct pci_device *pci) vp_cap = NULL; break; }
if (vp_cap) {
if (vp_cap && !vp_cap->cap) { vp_cap->cap = cap; vp_cap->bar = pci_config_readb(pci->bdf, cap + offsetof(structvirtio_pci_cap, bar));
... makes seabios use the first not the last and should do the trick, right?
cheers, Gerd
On Mi, 2015-07-01 at 10:08 +0200, Michael S. Tsirkin wrote:
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Is there an advantage to running them in a 16 bit mode?
Not really any more. Switching from 32bit mode back to whatever-was-active-before used to be problematic before we had smm mode support. In theory. Because you can't save/restore the complete x86 processor state. In practice we had surprisingly few problems, appearently linux boot loaders simply don't play dirty tricks.
cheers, Gerd
Hi,
Yes, seabios always allocates both mem and io.
What if it can't? E.g. too many devices.
First tries to move 64bit bars above 64g. Guess we better should exclude virtio devices here (like we do for xhci already).
Failing that it'll panic.
cheers, Gerd
On Wed, Jul 01, 2015 at 02:49:54PM +0200, Gerd Hoffmann wrote:
Hi,
Yes, seabios always allocates both mem and io.
What if it can't? E.g. too many devices.
First tries to move 64bit bars above 64g. Guess we better should exclude virtio devices here (like we do for xhci already).
Why? You can use config capability.
Failing that it'll panic.
cheers, Gerd
IO can't well go above 4G :) So eventually we'll make these express devices. For express the spec explicitly says devices must still work if IO is disabled. At that point maybe teaching bios to disable IO and keep working will have value.
On Wed, Jul 01, 2015 at 02:30:29PM +0200, Gerd Hoffmann wrote:
On Mi, 2015-07-01 at 10:08 +0200, Michael S. Tsirkin wrote:
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Is there an advantage to running them in a 16 bit mode?
Not really any more. Switching from 32bit mode back to whatever-was-active-before used to be problematic before we had smm mode support. In theory. Because you can't save/restore the complete x86 processor state. In practice we had surprisingly few problems, appearently linux boot loaders simply don't play dirty tricks.
cheers, Gerd
Interesting. Might not be true for non-linux loaders :)
Anyway we support SSM now so all should be well, right?
On Wed, Jul 01, 2015 at 03:50:50PM +0200, Michael S. Tsirkin wrote:
On Wed, Jul 01, 2015 at 02:30:29PM +0200, Gerd Hoffmann wrote:
On Mi, 2015-07-01 at 10:08 +0200, Michael S. Tsirkin wrote:
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Is there an advantage to running them in a 16 bit mode?
Not really any more. Switching from 32bit mode back to whatever-was-active-before used to be problematic before we had smm mode support. In theory. Because you can't save/restore the complete x86 processor state. In practice we had surprisingly few problems, appearently linux boot loaders simply don't play dirty tricks.
cheers, Gerd
Interesting. Might not be true for non-linux loaders :)
Anyway we support SSM now so all should be well, right?
Also a question: what's cheaper on kvm: use SMM to save/restore or access device through the config cap?
-- MST
Hi,
Not really any more. Switching from 32bit mode back to whatever-was-active-before used to be problematic before we had smm mode support. In theory. Because you can't save/restore the complete x86 processor state. In practice we had surprisingly few problems, appearently linux boot loaders simply don't play dirty tricks.
Interesting. Might not be true for non-linux loaders :)
No problems with modern windows too. DOS with emm386 being active could be more challenging.
Didn't dig that deep though, and as the ide driver runs in 16bit mode still there is an easy way around any isses should they pop up.
Anyway we support SSM now so all should be well, right?
Yes, with smm this issue is gone.
cheers, Gerd
On Wed, Jul 01, 2015 at 03:50:50PM +0200, Michael S. Tsirkin wrote:
On Wed, Jul 01, 2015 at 02:30:29PM +0200, Gerd Hoffmann wrote:
On Mi, 2015-07-01 at 10:08 +0200, Michael S. Tsirkin wrote:
On Tue, Jun 30, 2015 at 10:38:53AM +0200, Gerd Hoffmann wrote:
virtio version 1.0 registers can (and actually do in the qemu implementation) live in mmio space. So we must run the blk and scsi virtio drivers in 32bit mode, otherwise we can't access them.
This also allows to drop a bunch of GET_LOWFLAT calls from the virtio code in the following patches.
Signed-off-by: Gerd Hoffmann kraxel@redhat.com
Is there an advantage to running them in a 16 bit mode?
Not really any more. Switching from 32bit mode back to whatever-was-active-before used to be problematic before we had smm mode support. In theory. Because you can't save/restore the complete x86 processor state. In practice we had surprisingly few problems, appearently linux boot loaders simply don't play dirty tricks.
cheers, Gerd
Interesting. Might not be true for non-linux loaders :)
Without SMM, the only issue I've seen with "thunking" to 32bit mode was DOS-era programs (and in particular those that used emm386).
Anyway we support SSM now so all should be well, right?
With SMM, I haven't seen any problems. I don't doubt that some DOS-era programs might still have issues though. Also, I haven't tested Paolo's kvm smm support yet.
SeaBIOS already runs a number of drivers exclusively in 32bit mode: ahci, xhci, sdcard, ohci disks, pvscsi. Even without smm support, virtio is likely a good candidate to move to 32bit mode as I don't think there is a use case for running DOS-era programs on virtio disks. (Using 32bit only drivers results in smaller and easier to maintain code - indeed we'd like to move exclusively to 32bit drivers in the future.)
Cheers, -Kevin
-
Gerd Hoffmann -
Kevin O'Connor -
Michael S. Tsirkin