[Qemu-devel] [PATCH v2 00/22] virtio: add version 1.0 support

[Qemu-devel] [PATCH v2 00/22] virtio: add version 1.0 support

[Gerd Hoffmann]

  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(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 01/22] pci: allow to loop over capabilities

[Gerd Hoffmann]

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);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Gerd Hoffmann]

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)
-- 
1.8.3.1

Re: [Qemu-devel] [SeaBIOS] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Kevin O'Connor]

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

Re: [Qemu-devel] [SeaBIOS] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Michael S. Tsirkin]

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;
>      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)
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Michael S. Tsirkin]

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?

-- 
MST

Re: [Qemu-devel] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [SeaBIOS] [PATCH v2 02/22] virtio: run drivers in 32bit mode

[Kevin O'Connor]

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

[Qemu-devel] [PATCH v2 03/22] virtio: add struct vp_device

[Gerd Hoffmann]

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);
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [SeaBIOS] [PATCH v2 03/22] virtio: add struct vp_device

[Kevin O'Connor]

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

Re: [Qemu-devel] [SeaBIOS] [PATCH v2 03/22] virtio: add struct vp_device

[Gerd Hoffmann]

> > -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

[Qemu-devel] [PATCH v2 04/22] virtio: pass struct pci_device to vp_init_simple

[Gerd Hoffmann]

... 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));
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 05/22] virtio: add version 1.0 structs and #defines

[Gerd Hoffmann]

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
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 06/22] virtio: add version 0.9.5 struct

[Gerd Hoffmann]

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 */
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 07/22] virtio: find version 1.0 virtio capabilities

[Gerd Hoffmann]

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)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 07/22] virtio: find version 1.0 virtio capabilities

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH v2 07/22] virtio: find version 1.0 virtio capabilities

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [PATCH v2 07/22] virtio: find version 1.0 virtio capabilities

[Michael S. Tsirkin]

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(struct
> virtio_pci_cap, bar));
> 
> ... makes seabios use the first not the last and should do the trick,
> right?
> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [PATCH v2 07/22] virtio: find version 1.0 virtio capabilities

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [PATCH v2 07/22] virtio: find version 1.0 virtio capabilities

[Michael S. Tsirkin]

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.

-- 
MST

[Qemu-devel] [PATCH v2 08/22] virtio: create vp_cap struct for legacy bar

[Gerd Hoffmann]

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)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 09/22] virtio: add version 0.9.5 struct [fixup]

[Gerd Hoffmann]

---
 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 ---------------------------------- */
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 10/22] virtio: add read/write functions and macros

[Gerd Hoffmann]

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);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 11/22] virtio: make features 64bit, support version 1.0 features

[Gerd Hoffmann]

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)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 12/22] virtio: add version 1.0 support to vp_{get, set}_status

[Gerd Hoffmann]

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)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 13/22] virtio: add version 1.0 support to vp_get_isr

[Gerd Hoffmann]

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)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 14/22] virtio: add version 1.0 support to vp_reset

[Gerd Hoffmann]

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)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 15/22] virtio: add version 1.0 support to vp_notify

[Gerd Hoffmann]

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 {
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 16/22] virtio: remove unused vp_del_vq

[Gerd Hoffmann]

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);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 17/22] virtio: add version 1.0 support to vp_find_vq

[Gerd Hoffmann]

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:
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 18/22] virtio-scsi: fix initialization for version 1.0

[Gerd Hoffmann]

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++)
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 19/22] virtio-blk: fix initialization for version 1.0

[Gerd Hoffmann]

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);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 20/22] virtio: use version 1.0 if available (flip the big switch)

[Gerd Hoffmann]

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 |
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 21/22] virtio: also probe version 1.0 pci ids

[Gerd Hoffmann]

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);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 22/22] virtio: legacy cleanup

[Gerd Hoffmann]

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);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 00/22] virtio: add version 1.0 support

[Michael S. Tsirkin]

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