[PATCH 0/6] virtio: graceful failure with get == NULL

[PATCH 0/6] virtio: graceful failure with get == NULL

[Michael S. Tsirkin]

virtio 1.0 says device configuration is optional, but most drivers treat it as
mandatory.  Even if presented by device, guest bios might disable the BAR
holding that configuration, so we can't assume it's there, but we also don't
want to fail if not in case drivers can cope with it's absence - such as caif
or rng.

Add code to drivers to check presence of get callback and fail gracefully.

Michael S. Tsirkin (6):
  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

 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 ++++++
 net/9p/trans_virtio.c           | 6 ++++++
 6 files changed, 36 insertions(+)

-- 
MST

[PATCH 1/6] 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 2/6] 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 3/6] 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

[PATCH 4/6] 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 5/6] 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 6/6] 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 6/6] 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

Re: [PATCH 0/6] virtio: graceful failure with get == NULL

[Michael S. Tsirkin]

On Tue, Jan 13, 2015 at 04:43:07PM +0200, Michael S. Tsirkin wrote:
> virtio 1.0 says device configuration is optional, but most drivers treat it as
> mandatory.  Even if presented by device, guest bios might disable the BAR
> holding that configuration, so we can't assume it's there, but we also don't
> want to fail if not in case drivers can cope with it's absence - such as caif
> or rng.
> 
> Add code to drivers to check presence of get callback and fail gracefully.

Rusty, in case it's not clear: I'd like to hear
your opinion on these patches, since virtio pci modern
driver I'm now preparing for submission, depends on this.


> Michael S. Tsirkin (6):
>   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
> 
>  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 ++++++
>  net/9p/trans_virtio.c           | 6 ++++++
>  6 files changed, 36 insertions(+)
> 
> -- 
> MST
> 

Re: [PATCH 0/6] virtio: graceful failure with get == NULL

[Rusty Russell]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Tue, Jan 13, 2015 at 04:43:07PM +0200, Michael S. Tsirkin wrote:
>> virtio 1.0 says device configuration is optional, but most drivers treat it as
>> mandatory.  Even if presented by device, guest bios might disable the BAR
>> holding that configuration, so we can't assume it's there, but we also don't
>> want to fail if not in case drivers can cope with it's absence - such as caif
>> or rng.
>> 
>> Add code to drivers to check presence of get callback and fail gracefully.
>
> Rusty, in case it's not clear: I'd like to hear
> your opinion on these patches, since virtio pci modern
> driver I'm now preparing for submission, depends on this.

Just to note for anyone following, that these are now in my virtio-next
tree.

Cheers,
Rusty.