[PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

Changes since v2:
	handling for devices without config space (e.g. rng)
	reduce # of mappings for VQs

These patches seem to work fine on my virtio-1.0 qemu branch.
There haven't been any bugs since v2: just minor cleanups
and enhancements.
QEMU side is still undergoing polishing, but is already testable.

Rusty, what do you think?  Let's merge these for 3.20?
Also - will you be doing that merge window, or should I?

Michael S. Tsirkin (14):
  virtio_pci: drop virtio_config dependency
  virtio/9p: verify device has config space
  virtio/blk: verify device has config space
  virtio/console: verify device has config space
  virtio/net: verify device has config space
  virtio/scsi: verify device has config space
  virtio/balloon: verify device has config space
  mn10300: drop dead code
  pci: add pci_iomap_range
  s390: add pci_iomap_range
  virtio_pci: move probe/remove code to common
  virtio_pci: modern driver
  virtio_pci_modern: reduce number of mappings
  virtio_pci_modern: support devices with no config

Rusty Russell (2):
  virtio-pci: define layout for virtio 1.0
  virtio_pci: macros for PCI layout offsets

 arch/s390/include/asm/pci_io.h        |   1 +
 drivers/virtio/virtio_pci_common.h    |  33 +-
 include/asm-generic/pci_iomap.h       |  10 +
 include/uapi/linux/virtio_pci.h       |  94 ++++-
 arch/mn10300/unit-asb2305/pci-iomap.c |  35 --
 arch/s390/pci/pci.c                   |  34 +-
 drivers/block/virtio_blk.c            |   6 +
 drivers/char/virtio_console.c         |   6 +
 drivers/net/virtio_net.c              |   6 +
 drivers/scsi/virtio_scsi.c            |   6 +
 drivers/virtio/virtio_balloon.c       |   6 +
 drivers/virtio/virtio_pci_common.c    |  79 +++-
 drivers/virtio/virtio_pci_legacy.c    |  76 +---
 drivers/virtio/virtio_pci_modern.c    | 706 ++++++++++++++++++++++++++++++++++
 lib/pci_iomap.c                       |  35 +-
 net/9p/trans_virtio.c                 |   6 +
 drivers/virtio/Makefile               |   2 +-
 17 files changed, 1016 insertions(+), 125 deletions(-)
 delete mode 100644 arch/mn10300/unit-asb2305/pci-iomap.c
 create mode 100644 drivers/virtio/virtio_pci_modern.c

-- 
MST

[PATCH v3 01/16] virtio_pci: drop virtio_config dependency

[Michael S. Tsirkin]

virtio_pci does not depend on virtio_config:
let's not include it, users can pull it in as necessary.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/uapi/linux/virtio_pci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 35b552c..509d630 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -39,7 +39,7 @@
 #ifndef _LINUX_VIRTIO_PCI_H
 #define _LINUX_VIRTIO_PCI_H
 
-#include <linux/virtio_config.h>
+#include <linux/types.h>
 
 #ifndef VIRTIO_PCI_NO_LEGACY
 
-- 
MST

[PATCH v3 02/16] virtio/9p: verify device has config space

[Michael S. Tsirkin]

Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/9p needs config space access so make it
fail gracefully if not there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 net/9p/trans_virtio.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index daa749c..d8e376a 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -524,6 +524,12 @@ static int p9_virtio_probe(struct virtio_device *vdev)
 	int err;
 	struct virtio_chan *chan;
 
+	if (!vdev->config->get) {
+		dev_err(&vdev->dev, "%s failure: config access disabled\n",
+			__func__);
+		return -EINVAL;
+	}
+
 	chan = kmalloc(sizeof(struct virtio_chan), GFP_KERNEL);
 	if (!chan) {
 		pr_err("Failed to allocate virtio 9P channel\n");
-- 
MST

[PATCH v3 03/16] virtio/blk: verify device has config space

[Michael S. Tsirkin]

Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/blk needs config space access so make it
fail gracefully if not there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/block/virtio_blk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 7ef7c09..7164da8 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -575,6 +575,12 @@ static int virtblk_probe(struct virtio_device *vdev)
 	u16 min_io_size;
 	u8 physical_block_exp, alignment_offset;
 
+	if (!vdev->config->get) {
+		dev_err(&vdev->dev, "%s failure: config access disabled\n",
+			__func__);
+		return -EINVAL;
+	}
+
 	err = ida_simple_get(&vd_index_ida, 0, minor_to_index(1 << MINORBITS),
 			     GFP_KERNEL);
 	if (err < 0)
-- 
MST

[PATCH v3 04/16] virtio/console: verify device has config space

[Michael S. Tsirkin]

Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/console needs config space access so make it
fail gracefully if not there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/char/virtio_console.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index de03df9..26afb56 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1986,6 +1986,12 @@ static int virtcons_probe(struct virtio_device *vdev)
 	bool multiport;
 	bool early = early_put_chars != NULL;
 
+	if (!vdev->config->get) {
+		dev_err(&vdev->dev, "%s failure: config access disabled\n",
+			__func__);
+		return -EINVAL;
+	}
+
 	/* Ensure to read early_put_chars now */
 	barrier();
 
-- 
MST

Re: [PATCH v3 04/16] virtio/console: verify device has config space

[Amit Shah]

On (Wed) 14 Jan 2015 [19:27:35], Michael S. Tsirkin wrote:
> Some devices might not implement config space access
> (e.g. remoteproc used not to - before 3.9).
> virtio/console needs config space access so make it
> fail gracefully if not there.

Do we know any such devices?  Wondering what prompted this patch.  If
it's just theoretical, I'd rather let it be like this, and pull this
in when there's a device that doesn't have config space.

Also, just the console functionality (i.e. F_MULTIPORT is unset) is
available w/o config space access.  In fact, getting this patch in
would mean remoteproc wouldn't even run in its pre-config days...

> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index de03df9..26afb56 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1986,6 +1986,12 @@ static int virtcons_probe(struct virtio_device *vdev)
>  	bool multiport;
>  	bool early = early_put_chars != NULL;
>  
> +	if (!vdev->config->get) {
> +		dev_err(&vdev->dev, "%s failure: config access disabled\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +


		Amit

Re: [PATCH v3 04/16] virtio/console: verify device has config space

[Michael S. Tsirkin]

On Tue, Jan 20, 2015 at 04:10:40PM +0530, Amit Shah wrote:
> On (Wed) 14 Jan 2015 [19:27:35], Michael S. Tsirkin wrote:
> > Some devices might not implement config space access
> > (e.g. remoteproc used not to - before 3.9).
> > virtio/console needs config space access so make it
> > fail gracefully if not there.
> 
> Do we know any such devices?  Wondering what prompted this patch.  If
> it's just theoretical, I'd rather let it be like this, and pull this
> in when there's a device that doesn't have config space.

Yes, with virtio 1.0 config space can be in a separate BAR now.  If
that's not enabled by BIOS (e.g. out of space), we won't have config
space.

> Also, just the console functionality (i.e. F_MULTIPORT is unset) is
> available w/o config space access.

Supporting this by gracefully disabling F_MULTIPORT
would require getting this info from driver before
features are finalized.
Alternatively, check F_MULTIPORT and only fail if set?
Let me know, I'll cook up a patch.

> In fact, getting this patch in
> would mean remoteproc wouldn't even run in its pre-config days...

It seems to have get callback unconditionally now - or did I miss
something?


> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index de03df9..26afb56 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -1986,6 +1986,12 @@ static int virtcons_probe(struct virtio_device *vdev)
> >  	bool multiport;
> >  	bool early = early_put_chars != NULL;
> >  
> > +	if (!vdev->config->get) {
> > +		dev_err(&vdev->dev, "%s failure: config access disabled\n",
> > +			__func__);
> > +		return -EINVAL;
> > +	}
> > +
> 
> 
> 		Amit

Re: [PATCH v3 04/16] virtio/console: verify device has config space

[Amit Shah]

On (Tue) 20 Jan 2015 [13:09:55], Michael S. Tsirkin wrote:
> On Tue, Jan 20, 2015 at 04:10:40PM +0530, Amit Shah wrote:
> > On (Wed) 14 Jan 2015 [19:27:35], Michael S. Tsirkin wrote:
> > > Some devices might not implement config space access
> > > (e.g. remoteproc used not to - before 3.9).
> > > virtio/console needs config space access so make it
> > > fail gracefully if not there.
> > 
> > Do we know any such devices?  Wondering what prompted this patch.  If
> > it's just theoretical, I'd rather let it be like this, and pull this
> > in when there's a device that doesn't have config space.
> 
> Yes, with virtio 1.0 config space can be in a separate BAR now.  If
> that's not enabled by BIOS (e.g. out of space), we won't have config
> space.

I'm still not sure whether we should pull in this patch before
actually seeing a failure.

You do have a dev_err which tells why the probe failed, so it's an
acceptable compromise I suppose.

> > Also, just the console functionality (i.e. F_MULTIPORT is unset) is
> > available w/o config space access.
> 
> Supporting this by gracefully disabling F_MULTIPORT
> would require getting this info from driver before
> features are finalized.
> Alternatively, check F_MULTIPORT and only fail if set?
> Let me know, I'll cook up a patch.

Yes, failing only if F_MULTIPORT is set is a better option (if we have
to fail).

> > In fact, getting this patch in
> > would mean remoteproc wouldn't even run in its pre-config days...
> 
> It seems to have get callback unconditionally now - or did I miss
> something?

What I meant was remoteproc doesn't depend on the config space, only
uses the console functionality.  If remoteproc devices didn't expose a
config space, this patch would cause it to lose its console
functionality for no apparent reason.


		Amit

Re: [PATCH v3 04/16] virtio/console: verify device has config space

[Michael S. Tsirkin]

On Wed, Jan 21, 2015 at 11:44:52AM +0530, Amit Shah wrote:
> On (Tue) 20 Jan 2015 [13:09:55], Michael S. Tsirkin wrote:
> > On Tue, Jan 20, 2015 at 04:10:40PM +0530, Amit Shah wrote:
> > > On (Wed) 14 Jan 2015 [19:27:35], Michael S. Tsirkin wrote:
> > > > Some devices might not implement config space access
> > > > (e.g. remoteproc used not to - before 3.9).
> > > > virtio/console needs config space access so make it
> > > > fail gracefully if not there.
> > > 
> > > Do we know any such devices?  Wondering what prompted this patch.  If
> > > it's just theoretical, I'd rather let it be like this, and pull this
> > > in when there's a device that doesn't have config space.
> > 
> > Yes, with virtio 1.0 config space can be in a separate BAR now.  If
> > that's not enabled by BIOS (e.g. out of space), we won't have config
> > space.
> 
> I'm still not sure whether we should pull in this patch before
> actually seeing a failure.
> 
> You do have a dev_err which tells why the probe failed, so it's an
> acceptable compromise I suppose.
> 
> > > Also, just the console functionality (i.e. F_MULTIPORT is unset) is
> > > available w/o config space access.
> > 
> > Supporting this by gracefully disabling F_MULTIPORT
> > would require getting this info from driver before
> > features are finalized.
> > Alternatively, check F_MULTIPORT and only fail if set?
> > Let me know, I'll cook up a patch.
> 
> Yes, failing only if F_MULTIPORT is set is a better option (if we have
> to fail).

OK, that's easy I think - will send a patch on top.

> > > In fact, getting this patch in
> > > would mean remoteproc wouldn't even run in its pre-config days...
> > 
> > It seems to have get callback unconditionally now - or did I miss
> > something?
> 
> What I meant was remoteproc doesn't depend on the config space, only
> uses the console functionality.  If remoteproc devices didn't expose a
> config space, this patch would cause it to lose its console
> functionality for no apparent reason.
> 
> 
> 		Amit

Better than crashing on jump to NULL?

-- 
MST

[PATCH v3 05/16] virtio/net: verify device has config space

[Michael S. Tsirkin]

Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/net needs config space access so make it
fail gracefully if not there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio_net.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5ca9771..9bc1072 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1713,6 +1713,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 	struct virtnet_info *vi;
 	u16 max_queue_pairs;
 
+	if (!vdev->config->get) {
+		dev_err(&vdev->dev, "%s failure: config access disabled\n",
+			__func__);
+		return -EINVAL;
+	}
+
 	if (!virtnet_validate_features(vdev))
 		return -EINVAL;
 
-- 
MST

[PATCH v3 06/16] virtio/scsi: verify device has config space

[Michael S. Tsirkin]

Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/scsi needs config space access so make it
fail gracefully if not there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/scsi/virtio_scsi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index c52bb5d..f164f24 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -950,6 +950,12 @@ static int virtscsi_probe(struct virtio_device *vdev)
 	u32 num_queues;
 	struct scsi_host_template *hostt;
 
+	if (!vdev->config->get) {
+		dev_err(&vdev->dev, "%s failure: config access disabled\n",
+			__func__);
+		return -EINVAL;
+	}
+
 	/* We need to know how many queues before we allocate. */
 	num_queues = virtscsi_config_get(vdev, num_queues) ? : 1;
 
-- 
MST

[PATCH v3 07/16] virtio/balloon: verify device has config space

[Michael S. Tsirkin]

Some devices might not implement config space access
(e.g. remoteproc used not to - before 3.9).
virtio/balloon needs config space access so make it
fail gracefully if not there.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_balloon.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 50c5f42..3176ea4 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -466,6 +466,12 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	struct virtio_balloon *vb;
 	int err;
 
+	if (!vdev->config->get) {
+		dev_err(&vdev->dev, "%s failure: config access disabled\n",
+			__func__);
+		return -EINVAL;
+	}
+
 	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
 	if (!vb) {
 		err = -ENOMEM;
-- 
MST

[PATCH v3 08/16] mn10300: drop dead code

[Michael S. Tsirkin]

pci-iomap.c was (apparently, mistakenly) reintroduced as part of
commit 83c2dc15ce824450e7044b9f90cd529c25747ae0
    MN10300: Handle cacheable PCI regions in pci_iomap()
probably as side-effect of forward-porting the patch
from an old kernel.

It's not really needed: the generic pci_iomap does the right thing here.

The new file isn't compiled so it's safe to drop.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: trivial@kernel.org
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Can relevant people please ack this for merging through virtio tree?

 arch/mn10300/unit-asb2305/pci-iomap.c | 35 -----------------------------------
 1 file changed, 35 deletions(-)
 delete mode 100644 arch/mn10300/unit-asb2305/pci-iomap.c

diff --git a/arch/mn10300/unit-asb2305/pci-iomap.c b/arch/mn10300/unit-asb2305/pci-iomap.c
deleted file mode 100644
index bd65dae..0000000
--- a/arch/mn10300/unit-asb2305/pci-iomap.c
+++ /dev/null
@@ -1,35 +0,0 @@
-/* ASB2305 PCI I/O mapping handler
- *
- * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
- * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
- */
-#include <linux/pci.h>
-#include <linux/module.h>
-
-/*
- * Create a virtual mapping cookie for a PCI BAR (memory or IO)
- */
-void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
-{
-	resource_size_t start = pci_resource_start(dev, bar);
-	resource_size_t len = pci_resource_len(dev, bar);
-	unsigned long flags = pci_resource_flags(dev, bar);
-
-	if (!len || !start)
-		return NULL;
-
-	if ((flags & IORESOURCE_IO) || (flags & IORESOURCE_MEM)) {
-		if (flags & IORESOURCE_CACHEABLE && !(flags & IORESOURCE_IO))
-			return ioremap(start, len);
-		else
-			return ioremap_nocache(start, len);
-	}
-
-	return NULL;
-}
-EXPORT_SYMBOL(pci_iomap);
-- 
MST

Re: [PATCH v3 08/16] mn10300: drop dead code

[Bjorn Helgaas]

On Wed, Jan 14, 2015 at 07:27:48PM +0200, Michael S. Tsirkin wrote:
> pci-iomap.c was (apparently, mistakenly) reintroduced as part of
> commit 83c2dc15ce824450e7044b9f90cd529c25747ae0
>     MN10300: Handle cacheable PCI regions in pci_iomap()
> probably as side-effect of forward-porting the patch
> from an old kernel.
> 
> It's not really needed: the generic pci_iomap does the right thing here.
> 
> The new file isn't compiled so it's safe to drop.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Cc: trivial@kernel.org
> Cc: David Howells <dhowells@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> 
> Can relevant people please ack this for merging through virtio tree?
> 
>  arch/mn10300/unit-asb2305/pci-iomap.c | 35 -----------------------------------
>  1 file changed, 35 deletions(-)
>  delete mode 100644 arch/mn10300/unit-asb2305/pci-iomap.c
> 
> diff --git a/arch/mn10300/unit-asb2305/pci-iomap.c b/arch/mn10300/unit-asb2305/pci-iomap.c
> deleted file mode 100644
> index bd65dae..0000000
> --- a/arch/mn10300/unit-asb2305/pci-iomap.c
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/* ASB2305 PCI I/O mapping handler
> - *
> - * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
> - * Written by David Howells (dhowells@redhat.com)
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public Licence
> - * as published by the Free Software Foundation; either version
> - * 2 of the Licence, or (at your option) any later version.
> - */
> -#include <linux/pci.h>
> -#include <linux/module.h>
> -
> -/*
> - * Create a virtual mapping cookie for a PCI BAR (memory or IO)
> - */
> -void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
> -{
> -	resource_size_t start = pci_resource_start(dev, bar);
> -	resource_size_t len = pci_resource_len(dev, bar);
> -	unsigned long flags = pci_resource_flags(dev, bar);
> -
> -	if (!len || !start)
> -		return NULL;
> -
> -	if ((flags & IORESOURCE_IO) || (flags & IORESOURCE_MEM)) {
> -		if (flags & IORESOURCE_CACHEABLE && !(flags & IORESOURCE_IO))
> -			return ioremap(start, len);
> -		else
> -			return ioremap_nocache(start, len);
> -	}
> -
> -	return NULL;
> -}
> -EXPORT_SYMBOL(pci_iomap);
> -- 
> MST
> 

[PATCH v3 09/16] pci: add pci_iomap_range

[Michael S. Tsirkin]

Virtio drivers should map the part of the BAR they need, not necessarily
all of it.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---

Bjorn, can you please ack this for merging through the virtio tree?

 include/asm-generic/pci_iomap.h | 10 ++++++++++
 lib/pci_iomap.c                 | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index ce37349..7389c87 100644
--- a/include/asm-generic/pci_iomap.h
+++ b/include/asm-generic/pci_iomap.h
@@ -15,6 +15,9 @@ struct pci_dev;
 #ifdef CONFIG_PCI
 /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
 extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
+				     unsigned long offset,
+				     unsigned long maxlen);
 /* Create a virtual mapping cookie for a port on a given PCI device.
  * Do not call this directly, it exists to make it easier for architectures
  * to override */
@@ -30,6 +33,13 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
 {
 	return NULL;
 }
+
+static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
+					    unsigned long offset,
+					    unsigned long maxlen)
+{
+	return NULL;
+}
 #endif
 
 #endif /* __ASM_GENERIC_IO_H */
diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
index 0d83ea8..bcce5f1 100644
--- a/lib/pci_iomap.c
+++ b/lib/pci_iomap.c
@@ -10,10 +10,11 @@
 
 #ifdef CONFIG_PCI
 /**
- * pci_iomap - create a virtual mapping cookie for a PCI BAR
+ * pci_iomap_range - create a virtual mapping cookie for a PCI BAR
  * @dev: PCI device that owns the BAR
  * @bar: BAR number
- * @maxlen: length of the memory to map
+ * @offset: map memory at the given offset in BAR
+ * @maxlen: max length of the memory to map
  *
  * Using this function you will get a __iomem address to your device BAR.
  * You can access it using ioread*() and iowrite*(). These functions hide
@@ -21,16 +22,21 @@
  * you expect from them in the correct way.
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
- * the complete BAR without checking for its length first, pass %0 here.
+ * the complete BAR from offset to the end, pass %0 here.
  * */
-void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
+void __iomem *pci_iomap_range(struct pci_dev *dev,
+			      int bar,
+			      unsigned long offset,
+			      unsigned long maxlen)
 {
 	resource_size_t start = pci_resource_start(dev, bar);
 	resource_size_t len = pci_resource_len(dev, bar);
 	unsigned long flags = pci_resource_flags(dev, bar);
 
-	if (!len || !start)
+	if (len <= offset || !start)
 		return NULL;
+	len -= offset;
+	start += offset;
 	if (maxlen && len > maxlen)
 		len = maxlen;
 	if (flags & IORESOURCE_IO)
@@ -43,6 +49,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
 	/* What? */
 	return NULL;
 }
+EXPORT_SYMBOL(pci_iomap_range);
 
+/**
+ * pci_iomap - create a virtual mapping cookie for a PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @maxlen: length of the memory to map
+ *
+ * Using this function you will get a __iomem address to your device BAR.
+ * You can access it using ioread*() and iowrite*(). These functions hide
+ * the details if this is a MMIO or PIO address space and will just do what
+ * you expect from them in the correct way.
+ *
+ * @maxlen specifies the maximum length to map. If you want to get access to
+ * the complete BAR without checking for its length first, pass %0 here.
+ * */
+void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+	return pci_iomap_range(dev, bar, 0, maxlen);
+}
 EXPORT_SYMBOL(pci_iomap);
 #endif /* CONFIG_PCI */
-- 
MST

Re: [PATCH v3 09/16] pci: add pci_iomap_range

[Bjorn Helgaas]

On Wed, Jan 14, 2015 at 07:27:54PM +0200, Michael S. Tsirkin wrote:
> Virtio drivers should map the part of the BAR they need, not necessarily
> all of it.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Sorry it took so long for me to notice these!

> ---
> 
> Bjorn, can you please ack this for merging through the virtio tree?
> 
>  include/asm-generic/pci_iomap.h | 10 ++++++++++
>  lib/pci_iomap.c                 | 35 ++++++++++++++++++++++++++++++-----
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
> index ce37349..7389c87 100644
> --- a/include/asm-generic/pci_iomap.h
> +++ b/include/asm-generic/pci_iomap.h
> @@ -15,6 +15,9 @@ struct pci_dev;
>  #ifdef CONFIG_PCI
>  /* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
>  extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
> +extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
> +				     unsigned long offset,
> +				     unsigned long maxlen);
>  /* Create a virtual mapping cookie for a port on a given PCI device.
>   * Do not call this directly, it exists to make it easier for architectures
>   * to override */
> @@ -30,6 +33,13 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
>  {
>  	return NULL;
>  }
> +
> +static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
> +					    unsigned long offset,
> +					    unsigned long maxlen)
> +{
> +	return NULL;
> +}
>  #endif
>  
>  #endif /* __ASM_GENERIC_IO_H */
> diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
> index 0d83ea8..bcce5f1 100644
> --- a/lib/pci_iomap.c
> +++ b/lib/pci_iomap.c
> @@ -10,10 +10,11 @@
>  
>  #ifdef CONFIG_PCI
>  /**
> - * pci_iomap - create a virtual mapping cookie for a PCI BAR
> + * pci_iomap_range - create a virtual mapping cookie for a PCI BAR
>   * @dev: PCI device that owns the BAR
>   * @bar: BAR number
> - * @maxlen: length of the memory to map
> + * @offset: map memory at the given offset in BAR
> + * @maxlen: max length of the memory to map
>   *
>   * Using this function you will get a __iomem address to your device BAR.
>   * You can access it using ioread*() and iowrite*(). These functions hide
> @@ -21,16 +22,21 @@
>   * you expect from them in the correct way.
>   *
>   * @maxlen specifies the maximum length to map. If you want to get access to
> - * the complete BAR without checking for its length first, pass %0 here.
> + * the complete BAR from offset to the end, pass %0 here.
>   * */
> -void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
> +void __iomem *pci_iomap_range(struct pci_dev *dev,
> +			      int bar,
> +			      unsigned long offset,
> +			      unsigned long maxlen)
>  {
>  	resource_size_t start = pci_resource_start(dev, bar);
>  	resource_size_t len = pci_resource_len(dev, bar);
>  	unsigned long flags = pci_resource_flags(dev, bar);
>  
> -	if (!len || !start)
> +	if (len <= offset || !start)
>  		return NULL;
> +	len -= offset;
> +	start += offset;
>  	if (maxlen && len > maxlen)
>  		len = maxlen;
>  	if (flags & IORESOURCE_IO)
> @@ -43,6 +49,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
>  	/* What? */
>  	return NULL;
>  }
> +EXPORT_SYMBOL(pci_iomap_range);
>  
> +/**
> + * pci_iomap - create a virtual mapping cookie for a PCI BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR number
> + * @maxlen: length of the memory to map
> + *
> + * Using this function you will get a __iomem address to your device BAR.
> + * You can access it using ioread*() and iowrite*(). These functions hide
> + * the details if this is a MMIO or PIO address space and will just do what
> + * you expect from them in the correct way.
> + *
> + * @maxlen specifies the maximum length to map. If you want to get access to
> + * the complete BAR without checking for its length first, pass %0 here.
> + * */
> +void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
> +{
> +	return pci_iomap_range(dev, bar, 0, maxlen);
> +}
>  EXPORT_SYMBOL(pci_iomap);
>  #endif /* CONFIG_PCI */
> -- 
> MST
> 

[PATCH v3 10/16] s390: add pci_iomap_range

[Michael S. Tsirkin]

Virtio drivers should map the part of the range they need, not
necessarily all of it.
To this end, support mapping ranges within BAR on s390.
Since multiple ranges can now be mapped within a BAR, we keep track of
the number of mappings created, and only clear out the mapping for a BAR
when this number reaches 0.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Tested-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Heiko, Martin, can you please ack merging this through the virtio tree?
This was lightly tested by Sebastian Ott.

 arch/s390/include/asm/pci_io.h |  1 +
 arch/s390/pci/pci.c            | 34 +++++++++++++++++++++++++++-------
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/arch/s390/include/asm/pci_io.h b/arch/s390/include/asm/pci_io.h
index f664e96..1a9a98d 100644
--- a/arch/s390/include/asm/pci_io.h
+++ b/arch/s390/include/asm/pci_io.h
@@ -16,6 +16,7 @@
 struct zpci_iomap_entry {
 	u32 fh;
 	u8 bar;
+	u16 count;
 };
 
 extern struct zpci_iomap_entry *zpci_iomap_start;
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 3290f11..753a567 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -259,7 +259,10 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
 }
 
 /* Create a virtual mapping cookie for a PCI BAR */
-void __iomem *pci_iomap(struct pci_dev *pdev, int bar, unsigned long max)
+void __iomem *pci_iomap_range(struct pci_dev *pdev,
+			      int bar,
+			      unsigned long offset,
+			      unsigned long max)
 {
 	struct zpci_dev *zdev =	get_zdev(pdev);
 	u64 addr;
@@ -270,14 +273,27 @@ void __iomem *pci_iomap(struct pci_dev *pdev, int bar, unsigned long max)
 
 	idx = zdev->bars[bar].map_idx;
 	spin_lock(&zpci_iomap_lock);
-	zpci_iomap_start[idx].fh = zdev->fh;
-	zpci_iomap_start[idx].bar = bar;
+	if (zpci_iomap_start[idx].count++) {
+		BUG_ON(zpci_iomap_start[idx].fh != zdev->fh ||
+		       zpci_iomap_start[idx].bar != bar);
+	} else {
+		zpci_iomap_start[idx].fh = zdev->fh;
+		zpci_iomap_start[idx].bar = bar;
+	}
+	/* Detect overrun */
+	BUG_ON(!zpci_iomap_start[idx].count);
 	spin_unlock(&zpci_iomap_lock);
 
 	addr = ZPCI_IOMAP_ADDR_BASE | ((u64) idx << 48);
-	return (void __iomem *) addr;
+	return (void __iomem *) addr + offset;
 }
-EXPORT_SYMBOL_GPL(pci_iomap);
+EXPORT_SYMBOL_GPL(pci_iomap_range);
+
+void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+	return pci_iomap_range(dev, bar, 0, maxlen);
+}
+EXPORT_SYMBOL(pci_iomap);
 
 void pci_iounmap(struct pci_dev *pdev, void __iomem *addr)
 {
@@ -285,8 +301,12 @@ void pci_iounmap(struct pci_dev *pdev, void __iomem *addr)
 
 	idx = (((__force u64) addr) & ~ZPCI_IOMAP_ADDR_BASE) >> 48;
 	spin_lock(&zpci_iomap_lock);
-	zpci_iomap_start[idx].fh = 0;
-	zpci_iomap_start[idx].bar = 0;
+	/* Detect underrun */
+	BUG_ON(!zpci_iomap_start[idx].count);
+	if (!--zpci_iomap_start[idx].count) {
+		zpci_iomap_start[idx].fh = 0;
+		zpci_iomap_start[idx].bar = 0;
+	}
 	spin_unlock(&zpci_iomap_lock);
 }
 EXPORT_SYMBOL_GPL(pci_iounmap);
-- 
MST

Re: [PATCH v3 10/16] s390: add pci_iomap_range

[Sebastian Ott]

On Wed, 14 Jan 2015, Michael S. Tsirkin wrote:
> Virtio drivers should map the part of the range they need, not
> necessarily all of it.
> To this end, support mapping ranges within BAR on s390.
> Since multiple ranges can now be mapped within a BAR, we keep track of
> the number of mappings created, and only clear out the mapping for a BAR
> when this number reaches 0.
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Tested-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Heiko, Martin, can you please ack merging this through the virtio tree?
> This was lightly tested by Sebastian Ott.
> 
>  arch/s390/include/asm/pci_io.h |  1 +
>  arch/s390/pci/pci.c            | 34 +++++++++++++++++++++++++++-------
>  2 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/s390/include/asm/pci_io.h b/arch/s390/include/asm/pci_io.h
> index f664e96..1a9a98d 100644
> --- a/arch/s390/include/asm/pci_io.h
> +++ b/arch/s390/include/asm/pci_io.h
> @@ -16,6 +16,7 @@
>  struct zpci_iomap_entry {
>  	u32 fh;
>  	u8 bar;
> +	u16 count;
>  };
> 
>  extern struct zpci_iomap_entry *zpci_iomap_start;
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 3290f11..753a567 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -259,7 +259,10 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
>  }
> 
>  /* Create a virtual mapping cookie for a PCI BAR */
> -void __iomem *pci_iomap(struct pci_dev *pdev, int bar, unsigned long max)
> +void __iomem *pci_iomap_range(struct pci_dev *pdev,
> +			      int bar,
> +			      unsigned long offset,
> +			      unsigned long max)
>  {
>  	struct zpci_dev *zdev =	get_zdev(pdev);
>  	u64 addr;
> @@ -270,14 +273,27 @@ void __iomem *pci_iomap(struct pci_dev *pdev, int bar, unsigned long max)
> 
>  	idx = zdev->bars[bar].map_idx;
>  	spin_lock(&zpci_iomap_lock);
> -	zpci_iomap_start[idx].fh = zdev->fh;
> -	zpci_iomap_start[idx].bar = bar;
> +	if (zpci_iomap_start[idx].count++) {
> +		BUG_ON(zpci_iomap_start[idx].fh != zdev->fh ||
> +		       zpci_iomap_start[idx].bar != bar);
> +	} else {
> +		zpci_iomap_start[idx].fh = zdev->fh;
> +		zpci_iomap_start[idx].bar = bar;
> +	}
> +	/* Detect overrun */
> +	BUG_ON(!zpci_iomap_start[idx].count);
>  	spin_unlock(&zpci_iomap_lock);
> 
>  	addr = ZPCI_IOMAP_ADDR_BASE | ((u64) idx << 48);
> -	return (void __iomem *) addr;
> +	return (void __iomem *) addr + offset;
>  }
> -EXPORT_SYMBOL_GPL(pci_iomap);
> +EXPORT_SYMBOL_GPL(pci_iomap_range);
> +
> +void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
> +{
> +	return pci_iomap_range(dev, bar, 0, maxlen);
> +}
> +EXPORT_SYMBOL(pci_iomap);

As discussed earlier, could you please leave that as EXPORT_SYMBOL_GPL.
If there's a reason to have these interfaces as EXPORT_SYMBOL, we could
change all of them in an extra patch.

With this change integrated you can add
Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>

Regards,
Sebastian

> 
>  void pci_iounmap(struct pci_dev *pdev, void __iomem *addr)
>  {
> @@ -285,8 +301,12 @@ void pci_iounmap(struct pci_dev *pdev, void __iomem *addr)
> 
>  	idx = (((__force u64) addr) & ~ZPCI_IOMAP_ADDR_BASE) >> 48;
>  	spin_lock(&zpci_iomap_lock);
> -	zpci_iomap_start[idx].fh = 0;
> -	zpci_iomap_start[idx].bar = 0;
> +	/* Detect underrun */
> +	BUG_ON(!zpci_iomap_start[idx].count);
> +	if (!--zpci_iomap_start[idx].count) {
> +		zpci_iomap_start[idx].fh = 0;
> +		zpci_iomap_start[idx].bar = 0;
> +	}
>  	spin_unlock(&zpci_iomap_lock);
>  }
>  EXPORT_SYMBOL_GPL(pci_iounmap);
> -- 
> MST
> 
> 

Re: [PATCH v3 10/16] s390: add pci_iomap_range

[Rusty Russell]

Sebastian Ott <sebott@linux.vnet.ibm.com> writes:
> On Wed, 14 Jan 2015, Michael S. Tsirkin wrote:
>>  }
>> -EXPORT_SYMBOL_GPL(pci_iomap);
>> +EXPORT_SYMBOL_GPL(pci_iomap_range);
>> +
>> +void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
>> +{
>> +	return pci_iomap_range(dev, bar, 0, maxlen);
>> +}
>> +EXPORT_SYMBOL(pci_iomap);
>
> As discussed earlier, could you please leave that as EXPORT_SYMBOL_GPL.
> If there's a reason to have these interfaces as EXPORT_SYMBOL, we could
> change all of them in an extra patch.
>
> With this change integrated you can add
> Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>

OK, I've gone back and rebased virtio-next to fix this (I don't really
want to think too hard about the legal consequences of having a version
which isn't EXPORT_SYMBOL_GPL).

Thanks,
Rusty.

[PATCH v3 11/16] virtio_pci: move probe/remove code to common

[Michael S. Tsirkin]

Most of initialization is device-independent.
Let's move it to common.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci_common.h |  5 +--
 drivers/virtio/virtio_pci_common.c | 69 +++++++++++++++++++++++++++++++++-
 drivers/virtio/virtio_pci_legacy.c | 76 ++++----------------------------------
 3 files changed, 77 insertions(+), 73 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 5a49728..2b1e70d 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -127,8 +127,7 @@ const char *vp_bus_name(struct virtio_device *vdev);
  */
 int vp_set_vq_affinity(struct virtqueue *vq, int cpu);
 
-int virtio_pci_legacy_probe(struct pci_dev *pci_dev,
-			    const struct pci_device_id *id);
-void virtio_pci_legacy_remove(struct pci_dev *pci_dev);
+int virtio_pci_legacy_probe(struct virtio_pci_device *);
+void virtio_pci_legacy_remove(struct virtio_pci_device *);
 
 #endif
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 9756f21..457cbe2 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -464,15 +464,80 @@ static const struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
+static void virtio_pci_release_dev(struct device *_d)
+{
+	struct virtio_device *vdev = dev_to_virtio(_d);
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+	/* As struct device is a kobject, it's not safe to
+	 * free the memory (including the reference counter itself)
+	 * until it's release callback. */
+	kfree(vp_dev);
+}
+
 static int virtio_pci_probe(struct pci_dev *pci_dev,
 			    const struct pci_device_id *id)
 {
-	return virtio_pci_legacy_probe(pci_dev, id);
+	struct virtio_pci_device *vp_dev;
+	int rc;
+
+	/* allocate our structure and fill it out */
+	vp_dev = kzalloc(sizeof(struct virtio_pci_device), GFP_KERNEL);
+	if (!vp_dev)
+		return -ENOMEM;
+
+	pci_set_drvdata(pci_dev, vp_dev);
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
+	vp_dev->vdev.dev.release = virtio_pci_release_dev;
+	vp_dev->pci_dev = pci_dev;
+	INIT_LIST_HEAD(&vp_dev->virtqueues);
+	spin_lock_init(&vp_dev->lock);
+
+	/* Disable MSI/MSIX to bring device to a known good state. */
+	pci_msi_off(pci_dev);
+
+	/* enable the device */
+	rc = pci_enable_device(pci_dev);
+	if (rc)
+		goto err_enable_device;
+
+	rc = pci_request_regions(pci_dev, "virtio-pci");
+	if (rc)
+		goto err_request_regions;
+
+	rc = virtio_pci_legacy_probe(vp_dev);
+	if (rc)
+		goto err_probe;
+
+	pci_set_master(pci_dev);
+
+	rc = register_virtio_device(&vp_dev->vdev);
+	if (rc)
+		goto err_register;
+
+	return 0;
+
+err_register:
+	virtio_pci_legacy_remove(vp_dev);
+err_probe:
+	pci_release_regions(pci_dev);
+err_request_regions:
+	pci_disable_device(pci_dev);
+err_enable_device:
+	kfree(vp_dev);
+	return rc;
 }
 
 static void virtio_pci_remove(struct pci_dev *pci_dev)
 {
-     virtio_pci_legacy_remove(pci_dev);
+	struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+
+	unregister_virtio_device(&vp_dev->vdev);
+
+	virtio_pci_legacy_remove(pci_dev);
+
+	pci_release_regions(pci_dev);
+	pci_disable_device(pci_dev);
 }
 
 static struct pci_driver virtio_pci_driver = {
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
index a5486e6..256a527 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -211,23 +211,10 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
 	.set_vq_affinity = vp_set_vq_affinity,
 };
 
-static void virtio_pci_release_dev(struct device *_d)
-{
-	struct virtio_device *vdev = dev_to_virtio(_d);
-	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
-
-	/* As struct device is a kobject, it's not safe to
-	 * free the memory (including the reference counter itself)
-	 * until it's release callback. */
-	kfree(vp_dev);
-}
-
 /* the PCI probing function */
-int virtio_pci_legacy_probe(struct pci_dev *pci_dev,
-			    const struct pci_device_id *id)
+int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
 {
-	struct virtio_pci_device *vp_dev;
-	int err;
+	struct pci_dev *pci_dev = vp_dev->pci_dev;
 
 	/* We only own devices >= 0x1000 and <= 0x103f: leave the rest. */
 	if (pci_dev->device < 0x1000 || pci_dev->device > 0x103f)
@@ -239,41 +226,12 @@ int virtio_pci_legacy_probe(struct pci_dev *pci_dev,
 		return -ENODEV;
 	}
 
-	/* allocate our structure and fill it out */
-	vp_dev = kzalloc(sizeof(struct virtio_pci_device), GFP_KERNEL);
-	if (vp_dev == NULL)
-		return -ENOMEM;
-
-	vp_dev->vdev.dev.parent = &pci_dev->dev;
-	vp_dev->vdev.dev.release = virtio_pci_release_dev;
-	vp_dev->vdev.config = &virtio_pci_config_ops;
-	vp_dev->pci_dev = pci_dev;
-	INIT_LIST_HEAD(&vp_dev->virtqueues);
-	spin_lock_init(&vp_dev->lock);
-
-	/* Disable MSI/MSIX to bring device to a known good state. */
-	pci_msi_off(pci_dev);
-
-	/* enable the device */
-	err = pci_enable_device(pci_dev);
-	if (err)
-		goto out;
-
-	err = pci_request_regions(pci_dev, "virtio-pci");
-	if (err)
-		goto out_enable_device;
-
 	vp_dev->ioaddr = pci_iomap(pci_dev, 0, 0);
-	if (vp_dev->ioaddr == NULL) {
-		err = -ENOMEM;
-		goto out_req_regions;
-	}
+	if (!vp_dev->ioaddr)
+		return -ENOMEM;
 
 	vp_dev->isr = vp_dev->ioaddr + VIRTIO_PCI_ISR;
 
-	pci_set_drvdata(pci_dev, vp_dev);
-	pci_set_master(pci_dev);
-
 	/* we use the subsystem vendor/device id as the virtio vendor/device
 	 * id.  this allows us to use the same PCI vendor/device id for all
 	 * virtio devices and to identify the particular virtio driver by
@@ -281,36 +239,18 @@ int virtio_pci_legacy_probe(struct pci_dev *pci_dev,
 	vp_dev->vdev.id.vendor = pci_dev->subsystem_vendor;
 	vp_dev->vdev.id.device = pci_dev->subsystem_device;
 
+	vp_dev->vdev.config = &virtio_pci_config_ops;
+
 	vp_dev->config_vector = vp_config_vector;
 	vp_dev->setup_vq = setup_vq;
 	vp_dev->del_vq = del_vq;
 
-	/* finally register the virtio device */
-	err = register_virtio_device(&vp_dev->vdev);
-	if (err)
-		goto out_set_drvdata;
-
 	return 0;
-
-out_set_drvdata:
-	pci_iounmap(pci_dev, vp_dev->ioaddr);
-out_req_regions:
-	pci_release_regions(pci_dev);
-out_enable_device:
-	pci_disable_device(pci_dev);
-out:
-	kfree(vp_dev);
-	return err;
 }
 
-void virtio_pci_legacy_remove(struct pci_dev *pci_dev)
+void virtio_pci_legacy_remove(struct virtio_pci_device *vp_dev)
 {
-	struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
-
-	unregister_virtio_device(&vp_dev->vdev);
+	struct pci_dev *pci_dev = vp_dev->pci_dev;
 
-	vp_del_vqs(&vp_dev->vdev);
 	pci_iounmap(pci_dev, vp_dev->ioaddr);
-	pci_release_regions(pci_dev);
-	pci_disable_device(pci_dev);
 }
-- 
MST

[PATCH v3 12/16] virtio-pci: define layout for virtio 1.0

[Michael S. Tsirkin]

From: Rusty Russell <rusty@rustcorp.com.au>

Based on patches by Michael S. Tsirkin <mst@redhat.com>, but I found it
hard to follow so changed to use structures which are more
self-documenting.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/uapi/linux/virtio_pci.h | 62 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 509d630..4e05423 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -99,4 +99,66 @@
 /* Vector value used to disable MSI for queue */
 #define VIRTIO_MSI_NO_VECTOR            0xffff
 
+#ifndef VIRTIO_PCI_NO_MODERN
+
+/* IDs for different capabilities.  Must all exist. */
+
+/* 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 confiuration */
+#define VIRTIO_PCI_CAP_DEVICE_CFG	4
+
+/* 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 type_and_bar;	/* Upper 3 bits: bar.
+				 * Lower 3 is VIRTIO_PCI_CAP_*_CFG. */
+	__le32 offset;		/* Offset within bar. */
+	__le32 length;		/* Length. */
+};
+
+#define VIRTIO_PCI_CAP_BAR_SHIFT	5
+#define VIRTIO_PCI_CAP_BAR_MASK		0x7
+#define VIRTIO_PCI_CAP_TYPE_SHIFT	0
+#define VIRTIO_PCI_CAP_TYPE_MASK	0x7
+
+struct virtio_pci_notify_cap {
+	struct virtio_pci_cap cap;
+	__le32 notify_off_multiplier;	/* Multiplier for queue_notify_off. */
+};
+
+/* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */
+struct virtio_pci_common_cfg {
+	/* About the whole device. */
+	__le32 device_feature_select;	/* read-write */
+	__le32 device_feature;		/* read-only */
+	__le32 guest_feature_select;	/* read-write */
+	__le32 guest_feature;		/* read-write */
+	__le16 msix_config;		/* read-write */
+	__le16 num_queues;		/* read-only */
+	__u8 device_status;		/* read-write */
+	__u8 config_generation;		/* read-only */
+
+	/* About a specific virtqueue. */
+	__le16 queue_select;		/* read-write */
+	__le16 queue_size;		/* read-write, power of 2. */
+	__le16 queue_msix_vector;	/* read-write */
+	__le16 queue_enable;		/* read-write */
+	__le16 queue_notify_off;	/* read-only */
+	__le32 queue_desc_lo;		/* read-write */
+	__le32 queue_desc_hi;		/* read-write */
+	__le32 queue_avail_lo;		/* read-write */
+	__le32 queue_avail_hi;		/* read-write */
+	__le32 queue_used_lo;		/* read-write */
+	__le32 queue_used_hi;		/* read-write */
+};
+
+#endif /* VIRTIO_PCI_NO_MODERN */
+
 #endif
-- 
MST

[PATCH v3 13/16] virtio_pci: modern driver

[Michael S. Tsirkin]

Lightly tested against qemu.

One thing *not* implemented here is separate mappings
for descriptor/avail/used rings. That's nice to have,
will be done later after we have core support.

This also exposes the PCI layout to userspace, and
adds macros for PCI layout offsets:

QEMU wants it, so why not?  Trust, but verify.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci_common.h |  25 +-
 drivers/virtio/virtio_pci_common.c |  14 +-
 drivers/virtio/virtio_pci_modern.c | 592 +++++++++++++++++++++++++++++++++++++
 drivers/virtio/Makefile            |   2 +-
 4 files changed, 627 insertions(+), 6 deletions(-)
 create mode 100644 drivers/virtio/virtio_pci_modern.c

diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 2b1e70d..610c43f 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -53,12 +53,29 @@ struct virtio_pci_device {
 	struct virtio_device vdev;
 	struct pci_dev *pci_dev;
 
+	/* In legacy mode, these two point to within ->legacy. */
+	/* Where to read and clear interrupt */
+	u8 __iomem *isr;
+
+	/* Modern only fields */
+	/* The IO mapping for the PCI config space (non-legacy mode) */
+	struct virtio_pci_common_cfg __iomem *common;
+	/* Device-specific data (non-legacy mode)  */
+	void __iomem *device;
+
+	/* So we can sanity-check accesses. */
+	size_t device_len;
+
+	/* Capability for when we need to map notifications per-vq. */
+	int notify_map_cap;
+
+	/* Multiply queue_notify_off by this value. (non-legacy mode). */
+	u32 notify_offset_multiplier;
+
+	/* Legacy only field */
 	/* the IO mapping for the PCI config space */
 	void __iomem *ioaddr;
 
-	/* the IO mapping for ISR operation */
-	void __iomem *isr;
-
 	/* a list of queues so we can dispatch IRQs */
 	spinlock_t lock;
 	struct list_head virtqueues;
@@ -129,5 +146,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu);
 
 int virtio_pci_legacy_probe(struct virtio_pci_device *);
 void virtio_pci_legacy_remove(struct virtio_pci_device *);
+int virtio_pci_modern_probe(struct virtio_pci_device *);
+void virtio_pci_modern_remove(struct virtio_pci_device *);
 
 #endif
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 457cbe2..20c7638 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -505,6 +505,10 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
 	if (rc)
 		goto err_request_regions;
 
+	rc = virtio_pci_modern_probe(vp_dev);
+	if (rc != -ENODEV)
+		return rc;
+
 	rc = virtio_pci_legacy_probe(vp_dev);
 	if (rc)
 		goto err_probe;
@@ -518,7 +522,10 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
 	return 0;
 
 err_register:
-	virtio_pci_legacy_remove(vp_dev);
+	if (vp_dev->ioaddr)
+	     virtio_pci_legacy_remove(vp_dev);
+	else
+	     virtio_pci_modern_remove(vp_dev);
 err_probe:
 	pci_release_regions(pci_dev);
 err_request_regions:
@@ -534,7 +541,10 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
 
 	unregister_virtio_device(&vp_dev->vdev);
 
-	virtio_pci_legacy_remove(pci_dev);
+	if (vp_dev->ioaddr)
+		virtio_pci_legacy_remove(vp_dev);
+	else
+		virtio_pci_modern_remove(vp_dev);
 
 	pci_release_regions(pci_dev);
 	pci_disable_device(pci_dev);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
new file mode 100644
index 0000000..38c0a11
--- /dev/null
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -0,0 +1,592 @@
+/*
+ * Virtio PCI driver - legacy device support
+ *
+ * This module allows virtio devices to be used over a virtual PCI device.
+ * This can be used with QEMU based VMMs like KVM or Xen.
+ *
+ * Copyright IBM Corp. 2007
+ * Copyright Red Hat, Inc. 2014
+ *
+ * Authors:
+ *  Anthony Liguori  <aliguori@us.ibm.com>
+ *  Rusty Russell <rusty@rustcorp.com.au>
+ *  Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#define VIRTIO_PCI_NO_LEGACY
+#include "virtio_pci_common.h"
+
+static void __iomem *map_capability(struct pci_dev *dev, int off,
+				    size_t minlen,
+				    u32 align,
+				    u32 start, u32 size,
+				    size_t *len)
+{
+	u8 type_and_bar, bar;
+	u32 offset, length;
+	void __iomem *p;
+
+	pci_read_config_byte(dev, off + offsetof(struct virtio_pci_cap,
+						 type_and_bar),
+			     &type_and_bar);
+	pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, offset),
+			     &offset);
+	pci_read_config_dword(dev, off + offsetof(struct virtio_pci_cap, length),
+			      &length);
+
+	if (length <= start) {
+		dev_err(&dev->dev,
+			"virtio_pci: bad capability len %u (>%u expected)\n",
+			length, start);
+		return NULL;
+	}
+
+	if (length - start < minlen) {
+		dev_err(&dev->dev,
+			"virtio_pci: bad capability len %u (>=%zu expected)\n",
+			length, minlen);
+		return NULL;
+	}
+
+	length -= start;
+
+	if (start + offset < offset) {
+		dev_err(&dev->dev,
+			"virtio_pci: map wrap-around %u+%u\n",
+			start, offset);
+		return NULL;
+	}
+
+	offset += start;
+
+	if (offset & (align - 1)) {
+		dev_err(&dev->dev,
+			"virtio_pci: offset %u not aligned to %u\n",
+			offset, align);
+		return NULL;
+	}
+
+	if (length > size)
+		length = size;
+
+	if (len)
+		*len = length;
+
+	bar = (type_and_bar >> VIRTIO_PCI_CAP_BAR_SHIFT) &
+		VIRTIO_PCI_CAP_BAR_MASK;
+
+	if (minlen + offset < minlen ||
+	    minlen + offset > pci_resource_len(dev, bar)) {
+		dev_err(&dev->dev,
+			"virtio_pci: map virtio %zu@%u "
+			"out of range on bar %i length %lu\n",
+			minlen, offset,
+			bar, (unsigned long)pci_resource_len(dev, bar));
+		return NULL;
+	}
+
+	p = pci_iomap_range(dev, bar, offset, length);
+	if (!p)
+		dev_err(&dev->dev,
+			"virtio_pci: unable to map virtio %u@%u on bar %i\n",
+			length, offset, bar);
+	return p;
+}
+
+static void iowrite64_twopart(u64 val, __le32 __iomem *lo, __le32 __iomem *hi)
+{
+	iowrite32((u32)val, lo);
+	iowrite32(val >> 32, hi);
+}
+
+/* virtio config->get_features() implementation */
+static u64 vp_get_features(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	u64 features;
+
+	iowrite32(0, &vp_dev->common->device_feature_select);
+	features = ioread32(&vp_dev->common->device_feature);
+	iowrite32(1, &vp_dev->common->device_feature_select);
+	features |= ((u64)ioread32(&vp_dev->common->device_feature) << 32);
+
+	return features;
+}
+
+/* virtio config->finalize_features() implementation */
+static int vp_finalize_features(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+	/* Give virtio_ring a chance to accept features. */
+	vring_transport_features(vdev);
+
+	if (!__virtio_test_bit(vdev, VIRTIO_F_VERSION_1)) {
+		dev_err(&vdev->dev, "virtio: device uses modern interface "
+			"but does not have VIRTIO_F_VERSION_1\n");
+		return -EINVAL;
+	}
+
+	iowrite32(0, &vp_dev->common->guest_feature_select);
+	iowrite32((u32)vdev->features, &vp_dev->common->guest_feature);
+	iowrite32(1, &vp_dev->common->guest_feature_select);
+	iowrite32(vdev->features >> 32, &vp_dev->common->guest_feature);
+
+	return 0;
+}
+
+/* virtio config->get() implementation */
+static void vp_get(struct virtio_device *vdev, unsigned offset,
+		   void *buf, unsigned len)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	u8 b;
+	__le16 w;
+	__le32 l;
+
+	BUG_ON(offset + len > vp_dev->device_len);
+
+	switch (len) {
+	case 1:
+		b = ioread8(vp_dev->device + offset);
+		memcpy(buf, &b, sizeof b);
+		break;
+	case 2:
+		w = cpu_to_le16(ioread16(vp_dev->device + offset));
+		memcpy(buf, &w, sizeof w);
+		break;
+	case 4:
+		l = cpu_to_le32(ioread32(vp_dev->device + offset));
+		memcpy(buf, &l, sizeof l);
+		break;
+	case 8:
+		l = cpu_to_le32(ioread32(vp_dev->device + offset));
+		memcpy(buf, &l, sizeof l);
+		l = cpu_to_le32(ioread32(vp_dev->device + offset + sizeof l));
+		memcpy(buf + sizeof l, &l, sizeof l);
+		break;
+	default:
+		BUG();
+	}
+}
+
+/* the config->set() implementation.  it's symmetric to the config->get()
+ * implementation */
+static void vp_set(struct virtio_device *vdev, unsigned offset,
+		   const void *buf, unsigned len)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	u8 b;
+	__le16 w;
+	__le32 l;
+
+	BUG_ON(offset + len > vp_dev->device_len);
+
+	switch (len) {
+	case 1:
+		memcpy(&b, buf, sizeof b);
+		iowrite8(b, vp_dev->device + offset);
+		break;
+	case 2:
+		memcpy(&w, buf, sizeof w);
+		iowrite16(le16_to_cpu(w), vp_dev->device + offset);
+		break;
+	case 4:
+		memcpy(&l, buf, sizeof l);
+		iowrite32(le32_to_cpu(l), vp_dev->device + offset);
+		break;
+	case 8:
+		memcpy(&l, buf, sizeof l);
+		iowrite32(le32_to_cpu(l), vp_dev->device + offset);
+		memcpy(&l, buf + sizeof l, sizeof l);
+		iowrite32(le32_to_cpu(l), vp_dev->device + offset + sizeof l);
+		break;
+	default:
+		BUG();
+	}
+}
+
+static u32 vp_generation(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	return ioread8(&vp_dev->common->config_generation);
+}
+
+/* config->{get,set}_status() implementations */
+static u8 vp_get_status(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	return ioread8(&vp_dev->common->device_status);
+}
+
+static void vp_set_status(struct virtio_device *vdev, u8 status)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	/* We should never be setting status to 0. */
+	BUG_ON(status == 0);
+	iowrite8(status, &vp_dev->common->device_status);
+}
+
+static void vp_reset(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	/* 0 status means a reset. */
+	iowrite8(0, &vp_dev->common->device_status);
+	/* Flush out the status write, and flush in device writes,
+	 * including MSI-X interrupts, if any. */
+	ioread8(&vp_dev->common->device_status);
+	/* Flush pending VQ/configuration callbacks. */
+	vp_synchronize_vectors(vdev);
+}
+
+static u16 vp_config_vector(struct virtio_pci_device *vp_dev, u16 vector)
+{
+	/* Setup the vector used for configuration events */
+	iowrite16(vector, &vp_dev->common->msix_config);
+	/* Verify we had enough resources to assign the vector */
+	/* Will also flush the write out to device */
+	return ioread16(&vp_dev->common->msix_config);
+}
+
+static size_t vring_pci_size(u16 num)
+{
+	/* We only need a cacheline separation. */
+	return PAGE_ALIGN(vring_size(num, SMP_CACHE_BYTES));
+}
+
+static void *alloc_virtqueue_pages(int *num)
+{
+	void *pages;
+
+	/* TODO: allocate each queue chunk individually */
+	for (; *num && vring_pci_size(*num) > PAGE_SIZE; *num /= 2) {
+		pages = alloc_pages_exact(vring_pci_size(*num),
+					  GFP_KERNEL|__GFP_ZERO|__GFP_NOWARN);
+		if (pages)
+			return pages;
+	}
+
+	if (!*num)
+		return NULL;
+
+	/* Try to get a single page. You are my only hope! */
+	return alloc_pages_exact(vring_pci_size(*num), GFP_KERNEL|__GFP_ZERO);
+}
+
+static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
+				  struct virtio_pci_vq_info *info,
+				  unsigned index,
+				  void (*callback)(struct virtqueue *vq),
+				  const char *name,
+				  u16 msix_vec)
+{
+	struct virtio_pci_common_cfg __iomem *cfg = vp_dev->common;
+	struct virtqueue *vq;
+	u16 num, off;
+	int err;
+
+	if (index >= ioread16(&cfg->num_queues))
+		return ERR_PTR(-ENOENT);
+
+	/* Select the queue we're interested in */
+	iowrite16(index, &cfg->queue_select);
+
+	/* Check if queue is either not available or already active. */
+	num = ioread16(&cfg->queue_size);
+	if (!num || ioread8(&cfg->queue_enable))
+		return ERR_PTR(-ENOENT);
+
+	if (num & (num - 1)) {
+		dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* get offset of notification word for this vq */
+	off = ioread16(&cfg->queue_notify_off);
+
+	info->num = num;
+	info->msix_vector = msix_vec;
+
+	info->queue = alloc_virtqueue_pages(&info->num);
+	if (info->queue == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	/* create the vring */
+	vq = vring_new_virtqueue(index, info->num,
+				 SMP_CACHE_BYTES, &vp_dev->vdev,
+				 true, info->queue, vp_notify, callback, name);
+	if (!vq) {
+		err = -ENOMEM;
+		goto err_new_queue;
+	}
+
+	/* activate the queue */
+	iowrite16(num, &cfg->queue_size);
+	iowrite64_twopart(virt_to_phys(info->queue),
+			  &cfg->queue_desc_lo, &cfg->queue_desc_hi);
+	iowrite64_twopart(virt_to_phys(virtqueue_get_avail(vq)),
+			  &cfg->queue_avail_lo, &cfg->queue_avail_hi);
+	iowrite64_twopart(virt_to_phys(virtqueue_get_used(vq)),
+			  &cfg->queue_used_lo, &cfg->queue_used_hi);
+
+	vq->priv = (void __force *)map_capability(vp_dev->pci_dev,
+				  vp_dev->notify_map_cap, 2, 2,
+				  off * vp_dev->notify_offset_multiplier, 2,
+				  NULL);
+
+	if (!vq->priv) {
+		err = -ENOMEM;
+		goto err_map_notify;
+	}
+
+	if (msix_vec != VIRTIO_MSI_NO_VECTOR) {
+		iowrite16(msix_vec, &cfg->queue_msix_vector);
+		msix_vec = ioread16(&cfg->queue_msix_vector);
+		if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
+			err = -EBUSY;
+			goto err_assign_vector;
+		}
+	}
+
+	return vq;
+
+err_assign_vector:
+	pci_iounmap(vp_dev->pci_dev, (void __iomem __force *)vq->priv);
+err_map_notify:
+	vring_del_virtqueue(vq);
+err_new_queue:
+	free_pages_exact(info->queue, vring_pci_size(info->num));
+	return ERR_PTR(err);
+}
+
+static int vp_modern_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+			      struct virtqueue *vqs[],
+			      vq_callback_t *callbacks[],
+			      const char *names[])
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	struct virtqueue *vq;
+	int rc = vp_find_vqs(vdev, nvqs, vqs, callbacks, names);
+
+	if (rc)
+		return rc;
+
+	/* Select and activate all queues. Has to be done last: once we do
+	 * this, there's no way to go back except reset.
+	 */
+	list_for_each_entry(vq, &vdev->vqs, list) {
+		iowrite16(vq->index, &vp_dev->common->queue_select);
+		iowrite8(1, &vp_dev->common->queue_enable);
+	}
+
+	return 0;
+}
+
+static void del_vq(struct virtio_pci_vq_info *info)
+{
+	struct virtqueue *vq = info->vq;
+	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
+
+	iowrite16(vq->index, &vp_dev->common->queue_select);
+
+	if (vp_dev->msix_enabled) {
+		iowrite16(VIRTIO_MSI_NO_VECTOR,
+			  &vp_dev->common->queue_msix_vector);
+		/* Flush the write out to device */
+		ioread16(&vp_dev->common->queue_msix_vector);
+	}
+
+	pci_iounmap(vp_dev->pci_dev, (void __force __iomem *)vq->priv);
+
+	vring_del_virtqueue(vq);
+
+	free_pages_exact(info->queue, vring_pci_size(info->num));
+}
+
+static const struct virtio_config_ops virtio_pci_config_ops = {
+	.get		= vp_get,
+	.set		= vp_set,
+	.generation	= vp_generation,
+	.get_status	= vp_get_status,
+	.set_status	= vp_set_status,
+	.reset		= vp_reset,
+	.find_vqs	= vp_modern_find_vqs,
+	.del_vqs	= vp_del_vqs,
+	.get_features	= vp_get_features,
+	.finalize_features = vp_finalize_features,
+	.bus_name	= vp_bus_name,
+	.set_vq_affinity = vp_set_vq_affinity,
+};
+
+/**
+ * virtio_pci_find_capability - walk capabilities to find device info.
+ * @dev: the pci device
+ * @cfg_type: the VIRTIO_PCI_CAP_* value we seek
+ * @ioresource_types: IORESOURCE_MEM and/or IORESOURCE_IO.
+ *
+ * Returns offset of the capability, or 0.
+ */
+static inline int virtio_pci_find_capability(struct pci_dev *dev, u8 cfg_type,
+					     u32 ioresource_types)
+{
+	int pos;
+
+	for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
+	     pos > 0;
+	     pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
+		u8 type_and_bar, type, bar;
+		pci_read_config_byte(dev, pos + offsetof(struct virtio_pci_cap,
+							 type_and_bar),
+				     &type_and_bar);
+
+		type = (type_and_bar >> VIRTIO_PCI_CAP_TYPE_SHIFT) &
+			VIRTIO_PCI_CAP_TYPE_MASK;
+		bar = (type_and_bar >> VIRTIO_PCI_CAP_BAR_SHIFT) &
+			VIRTIO_PCI_CAP_BAR_MASK;
+
+		/* Ignore structures with reserved BAR values */
+		if (bar > 0x5)
+			continue;
+
+		if (type == cfg_type) {
+			if (pci_resource_len(dev, bar) &&
+			    pci_resource_flags(dev, bar) & ioresource_types)
+				return pos;
+		}
+	}
+	return 0;
+}
+
+static void virtio_pci_release_dev(struct device *_d)
+{
+	struct virtio_device *vdev = dev_to_virtio(_d);
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+	kfree(vp_dev);
+}
+
+/* TODO: validate the ABI statically. */
+static inline void check_offsets(void)
+{
+}
+
+/* the PCI probing function */
+int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
+{
+	struct pci_dev *pci_dev = vp_dev->pci_dev;
+	int err, common, isr, notify, device;
+	u32 notify_length;
+
+	check_offsets();
+
+	/* We only own devices >= 0x1000 and <= 0x107f: leave the rest. */
+	if (pci_dev->device < 0x1000 || pci_dev->device > 0x107f)
+		return -ENODEV;
+
+	if (pci_dev->device < 0x1040) {
+		/* Transitional devices: use the PCI subsystem device id as
+		 * virtio device id, same as legacy driver always did.
+		 */
+		vp_dev->vdev.id.device = pci_dev->subsystem_device;
+	} else {
+		/* Modern devices: simply use PCI device id, but start from 0x1040. */
+		vp_dev->vdev.id.device = pci_dev->device - 0x1040;
+	}
+	vp_dev->vdev.id.vendor = pci_dev->subsystem_vendor;
+
+	if (virtio_device_is_legacy_only(vp_dev->vdev.id))
+		return -ENODEV;
+
+	/* check for a common config: if not, use legacy mode (bar 0). */
+	common = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_COMMON_CFG,
+					    IORESOURCE_IO | IORESOURCE_MEM);
+	if (!common) {
+		dev_info(&pci_dev->dev,
+			 "virtio_pci: leaving for legacy driver\n");
+		return -ENODEV;
+	}
+
+	/* If common is there, these should be too... */
+	isr = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_ISR_CFG,
+					 IORESOURCE_IO | IORESOURCE_MEM);
+	notify = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_NOTIFY_CFG,
+					    IORESOURCE_IO | IORESOURCE_MEM);
+	if (!isr || !notify) {
+		dev_err(&pci_dev->dev,
+			"virtio_pci: missing capabilities %i/%i/%i\n",
+			common, isr, notify);
+		return -EINVAL;
+	}
+
+	/* Device capability is only mandatory for devices that have
+	 * device-specific configuration.
+	 */
+	device = virtio_pci_find_capability(pci_dev, VIRTIO_PCI_CAP_DEVICE_CFG,
+					    IORESOURCE_IO | IORESOURCE_MEM);
+
+	err = -EINVAL;
+	vp_dev->common = map_capability(pci_dev, common,
+					sizeof(struct virtio_pci_common_cfg), 4,
+					0, sizeof(struct virtio_pci_common_cfg),
+					NULL);
+	if (!vp_dev->common)
+		goto err_map_common;
+	vp_dev->isr = map_capability(pci_dev, isr, sizeof(u8), 1,
+				     0, 1,
+				     NULL);
+	if (!vp_dev->isr)
+		goto err_map_isr;
+
+	/* Read notify_off_multiplier from config space. */
+	pci_read_config_dword(pci_dev,
+			      notify + offsetof(struct virtio_pci_notify_cap,
+						notify_off_multiplier),
+			      &vp_dev->notify_offset_multiplier);
+	/* Read notify length from config space. */
+	pci_read_config_dword(pci_dev,
+			      notify + offsetof(struct virtio_pci_notify_cap,
+						cap.length),
+			      &notify_length);
+
+	vp_dev->notify_map_cap = notify;
+
+	/* Again, we don't know how much we should map, but PAGE_SIZE
+	 * is more than enough for all existing devices.
+	 */
+	if (device) {
+		vp_dev->device = map_capability(pci_dev, device, 0, 4,
+						0, PAGE_SIZE,
+						&vp_dev->device_len);
+		if (!vp_dev->device)
+			goto err_map_device;
+	}
+
+	vp_dev->vdev.config = &virtio_pci_config_ops;
+
+	vp_dev->config_vector = vp_config_vector;
+	vp_dev->setup_vq = setup_vq;
+	vp_dev->del_vq = del_vq;
+
+	return 0;
+
+err_map_device:
+	pci_iounmap(pci_dev, vp_dev->isr);
+err_map_isr:
+	pci_iounmap(pci_dev, vp_dev->common);
+err_map_common:
+	return err;
+}
+
+void virtio_pci_modern_remove(struct virtio_pci_device *vp_dev)
+{
+	struct pci_dev *pci_dev = vp_dev->pci_dev;
+
+	if (vp_dev->device)
+		pci_iounmap(pci_dev, vp_dev->device);
+	pci_iounmap(pci_dev, vp_dev->isr);
+	pci_iounmap(pci_dev, vp_dev->common);
+}
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index bf5104b..bd230d1 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -1,5 +1,5 @@
 obj-$(CONFIG_VIRTIO) += virtio.o virtio_ring.o
 obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o
 obj-$(CONFIG_VIRTIO_PCI) += virtio_pci.o
-virtio_pci-y := virtio_pci_legacy.o virtio_pci_common.o
+virtio_pci-y := virtio_pci_modern.o virtio_pci_legacy.o virtio_pci_common.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
-- 
MST

[PATCH v3 14/16] virtio_pci: macros for PCI layout offsets

[Michael S. Tsirkin]

From: Rusty Russell <rusty@rustcorp.com.au>

QEMU wants it, so why not?  Trust, but verify.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/uapi/linux/virtio_pci.h    | 30 ++++++++++++++++++++
 drivers/virtio/virtio_pci_modern.c | 58 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 4e05423..e841edd 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -159,6 +159,36 @@ struct virtio_pci_common_cfg {
 	__le32 queue_used_hi;		/* read-write */
 };
 
+/* Macro versions of offsets for the Old Timers! */
+#define VIRTIO_PCI_CAP_VNDR		0
+#define VIRTIO_PCI_CAP_NEXT		1
+#define VIRTIO_PCI_CAP_LEN		2
+#define VIRTIO_PCI_CAP_TYPE_AND_BAR	3
+#define VIRTIO_PCI_CAP_OFFSET		4
+#define VIRTIO_PCI_CAP_LENGTH		8
+
+#define VIRTIO_PCI_NOTIFY_CAP_MULT	12
+
+#define VIRTIO_PCI_COMMON_DFSELECT	0
+#define VIRTIO_PCI_COMMON_DF		4
+#define VIRTIO_PCI_COMMON_GFSELECT	8
+#define VIRTIO_PCI_COMMON_GF		12
+#define VIRTIO_PCI_COMMON_MSIX		16
+#define VIRTIO_PCI_COMMON_NUMQ		18
+#define VIRTIO_PCI_COMMON_STATUS	20
+#define VIRTIO_PCI_COMMON_CFGGENERATION	21
+#define VIRTIO_PCI_COMMON_Q_SELECT	22
+#define VIRTIO_PCI_COMMON_Q_SIZE	24
+#define VIRTIO_PCI_COMMON_Q_MSIX	26
+#define VIRTIO_PCI_COMMON_Q_ENABLE	28
+#define VIRTIO_PCI_COMMON_Q_NOFF	30
+#define VIRTIO_PCI_COMMON_Q_DESCLO	32
+#define VIRTIO_PCI_COMMON_Q_DESCHI	36
+#define VIRTIO_PCI_COMMON_Q_AVAILLO	40
+#define VIRTIO_PCI_COMMON_Q_AVAILHI	44
+#define VIRTIO_PCI_COMMON_Q_USEDLO	48
+#define VIRTIO_PCI_COMMON_Q_USEDHI	52
+
 #endif /* VIRTIO_PCI_NO_MODERN */
 
 #endif
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 38c0a11..5e0d309 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -469,9 +469,65 @@ static void virtio_pci_release_dev(struct device *_d)
 	kfree(vp_dev);
 }
 
-/* TODO: validate the ABI statically. */
+/* This is part of the ABI.  Don't screw with it. */
 static inline void check_offsets(void)
 {
+	/* Note: disk space was harmed in compilation of this function. */
+	BUILD_BUG_ON(VIRTIO_PCI_CAP_VNDR !=
+		     offsetof(struct virtio_pci_cap, cap_vndr));
+	BUILD_BUG_ON(VIRTIO_PCI_CAP_NEXT !=
+		     offsetof(struct virtio_pci_cap, cap_next));
+	BUILD_BUG_ON(VIRTIO_PCI_CAP_LEN !=
+		     offsetof(struct virtio_pci_cap, cap_len));
+	BUILD_BUG_ON(VIRTIO_PCI_CAP_TYPE_AND_BAR !=
+		     offsetof(struct virtio_pci_cap, type_and_bar));
+	BUILD_BUG_ON(VIRTIO_PCI_CAP_OFFSET !=
+		     offsetof(struct virtio_pci_cap, offset));
+	BUILD_BUG_ON(VIRTIO_PCI_CAP_LENGTH !=
+		     offsetof(struct virtio_pci_cap, length));
+	BUILD_BUG_ON(VIRTIO_PCI_NOTIFY_CAP_MULT !=
+		     offsetof(struct virtio_pci_notify_cap,
+			      notify_off_multiplier));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_DFSELECT !=
+		     offsetof(struct virtio_pci_common_cfg,
+			      device_feature_select));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_DF !=
+		     offsetof(struct virtio_pci_common_cfg, device_feature));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_GFSELECT !=
+		     offsetof(struct virtio_pci_common_cfg,
+			      guest_feature_select));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_GF !=
+		     offsetof(struct virtio_pci_common_cfg, guest_feature));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_MSIX !=
+		     offsetof(struct virtio_pci_common_cfg, msix_config));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_NUMQ !=
+		     offsetof(struct virtio_pci_common_cfg, num_queues));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_STATUS !=
+		     offsetof(struct virtio_pci_common_cfg, device_status));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_CFGGENERATION !=
+		     offsetof(struct virtio_pci_common_cfg, config_generation));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_SELECT !=
+		     offsetof(struct virtio_pci_common_cfg, queue_select));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_SIZE !=
+		     offsetof(struct virtio_pci_common_cfg, queue_size));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_MSIX !=
+		     offsetof(struct virtio_pci_common_cfg, queue_msix_vector));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_ENABLE !=
+		     offsetof(struct virtio_pci_common_cfg, queue_enable));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_NOFF !=
+		     offsetof(struct virtio_pci_common_cfg, queue_notify_off));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_DESCLO !=
+		     offsetof(struct virtio_pci_common_cfg, queue_desc_lo));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_DESCHI !=
+		     offsetof(struct virtio_pci_common_cfg, queue_desc_hi));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_AVAILLO !=
+		     offsetof(struct virtio_pci_common_cfg, queue_avail_lo));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_AVAILHI !=
+		     offsetof(struct virtio_pci_common_cfg, queue_avail_hi));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_USEDLO !=
+		     offsetof(struct virtio_pci_common_cfg, queue_used_lo));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_USEDHI !=
+		     offsetof(struct virtio_pci_common_cfg, queue_used_hi));
 }
 
 /* the PCI probing function */
-- 
MST

[PATCH v3 15/16] virtio_pci_modern: reduce number of mappings

[Michael S. Tsirkin]

We don't know the # of VQs that drivers are going to use so it's hard to
predict how much memory we'll need to map. However, the relevant
capability does give us an upper limit.
If that's below a page, we can reduce the number of required
mappings by mapping it all once ahead of the time.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci_common.h |  3 ++
 drivers/virtio/virtio_pci_modern.c | 57 ++++++++++++++++++++++++++++++++------
 2 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index 610c43f..d391805 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -62,8 +62,11 @@ struct virtio_pci_device {
 	struct virtio_pci_common_cfg __iomem *common;
 	/* Device-specific data (non-legacy mode)  */
 	void __iomem *device;
+	/* Base of vq notifications (non-legacy mode). */
+	void __iomem *notify_base;
 
 	/* So we can sanity-check accesses. */
+	size_t notify_len;
 	size_t device_len;
 
 	/* Capability for when we need to map notifications per-vq. */
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 5e0d309..9c000e9a 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -333,10 +333,26 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
 	iowrite64_twopart(virt_to_phys(virtqueue_get_used(vq)),
 			  &cfg->queue_used_lo, &cfg->queue_used_hi);
 
-	vq->priv = (void __force *)map_capability(vp_dev->pci_dev,
-				  vp_dev->notify_map_cap, 2, 2,
-				  off * vp_dev->notify_offset_multiplier, 2,
-				  NULL);
+	if (vp_dev->notify_base) {
+		/* offset should not wrap */
+		if ((u64)off * vp_dev->notify_offset_multiplier + 2
+		    > vp_dev->notify_len) {
+			dev_warn(&vp_dev->pci_dev->dev,
+				 "bad notification offset %u (x %u) "
+				 "for queue %u > %zd",
+				 off, vp_dev->notify_offset_multiplier,
+				 index, vp_dev->notify_len);
+			err = -EINVAL;
+			goto err_map_notify;
+		}
+		vq->priv = (void __force *)vp_dev->notify_base +
+			off * vp_dev->notify_offset_multiplier;
+	} else {
+		vq->priv = (void __force *)map_capability(vp_dev->pci_dev,
+					  vp_dev->notify_map_cap, 2, 2,
+					  off * vp_dev->notify_offset_multiplier, 2,
+					  NULL);
+	}
 
 	if (!vq->priv) {
 		err = -ENOMEM;
@@ -355,7 +371,8 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
 	return vq;
 
 err_assign_vector:
-	pci_iounmap(vp_dev->pci_dev, (void __iomem __force *)vq->priv);
+	if (!vp_dev->notify_base)
+		pci_iounmap(vp_dev->pci_dev, (void __iomem __force *)vq->priv);
 err_map_notify:
 	vring_del_virtqueue(vq);
 err_new_queue:
@@ -400,7 +417,8 @@ static void del_vq(struct virtio_pci_vq_info *info)
 		ioread16(&vp_dev->common->queue_msix_vector);
 	}
 
-	pci_iounmap(vp_dev->pci_dev, (void __force __iomem *)vq->priv);
+	if (!vp_dev->notify_base)
+		pci_iounmap(vp_dev->pci_dev, (void __force __iomem *)vq->priv);
 
 	vring_del_virtqueue(vq);
 
@@ -536,6 +554,7 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
 	struct pci_dev *pci_dev = vp_dev->pci_dev;
 	int err, common, isr, notify, device;
 	u32 notify_length;
+	u32 notify_offset;
 
 	check_offsets();
 
@@ -602,13 +621,30 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
 			      notify + offsetof(struct virtio_pci_notify_cap,
 						notify_off_multiplier),
 			      &vp_dev->notify_offset_multiplier);
-	/* Read notify length from config space. */
+	/* Read notify length and offset from config space. */
 	pci_read_config_dword(pci_dev,
 			      notify + offsetof(struct virtio_pci_notify_cap,
 						cap.length),
 			      &notify_length);
 
-	vp_dev->notify_map_cap = notify;
+	pci_read_config_dword(pci_dev,
+			      notify + offsetof(struct virtio_pci_notify_cap,
+						cap.length),
+			      &notify_offset);
+
+	/* We don't know how many VQs we'll map, ahead of the time.
+	 * If notify length is small, map it all now.
+	 * Otherwise, map each VQ individually later.
+	 */
+	if ((u64)notify_length + (notify_offset % PAGE_SIZE) <= PAGE_SIZE) {
+		vp_dev->notify_base = map_capability(pci_dev, notify, 2, 2,
+						     0, notify_length,
+						     &vp_dev->notify_len);
+		if (!vp_dev->notify_base)
+			goto err_map_notify;
+	} else {
+		vp_dev->notify_map_cap = notify;
+	}
 
 	/* Again, we don't know how much we should map, but PAGE_SIZE
 	 * is more than enough for all existing devices.
@@ -630,6 +666,9 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
 	return 0;
 
 err_map_device:
+	if (vp_dev->notify_base)
+		pci_iounmap(pci_dev, vp_dev->notify_base);
+err_map_notify:
 	pci_iounmap(pci_dev, vp_dev->isr);
 err_map_isr:
 	pci_iounmap(pci_dev, vp_dev->common);
@@ -643,6 +682,8 @@ void virtio_pci_modern_remove(struct virtio_pci_device *vp_dev)
 
 	if (vp_dev->device)
 		pci_iounmap(pci_dev, vp_dev->device);
+	if (vp_dev->notify_base)
+		pci_iounmap(pci_dev, vp_dev->notify_base);
 	pci_iounmap(pci_dev, vp_dev->isr);
 	pci_iounmap(pci_dev, vp_dev->common);
 }
-- 
MST

[PATCH v3 16/16] virtio_pci_modern: support devices with no config

[Michael S. Tsirkin]

Virtio 1.0 spec lists device config as optional.
Set get/set callbacks to NULL. Drivers can check that
and fail gracefully.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_pci_modern.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 9c000e9a..b877705 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -425,6 +425,21 @@ static void del_vq(struct virtio_pci_vq_info *info)
 	free_pages_exact(info->queue, vring_pci_size(info->num));
 }
 
+static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
+	.get		= NULL,
+	.set		= NULL,
+	.generation	= vp_generation,
+	.get_status	= vp_get_status,
+	.set_status	= vp_set_status,
+	.reset		= vp_reset,
+	.find_vqs	= vp_modern_find_vqs,
+	.del_vqs	= vp_del_vqs,
+	.get_features	= vp_get_features,
+	.finalize_features = vp_finalize_features,
+	.bus_name	= vp_bus_name,
+	.set_vq_affinity = vp_set_vq_affinity,
+};
+
 static const struct virtio_config_ops virtio_pci_config_ops = {
 	.get		= vp_get,
 	.set		= vp_set,
@@ -655,9 +670,11 @@ int virtio_pci_modern_probe(struct virtio_pci_device *vp_dev)
 						&vp_dev->device_len);
 		if (!vp_dev->device)
 			goto err_map_device;
-	}
 
-	vp_dev->vdev.config = &virtio_pci_config_ops;
+		vp_dev->vdev.config = &virtio_pci_config_ops;
+	} else {
+		vp_dev->vdev.config = &virtio_pci_config_nodev_ops;
+	}
 
 	vp_dev->config_vector = vp_config_vector;
 	vp_dev->setup_vq = setup_vq;
-- 
MST

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

  Hi,

> QEMU side is still undergoing polishing, but is already testable.

Looked art the qemu side for now, commenting here due to qemu patches
not being posted to the list yet (guess that is at least partly already
on your todo list):

 * Both legacy and modern should be switchable.  Modern needs to be
   switchable _anyway_ for backward compatibility with old qemu
   versions.  But legacy should be too, for testing purposes and for
   new device types which don't need legacy in the first place.

 * I'd like to see some more flexibility in the pci bar layout.  Stuff
   I have in mind:
    - New devices which don't need a legacy bar can use bar 0 for
      modern.
    - One MMIO bar is enough, we can place both virtio regions and
      msi-x regions there.  I'd suggest to add msi-x sub-regions to
      the modern bar.

 * What is the reason for making the modern bar 8M in size?  Looks a bit
   excessive, given that only 64k or so of that are actually used ...

virtio-scsi seems to be broken, at least my usual fedora guest didn't
boot up from virtio-scsi disk when using a guest kernel with this patch
series applied.

cheers,
  Gerd

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Thu, Jan 15, 2015 at 10:18:18PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > QEMU side is still undergoing polishing, but is already testable.
> 
> Looked art the qemu side for now, commenting here due to qemu patches
> not being posted to the list yet (guess that is at least partly already
> on your todo list):

Absolutely.

>  * Both legacy and modern should be switchable.  Modern needs to be
>    switchable _anyway_ for backward compatibility with old qemu
>    versions.  But legacy should be too, for testing purposes and for
>    new device types which don't need legacy in the first place.

I agree.

>  * I'd like to see some more flexibility in the pci bar layout.  Stuff
>    I have in mind:
>     - New devices which don't need a legacy bar can use bar 0 for
>       modern.
>     - One MMIO bar is enough, we can place both virtio regions and
>       msi-x regions there.  I'd suggest to add msi-x sub-regions to
>       the modern bar.

Why exactly? It seems simpler to separate things, extra BARs
have no cost.

>  * What is the reason for making the modern bar 8M in size?  Looks a bit
>    excessive, given that only 64k or so of that are actually used ...

I use a page per VQ for architectures that can locate the offset of the
accessed page that triggered EPT violation faster than the offset within
page.  I think this is the case for SVM.

> 
> virtio-scsi seems to be broken, at least my usual fedora guest didn't
> boot up from virtio-scsi disk when using a guest kernel with this patch
> series applied.
> 
> cheers,
>   Gerd

I'll re-test. Do other devices work for you? Thanks!

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

  Hi,

> >  * I'd like to see some more flexibility in the pci bar layout.  Stuff
> >    I have in mind:
> >     - New devices which don't need a legacy bar can use bar 0 for
> >       modern.
> >     - One MMIO bar is enough, we can place both virtio regions and
> >       msi-x regions there.  I'd suggest to add msi-x sub-regions to
> >       the modern bar.
> 
> Why exactly? It seems simpler to separate things, extra BARs
> have no cost.

Well, there are only six BARs.  legacy bar, modern mmio bar, msi-x bar,
modern io bar (for fast isr).  That already four out of six ...

We have a mmio bar, which we partition into subregions for virtio-1.0
anyway.  Also placing msi-x there is a single msix_init() call.  xhci is
doing that too:

    00000000febf0000-00000000febf3fff (prio 1, RW): xhci
      00000000febf0000-00000000febf003f (prio 0, RW): capabilities
      00000000febf0040-00000000febf043f (prio 0, RW): operational
      00000000febf0440-00000000febf044f (prio 0, RW): usb3 port #1
      00000000febf0450-00000000febf045f (prio 0, RW): usb3 port #2
      00000000febf0460-00000000febf046f (prio 0, RW): usb3 port #3
      00000000febf0470-00000000febf047f (prio 0, RW): usb3 port #4
      00000000febf0480-00000000febf048f (prio 0, RW): usb2 port #1
      00000000febf0490-00000000febf049f (prio 0, RW): usb2 port #2
      00000000febf04a0-00000000febf04af (prio 0, RW): usb2 port #3
      00000000febf04b0-00000000febf04bf (prio 0, RW): usb2 port #4
      00000000febf1000-00000000febf121f (prio 0, RW): runtime
      00000000febf2000-00000000febf281f (prio 0, RW): doorbell
      00000000febf3000-00000000febf30ff (prio 0, RW): msix-table
      00000000febf3800-00000000febf3807 (prio 0, RW): msix-pba


> >  * What is the reason for making the modern bar 8M in size?  Looks a bit
> >    excessive, given that only 64k or so of that are actually used ...
> 
> I use a page per VQ for architectures that can locate the offset of the
> accessed page that triggered EPT violation faster than the offset within
> page.  I think this is the case for SVM.

8M still looks excessive, given that we typically have a small number of
queues per device.  Do you allocate address space for the maximum
possible number of queues unconditionally?

> > virtio-scsi seems to be broken, at least my usual fedora guest didn't
> > boot up from virtio-scsi disk when using a guest kernel with this patch
> > series applied.
> 
> I'll re-test. Do other devices work for you? Thanks!

Didn't came very far yet in my testing due to the guest not booting.  I
plan to try other storage for the image, but didn't found the time yet.

I've tried to boot a F21 live iso with virtio-net (legacy guest driver
obviously), which seems to work ok in light testing.

BTW: is there a tool (or pciutils patch) which can decode the virtio
capabilities?

cheers,
  Gerd

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Fri, Jan 16, 2015 at 09:32:26AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > >  * I'd like to see some more flexibility in the pci bar layout.  Stuff
> > >    I have in mind:
> > >     - New devices which don't need a legacy bar can use bar 0 for
> > >       modern.
> > >     - One MMIO bar is enough, we can place both virtio regions and
> > >       msi-x regions there.  I'd suggest to add msi-x sub-regions to
> > >       the modern bar.
> > 
> > Why exactly? It seems simpler to separate things, extra BARs
> > have no cost.
> 
> Well, there are only six BARs.  legacy bar, modern mmio bar, msi-x bar,
> modern io bar (for fast isr).  That already four out of six ...

So?  If we need to add another bar, we'll change things.

> We have a mmio bar, which we partition into subregions for virtio-1.0
> anyway.  Also placing msi-x there is a single msix_init() call.  xhci is
> doing that too:
> 
>     00000000febf0000-00000000febf3fff (prio 1, RW): xhci
>       00000000febf0000-00000000febf003f (prio 0, RW): capabilities
>       00000000febf0040-00000000febf043f (prio 0, RW): operational
>       00000000febf0440-00000000febf044f (prio 0, RW): usb3 port #1
>       00000000febf0450-00000000febf045f (prio 0, RW): usb3 port #2
>       00000000febf0460-00000000febf046f (prio 0, RW): usb3 port #3
>       00000000febf0470-00000000febf047f (prio 0, RW): usb3 port #4
>       00000000febf0480-00000000febf048f (prio 0, RW): usb2 port #1
>       00000000febf0490-00000000febf049f (prio 0, RW): usb2 port #2
>       00000000febf04a0-00000000febf04af (prio 0, RW): usb2 port #3
>       00000000febf04b0-00000000febf04bf (prio 0, RW): usb2 port #4
>       00000000febf1000-00000000febf121f (prio 0, RW): runtime
>       00000000febf2000-00000000febf281f (prio 0, RW): doorbell
>       00000000febf3000-00000000febf30ff (prio 0, RW): msix-table
>       00000000febf3800-00000000febf3807 (prio 0, RW): msix-pba
> 

However that one is non prefetheable, and I prefer putting
virtio caps in a prefetcheable bar.

> > >  * What is the reason for making the modern bar 8M in size?  Looks a bit
> > >    excessive, given that only 64k or so of that are actually used ...
> > 
> > I use a page per VQ for architectures that can locate the offset of the
> > accessed page that triggered EPT violation faster than the offset within
> > page.  I think this is the case for SVM.
> 
> 8M still looks excessive, given that we typically have a small number of
> queues per device.  Do you allocate address space for the maximum
> possible number of queues unconditionally?

Yes, simpler this way. I'll check if I can find the real # of VQs.

> > > virtio-scsi seems to be broken, at least my usual fedora guest didn't
> > > boot up from virtio-scsi disk when using a guest kernel with this patch
> > > series applied.
> > 
> > I'll re-test. Do other devices work for you? Thanks!
> 
> Didn't came very far yet in my testing due to the guest not booting.

Try virtio-blk - that worked for me.

>  I
> plan to try other storage for the image, but didn't found the time yet.
> 
> I've tried to boot a F21 live iso with virtio-net (legacy guest driver
> obviously), which seems to work ok in light testing.
> 
> BTW: is there a tool (or pciutils patch) which can decode the virtio
> capabilities?
> 
> cheers,
>   Gerd
> 

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

  Hi,

> > > > virtio-scsi seems to be broken, at least my usual fedora guest didn't
> > > > boot up from virtio-scsi disk when using a guest kernel with this patch
> > > > series applied.
> > > 
> > > I'll re-test. Do other devices work for you? Thanks!
> > 
> > Didn't came very far yet in my testing due to the guest not booting.
> 
> Try virtio-blk - that worked for me.

Nope.  Booted from ide now, looked in /sys/bus/virtio/devices:

lrwxrwxrwx. 1 root root 0 Jan 16 14:15 virtio0
-> ../../../devices/pci0000:00/0000:00:0b.0/virtio0

lspci:

00:09.0 Ethernet controller: Red Hat, Inc Virtio network device
00:0a.0 Communication controller: Red Hat, Inc Virtio console
00:0b.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon

net + serial are missing ...

I'm running 3.19-rc4 with this series applied in the guest, maybe I need
more?

On the qemu side it is your virtio-1.0 branch from yesterday, rebased to
master.

cheers,
  Gerd

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

  Hi,

> I'm running 3.19-rc4 with this series applied in the guest, maybe I need
> more?

Tried your vhost branch, rebased to 3.19-rc5 -- same result.  Hmm.

cheers,
  Gerd

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 11:54:43AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > I'm running 3.19-rc4 with this series applied in the guest, maybe I need
> > more?
> 
> Tried your vhost branch, rebased to 3.19-rc5 -- same result.  Hmm.
> 
> cheers,
>   Gerd
> 

Yes, it was all completely broken.
Looks like after making what looked like cosmetic changes, I run legacy
driver on new qemu and new driver on legacy qemu, but never new driver
on new qemu.

And sure enough, the "cosmetic" change inverted the logic testing probe
routine for success.

Sorry about that :(

I now posted fixed up patches: seem to work with net and block for me.

The easiest way to get the is probably from my git trees.

Thanks a lot for the reports!

-- 
MST

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 788 bytes --]

  Hi,

> BTW: is there a tool (or pciutils patch) which can decode the virtio
> capabilities?

Searched for a patch today, and all google found me was this mail asking
for one :-o

So I went ahead and coded one up.  Attached.

While hacking it up I've noticed spec doesn't match reality.  The
"Virtio Structure PCI Capabilities" section here ...

http://docs.oasis-open.org/virtio/virtio/v1.0/cs01/virtio-v1.0-cs01.html#x1-690004

... doesn't match what qemu is doing.  Huh?


Also note:

[root@fedora ~]# ~kraxel/projects/pciutils/lspci -vvsa
00:0a.0 Communication controller: Red Hat, Inc Virtio console
[ ... ]
	Capabilities: [64] VirtIO: Notify
		BAR=2 offset=00003000 size=00400000
		multiplier=00010000
[ ... ]

Why multiplier is 64k instead of 4k?  Just a tyops?


cheers,
  Gerd


[-- Attachment #2: 0001-add-virtio-vendor-caps.patch --]
[-- Type: text/x-patch, Size: 3354 bytes --]

>From 0d5d99c20b079c986128c3d98c4bbaba51ebe65c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 19 Jan 2015 11:46:37 +0100
Subject: [PATCH] add virtio vendor caps

---
 Makefile         |  2 +-
 ls-caps-vendor.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ls-caps.c        |  9 ++++++++-
 lspci.h          |  4 ++++
 4 files changed, 70 insertions(+), 2 deletions(-)
 create mode 100644 ls-caps-vendor.c

diff --git a/Makefile b/Makefile
index 8d49afa..39a07d1 100644
--- a/Makefile
+++ b/Makefile
@@ -69,7 +69,7 @@ force:
 lib/config.h lib/config.mk:
 	cd lib && ./configure
 
-lspci: lspci.o ls-vpd.o ls-caps.o ls-ecaps.o ls-kernel.o ls-tree.o ls-map.o common.o lib/$(PCILIB)
+lspci: lspci.o ls-vpd.o ls-caps.o ls-caps-vendor.o ls-ecaps.o ls-kernel.o ls-tree.o ls-map.o common.o lib/$(PCILIB)
 setpci: setpci.o common.o lib/$(PCILIB)
 
 LSPCIINC=lspci.h pciutils.h $(PCIINC)
diff --git a/ls-caps-vendor.c b/ls-caps-vendor.c
new file mode 100644
index 0000000..5f8b3e1
--- /dev/null
+++ b/ls-caps-vendor.c
@@ -0,0 +1,57 @@
+/*
+ *	The PCI Utilities -- Show Vendor-specific Capabilities
+ *
+ *	Copyright (c) 2014 Gerd Hoffmann <kraxel@redhat.com>
+ *
+ *	Can be freely distributed and used under the terms of the GNU GPL.
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "lspci.h"
+
+void
+show_vendor_caps_virtio(struct device *d, int where, int cap)
+{
+  int length = BITS(cap, 0, 8);
+  int type = BITS(cap, 8, 5);
+  int bar = BITS(cap, 13, 3);
+  char *tname;
+
+  if (length < 12)
+    return;
+  if (!config_fetch(d, where, length))
+    return;
+
+  switch (type) {
+  case 1:
+      tname = "CommonCfg";
+      break;
+  case 2:
+      tname = "Notify";
+      break;
+  case 3:
+      tname = "ISR";
+      break;
+  case 4:
+      tname = "DeviceCfg";
+      break;
+  default:
+      tname = "<unknown>";
+      break;
+  }
+
+  printf("VirtIO: %s\n", tname);
+
+  if (verbose < 2)
+    return;
+
+  printf("\t\tBAR=%d offset=%08x size=%08x\n", bar,
+         get_conf_long(d, where+4),
+         get_conf_long(d, where+8));
+  if (type != 2 || length < 16)
+    return;
+  printf("\t\tmultiplier=%08x\n",
+         get_conf_long(d, where+12));
+}
diff --git a/ls-caps.c b/ls-caps.c
index 7de55ef..438b580 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -1315,7 +1315,14 @@ show_caps(struct device *d, int where)
 	      cap_ht(d, where, cap);
 	      break;
 	    case PCI_CAP_ID_VNDR:
-	      printf("Vendor Specific Information: Len=%02x <?>\n", BITS(cap, 0, 8));
+              switch (get_conf_word(d, PCI_VENDOR_ID)) {
+              case 0x1af4: /* Red Hat, devices 0x1000 -> 0x107f are virtio */
+                show_vendor_caps_virtio(d, where, cap);
+                break;
+              default:
+                printf("Vendor Specific Information: Len=%02x <?>\n", BITS(cap, 0, 8));
+                break;
+              }
 	      break;
 	    case PCI_CAP_ID_DBG:
 	      cap_debug_port(cap);
diff --git a/lspci.h b/lspci.h
index 86429b2..5ca9899 100644
--- a/lspci.h
+++ b/lspci.h
@@ -70,6 +70,10 @@ void show_caps(struct device *d, int where);
 
 void show_ext_caps(struct device *d);
 
+/* ls-caps-vendor.c */
+
+void show_vendor_caps_virtio(struct device *d, int where, int cap);
+
 /* ls-kernel.c */
 
 void show_kernel_machine(struct device *d UNUSED);
-- 
1.8.3.1

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 12:07:00PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > BTW: is there a tool (or pciutils patch) which can decode the virtio
> > capabilities?
> 
> Searched for a patch today, and all google found me was this mail asking
> for one :-o
> 
> So I went ahead and coded one up.  Attached.
> 
> While hacking it up I've noticed spec doesn't match reality.  The
> "Virtio Structure PCI Capabilities" section here ...
> 
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs01/virtio-v1.0-cs01.html#x1-690004
> 
> ... doesn't match what qemu is doing.  Huh?

Wow! And I made the same bug in the guest too.
Good catch, will fix both up.

> 
> Also note:
> 
> [root@fedora ~]# ~kraxel/projects/pciutils/lspci -vvsa
> 00:0a.0 Communication controller: Red Hat, Inc Virtio console
> [ ... ]
> 	Capabilities: [64] VirtIO: Notify
> 		BAR=2 offset=00003000 size=00400000
> 		multiplier=00010000
> [ ... ]
> 
> Why multiplier is 64k instead of 4k?  Just a tyops?
> 
> 
> cheers,
>   Gerd
> 

Just in case :) I'll make it smaller, will address your concerns
about BAR size in a simple way.


> >From 0d5d99c20b079c986128c3d98c4bbaba51ebe65c Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@redhat.com>
> Date: Mon, 19 Jan 2015 11:46:37 +0100
> Subject: [PATCH] add virtio vendor caps
> 
> ---
>  Makefile         |  2 +-
>  ls-caps-vendor.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  ls-caps.c        |  9 ++++++++-
>  lspci.h          |  4 ++++
>  4 files changed, 70 insertions(+), 2 deletions(-)
>  create mode 100644 ls-caps-vendor.c
> 
> diff --git a/Makefile b/Makefile
> index 8d49afa..39a07d1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -69,7 +69,7 @@ force:
>  lib/config.h lib/config.mk:
>  	cd lib && ./configure
>  
> -lspci: lspci.o ls-vpd.o ls-caps.o ls-ecaps.o ls-kernel.o ls-tree.o ls-map.o common.o lib/$(PCILIB)
> +lspci: lspci.o ls-vpd.o ls-caps.o ls-caps-vendor.o ls-ecaps.o ls-kernel.o ls-tree.o ls-map.o common.o lib/$(PCILIB)
>  setpci: setpci.o common.o lib/$(PCILIB)
>  
>  LSPCIINC=lspci.h pciutils.h $(PCIINC)
> diff --git a/ls-caps-vendor.c b/ls-caps-vendor.c
> new file mode 100644
> index 0000000..5f8b3e1
> --- /dev/null
> +++ b/ls-caps-vendor.c
> @@ -0,0 +1,57 @@
> +/*
> + *	The PCI Utilities -- Show Vendor-specific Capabilities
> + *
> + *	Copyright (c) 2014 Gerd Hoffmann <kraxel@redhat.com>
> + *
> + *	Can be freely distributed and used under the terms of the GNU GPL.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "lspci.h"
> +
> +void
> +show_vendor_caps_virtio(struct device *d, int where, int cap)
> +{
> +  int length = BITS(cap, 0, 8);
> +  int type = BITS(cap, 8, 5);
> +  int bar = BITS(cap, 13, 3);
> +  char *tname;
> +
> +  if (length < 12)
> +    return;
> +  if (!config_fetch(d, where, length))
> +    return;
> +
> +  switch (type) {
> +  case 1:
> +      tname = "CommonCfg";
> +      break;
> +  case 2:
> +      tname = "Notify";
> +      break;
> +  case 3:
> +      tname = "ISR";
> +      break;
> +  case 4:
> +      tname = "DeviceCfg";
> +      break;
> +  default:
> +      tname = "<unknown>";
> +      break;
> +  }
> +
> +  printf("VirtIO: %s\n", tname);
> +
> +  if (verbose < 2)
> +    return;
> +
> +  printf("\t\tBAR=%d offset=%08x size=%08x\n", bar,
> +         get_conf_long(d, where+4),
> +         get_conf_long(d, where+8));
> +  if (type != 2 || length < 16)
> +    return;
> +  printf("\t\tmultiplier=%08x\n",
> +         get_conf_long(d, where+12));
> +}
> diff --git a/ls-caps.c b/ls-caps.c
> index 7de55ef..438b580 100644
> --- a/ls-caps.c
> +++ b/ls-caps.c
> @@ -1315,7 +1315,14 @@ show_caps(struct device *d, int where)
>  	      cap_ht(d, where, cap);
>  	      break;
>  	    case PCI_CAP_ID_VNDR:
> -	      printf("Vendor Specific Information: Len=%02x <?>\n", BITS(cap, 0, 8));
> +              switch (get_conf_word(d, PCI_VENDOR_ID)) {
> +              case 0x1af4: /* Red Hat, devices 0x1000 -> 0x107f are virtio */
> +                show_vendor_caps_virtio(d, where, cap);
> +                break;
> +              default:
> +                printf("Vendor Specific Information: Len=%02x <?>\n", BITS(cap, 0, 8));
> +                break;
> +              }
>  	      break;
>  	    case PCI_CAP_ID_DBG:
>  	      cap_debug_port(cap);
> diff --git a/lspci.h b/lspci.h
> index 86429b2..5ca9899 100644
> --- a/lspci.h
> +++ b/lspci.h
> @@ -70,6 +70,10 @@ void show_caps(struct device *d, int where);
>  
>  void show_ext_caps(struct device *d);
>  
> +/* ls-caps-vendor.c */
> +
> +void show_vendor_caps_virtio(struct device *d, int where, int cap);
> +
>  /* ls-kernel.c */
>  
>  void show_kernel_machine(struct device *d UNUSED);
> -- 
> 1.8.3.1
> 

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Mon, Jan 19, 2015 at 12:07:00PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > BTW: is there a tool (or pciutils patch) which can decode the virtio
> > capabilities?
> 
> Searched for a patch today, and all google found me was this mail asking
> for one :-o
> 
> So I went ahead and coded one up.  Attached.
> 
> While hacking it up I've noticed spec doesn't match reality.  The
> "Virtio Structure PCI Capabilities" section here ...
> 
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs01/virtio-v1.0-cs01.html#x1-690004
> 
> ... doesn't match what qemu is doing.  Huh?

Thanks a lot for the report, and the tool!
I sent patches to fix this all up.
You can try my qemu and linux virtio-net branches now,
they should be spec compliant.

-- 
MST

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

  Hi,

> > While hacking it up I've noticed spec doesn't match reality.  The
> > "Virtio Structure PCI Capabilities" section here ...
> > 
> > http://docs.oasis-open.org/virtio/virtio/v1.0/cs01/virtio-v1.0-cs01.html#x1-690004
> > 
> > ... doesn't match what qemu is doing.  Huh?
> 
> Thanks a lot for the report, and the tool!

pciutils patch needs an update too (because I've made it decode the
broken qemu capabilities).  Will come as separate mail with proper
commit message in a moment.

> I sent patches to fix this all up.
> You can try my qemu and linux virtio-net branches now,
> they should be spec compliant.

Yep, much better.  caps match spec, the system boots fine from
virtio-scsi, and the nfs-mounts (using virtio-net) are there too.

cheers,
  Gerd

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Wed, Jan 21, 2015 at 12:31:08PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > > While hacking it up I've noticed spec doesn't match reality.  The
> > > "Virtio Structure PCI Capabilities" section here ...
> > > 
> > > http://docs.oasis-open.org/virtio/virtio/v1.0/cs01/virtio-v1.0-cs01.html#x1-690004
> > > 
> > > ... doesn't match what qemu is doing.  Huh?
> > 
> > Thanks a lot for the report, and the tool!
> 
> pciutils patch needs an update too (because I've made it decode the
> broken qemu capabilities).  Will come as separate mail with proper
> commit message in a moment.
> 
> > I sent patches to fix this all up.
> > You can try my qemu and linux virtio-net branches now,
> > they should be spec compliant.
> 
> Yep, much better.  caps match spec, the system boots fine from
> virtio-scsi, and the nfs-mounts (using virtio-net) are there too.
> 
> cheers,
>   Gerd


Thanks!
Want to respond to the patches with a tested-by tag?

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Gerd Hoffmann]

On Mi, 2015-01-21 at 13:36 +0200, Michael S. Tsirkin wrote:
> Want to respond to the patches with a tested-by tag?

Done.

Making multiplier smaller isn't in there yet, right?

cheers,
  Gerd

Re: [PATCH v3 00/16] virtio-pci: towards virtio 1.0 guest support

[Michael S. Tsirkin]

On Wed, Jan 21, 2015 at 02:43:11PM +0100, Gerd Hoffmann wrote:
> On Mi, 2015-01-21 at 13:36 +0200, Michael S. Tsirkin wrote:
> > Want to respond to the patches with a tested-by tag?
> 
> Done.
> 
> Making multiplier smaller isn't in there yet, right?
> 
> cheers,
>   Gerd

No - but I'll make sure to address this before I
post a new version.

-- 
MST