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 e2dbd3c..ef99d57 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..a1e9939 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_low(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); }
On Thu, Jun 25, 2015 at 10:17:21AM +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.
It sounds like this patch is in preparation for some other patches. It would help to see those other patches as well.
[...]
-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_low(sizeof(*vp));
This moves the ioaddr from "fseg" memory to "low" memory. Is the ioaddr something that is going to change during runtime in the future, or is this change because some other state in 'vp_device' requires read/write support at runtime?
-Kevin
On Do, 2015-06-25 at 09:10 -0400, Kevin O'Connor wrote:
On Thu, Jun 25, 2015 at 10:17:21AM +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.
It sounds like this patch is in preparation for some other patches. It would help to see those other patches as well.
Yes. Still coding though ...
[...]
-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_low(sizeof(*vp));
This moves the ioaddr from "fseg" memory to "low" memory. Is the ioaddr something that is going to change during runtime in the future,
No.
or is this change because some other state in 'vp_device' requires read/write support at runtime?
We need more information, for virtio-1.0 mode. Not fully sure yet there is anything which changes after init. I don't think so ...
cheers, Gerd
Hi,
It sounds like this patch is in preparation for some other patches. It would help to see those other patches as well.
Yes. Still coding though ...
Patch series adds support for virtio-1.0. Still in progress. virtio-scsi works now (tested using live iso boot), so I've pushed a sneak preview:
https://www.kraxel.org/cgit/seabios/log/?h=virtio
Patches are in very small pieces, I probably squash them a bit before posting to the list, especially the ones adding virtio 1.0 #defines and structs to the headers.
They also need cleanups. I carefully added loads of GET_LOWFLAT() macros. After doing so I've noticed I have to run this in 32bit mode anyway because otherwise readl+friends don't work ...
They have some verbose debug output atm.
virtio-blk is still to be done.
enjoy, Gerd
On Fr, 2015-06-26 at 12:19 +0200, Gerd Hoffmann wrote:
Hi,
virtio-blk is still to be done.
Works too now. Cleanups to be done next week.
Started cleanups, pushed updates to the branch: https://www.kraxel.org/cgit/seabios/log/?h=virtio
Patches are in a better state now. Finishing touches and posting next week. Really.
With this branch booting Fedora 22 with non-transitional virtio devices (i.e. disable-legacy=on,disable-modern=off) works fine.
cheers, Gerd
On Fri, Jun 26, 2015 at 10:46:10AM +0200, Gerd Hoffmann wrote:
Hi,
It sounds like this patch is in preparation for some other patches. It would help to see those other patches as well.
Yes. Still coding though ...
Patch series adds support for virtio-1.0. Still in progress. virtio-scsi works now (tested using live iso boot), so I've pushed a sneak preview:
https://www.kraxel.org/cgit/seabios/log/?h=virtio
Patches are in very small pieces, I probably squash them a bit before posting to the list, especially the ones adding virtio 1.0 #defines and structs to the headers.
They also need cleanups. I carefully added loads of GET_LOWFLAT() macros. After doing so I've noticed I have to run this in 32bit mode anyway because otherwise readl+friends don't work ...
They have some verbose debug output atm.
virtio-blk is still to be done.
enjoy, Gerd
You could use the config capability if you wanted to, though that multiplies the cost of each access by a factor of ~4.
I noticed that QEMU doesn't support it but it must, I'll fix it up now.
Hi,
You could use the config capability if you wanted to, though that multiplies the cost of each access by a factor of ~4.
We are moving to 32bit drivers _anyway_, some of our drivers run in 32bit mode already, and with SMM mode landing in kvm now we have a reliable way to transition to 32bit mode and back, even in corner cases like guest calling bios from vm86 mode.
So, I don't feel like using the config capability. Might be useful for virtio-net driver in ipxe though, didn't look there yet.
I noticed that QEMU doesn't support it but it must, I'll fix it up now.
I've also noticed we are back to 8M pci bars for modern/transitional virtio devices, due to supporting up to 1024 virt queues now I think.
Should we introduce a per-device virt queue limit? Supporting 1024 queues makes sense for virtio-scsi, virtio-net and maybe virtio-serial. Other devices will never ever come even close to 1024 virt queues ...
cheers, Gerd
-
Gerd Hoffmann -
Kevin O'Connor -
Michael S. Tsirkin