From b92b1b89a33c172c075edccf6afb0edc41d851fd Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Fri, 19 Oct 2012 14:03:33 +0100 Subject: virtio: force vring descriptors to be allocated from lowmem Virtio devices may attempt to add descriptors to a virtqueue from atomic context using GFP_ATOMIC allocation. This is problematic because such allocations can fall outside of the lowmem mapping, causing virt_to_phys to report bogus physical addresses which are subsequently passed to userspace via the buffers for the virtual device. This patch masks out __GFP_HIGH and __GFP_HIGHMEM from the requested flags when allocating descriptors for a virtqueue. If an atomic allocation is requested and later fails, we will return -ENOSPC which will be handled by the driver. Cc: stable@kernel.org Cc: Sasha Levin Signed-off-by: Will Deacon Signed-off-by: Rusty Russell --- drivers/virtio/virtio_ring.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index e639584b2dbd..286c30cb393d 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -135,6 +135,13 @@ static int vring_add_indirect(struct vring_virtqueue *vq, unsigned head; int i; + /* + * We require lowmem mappings for the descriptors because + * otherwise virt_to_phys will give us bogus addresses in the + * virtqueue. + */ + gfp &= ~(__GFP_HIGHMEM | __GFP_HIGH); + desc = kmalloc((out + in) * sizeof(struct vring_desc), gfp); if (!desc) return -ENOMEM; -- cgit v1.2.3 From 1ce6853aa0f8e1cc3ae811a85d50cde6ad0ef735 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Tue, 16 Oct 2012 23:56:13 +1030 Subject: virtio-pci: use module_pci_driver to simplify the code Use the module_pci_driver() macro to make the code simpler by eliminating module_init and module_exit calls. dpatch engine is used to auto generate this patch. (https://github.com/weiyj/dpatch) Signed-off-by: Wei Yongjun Signed-off-by: Rusty Russell --- drivers/virtio/virtio_pci.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index c33aea36598a..b59237c1d540 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -830,16 +830,4 @@ static struct pci_driver virtio_pci_driver = { #endif }; -static int __init virtio_pci_init(void) -{ - return pci_register_driver(&virtio_pci_driver); -} - -module_init(virtio_pci_init); - -static void __exit virtio_pci_exit(void) -{ - pci_unregister_driver(&virtio_pci_driver); -} - -module_exit(virtio_pci_exit); +module_pci_driver(virtio_pci_driver); -- cgit v1.2.3 From 06ca287dbac9cc19d04ac2901b8c4882c03795ff Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 16 Oct 2012 23:56:14 +1030 Subject: virtio: move queue_index and num_free fields into core struct virtqueue. They're generic concepts, so hoist them. This also avoids accessor functions (though kept around for merge with DaveM's net tree). This goes even further than Jason Wang's 17bb6d4088 patch ("virtio-ring: move queue_index to vring_virtqueue") which moved the queue_index from the specific transport. Acked-by: Michael S. Tsirkin Signed-off-by: Rusty Russell --- drivers/virtio/virtio_mmio.c | 4 ++-- drivers/virtio/virtio_pci.c | 6 ++---- drivers/virtio/virtio_ring.c | 34 +++++++++++----------------------- 3 files changed, 15 insertions(+), 29 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 6b1b7e184939..5a0e1d32ce13 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -225,7 +225,7 @@ static void vm_notify(struct virtqueue *vq) /* We write the queue's selector into the notification register to * signal the other end */ - writel(virtqueue_get_queue_index(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); + writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY); } /* Notify all virtqueues on an interrupt. */ @@ -266,7 +266,7 @@ static void vm_del_vq(struct virtqueue *vq) struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev); struct virtio_mmio_vq_info *info = vq->priv; unsigned long flags, size; - unsigned int index = virtqueue_get_queue_index(vq); + unsigned int index = vq->index; spin_lock_irqsave(&vm_dev->lock, flags); list_del(&info->node); diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index b59237c1d540..e3ecc94591ad 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -203,8 +203,7 @@ static void vp_notify(struct virtqueue *vq) /* we write the queue's selector into the notification register to * signal the other end */ - iowrite16(virtqueue_get_queue_index(vq), - vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); + iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_NOTIFY); } /* Handle a configuration change: Tell driver if it wants to know. */ @@ -479,8 +478,7 @@ static void vp_del_vq(struct virtqueue *vq) list_del(&info->node); spin_unlock_irqrestore(&vp_dev->lock, flags); - iowrite16(virtqueue_get_queue_index(vq), - vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL); + iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL); if (vp_dev->msix_enabled) { iowrite16(VIRTIO_MSI_NO_VECTOR, diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 286c30cb393d..33a4ce009bcc 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -93,8 +93,6 @@ struct vring_virtqueue /* Host publishes avail event idx */ bool event; - /* Number of free buffers */ - unsigned int num_free; /* Head of free buffer list. */ unsigned int free_head; /* Number we've added since last sync. */ @@ -106,9 +104,6 @@ struct vring_virtqueue /* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq); - /* Index of the queue */ - int queue_index; - #ifdef DEBUG /* They're supposed to lock for us. */ unsigned int in_use; @@ -167,7 +162,7 @@ static int vring_add_indirect(struct vring_virtqueue *vq, desc[i-1].next = 0; /* We're about to use a buffer */ - vq->num_free--; + vq->vq.num_free--; /* Use a single buffer which doesn't continue */ head = vq->free_head; @@ -181,13 +176,6 @@ static int vring_add_indirect(struct vring_virtqueue *vq, return head; } -int virtqueue_get_queue_index(struct virtqueue *_vq) -{ - struct vring_virtqueue *vq = to_vvq(_vq); - return vq->queue_index; -} -EXPORT_SYMBOL_GPL(virtqueue_get_queue_index); - /** * virtqueue_add_buf - expose buffer to other end * @vq: the struct virtqueue we're talking about. @@ -235,7 +223,7 @@ int virtqueue_add_buf(struct virtqueue *_vq, /* If the host supports indirect descriptor tables, and we have multiple * buffers, then go indirect. FIXME: tune this threshold */ - if (vq->indirect && (out + in) > 1 && vq->num_free) { + if (vq->indirect && (out + in) > 1 && vq->vq.num_free) { head = vring_add_indirect(vq, sg, out, in, gfp); if (likely(head >= 0)) goto add_head; @@ -244,9 +232,9 @@ int virtqueue_add_buf(struct virtqueue *_vq, BUG_ON(out + in > vq->vring.num); BUG_ON(out + in == 0); - if (vq->num_free < out + in) { + if (vq->vq.num_free < out + in) { pr_debug("Can't add buf len %i - avail = %i\n", - out + in, vq->num_free); + out + in, vq->vq.num_free); /* FIXME: for historical reasons, we force a notify here if * there are outgoing parts to the buffer. Presumably the * host should service the ring ASAP. */ @@ -257,7 +245,7 @@ int virtqueue_add_buf(struct virtqueue *_vq, } /* We're about to use some buffers from the free list. */ - vq->num_free -= out + in; + vq->vq.num_free -= out + in; head = vq->free_head; for (i = vq->free_head; out; i = vq->vring.desc[i].next, out--) { @@ -303,7 +291,7 @@ add_head: pr_debug("Added buffer head %i to %p\n", head, vq); END_USE(vq); - return vq->num_free; + return vq->vq.num_free; } EXPORT_SYMBOL_GPL(virtqueue_add_buf); @@ -400,13 +388,13 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) while (vq->vring.desc[i].flags & VRING_DESC_F_NEXT) { i = vq->vring.desc[i].next; - vq->num_free++; + vq->vq.num_free++; } vq->vring.desc[i].next = vq->free_head; vq->free_head = head; /* Plus final descriptor */ - vq->num_free++; + vq->vq.num_free++; } static inline bool more_used(const struct vring_virtqueue *vq) @@ -606,7 +594,7 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq) return buf; } /* That should have freed everything. */ - BUG_ON(vq->num_free != vq->vring.num); + BUG_ON(vq->vq.num_free != vq->vring.num); END_USE(vq); return NULL; @@ -660,12 +648,13 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, vq->vq.callback = callback; vq->vq.vdev = vdev; vq->vq.name = name; + vq->vq.num_free = num; + vq->vq.index = index; vq->notify = notify; vq->weak_barriers = weak_barriers; vq->broken = false; vq->last_used_idx = 0; vq->num_added = 0; - vq->queue_index = index; list_add_tail(&vq->vq.list, &vdev->vqs); #ifdef DEBUG vq->in_use = false; @@ -680,7 +669,6 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; /* Put everything in free lists. */ - vq->num_free = num; vq->free_head = 0; for (i = 0; i < num-1; i++) { vq->vring.desc[i].next = i+1; -- cgit v1.2.3 From 98e8c6bc66048db6f921ccd5b24f0e09804cfcca Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 16 Oct 2012 23:56:15 +1030 Subject: virtio: make virtqueue_add_buf() returning 0 on success, not capacity. Now noone relies on this behavior, we simplify virtqueue_add_buf() so it return 0 or -errno. Signed-off-by: Rusty Russell Acked-by: Michael S. Tsirkin --- drivers/virtio/virtio_ring.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 33a4ce009bcc..ffd7e7da5d3b 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -188,10 +188,7 @@ static int vring_add_indirect(struct vring_virtqueue *vq, * Caller must ensure we don't call this with other virtqueue operations * at the same time (except where noted). * - * Returns remaining capacity of queue or a negative error - * (ie. ENOSPC). Note that it only really makes sense to treat all - * positive return values as "available": indirect buffers mean that - * we can put an entire sg[] array inside a single queue entry. + * Returns zero or a negative error (ie. ENOSPC, ENOMEM). */ int virtqueue_add_buf(struct virtqueue *_vq, struct scatterlist sg[], @@ -291,7 +288,7 @@ add_head: pr_debug("Added buffer head %i to %p\n", head, vq); END_USE(vq); - return vq->vq.num_free; + return 0; } EXPORT_SYMBOL_GPL(virtqueue_add_buf); -- cgit v1.2.3 From 800ba5eabf13485fecbf468f6b2999608413d176 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 31 Oct 2012 09:27:22 +1030 Subject: virtio: Convert dev_printk(KERN_ to dev_( dev_ calls take less code than dev_printk(KERN_ and reducing object size is good. Convert if (printk_ratelimit()) dev_printk to dev__ratelimited. Signed-off-by: Joe Perches Signed-off-by: Rusty Russell --- drivers/virtio/virtio_balloon.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 0908e6044333..586395cca5fe 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -130,10 +130,9 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num) struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN); if (!page) { - if (printk_ratelimit()) - dev_printk(KERN_INFO, &vb->vdev->dev, - "Out of puff! Can't get %zu pages\n", - num); + dev_info_ratelimited(&vb->vdev->dev, + "Out of puff! Can't get %zu pages\n", + num); /* Sleep for at least 1/5 of a second before retry. */ msleep(200); break; -- cgit v1.2.3 From 40f9938c4c69183c5ceaca74b77aff636cc79623 Mon Sep 17 00:00:00 2001 From: Pawel Moll Date: Thu, 22 Nov 2012 12:30:24 +1030 Subject: virtio-mmio: Fix irq parsing in command line parameter When the resource_size_t is 64-bit long, the sscanf() on the virtio device command line paramter string may return wrong value because its format was defined as "%u". Fixed by using an intermediate local value of a known length. Also added cleaned up the resource creation and added extra comments to make the parameters parsing easier to follow. Reported-by: Lee Jones Signed-off-by: Pawel Moll Signed-off-by: Rusty Russell --- drivers/virtio/virtio_mmio.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 5a0e1d32ce13..634f80bcdbd7 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -521,25 +521,33 @@ static int vm_cmdline_set(const char *device, int err; struct resource resources[2] = {}; char *str; - long long int base; + long long int base, size; + unsigned int irq; int processed, consumed = 0; struct platform_device *pdev; - resources[0].flags = IORESOURCE_MEM; - resources[1].flags = IORESOURCE_IRQ; - - resources[0].end = memparse(device, &str) - 1; + /* Consume "size" part of the command line parameter */ + size = memparse(device, &str); + /* Get "@:[:]" chunks */ processed = sscanf(str, "@%lli:%u%n:%d%n", - &base, &resources[1].start, &consumed, + &base, &irq, &consumed, &vm_cmdline_id, &consumed); - if (processed < 2 || processed > 3 || str[consumed]) + /* + * sscanf() must processes at least 2 chunks; also there + * must be no extra characters after the last chunk, so + * str[consumed] must be '\0' + */ + if (processed < 2 || str[consumed]) return -EINVAL; + resources[0].flags = IORESOURCE_MEM; resources[0].start = base; - resources[0].end += base; - resources[1].end = resources[1].start; + resources[0].end = base + size - 1; + + resources[1].flags = IORESOURCE_IRQ; + resources[1].start = resources[1].end = irq; if (!vm_cmdline_parent_registered) { err = device_register(&vm_cmdline_parent); -- cgit v1.2.3 From 9bffdca8c64a72ac54c47a552734ab457bc720d4 Mon Sep 17 00:00:00 2001 From: Wanlong Gao Date: Tue, 11 Dec 2012 11:04:50 +1030 Subject: virtio: use dev_to_virtio wrapper in virtio Use dev_to_virtio wrapper in virtio to make code clearly. Cc: Rusty Russell Cc: "Michael S. Tsirkin" Signed-off-by: Wanlong Gao Signed-off-by: Rusty Russell --- drivers/virtio/virtio.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 1e8659ca27ef..1346ae8e14f3 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -10,33 +10,32 @@ static DEFINE_IDA(virtio_index_ida); static ssize_t device_show(struct device *_d, struct device_attribute *attr, char *buf) { - struct virtio_device *dev = container_of(_d,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_d); return sprintf(buf, "0x%04x\n", dev->id.device); } static ssize_t vendor_show(struct device *_d, struct device_attribute *attr, char *buf) { - struct virtio_device *dev = container_of(_d,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_d); return sprintf(buf, "0x%04x\n", dev->id.vendor); } static ssize_t status_show(struct device *_d, struct device_attribute *attr, char *buf) { - struct virtio_device *dev = container_of(_d,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_d); return sprintf(buf, "0x%08x\n", dev->config->get_status(dev)); } static ssize_t modalias_show(struct device *_d, struct device_attribute *attr, char *buf) { - struct virtio_device *dev = container_of(_d,struct virtio_device,dev); - + struct virtio_device *dev = dev_to_virtio(_d); return sprintf(buf, "virtio:d%08Xv%08X\n", dev->id.device, dev->id.vendor); } static ssize_t features_show(struct device *_d, struct device_attribute *attr, char *buf) { - struct virtio_device *dev = container_of(_d, struct virtio_device, dev); + struct virtio_device *dev = dev_to_virtio(_d); unsigned int i; ssize_t len = 0; @@ -71,7 +70,7 @@ static inline int virtio_id_match(const struct virtio_device *dev, static int virtio_dev_match(struct device *_dv, struct device_driver *_dr) { unsigned int i; - struct virtio_device *dev = container_of(_dv,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_dv); const struct virtio_device_id *ids; ids = container_of(_dr, struct virtio_driver, driver)->id_table; @@ -83,7 +82,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr) static int virtio_uevent(struct device *_dv, struct kobj_uevent_env *env) { - struct virtio_device *dev = container_of(_dv,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_dv); return add_uevent_var(env, "MODALIAS=virtio:d%08Xv%08X", dev->id.device, dev->id.vendor); @@ -111,7 +110,7 @@ EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature); static int virtio_dev_probe(struct device *_d) { int err, i; - struct virtio_device *dev = container_of(_d,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_d); struct virtio_driver *drv = container_of(dev->dev.driver, struct virtio_driver, driver); u32 device_features; @@ -152,7 +151,7 @@ static int virtio_dev_probe(struct device *_d) static int virtio_dev_remove(struct device *_d) { - struct virtio_device *dev = container_of(_d,struct virtio_device,dev); + struct virtio_device *dev = dev_to_virtio(_d); struct virtio_driver *drv = container_of(dev->dev.driver, struct virtio_driver, driver); -- cgit v1.2.3 From 9a2bdcc85d28506d4e5d4a9618fb133a3f40945d Mon Sep 17 00:00:00 2001 From: Wanlong Gao Date: Mon, 10 Dec 2012 16:38:33 +0800 Subject: virtio: add drv_to_virtio to make code clearly Add drv_to_virtio wrapper to get virtio_driver from device_driver. Cc: Rusty Russell Cc: "Michael S. Tsirkin" Signed-off-by: Wanlong Gao Signed-off-by: Rusty Russell --- drivers/virtio/virtio.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'drivers/virtio') diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 1346ae8e14f3..1c01ac3fad08 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -73,7 +73,7 @@ static int virtio_dev_match(struct device *_dv, struct device_driver *_dr) struct virtio_device *dev = dev_to_virtio(_dv); const struct virtio_device_id *ids; - ids = container_of(_dr, struct virtio_driver, driver)->id_table; + ids = drv_to_virtio(_dr)->id_table; for (i = 0; ids[i].device; i++) if (virtio_id_match(dev, &ids[i])) return 1; @@ -97,8 +97,7 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev, unsigned int fbit) { unsigned int i; - struct virtio_driver *drv = container_of(vdev->dev.driver, - struct virtio_driver, driver); + struct virtio_driver *drv = drv_to_virtio(vdev->dev.driver); for (i = 0; i < drv->feature_table_size; i++) if (drv->feature_table[i] == fbit) @@ -111,8 +110,7 @@ static int virtio_dev_probe(struct device *_d) { int err, i; struct virtio_device *dev = dev_to_virtio(_d); - struct virtio_driver *drv = container_of(dev->dev.driver, - struct virtio_driver, driver); + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver); u32 device_features; /* We have a driver! */ @@ -152,8 +150,7 @@ static int virtio_dev_probe(struct device *_d) static int virtio_dev_remove(struct device *_d) { struct virtio_device *dev = dev_to_virtio(_d); - struct virtio_driver *drv = container_of(dev->dev.driver, - struct virtio_driver, driver); + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver); drv->remove(dev); -- cgit v1.2.3