[PATCH 0/2] virtio-mmio updates for 3.7

[PATCH 0/2] virtio-mmio updates for 3.7

[Pawel Moll]

Hi Rusty,

Would you be so kind and consider getting those two small fixes into
3.7 merge? All credits go to Brian, all shame on me :-)

Cheers!

Pawel

Brian Foley (2):
  virtio_mmio: fix off by one error allocating queue
  virtio_mmio: Don't attempt to create empty virtqueues

 drivers/virtio/virtio_mmio.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] virtio_mmio: fix off by one error allocating queue

[Pawel Moll]

From: Brian Foley <brian.foley@arm.com>

vm_setup_vq fails to allow VirtQueues needing only 2 pages of
storage, as it should. Found with a kernel using 64kB pages, but
can be provoked if a virtio device reports QueueNumMax where the
descriptor table and available ring fit in one page, and the used
ring on the second (<= 227 descriptors with 4kB pages and <= 3640
with 64kB pages.)

Signed-off-by: Brian Foley <brian.foley@arm.com>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 drivers/virtio/virtio_mmio.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 453db0c..58e2d78 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -335,8 +335,8 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index,
 	while (1) {
 		size = PAGE_ALIGN(vring_size(info->num,
 				VIRTIO_MMIO_VRING_ALIGN));
-		/* Already smallest possible allocation? */
-		if (size <= VIRTIO_MMIO_VRING_ALIGN * 2) {
+		/* Did the last iter shrink the queue below minimum size? */
+		if (size < VIRTIO_MMIO_VRING_ALIGN * 2) {
 			err = -ENOMEM;
 			goto error_alloc_pages;
 		}
-- 
1.7.9.5

[PATCH 2/2] virtio_mmio: Don't attempt to create empty virtqueues

[Pawel Moll]

From: Brian Foley <brian.foley@arm.com>

If a virtio device reports a QueueNumMax of 0, vring_new_virtqueue()
doesn't check this, and thanks to an unsigned (i < num - 1) loop
guard, scribbles over memory when initialising the free list.

Avoid by not trying to create zero-descriptor queues, as there's no
way to do any I/O with one.

Signed-off-by: Brian Foley <brian.foley@arm.com>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 drivers/virtio/virtio_mmio.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 58e2d78..6979c1b 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -332,6 +332,16 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index,
 	 * and two rings (which makes it "alignment_size * 2")
 	 */
 	info->num = readl(vm_dev->base + VIRTIO_MMIO_QUEUE_NUM_MAX);
+
+	/* If the device reports a 0 entry queue, we won't be able to
+	 * use it to perform I/O, and vring_new_virtqueue() can't create
+	 * empty queues anyway, so don't bother to set up the device.
+	 */
+	if (info->num == 0) {
+		err = -ENOENT;
+		goto error_alloc_pages;
+	}
+
 	while (1) {
 		size = PAGE_ALIGN(vring_size(info->num,
 				VIRTIO_MMIO_VRING_ALIGN));
-- 
1.7.9.5

Re: [PATCH 0/2] virtio-mmio updates for 3.7

[Rusty Russell]

Pawel Moll <pawel.moll@arm.com> writes:

> Hi Rusty,
>
> Would you be so kind and consider getting those two small fixes into
> 3.7 merge? All credits go to Brian, all shame on me :-)

Should these also be cc'd to stable@kernel.org?

Cheers,
Rusty.

Re: [PATCH 0/2] virtio-mmio updates for 3.7

[Pawel Moll]

On Tue, 2012-09-25 at 01:11 +0100, Rusty Russell wrote:
> > Would you be so kind and consider getting those two small fixes into
> > 3.7 merge? All credits go to Brian, all shame on me :-)
> 
> Should these also be cc'd to stable@kernel.org?

I was thinking about it, but as the problem manifests itself mainly on
architectures with large page size, the impact is limited.

But if you think it's worth it, I will repost with Cc: stable.

Cheers!

Paweł


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/2] virtio-mmio updates for 3.7

[Rusty Russell]

Pawel Moll <pawel.moll@arm.com> writes:

> On Tue, 2012-09-25 at 01:11 +0100, Rusty Russell wrote:
>> > Would you be so kind and consider getting those two small fixes into
>> > 3.7 merge? All credits go to Brian, all shame on me :-)
>> 
>> Should these also be cc'd to stable@kernel.org?
>
> I was thinking about it, but as the problem manifests itself mainly on
> architectures with large page size, the impact is limited.
>
> But if you think it's worth it, I will repost with Cc: stable.

I agree with you. But if it's not CC' stable, it's not for 3.7 either,
since at this late stage the rules are basically synonymous.

Cheers,
Rusty.

Re: [PATCH 0/2] virtio-mmio updates for 3.7

[Pawel Moll]

On Wed, 2012-09-26 at 01:10 +0100, Rusty Russell wrote:
> > I was thinking about it, but as the problem manifests itself mainly on
> > architectures with large page size, the impact is limited.
> >
> > But if you think it's worth it, I will repost with Cc: stable.
> 
> I agree with you. But if it's not CC' stable, it's not for 3.7 either,
> since at this late stage the rules are basically synonymous.

Ok, let's postpone it till 3.8 for now, than. If we find out that this
is a real problem I'll re-post it with Cc: stable around 3.7-rc2, 3.

Cheers!

Pawel