Linux Virtualization - Jan 2015 - [PATCH 0/6] virtio: graceful failure with get == NULL

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

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

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

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

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

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

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

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
>

"Michael S. Tsirkin" <mst at 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.