* [PATCH v3 10/25] virtio: add API to enable VQs early
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
virtio spec 0.9.X requires DRIVER_OK to be set before
VQs are used, but some drivers use VQs before probe
function returns.
Since DRIVER_OK is set after probe, this violates the spec.
Even though under virtio 1.0 transitional devices support this
behaviour, we want to make it possible for those early callers to become
spec compliant and eventually support non-transitional devices.
Add API for drivers to call before using VQs.
Sets DRIVER_OK internally.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
include/linux/virtio_config.h | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index e8f8f71..e36403b 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -109,6 +109,23 @@ struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev,
return vq;
}
+/**
+ * virtio_enable_vqs_early - enable vq use in probe function
+ * @vdev: the device
+ *
+ * Driver must call this to use vqs in the probe function.
+ *
+ * Note: vqs are enabled automatically after probe returns.
+ */
+static inline
+void virtio_enable_vqs_early(struct virtio_device *dev)
+{
+ unsigned status = dev->config->get_status(dev);
+
+ BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
+ dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+}
+
static inline
const char *virtio_bus_name(struct virtio_device *vdev)
{
--
MST
^ permalink raw reply related
* [PATCH v3 09/25] virtio_net: minor cleanup
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
goto done;
done:
return;
is ugly, it was put there to make diff review easier.
replace by open-coded return.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
drivers/net/virtio_net.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 23e4a69..ef04d23 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1403,7 +1403,7 @@ static void virtnet_config_changed_work(struct work_struct *work)
if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
struct virtio_net_config, status, &v) < 0)
- goto done;
+ return;
if (v & VIRTIO_NET_S_ANNOUNCE) {
netdev_notify_peers(vi->dev);
@@ -1414,7 +1414,7 @@ static void virtnet_config_changed_work(struct work_struct *work)
v &= VIRTIO_NET_S_LINK_UP;
if (vi->status == v)
- goto done;
+ return;
vi->status = v;
@@ -1425,8 +1425,6 @@ static void virtnet_config_changed_work(struct work_struct *work)
netif_carrier_off(vi->dev);
netif_tx_stop_all_queues(vi->dev);
}
-done:
- return;
}
static void virtnet_config_changed(struct virtio_device *vdev)
--
MST
^ permalink raw reply related
* [PATCH v3 08/25] virtio-net: drop config_mutex
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
config_mutex served two purposes: prevent multiple concurrent config
change handlers, and synchronize access to config_enable flag.
Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1
workqueue: make all workqueues non-reentrant
all workqueues are non-reentrant, and config_enable
is now gone.
Get rid of the unnecessary lock.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
drivers/net/virtio_net.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 743fb04..23e4a69 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -132,9 +132,6 @@ struct virtnet_info {
/* Work struct for config space updates */
struct work_struct config_work;
- /* Lock for config space updates */
- struct mutex config_lock;
-
/* Does the affinity hint is set for virtqueues? */
bool affinity_hint_set;
@@ -1404,7 +1401,6 @@ static void virtnet_config_changed_work(struct work_struct *work)
container_of(work, struct virtnet_info, config_work);
u16 v;
- mutex_lock(&vi->config_lock);
if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
struct virtio_net_config, status, &v) < 0)
goto done;
@@ -1430,7 +1426,7 @@ static void virtnet_config_changed_work(struct work_struct *work)
netif_tx_stop_all_queues(vi->dev);
}
done:
- mutex_unlock(&vi->config_lock);
+ return;
}
static void virtnet_config_changed(struct virtio_device *vdev)
@@ -1751,7 +1747,6 @@ static int virtnet_probe(struct virtio_device *vdev)
u64_stats_init(&virtnet_stats->rx_syncp);
}
- mutex_init(&vi->config_lock);
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
/* If we can receive ANY GSO packets, we must allocate large ones. */
--
MST
^ permalink raw reply related
* [PATCH v3 07/25] virtio_net: drop config_enable
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
Now that virtio core ensures config changes don't arrive during probing,
drop config_enable flag in virtio net.
On removal, flush is now sufficient to guarantee that no change work is
queued.
This help simplify the driver, and will allow setting DRIVER_OK earlier
without losing config change notifications.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
drivers/net/virtio_net.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 59caa06..743fb04 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -123,9 +123,6 @@ struct virtnet_info {
/* Host can handle any s/g split between our header and packet data */
bool any_header_sg;
- /* enable config space updates */
- bool config_enable;
-
/* Active statistics */
struct virtnet_stats __percpu *stats;
@@ -1408,9 +1405,6 @@ static void virtnet_config_changed_work(struct work_struct *work)
u16 v;
mutex_lock(&vi->config_lock);
- if (!vi->config_enable)
- goto done;
-
if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS,
struct virtio_net_config, status, &v) < 0)
goto done;
@@ -1758,7 +1752,6 @@ static int virtnet_probe(struct virtio_device *vdev)
}
mutex_init(&vi->config_lock);
- vi->config_enable = true;
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
/* If we can receive ANY GSO packets, we must allocate large ones. */
@@ -1875,17 +1868,13 @@ static void virtnet_remove(struct virtio_device *vdev)
unregister_hotcpu_notifier(&vi->nb);
- /* Prevent config work handler from accessing the device. */
- mutex_lock(&vi->config_lock);
- vi->config_enable = false;
- mutex_unlock(&vi->config_lock);
+ /* Make sure no work handler is accessing the device. */
+ flush_work(&vi->config_work);
unregister_netdev(vi->dev);
remove_vq_common(vi);
- flush_work(&vi->config_work);
-
free_percpu(vi->stats);
free_netdev(vi->dev);
}
@@ -1898,10 +1887,8 @@ static int virtnet_freeze(struct virtio_device *vdev)
unregister_hotcpu_notifier(&vi->nb);
- /* Prevent config work handler from accessing the device */
- mutex_lock(&vi->config_lock);
- vi->config_enable = false;
- mutex_unlock(&vi->config_lock);
+ /* Make sure no work handler is accessing the device */
+ flush_work(&vi->config_work);
netif_device_detach(vi->dev);
cancel_delayed_work_sync(&vi->refill);
@@ -1916,8 +1903,6 @@ static int virtnet_freeze(struct virtio_device *vdev)
remove_vq_common(vi);
- flush_work(&vi->config_work);
-
return 0;
}
@@ -1941,10 +1926,6 @@ static int virtnet_restore(struct virtio_device *vdev)
netif_device_attach(vi->dev);
- mutex_lock(&vi->config_lock);
- vi->config_enable = true;
- mutex_unlock(&vi->config_lock);
-
rtnl_lock();
virtnet_set_queues(vi, vi->curr_queue_pairs);
rtnl_unlock();
--
MST
^ permalink raw reply related
* [PATCH v3 06/25] virtio-blk: drop config_mutex
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
config_mutex served two purposes: prevent multiple concurrent config
change handlers, and synchronize access to config_enable flag.
Since commit dbf2576e37da0fcc7aacbfbb9fd5d3de7888a3c1
workqueue: make all workqueues non-reentrant
all workqueues are non-reentrant, and config_enable
is now gone.
Get rid of the unnecessary lock.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
drivers/block/virtio_blk.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 91272f1a..89ba8d6 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -41,9 +41,6 @@ struct virtio_blk
/* Process context for config space updates */
struct work_struct config_work;
- /* Lock for config space updates */
- struct mutex config_lock;
-
/* What host tells us, plus 2 for header & tailer. */
unsigned int sg_elems;
@@ -344,8 +341,6 @@ static void virtblk_config_changed_work(struct work_struct *work)
char *envp[] = { "RESIZE=1", NULL };
u64 capacity, size;
- mutex_lock(&vblk->config_lock);
-
/* Host must always specify the capacity. */
virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
@@ -369,8 +364,6 @@ static void virtblk_config_changed_work(struct work_struct *work)
set_capacity(vblk->disk, capacity);
revalidate_disk(vblk->disk);
kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
-
- mutex_unlock(&vblk->config_lock);
}
static void virtblk_config_changed(struct virtio_device *vdev)
@@ -601,7 +594,6 @@ static int virtblk_probe(struct virtio_device *vdev)
vblk->vdev = vdev;
vblk->sg_elems = sg_elems;
- mutex_init(&vblk->config_lock);
INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
--
MST
^ permalink raw reply related
* [PATCH v3 05/25] virtio_blk: drop config_enable
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
Now that virtio core ensures config changes don't
arrive during probing, drop config_enable flag
in virtio blk.
On removal, flush is now sufficient to guarantee that
no change work is queued.
This help simplify the driver, and will allow
setting DRIVER_OK earlier without losing config
change notifications.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
drivers/block/virtio_blk.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0a58140..91272f1a 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -44,9 +44,6 @@ struct virtio_blk
/* Lock for config space updates */
struct mutex config_lock;
- /* enable config space updates */
- bool config_enable;
-
/* What host tells us, plus 2 for header & tailer. */
unsigned int sg_elems;
@@ -348,8 +345,6 @@ static void virtblk_config_changed_work(struct work_struct *work)
u64 capacity, size;
mutex_lock(&vblk->config_lock);
- if (!vblk->config_enable)
- goto done;
/* Host must always specify the capacity. */
virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity);
@@ -374,7 +369,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
set_capacity(vblk->disk, capacity);
revalidate_disk(vblk->disk);
kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
-done:
+
mutex_unlock(&vblk->config_lock);
}
@@ -609,7 +604,6 @@ static int virtblk_probe(struct virtio_device *vdev)
mutex_init(&vblk->config_lock);
INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
- vblk->config_enable = true;
err = init_vq(vblk);
if (err)
@@ -771,10 +765,8 @@ static void virtblk_remove(struct virtio_device *vdev)
int index = vblk->index;
int refc;
- /* Prevent config work handler from accessing the device. */
- mutex_lock(&vblk->config_lock);
- vblk->config_enable = false;
- mutex_unlock(&vblk->config_lock);
+ /* Make sure no work handler is accessing the device. */
+ flush_work(&vblk->config_work);
del_gendisk(vblk->disk);
blk_cleanup_queue(vblk->disk->queue);
@@ -784,8 +776,6 @@ static void virtblk_remove(struct virtio_device *vdev)
/* Stop all the virtqueues. */
vdev->config->reset(vdev);
- flush_work(&vblk->config_work);
-
refc = atomic_read(&disk_to_dev(vblk->disk)->kobj.kref.refcount);
put_disk(vblk->disk);
vdev->config->del_vqs(vdev);
@@ -805,11 +795,7 @@ static int virtblk_freeze(struct virtio_device *vdev)
/* Ensure we don't receive any more interrupts */
vdev->config->reset(vdev);
- /* Prevent config work handler from accessing the device. */
- mutex_lock(&vblk->config_lock);
- vblk->config_enable = false;
- mutex_unlock(&vblk->config_lock);
-
+ /* Make sure no work handler is accessing the device. */
flush_work(&vblk->config_work);
blk_mq_stop_hw_queues(vblk->disk->queue);
@@ -823,7 +809,6 @@ static int virtblk_restore(struct virtio_device *vdev)
struct virtio_blk *vblk = vdev->priv;
int ret;
- vblk->config_enable = true;
ret = init_vq(vdev->priv);
if (!ret)
blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
--
MST
^ permalink raw reply related
* [PATCH v3 04/25] virtio: defer config changed notifications
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
Defer config changed notifications that arrive during
probe/scan/freeze/restore.
This will allow drivers to set DRIVER_OK earlier, without worrying about
racing with config change interrupts.
This change will also benefit old hypervisors (before 2009)
that send interrupts without checking DRIVER_OK: previously,
the callback could race with driver-specific initialization.
This will also help simplify drivers.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
include/linux/virtio.h | 6 ++++++
drivers/virtio/virtio.c | 57 +++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 8df7ba8..5636b11 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -79,6 +79,9 @@ bool virtqueue_is_broken(struct virtqueue *vq);
* virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
* @failed: saved value for CONFIG_S_FAILED bit (for restore)
+ * @config_enabled: configuration change reporting enabled
+ * @config_changed: configuration change reported while disabled
+ * @config_lock: protects configuration change reporting
* @dev: underlying device.
* @id: the device type identification (used to match it with a driver).
* @config: the configuration ops for this device.
@@ -90,6 +93,9 @@ bool virtqueue_is_broken(struct virtqueue *vq);
struct virtio_device {
int index;
bool failed;
+ bool config_enabled;
+ bool config_changed;
+ spinlock_t config_lock;
struct device dev;
struct virtio_device_id id;
const struct virtio_config_ops *config;
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 8216b73..2536701 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -117,6 +117,42 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
}
EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
+static void __virtio_config_changed(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ if (!dev->config_enabled)
+ dev->config_changed = true;
+ else if (drv && drv->config_changed)
+ drv->config_changed(dev);
+}
+
+void virtio_config_changed(struct virtio_device *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->config_lock, flags);
+ __virtio_config_changed(dev);
+ spin_unlock_irqrestore(&dev->config_lock, flags);
+}
+EXPORT_SYMBOL_GPL(virtio_config_changed);
+
+static void virtio_config_disable(struct virtio_device *dev)
+{
+ spin_lock_irq(&dev->config_lock);
+ dev->config_enabled = false;
+ spin_unlock_irq(&dev->config_lock);
+}
+
+static void virtio_config_enable(struct virtio_device *dev)
+{
+ spin_lock_irq(&dev->config_lock);
+ dev->config_enabled = true;
+ __virtio_config_changed(dev);
+ dev->config_changed = false;
+ spin_unlock_irq(&dev->config_lock);
+}
+
static int virtio_dev_probe(struct device *_d)
{
int err, i;
@@ -153,6 +189,8 @@ static int virtio_dev_probe(struct device *_d)
add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
if (drv->scan)
drv->scan(dev);
+
+ virtio_config_enable(dev);
}
return err;
@@ -163,6 +201,8 @@ static int virtio_dev_remove(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ virtio_config_disable(dev);
+
drv->remove(dev);
/* Driver should have reset device. */
@@ -211,6 +251,10 @@ int register_virtio_device(struct virtio_device *dev)
dev->index = err;
dev_set_name(&dev->dev, "virtio%u", dev->index);
+ spin_lock_init(&dev->config_lock);
+ dev->config_enabled = false;
+ dev->config_changed = false;
+
/* We always start by resetting the device, in case a previous
* driver messed it up. This also tests that code path a little. */
dev->config->reset(dev);
@@ -239,20 +283,13 @@ void unregister_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(unregister_virtio_device);
-void virtio_config_changed(struct virtio_device *dev)
-{
- struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
-
- if (drv && drv->config_changed)
- drv->config_changed(dev);
-}
-EXPORT_SYMBOL_GPL(virtio_config_changed);
-
#ifdef CONFIG_PM_SLEEP
int virtio_device_freeze(struct virtio_device *dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ virtio_config_disable(dev);
+
dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
if (drv && drv->freeze)
@@ -297,6 +334,8 @@ int virtio_device_restore(struct virtio_device *dev)
/* Finally, tell the device we're all set */
add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+ virtio_config_enable(dev);
+
return 0;
}
EXPORT_SYMBOL_GPL(virtio_device_restore);
--
MST
^ permalink raw reply related
* [PATCH v3 03/25] virtio-pci: move freeze/restore to virtio core
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
This is in preparation to extending config changed event handling
in core.
Wrapping these in an API also seems to make for a cleaner code.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
include/linux/virtio.h | 6 +++++
drivers/virtio/virtio.c | 54 +++++++++++++++++++++++++++++++++++++++++++++
drivers/virtio/virtio_pci.c | 54 ++-------------------------------------------
3 files changed, 62 insertions(+), 52 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 3c19bd3..8df7ba8 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -78,6 +78,7 @@ bool virtqueue_is_broken(struct virtqueue *vq);
/**
* virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
+ * @failed: saved value for CONFIG_S_FAILED bit (for restore)
* @dev: underlying device.
* @id: the device type identification (used to match it with a driver).
* @config: the configuration ops for this device.
@@ -88,6 +89,7 @@ bool virtqueue_is_broken(struct virtqueue *vq);
*/
struct virtio_device {
int index;
+ bool failed;
struct device dev;
struct virtio_device_id id;
const struct virtio_config_ops *config;
@@ -109,6 +111,10 @@ void unregister_virtio_device(struct virtio_device *dev);
void virtio_break_device(struct virtio_device *dev);
void virtio_config_changed(struct virtio_device *dev);
+#ifdef CONFIG_PM_SLEEP
+int virtio_device_freeze(struct virtio_device *dev);
+int virtio_device_restore(struct virtio_device *dev);
+#endif
/**
* virtio_driver - operations for a virtio I/O driver
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 3980687..8216b73 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -248,6 +248,60 @@ void virtio_config_changed(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(virtio_config_changed);
+#ifdef CONFIG_PM_SLEEP
+int virtio_device_freeze(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
+
+ if (drv && drv->freeze)
+ return drv->freeze(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_device_freeze);
+
+int virtio_device_restore(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ /* We always start by resetting the device, in case a previous
+ * driver messed it up. */
+ dev->config->reset(dev);
+
+ /* Acknowledge that we've seen the device. */
+ add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+
+ /* Maybe driver failed before freeze.
+ * Restore the failed status, for debugging. */
+ if (dev->failed)
+ add_status(dev, VIRTIO_CONFIG_S_FAILED);
+
+ if (!drv)
+ return 0;
+
+ /* We have a driver! */
+ add_status(dev, VIRTIO_CONFIG_S_DRIVER);
+
+ dev->config->finalize_features(dev);
+
+ if (drv->restore) {
+ int ret = drv->restore(dev);
+ if (ret) {
+ add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
+ }
+ }
+
+ /* Finally, tell the device we're all set */
+ add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_device_restore);
+#endif
+
static int virtio_init(void)
{
if (bus_register(&virtio_bus) != 0)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index f39f4e7..d34ebfa 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -57,9 +57,6 @@ struct virtio_pci_device
/* Vectors allocated, excluding per-vq vectors if any */
unsigned msix_used_vectors;
- /* Status saved during hibernate/restore */
- u8 saved_status;
-
/* Whether we have vector per vq */
bool per_vq_vectors;
};
@@ -764,16 +761,9 @@ static int virtio_pci_freeze(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
- struct virtio_driver *drv;
int ret;
- drv = container_of(vp_dev->vdev.dev.driver,
- struct virtio_driver, driver);
-
- ret = 0;
- vp_dev->saved_status = vp_get_status(&vp_dev->vdev);
- if (drv && drv->freeze)
- ret = drv->freeze(&vp_dev->vdev);
+ ret = virtio_device_freeze(&vp_dev->vdev);
if (!ret)
pci_disable_device(pci_dev);
@@ -784,54 +774,14 @@ static int virtio_pci_restore(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
- struct virtio_driver *drv;
- unsigned status = 0;
int ret;
- drv = container_of(vp_dev->vdev.dev.driver,
- struct virtio_driver, driver);
-
ret = pci_enable_device(pci_dev);
if (ret)
return ret;
pci_set_master(pci_dev);
- /* We always start by resetting the device, in case a previous
- * driver messed it up. */
- vp_reset(&vp_dev->vdev);
-
- /* Acknowledge that we've seen the device. */
- status |= VIRTIO_CONFIG_S_ACKNOWLEDGE;
- vp_set_status(&vp_dev->vdev, status);
-
- /* Maybe driver failed before freeze.
- * Restore the failed status, for debugging. */
- status |= vp_dev->saved_status & VIRTIO_CONFIG_S_FAILED;
- vp_set_status(&vp_dev->vdev, status);
-
- if (!drv)
- return 0;
-
- /* We have a driver! */
- status |= VIRTIO_CONFIG_S_DRIVER;
- vp_set_status(&vp_dev->vdev, status);
-
- vp_finalize_features(&vp_dev->vdev);
-
- if (drv->restore) {
- ret = drv->restore(&vp_dev->vdev);
- if (ret) {
- status |= VIRTIO_CONFIG_S_FAILED;
- vp_set_status(&vp_dev->vdev, status);
- return ret;
- }
- }
-
- /* Finally, tell the device we're all set */
- status |= VIRTIO_CONFIG_S_DRIVER_OK;
- vp_set_status(&vp_dev->vdev, status);
-
- return ret;
+ return virtio_device_restore(&vp_dev->vdev);
}
static const struct dev_pm_ops virtio_pci_pm_ops = {
--
MST
^ permalink raw reply related
* [PATCH v3 02/25] virtio: unify config_changed handling
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, Wolfram Sang, Heiko Carstens, Sudeep Dutt, virtualization,
linux-s390, linux-scsi, Christian Borntraeger, v9fs-developer,
Siva Yerramreddy, Martin Schwidefsky, Paolo Bonzini, netdev,
Ashutosh Dixit, Greg Kroah-Hartman, Amit Shah, linux390,
Andrew Morton, David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
Replace duplicated code in all transports with a single wrapper in
virtio.c.
The only functional change is in virtio_mmio.c: if a buggy device sends
us an interrupt before driver is set, we previously returned IRQ_NONE,
now we return IRQ_HANDLED.
As this must not happen in practice, this does not look like a big deal.
See also commit 3fff0179e33cd7d0a688dab65700c46ad089e934
virtio-pci: do not oops on config change if driver not loaded.
for the original motivation behind the driver check.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
include/linux/virtio.h | 2 ++
drivers/misc/mic/card/mic_virtio.c | 6 +-----
drivers/s390/kvm/kvm_virtio.c | 9 +--------
drivers/s390/kvm/virtio_ccw.c | 6 +-----
drivers/virtio/virtio.c | 9 +++++++++
drivers/virtio/virtio_mmio.c | 7 ++-----
drivers/virtio/virtio_pci.c | 6 +-----
7 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b46671e..3c19bd3 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -108,6 +108,8 @@ void unregister_virtio_device(struct virtio_device *dev);
void virtio_break_device(struct virtio_device *dev);
+void virtio_config_changed(struct virtio_device *dev);
+
/**
* virtio_driver - operations for a virtio I/O driver
* @driver: underlying device driver (populate name and owner).
diff --git a/drivers/misc/mic/card/mic_virtio.c b/drivers/misc/mic/card/mic_virtio.c
index f14b600..e647947 100644
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -462,16 +462,12 @@ static void mic_handle_config_change(struct mic_device_desc __iomem *d,
struct mic_device_ctrl __iomem *dc
= (void __iomem *)d + mic_aligned_desc_size(d);
struct mic_vdev *mvdev = (struct mic_vdev *)ioread64(&dc->vdev);
- struct virtio_driver *drv;
if (ioread8(&dc->config_change) != MIC_VIRTIO_PARAM_CONFIG_CHANGED)
return;
dev_dbg(mdrv->dev, "%s %d\n", __func__, __LINE__);
- drv = container_of(mvdev->vdev.dev.driver,
- struct virtio_driver, driver);
- if (drv->config_changed)
- drv->config_changed(&mvdev->vdev);
+ virtio_config_changed(&mvdev->vdev);
iowrite8(1, &dc->guest_ack);
}
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index a134965..6431290 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -406,15 +406,8 @@ static void kvm_extint_handler(struct ext_code ext_code,
switch (param) {
case VIRTIO_PARAM_CONFIG_CHANGED:
- {
- struct virtio_driver *drv;
- drv = container_of(vq->vdev->dev.driver,
- struct virtio_driver, driver);
- if (drv->config_changed)
- drv->config_changed(vq->vdev);
-
+ virtio_config_changed(vq->vdev);
break;
- }
case VIRTIO_PARAM_DEV_ADD:
schedule_work(&hotplug_work);
break;
diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index d2c0b44..6cbe6ef 100644
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -940,11 +940,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
vring_interrupt(0, vq);
}
if (test_bit(0, &vcdev->indicators2)) {
- drv = container_of(vcdev->vdev.dev.driver,
- struct virtio_driver, driver);
-
- if (drv && drv->config_changed)
- drv->config_changed(&vcdev->vdev);
+ virtio_config_changed(&vcdev->vdev);
clear_bit(0, &vcdev->indicators2);
}
}
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index fed0ce1..3980687 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -239,6 +239,15 @@ void unregister_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(unregister_virtio_device);
+void virtio_config_changed(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ if (drv && drv->config_changed)
+ drv->config_changed(dev);
+}
+EXPORT_SYMBOL_GPL(virtio_config_changed);
+
static int virtio_init(void)
{
if (bus_register(&virtio_bus) != 0)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index c600ccf..ef9a165 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -234,8 +234,6 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
{
struct virtio_mmio_device *vm_dev = opaque;
struct virtio_mmio_vq_info *info;
- struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
- struct virtio_driver, driver);
unsigned long status;
unsigned long flags;
irqreturn_t ret = IRQ_NONE;
@@ -244,9 +242,8 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
- if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
- && vdrv && vdrv->config_changed) {
- vdrv->config_changed(&vm_dev->vdev);
+ if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
+ virtio_config_changed(&vm_dev->vdev);
ret = IRQ_HANDLED;
}
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index add40d0..f39f4e7 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -211,12 +211,8 @@ static bool vp_notify(struct virtqueue *vq)
static irqreturn_t vp_config_changed(int irq, void *opaque)
{
struct virtio_pci_device *vp_dev = opaque;
- struct virtio_driver *drv;
- drv = container_of(vp_dev->vdev.dev.driver,
- struct virtio_driver, driver);
- if (drv && drv->config_changed)
- drv->config_changed(&vp_dev->vdev);
+ virtio_config_changed(&vp_dev->vdev);
return IRQ_HANDLED;
}
--
MST
^ permalink raw reply related
* [PATCH v3 01/25] virtio_pci: fix virtio spec compliance on restore
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
stable, virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
On restore, virtio pci does the following:
+ set features
+ init vqs etc - device can be used at this point!
+ set ACKNOWLEDGE,DRIVER and DRIVER_OK status bits
This is in violation of the virtio spec, which
requires the following order:
- ACKNOWLEDGE
- DRIVER
- init vqs
- DRIVER_OK
This behaviour will break with hypervisors that assume spec compliant
behaviour. It seems like a good idea to have this patch applied to
stable branches to reduce the support butden for the hypervisors.
Cc: stable@vger.kernel.org
Cc: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_pci.c | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3d1463c..add40d0 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -789,6 +789,7 @@ static int virtio_pci_restore(struct device *dev)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
struct virtio_driver *drv;
+ unsigned status = 0;
int ret;
drv = container_of(vp_dev->vdev.dev.driver,
@@ -799,14 +800,40 @@ static int virtio_pci_restore(struct device *dev)
return ret;
pci_set_master(pci_dev);
+ /* We always start by resetting the device, in case a previous
+ * driver messed it up. */
+ vp_reset(&vp_dev->vdev);
+
+ /* Acknowledge that we've seen the device. */
+ status |= VIRTIO_CONFIG_S_ACKNOWLEDGE;
+ vp_set_status(&vp_dev->vdev, status);
+
+ /* Maybe driver failed before freeze.
+ * Restore the failed status, for debugging. */
+ status |= vp_dev->saved_status & VIRTIO_CONFIG_S_FAILED;
+ vp_set_status(&vp_dev->vdev, status);
+
+ if (!drv)
+ return 0;
+
+ /* We have a driver! */
+ status |= VIRTIO_CONFIG_S_DRIVER;
+ vp_set_status(&vp_dev->vdev, status);
+
vp_finalize_features(&vp_dev->vdev);
- if (drv && drv->restore)
+ if (drv->restore) {
ret = drv->restore(&vp_dev->vdev);
+ if (ret) {
+ status |= VIRTIO_CONFIG_S_FAILED;
+ vp_set_status(&vp_dev->vdev, status);
+ return ret;
+ }
+ }
/* Finally, tell the device we're all set */
- if (!ret)
- vp_set_status(&vp_dev->vdev, vp_dev->saved_status);
+ status |= VIRTIO_CONFIG_S_DRIVER_OK;
+ vp_set_status(&vp_dev->vdev, status);
return ret;
}
--
MST
^ permalink raw reply related
* [PATCH v3 25/25] virtio-rng: refactor probe error handling
From: Michael S. Tsirkin @ 2014-10-12 11:48 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini,
Matt Mackall, Herbert Xu, Amos Kong, Sasha Levin
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
Code like
vi->vq = NULL;
kfree(vi)
does not make sense.
Clean it up, use goto error labels for cleanup.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/hw_random/virtio-rng.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 132c9cc..72295ea 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -109,8 +109,8 @@ static int probe_common(struct virtio_device *vdev)
vi->index = index = ida_simple_get(&rng_index_ida, 0, 0, GFP_KERNEL);
if (index < 0) {
- kfree(vi);
- return index;
+ err = index;
+ goto err_ida;
}
sprintf(vi->name, "virtio_rng.%d", index);
init_completion(&vi->have_data);
@@ -128,13 +128,16 @@ static int probe_common(struct virtio_device *vdev)
vi->vq = virtio_find_single_vq(vdev, random_recv_done, "input");
if (IS_ERR(vi->vq)) {
err = PTR_ERR(vi->vq);
- vi->vq = NULL;
- kfree(vi);
- ida_simple_remove(&rng_index_ida, index);
- return err;
+ goto err_find;
}
return 0;
+
+err_find:
+ ida_simple_remove(&rng_index_ida, index);
+err_ida:
+ kfree(vi);
+ return err;
}
static void remove_common(struct virtio_device *vdev)
--
MST
^ permalink raw reply related
* [PATCH v3 00/25] virtio: fix spec compliance issues
From: Michael S. Tsirkin @ 2014-10-12 11:46 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, kvm, linux-scsi, Christian Borntraeger, netdev,
virtualization, Paolo Bonzini, Amit Shah, v9fs-developer,
David S. Miller
Rusty, please review this, and consider for this merge window.
This fixes the following virtio spec compliance issues:
1. on restore, drivers use device before setting ACKNOWLEDGE and DRIVER bits
2. on probe, drivers aren't prepared to handle config interrupts
arriving before probe returns
3. on probe, drivers use device before DRIVER_OK it set
Note that 1 is a clear violation of virtio spec 0.9 and 1.0,
so I am proposing the fix for stable. OTOH 2 is against 1.0 rules but
is a known documented bug in many drivers, so let's fix it going
forward, but it does not seem to be worth it to backport
the changes.
An error handling bugfix for virtio-net and a fix for a
theoretical race condition in virtio scsi is also included.
2 is merely a theoretical race condition, but it seems important
to address to make sure that changes to address 3 do not introduce
instability.
I also included patch to drop scan callback use from virtio
scsi: using this callback creates asymmetry between probe
and resume, and we actually had to add code comments so
people can figure out the flow. Code flow becomes clearer
with the new API.
After this change, the only user of the scan callback is virtio rng.
It's possible to modify it trivially and then drop scan callback
from core, but I'm rather inclined to look at ways to
make some rng core changes so that we don't need to
have so many variables tracking device state.
So this is deferred for now.
Michael S. Tsirkin (24):
virtio_pci: fix virtio spec compliance on restore
virtio: unify config_changed handling
virtio-pci: move freeze/restore to virtio core
virtio: defer config changed notifications
virtio_blk: drop config_enable
virtio-blk: drop config_mutex
virtio_net: drop config_enable
virtio-net: drop config_mutex
virtio_net: minor cleanup
virtio: add API to enable VQs early
virtio_net: enable VQs early
virtio_blk: enable VQs early
virtio_console: enable VQs early
9p/trans_virtio: enable VQs early
virtio_net: fix use after free on allocation failure
virtio_scsi: move kick event out from virtscsi_init
virtio_blk: enable VQs early on restore
virtio_scsi: enable VQs early on restore
virtio_console: enable VQs early on restore
virtio_net: enable VQs early on restore
virtio_scsi: fix race on device removal
virtio_balloon: enable VQs early on restore
virtio_scsi: drop scan callback
virtio-rng: refactor probe error handling
Paolo Bonzini (1):
virito_scsi: use freezable WQ for events
include/linux/virtio.h | 14 +++++
include/linux/virtio_config.h | 17 ++++++
drivers/block/virtio_blk.c | 40 ++++----------
drivers/char/hw_random/virtio-rng.c | 15 +++---
drivers/char/virtio_console.c | 4 ++
drivers/misc/mic/card/mic_virtio.c | 6 +--
drivers/net/virtio_net.c | 44 +++++-----------
drivers/s390/kvm/kvm_virtio.c | 9 +---
drivers/s390/kvm/virtio_ccw.c | 6 +--
drivers/scsi/virtio_scsi.c | 42 +++++++++------
drivers/virtio/virtio.c | 102 ++++++++++++++++++++++++++++++++++++
drivers/virtio/virtio_balloon.c | 2 +
drivers/virtio/virtio_mmio.c | 7 +--
drivers/virtio/virtio_pci.c | 33 ++----------
net/9p/trans_virtio.c | 2 +
15 files changed, 206 insertions(+), 137 deletions(-)
--
MST
^ permalink raw reply
* [PATCH v3 21/25] virito_scsi: use freezable WQ for events
From: Michael S. Tsirkin @ 2014-10-12 11:48 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini,
James E.J. Bottomley
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
From: Paolo Bonzini <pbonzini@redhat.com>
Michael S. Tsirkin noticed a race condition:
we reset device on freeze, but system WQ is still
running so it might try adding bufs to a VQ meanwhile.
To fix, switch to handling events from the freezable WQ.
Reported-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/scsi/virtio_scsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 2b565b3..501838d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -390,7 +390,7 @@ static void virtscsi_complete_event(struct virtio_scsi *vscsi, void *buf)
{
struct virtio_scsi_event_node *event_node = buf;
- schedule_work(&event_node->work);
+ queue_work(system_freezable_wq, &event_node->work);
}
static void virtscsi_event_done(struct virtqueue *vq)
--
MST
^ permalink raw reply related
* [PATCH v3 17/25] virtio_blk: enable VQs early on restore
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after restore returns, virtio block violated
this rule on restore by restarting queues, which might in theory
cause the VQ to be used directly within restore.
To fix, call virtio_enable_vqs_early before using starting queues.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/block/virtio_blk.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 46b04bf..1c95af5 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -804,10 +804,13 @@ static int virtblk_restore(struct virtio_device *vdev)
int ret;
ret = init_vq(vdev->priv);
- if (!ret)
- blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
+ if (ret)
+ return ret;
+
+ virtio_enable_vqs_early(vdev);
- return ret;
+ blk_mq_start_stopped_hw_queues(vblk->disk->queue, true);
+ return 0;
}
#endif
--
MST
^ permalink raw reply related
* [PATCH v3 16/25] virtio_scsi: move kick event out from virtscsi_init
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini,
James E.J. Bottomley
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
We currently kick event within virtscsi_init,
before host is fully initialized.
This can in theory confuse guest if device
consumes the buffers immediately.
To fix, move virtscsi_kick_event_all out to scan/restore.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/scsi/virtio_scsi.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index eee1bc0..0642ce3 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -853,7 +853,11 @@ static void virtscsi_init_vq(struct virtio_scsi_vq *virtscsi_vq,
static void virtscsi_scan(struct virtio_device *vdev)
{
- struct Scsi_Host *shost = (struct Scsi_Host *)vdev->priv;
+ struct Scsi_Host *shost = virtio_scsi_host(vdev);
+ struct virtio_scsi *vscsi = shost_priv(shost);
+
+ if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
+ virtscsi_kick_event_all(vscsi);
scsi_scan_host(shost);
}
@@ -916,9 +920,6 @@ static int virtscsi_init(struct virtio_device *vdev,
virtscsi_config_set(vdev, cdb_size, VIRTIO_SCSI_CDB_SIZE);
virtscsi_config_set(vdev, sense_size, VIRTIO_SCSI_SENSE_SIZE);
- if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
- virtscsi_kick_event_all(vscsi);
-
err = 0;
out:
@@ -1048,8 +1049,13 @@ static int virtscsi_restore(struct virtio_device *vdev)
return err;
err = register_hotcpu_notifier(&vscsi->nb);
- if (err)
+ if (err) {
vdev->config->del_vqs(vdev);
+ return err;
+ }
+
+ if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
+ virtscsi_kick_event_all(vscsi);
return err;
}
--
MST
^ permalink raw reply related
* [PATCH v3 13/25] virtio_console: enable VQs early
From: Michael S. Tsirkin @ 2014-10-12 11:47 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, virtualization, linux-scsi, linux-s390,
v9fs-developer, netdev, kvm, Amit Shah, Cornelia Huck,
Christian Borntraeger, David S. Miller, Paolo Bonzini,
Arnd Bergmann, Greg Kroah-Hartman
In-Reply-To: <1413114332-626-1-git-send-email-mst@redhat.com>
virtio spec requires drivers to set DRIVER_OK before using VQs.
This is set automatically after probe returns, virtio console violated this
rule by adding inbufs, which causes the VQ to be used directly within
probe.
To fix, call virtio_enable_vqs_early before using VQs.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/char/virtio_console.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b585b47..6ebe8f6 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1449,6 +1449,8 @@ static int add_port(struct ports_device *portdev, u32 id)
spin_lock_init(&port->outvq_lock);
init_waitqueue_head(&port->waitqueue);
+ virtio_enable_vqs_early(portdev->vdev);
+
/* Fill the in_vq with buffers so the host can send us data. */
nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock);
if (!nr_added_bufs) {
--
MST
^ permalink raw reply related
* Re: IPv6 FIB related crash with MACVLANs in 3.9.11+ kernel.
From: Vladislav Yasevich @ 2014-10-12 11:42 UTC (permalink / raw)
To: Ben Greear; +Cc: Cong Wang, Hannes Frederic Sowa, Hongmei Li, netdev
In-Reply-To: <5429CE26.6000108@candelatech.com>
On Mon, Sep 29, 2014 at 5:24 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 09/29/2014 01:39 PM, Cong Wang wrote:
>> On Mon, Sep 29, 2014 at 12:48 PM, Ben Greear <greearb@candelatech.com> wrote:
>>>
>>> We are going to be running up to 20 concurrent scripts that
>>> will be dumping routes & ips, and configuring ip addresses
>>> and routes during bringup.
>>>
>>> I don't have a simple stand-alone way to reproduce this,
>>> but at least when we reported the problem is was very easy
>>> for us to reproduce with our tool.
>>>
>>> Maybe 20 scripts running in parallel that randomly configured and dumped routes
>>> and ip addresses on random interfaces would do the trick?
>>>
>>
>> I think the most important question is why is this related with macvlan?
>> IPv6 is L3 while macvlan pure L2, if this is a IPv6 routing bug, it should
>> not be limited to macvlan.
>
> We saw it using mac-vlans on top of ixgbe. Probably it can be reproduced
> elsewhere, but since we had a good test case, we didn't bother trying lots
> of other combinations.
>
> Just enabling some debug code caused the problem to be much harder
> to reproduce, so it is probably some sort of race. Maybe lots of mac-vlans
> make it easier to hit.
>
>> What does your IPv6 routing table look like? And how do you configure those
>> macvlan interfaces?
>
> Each interface would have it's own routing table, with at least as subnet
> route. I'm not sure we were adding a default gateway or not.
>
> The main routing table would also have some auto-created routes
> I think.
>
> It is configured using 'ip' to add and dump routes.
>
Ben
Just saw this thread. Can you check if this commit makes a difference:
commit 40b8fe45d1f094e3babe7b2dc2b71557ab71401d
Author: Vlad Yasevich <vyasevich@gmail.com>
Date: Mon Sep 22 16:34:17 2014 -0400
macvtap: Fix race between device delete and open.
Thanks
-vlad
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc http://www.candelatech.com
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] brcmfmac: dhd_sdio.c: Cleaning up missing null-terminate in conjunction with strncpy
From: Rickard Strandqvist @ 2014-10-12 11:42 UTC (permalink / raw)
To: Brett Rudley, Arend van Spriel, linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: Rickard Strandqvist, Hante Meuleman, Pieter-Paul Giesberts,
Daniel Kim, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1413114134-18044-1-git-send-email-rickard_strandqvist-IW2WV5XWFqGZkjO+N0TKoMugMpMbD5Xr@public.gmane.org>
Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
And changed from using strncat to strlcat to simplify code.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist-IW2WV5XWFqGZkjO+N0TKoMugMpMbD5Xr@public.gmane.org>
---
drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 25 ++++++++++----------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
index f55f625..d20d4e6 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
@@ -670,7 +670,6 @@ static int brcmf_sdio_get_fwnames(struct brcmf_chip *ci,
struct brcmf_sdio_dev *sdiodev)
{
int i;
- uint fw_len, nv_len;
char end;
for (i = 0; i < ARRAY_SIZE(brcmf_fwname_data); i++) {
@@ -684,25 +683,25 @@ static int brcmf_sdio_get_fwnames(struct brcmf_chip *ci,
return -ENODEV;
}
- fw_len = sizeof(sdiodev->fw_name) - 1;
- nv_len = sizeof(sdiodev->nvram_name) - 1;
/* check if firmware path is provided by module parameter */
if (brcmf_firmware_path[0] != '\0') {
- strncpy(sdiodev->fw_name, brcmf_firmware_path, fw_len);
- strncpy(sdiodev->nvram_name, brcmf_firmware_path, nv_len);
- fw_len -= strlen(sdiodev->fw_name);
- nv_len -= strlen(sdiodev->nvram_name);
+ strlcpy(sdiodev->fw_name, brcmf_firmware_path,
+ sizeof(sdiodev->fw_name));
+ strlcpy(sdiodev->nvram_name, brcmf_firmware_path,
+ sizeof(sdiodev->nvram_name));
end = brcmf_firmware_path[strlen(brcmf_firmware_path) - 1];
if (end != '/') {
- strncat(sdiodev->fw_name, "/", fw_len);
- strncat(sdiodev->nvram_name, "/", nv_len);
- fw_len--;
- nv_len--;
+ strlcat(sdiodev->fw_name, "/",
+ sizeof(sdiodev->fw_name));
+ strlcat(sdiodev->nvram_name, "/",
+ sizeof(sdiodev->nvram_name));
}
}
- strncat(sdiodev->fw_name, brcmf_fwname_data[i].bin, fw_len);
- strncat(sdiodev->nvram_name, brcmf_fwname_data[i].nv, nv_len);
+ strlcat(sdiodev->fw_name, brcmf_fwname_data[i].bin,
+ sizeof(sdiodev->fw_name));
+ strlcat(sdiodev->nvram_name, brcmf_fwname_data[i].nv,
+ sizeof(sdiodev->nvram_name));
return 0;
}
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2] brcmfmac: dhd_sdio.c: Cleaning up missing null-terminate in conjunction with strncpy
From: Rickard Strandqvist @ 2014-10-12 11:42 UTC (permalink / raw)
To: Brett Rudley, Arend van Spriel, linville
Cc: Rickard Strandqvist, Hante Meuleman, Pieter-Paul Giesberts,
Daniel Kim, linux-wireless, brcm80211-dev-list, netdev,
linux-kernel
Now when I did the patch again I just want to ensure that this is not any other errors.
Although strncpy use before was wrong, it did filled the remaining part of the string with null
characters. This is not something that is needed?
And a part that is not obvious right, so check.
When brcmf_firmware_path[0] == '\0' we do not want to do like.
if (brcmf_firmware_path[0] != '\0') {
...
} else {
sdiodev->fw_name[0] = '\0';
sdiodev->nvram_name[0] = '\0';
}
Kind regards
Rickard Strandqvist
Rickard Strandqvist (1):
net: wireless: brcm80211: brcmfmac: dhd_sdio.c: Cleaning up missing null-terminate in conjunction with
strncpy
drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 25 ++++++++++----------
1 file changed, 12 insertions(+), 13 deletions(-)
--
1.7.10.4
^ permalink raw reply
* Re: [PATCH] net: wireless: brcm80211: brcmfmac: dhd_sdio.c: Cleaning up missing null-terminate in conjunction with strncpy
From: Rickard Strandqvist @ 2014-10-12 11:05 UTC (permalink / raw)
To: Arend van Spriel
Cc: Brett Rudley, Hante Meuleman, John W. Linville,
Pieter-Paul Giesberts, Daniel Kim,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list-dY08KVG/lbpWk0Htik3J/w, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <543A36A0.8090300-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
2014-10-12 10:06 GMT+02:00 Arend van Spriel <arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>:
> On 12-10-14 01:52, Rickard Strandqvist wrote:
>> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
>> And changed from using strncpy to strlcpy to simplify code.
>
> Looks good to me. Just two small process related remarks:
>
> - It is sufficient to prefix the patch with brcmfmac (skip net:...).
> - Send the patch to the wireless maintainer, ie. John Linville.
Hi
Sure, no problem.
Sending a new patch in a moment...
Kind regards
Rickard Strandqvist
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V1 net-next 1/2] pgtable: Add API to query if write combining is available
From: Moshe Lazer @ 2014-10-12 9:54 UTC (permalink / raw)
To: David Miller; +Cc: ogerlitz, jackm, talal, yevgenyp, netdev, amirv, moshel
In-Reply-To: <20141008.122450.1919301744382855426.davem@davemloft.net>
On 10/8/2014 7:24 PM, David Miller wrote:
> From: Moshe Lazer <moshel@dev.mellanox.co.il>
> Date: Wed, 08 Oct 2014 11:44:57 +0300
>
>>> #if defined(__i386__) || defined(__x86_64__)
>>> if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
>>> tmp = pgprot_noncached(tmp);
>>> else
>>> tmp = pgprot_writecombine(tmp);
>>> #elif defined(__powerpc__)
>>> pgprot_val(tmp) |= _PAGE_NO_CACHE;
>>> if (map->type == _DRM_REGISTERS)
>>> pgprot_val(tmp) |= _PAGE_GUARDED;
>>> #elif defined(__ia64__)
>>> if (efi_range_is_wc(vma->vm_start, vma->vm_end -
>>> vma->vm_start))
>>> tmp = pgprot_writecombine(tmp);
>>> else
>>> tmp = pgprot_noncached(tmp);
>>> #elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
>>> tmp = pgprot_noncached(tmp);
>>> #endif
>> The idea was to provide an indication as for whether the arch supports
>> write-combining in general.
>> If we want to benefit from blue flame operations, we need to map the
>> blue flame registers as write combining - otherwise there is no
>> benefit. So we would like to know if write combining is supported by
>> the system or not.
>>
> You completely miss my point. On a given architectuire it might be
> _illegal_ to map certain address ranges as write-combining without
> checks like the ones above that ia64 needs.
>
> Therefore your proposed interface is by definition insufficient.
Thanks David, I'll try to clarify my point.
For me the writecombine_available() is a way to know if the
pgprot_writecombine() is effective or just cover call to the
pgprot_noncached().
I want to use the writecombine_available() regardless to the mapping
address.
For example in mlx4 query_device I want to indicate that blue-flame is
not supported if `writecombine_available() == false`.
In this case we don't have the mapping address yet.
Later on if an arch has write-combining (writecombine_available() ==
true) when we try to map the blue-flame registers (in mlx4_ib_mmap):
vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
if (io_remap_pfn_range(vma, vma->vm_start,
to_mucontext(context)->uar.pfn +
dev->dev->caps.num_uars,
PAGE_SIZE, vma->vm_page_prot))
return -EAGAIN;
I can be sure that pgprot_writecombine() is not a cover for
pgprot_noncached().
The address checks that you mentioned should be part of
io_remap_pfn_range, this function should fail if the vma->vm_start is
not compatible to the vma->vm_page_prot.
Please let me know if I misunderstood something.
^ permalink raw reply
* Re: [PATCH net-next RFC 1/3] virtio: support for urgent descriptors
From: Michael S. Tsirkin @ 2014-10-12 9:27 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, netdev, linux-kernel, virtualization, linux-api
In-Reply-To: <1413011806-3813-2-git-send-email-jasowang@redhat.com>
On Sat, Oct 11, 2014 at 03:16:44PM +0800, Jason Wang wrote:
> Below should be useful for some experiments Jason is doing.
> I thought I'd send it out for early review/feedback.
>
> event idx feature allows us to defer interrupts until
> a specific # of descriptors were used.
> Sometimes it might be useful to get an interrupt after
> a specific descriptor, regardless.
> This adds a descriptor flag for this, and an API
> to create an urgent output descriptor.
> This is still an RFC:
> we'll need a feature bit for drivers to detect this,
> but we've run out of feature bits for virtio 0.X.
> For experimentation purposes, drivers can assume
> this is set, or add a driver-specific feature bit.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
I see that as compared to my original patch, you have
added a new flag: VRING_AVAIL_F_NO_URGENT_INTERRUPT
I don't think it's necessary, see below.
As such, I think this patch should be split:
- original patch adding support for urgent descriptors
- a patch adding virtqueue_enable/disable_cb_urgent(_prepare)?
> ---
> drivers/virtio/virtio_ring.c | 75 +++++++++++++++++++++++++++++++++++++---
> include/linux/virtio.h | 14 ++++++++
> include/uapi/linux/virtio_ring.h | 5 ++-
> 3 files changed, 89 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 4d08f45a..a5188c6 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -115,6 +115,7 @@ static inline struct scatterlist *sg_next_arr(struct scatterlist *sg,
>
> /* Set up an indirect table of descriptors and add it to the queue. */
> static inline int vring_add_indirect(struct vring_virtqueue *vq,
> + bool urgent,
> struct scatterlist *sgs[],
> struct scatterlist *(*next)
> (struct scatterlist *, unsigned int *),
> @@ -173,6 +174,8 @@ static inline int vring_add_indirect(struct vring_virtqueue *vq,
> /* Use a single buffer which doesn't continue */
> head = vq->free_head;
> vq->vring.desc[head].flags = VRING_DESC_F_INDIRECT;
> + if (urgent)
> + vq->vring.desc[head].flags |= VRING_DESC_F_URGENT;
> vq->vring.desc[head].addr = virt_to_phys(desc);
> /* kmemleak gives a false positive, as it's hidden by virt_to_phys */
> kmemleak_ignore(desc);
> @@ -185,6 +188,7 @@ static inline int vring_add_indirect(struct vring_virtqueue *vq,
> }
>
> static inline int virtqueue_add(struct virtqueue *_vq,
> + bool urgent,
> struct scatterlist *sgs[],
> struct scatterlist *(*next)
> (struct scatterlist *, unsigned int *),
> @@ -227,7 +231,7 @@ static inline int virtqueue_add(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 && total_sg > 1 && vq->vq.num_free) {
> - head = vring_add_indirect(vq, sgs, next, total_sg, total_out,
> + head = vring_add_indirect(vq, urgent, sgs, next, total_sg, total_out,
> total_in,
> out_sgs, in_sgs, gfp);
> if (likely(head >= 0))
> @@ -256,6 +260,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> for (n = 0; n < out_sgs; n++) {
> for (sg = sgs[n]; sg; sg = next(sg, &total_out)) {
> vq->vring.desc[i].flags = VRING_DESC_F_NEXT;
> + if (urgent) {
> + vq->vring.desc[head].flags |= VRING_DESC_F_URGENT;
> + urgent = false;
> + }
> vq->vring.desc[i].addr = sg_phys(sg);
> vq->vring.desc[i].len = sg->length;
> prev = i;
> @@ -265,6 +273,10 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> for (; n < (out_sgs + in_sgs); n++) {
> for (sg = sgs[n]; sg; sg = next(sg, &total_in)) {
> vq->vring.desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE;
> + if (urgent) {
> + vq->vring.desc[head].flags |= VRING_DESC_F_URGENT;
> + urgent = false;
> + }
> vq->vring.desc[i].addr = sg_phys(sg);
> vq->vring.desc[i].len = sg->length;
> prev = i;
> @@ -305,6 +317,8 @@ add_head:
>
> /**
> * virtqueue_add_sgs - expose buffers to other end
> + * @urgent: in case virtqueue_enable_cb_delayed was called, cause an interrupt
> + * after this descriptor was completed
> * @vq: the struct virtqueue we're talking about.
> * @sgs: array of terminated scatterlists.
> * @out_num: the number of scatterlists readable by other side
> @@ -337,7 +351,7 @@ int virtqueue_add_sgs(struct virtqueue *_vq,
> for (sg = sgs[i]; sg; sg = sg_next(sg))
> total_in++;
> }
> - return virtqueue_add(_vq, sgs, sg_next_chained,
> + return virtqueue_add(_vq, false, sgs, sg_next_chained,
> total_out, total_in, out_sgs, in_sgs, data, gfp);
> }
> EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
> @@ -360,11 +374,35 @@ int virtqueue_add_outbuf(struct virtqueue *vq,
> void *data,
> gfp_t gfp)
> {
> - return virtqueue_add(vq, &sg, sg_next_arr, num, 0, 1, 0, data, gfp);
> + return virtqueue_add(vq, false, &sg, sg_next_arr, num, 0, 1, 0, data, gfp);
> }
> EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
>
> /**
> + * virtqueue_add_outbuf - expose output buffers to other end
> + * in case virtqueue_enable_cb_delayed was called, cause an interrupt
> + * after this descriptor was completed
> + * @vq: the struct virtqueue we're talking about.
> + * @sgs: array of scatterlists (need not be terminated!)
> + * @num: the number of scatterlists readable by other side
> + * @data: the token identifying the buffer.
> + * @gfp: how to do memory allocations (if necessary).
> + *
> + * Caller must ensure we don't call this with other virtqueue operations
> + * at the same time (except where noted).
> + *
> + * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
> + */
> +int virtqueue_add_outbuf_urgent(struct virtqueue *vq,
> + struct scatterlist sg[], unsigned int num,
> + void *data,
> + gfp_t gfp)
> +{
> + return virtqueue_add(vq, true, &sg, sg_next_arr, num, 0, 1, 0, data, gfp);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_add_outbuf_urgent);
> +
> +/**
> * virtqueue_add_inbuf - expose input buffers to other end
> * @vq: the struct virtqueue we're talking about.
> * @sgs: array of scatterlists (need not be terminated!)
> @@ -382,7 +420,7 @@ int virtqueue_add_inbuf(struct virtqueue *vq,
> void *data,
> gfp_t gfp)
> {
> - return virtqueue_add(vq, &sg, sg_next_arr, 0, num, 0, 1, data, gfp);
> + return virtqueue_add(vq, false, &sg, sg_next_arr, 0, num, 0, 1, data, gfp);
> }
> EXPORT_SYMBOL_GPL(virtqueue_add_inbuf);
>
> @@ -595,6 +633,14 @@ void virtqueue_disable_cb(struct virtqueue *_vq)
> }
> EXPORT_SYMBOL_GPL(virtqueue_disable_cb);
>
> +void virtqueue_disable_cb_urgent(struct virtqueue *_vq)
> +{
> + struct vring_virtqueue *vq = to_vvq(_vq);
> +
> + vq->vring.avail->flags |= VRING_AVAIL_F_NO_URGENT_INTERRUPT;
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_disable_cb_urgent);
> +
> /**
> * virtqueue_enable_cb_prepare - restart callbacks after disable_cb
> * @vq: the struct virtqueue we're talking about.
> @@ -626,6 +672,19 @@ unsigned virtqueue_enable_cb_prepare(struct virtqueue *_vq)
> }
> EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare);
>
> +unsigned virtqueue_enable_cb_prepare_urgent(struct virtqueue *_vq)
> +{
> + struct vring_virtqueue *vq = to_vvq(_vq);
> + u16 last_used_idx;
> +
> + START_USE(vq);
> + vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_URGENT_INTERRUPT;
> + last_used_idx = vq->last_used_idx;
> + END_USE(vq);
> + return last_used_idx;
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_enable_cb_prepare_urgent);
> +
You can implement virtqueue_enable_cb_prepare_urgent
simply by clearing ~VRING_AVAIL_F_NO_INTERRUPT.
The effect is same: host sends interrupts only if there
is an urgent descriptor.
> /**
> * virtqueue_poll - query pending used buffers
> * @vq: the struct virtqueue we're talking about.
> @@ -662,6 +721,14 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
> }
> EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
>
> +bool virtqueue_enable_cb_urgent(struct virtqueue *_vq)
> +{
> + unsigned last_used_idx = virtqueue_enable_cb_prepare_urgent(_vq);
> +
> + return !virtqueue_poll(_vq, last_used_idx);
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_enable_cb_urgent);
> +
> /**
> * virtqueue_enable_cb_delayed - restart callbacks after disable_cb.
> * @vq: the struct virtqueue we're talking about.
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index b46671e..68be5f2 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -39,6 +39,12 @@ int virtqueue_add_outbuf(struct virtqueue *vq,
> void *data,
> gfp_t gfp);
>
> +int virtqueue_add_outbuf_urgent(struct virtqueue *vq,
> + struct scatterlist sg[], unsigned int num,
> + void *data,
> + gfp_t gfp);
> +
> +
> int virtqueue_add_inbuf(struct virtqueue *vq,
> struct scatterlist sg[], unsigned int num,
> void *data,
> @@ -61,10 +67,18 @@ void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
>
> void virtqueue_disable_cb(struct virtqueue *vq);
>
> +void virtqueue_disable_cb_urgent(struct virtqueue *vq);
> +
> bool virtqueue_enable_cb(struct virtqueue *vq);
>
> +bool virtqueue_enable_cb_urgent(struct virtqueue *vq);
> +
> +bool virtqueue_enable_cb_urgent(struct virtqueue *vq);
> +
> unsigned virtqueue_enable_cb_prepare(struct virtqueue *vq);
>
> +unsigned virtqueue_enable_cb_prepare_urgent(struct virtqueue *vq);
> +
> bool virtqueue_poll(struct virtqueue *vq, unsigned);
>
> bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
> diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
> index a99f9b7..daf5bb0 100644
> --- a/include/uapi/linux/virtio_ring.h
> +++ b/include/uapi/linux/virtio_ring.h
> @@ -39,6 +39,9 @@
> #define VRING_DESC_F_WRITE 2
> /* This means the buffer contains a list of buffer descriptors. */
> #define VRING_DESC_F_INDIRECT 4
> +/* This means the descriptor should cause an interrupt
> + * ignoring avail event idx. */
> +#define VRING_DESC_F_URGENT 8
>
> /* The Host uses this in used->flags to advise the Guest: don't kick me when
> * you add a buffer. It's unreliable, so it's simply an optimization. Guest
> @@ -48,7 +51,7 @@
> * when you consume a buffer. It's unreliable, so it's simply an
> * optimization. */
> #define VRING_AVAIL_F_NO_INTERRUPT 1
> -
> +#define VRING_AVAIL_F_NO_URGENT_INTERRUPT 2
> /* We support indirect buffer descriptors */
> #define VIRTIO_RING_F_INDIRECT_DESC 28
>
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH] net: wireless: brcm80211: brcmfmac: dhd_sdio.c: Cleaning up missing null-terminate in conjunction with strncpy
From: Arend van Spriel @ 2014-10-12 8:06 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Brett Rudley, Hante Meuleman, John W. Linville,
Pieter-Paul Giesberts, Daniel Kim, linux-wireless,
brcm80211-dev-list, netdev, linux-kernel
In-Reply-To: <1413071551-15372-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
On 12-10-14 01:52, Rickard Strandqvist wrote:
> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
> And changed from using strncpy to strlcpy to simplify code.
Looks good to me. Just two small process related remarks:
- It is sufficient to prefix the patch with brcmfmac (skip net:...).
- Send the patch to the wireless maintainer, ie. John Linville.
Acked-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c | 25 ++++++++++----------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
> index f55f625..d20d4e6 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_sdio.c
> @@ -670,7 +670,6 @@ static int brcmf_sdio_get_fwnames(struct brcmf_chip *ci,
> struct brcmf_sdio_dev *sdiodev)
> {
> int i;
> - uint fw_len, nv_len;
> char end;
>
> for (i = 0; i < ARRAY_SIZE(brcmf_fwname_data); i++) {
> @@ -684,25 +683,25 @@ static int brcmf_sdio_get_fwnames(struct brcmf_chip *ci,
> return -ENODEV;
> }
>
> - fw_len = sizeof(sdiodev->fw_name) - 1;
> - nv_len = sizeof(sdiodev->nvram_name) - 1;
> /* check if firmware path is provided by module parameter */
> if (brcmf_firmware_path[0] != '\0') {
> - strncpy(sdiodev->fw_name, brcmf_firmware_path, fw_len);
> - strncpy(sdiodev->nvram_name, brcmf_firmware_path, nv_len);
> - fw_len -= strlen(sdiodev->fw_name);
> - nv_len -= strlen(sdiodev->nvram_name);
> + strlcpy(sdiodev->fw_name, brcmf_firmware_path,
> + sizeof(sdiodev->fw_name));
> + strlcpy(sdiodev->nvram_name, brcmf_firmware_path,
> + sizeof(sdiodev->nvram_name));
>
> end = brcmf_firmware_path[strlen(brcmf_firmware_path) - 1];
> if (end != '/') {
> - strncat(sdiodev->fw_name, "/", fw_len);
> - strncat(sdiodev->nvram_name, "/", nv_len);
> - fw_len--;
> - nv_len--;
> + strlcat(sdiodev->fw_name, "/",
> + sizeof(sdiodev->fw_name));
> + strlcat(sdiodev->nvram_name, "/",
> + sizeof(sdiodev->nvram_name));
> }
> }
> - strncat(sdiodev->fw_name, brcmf_fwname_data[i].bin, fw_len);
> - strncat(sdiodev->nvram_name, brcmf_fwname_data[i].nv, nv_len);
> + strlcat(sdiodev->fw_name, brcmf_fwname_data[i].bin,
> + sizeof(sdiodev->fw_name));
> + strlcat(sdiodev->nvram_name, brcmf_fwname_data[i].nv,
> + sizeof(sdiodev->nvram_name));
>
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH net 2/3] net: sctp: fix panic on duplicate ASCONF chunks
From: Vladislav Yasevich @ 2014-10-12 7:15 UTC (permalink / raw)
To: Neil Horman
Cc: Daniel Borkmann, David Miller, linux-sctp@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <20141012014233.GA16360@localhost.localdomain>
I prefer we stay away from WARN-ONs, but pr_debug might be useful.
-vlad
On Sat, Oct 11, 2014 at 9:42 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Sat, Oct 11, 2014 at 12:02:31AM +0200, Daniel Borkmann wrote:
>> On 10/10/2014 05:39 PM, Neil Horman wrote:
>> ...
>> >Is it worth adding a WARN_ON, to indicate that two ASCONF chunks have been
>> >received with duplicate serials?
>>
>> Don't think so, as this would be triggerable from outside.
>>
> WARN_ON_ONCE then, per serial number?
> Neil
>
>> Thanks,
>> Daniel
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply
* Re: [PATCH net 2/3] net: sctp: fix panic on duplicate ASCONF chunks
From: Neil Horman @ 2014-10-12 1:42 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev, Vlad Yasevich
In-Reply-To: <54385777.7090907@redhat.com>
On Sat, Oct 11, 2014 at 12:02:31AM +0200, Daniel Borkmann wrote:
> On 10/10/2014 05:39 PM, Neil Horman wrote:
> ...
> >Is it worth adding a WARN_ON, to indicate that two ASCONF chunks have been
> >received with duplicate serials?
>
> Don't think so, as this would be triggerable from outside.
>
WARN_ON_ONCE then, per serial number?
Neil
> Thanks,
> Daniel
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox