This series implements support for the virtio device suspend feature
that is under discussion. Unfortunately, the virtio mailing list is
currently being migrated, so recent discussion of the proposal is not
archived anywhere. There current version of the proposal is a
combination of [1] and [2].
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lists.oasis-open.org/archives/virtio-comment/202402/msg00088.html
v1 -> v2:
- Check for device removal while waiting for suspend bit.
- Don't try to suspend uninitialized deivces.
- Use msleep instead of mdelay.
David Stevens (1):
virtio: Add support for the virtio suspend feature
drivers/virtio/virtio.c | 60 ++++++++++++++++++++++++++++++
drivers/virtio/virtio_pci_common.c | 34 ++++++++---------
drivers/virtio/virtio_pci_modern.c | 19 ++++++++++
include/linux/virtio.h | 8 ++++
include/uapi/linux/virtio_config.h | 10 ++++-
5 files changed, 112 insertions(+), 19 deletions(-)
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
--
2.44.0.769.g3c40516874-goog
Add support for the VIRTIO_F_SUSPEND feature. When this feature is
negotiated, power management can use it to suspend virtio devices
instead of resorting to resetting the devices entirely.
Signed-off-by: David Stevens <[email protected]>
---
drivers/virtio/virtio.c | 60 ++++++++++++++++++++++++++++++
drivers/virtio/virtio_pci_common.c | 34 ++++++++---------
drivers/virtio/virtio_pci_modern.c | 19 ++++++++++
include/linux/virtio.h | 8 ++++
include/uapi/linux/virtio_config.h | 10 ++++-
5 files changed, 112 insertions(+), 19 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index f4080692b351..c7685a0dc995 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/virtio.h>
+#include <linux/delay.h>
#include <linux/spinlock.h>
#include <linux/virtio_config.h>
#include <linux/virtio_anchor.h>
@@ -498,6 +499,13 @@ void unregister_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(unregister_virtio_device);
+void virtio_device_mark_removed(struct virtio_device *dev)
+{
+ /* Pairs with READ_ONCE() in virtio_device_set_suspend_bit(). */
+ WRITE_ONCE(dev->removed, true);
+}
+EXPORT_SYMBOL_GPL(virtio_device_mark_removed);
+
#ifdef CONFIG_PM_SLEEP
int virtio_device_freeze(struct virtio_device *dev)
{
@@ -580,6 +588,58 @@ int virtio_device_restore(struct virtio_device *dev)
return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_restore);
+
+static int virtio_device_set_suspend_bit(struct virtio_device *dev, bool enabled)
+{
+ u8 status, target;
+
+ status = dev->config->get_status(dev);
+ if (enabled)
+ target = status | VIRTIO_CONFIG_S_SUSPEND;
+ else
+ target = status & ~VIRTIO_CONFIG_S_SUSPEND;
+
+ if (target == status)
+ return 0;
+
+ dev->config->set_status(dev, target);
+
+ while ((status = dev->config->get_status(dev)) != target) {
+ if (status & VIRTIO_CONFIG_S_NEEDS_RESET)
+ return -EIO;
+ /* Pairs with WRITE_ONCE() in virtio_device_mark_removed(). */
+ if (READ_ONCE(dev->removed))
+ return -EIO;
+ msleep(10);
+ }
+ return 0;
+}
+
+bool virtio_device_can_suspend(struct virtio_device *dev)
+{
+ return virtio_has_feature(dev, VIRTIO_F_SUSPEND) &&
+ (dev->config->get_status(dev) & VIRTIO_CONFIG_S_FEATURES_OK);
+}
+EXPORT_SYMBOL_GPL(virtio_device_can_suspend);
+
+int virtio_device_suspend(struct virtio_device *dev)
+{
+ return virtio_device_set_suspend_bit(dev, true);
+}
+EXPORT_SYMBOL_GPL(virtio_device_suspend);
+
+bool virtio_device_can_resume(struct virtio_device *dev)
+{
+ return virtio_has_feature(dev, VIRTIO_F_SUSPEND) &&
+ (dev->config->get_status(dev) & VIRTIO_CONFIG_S_SUSPEND);
+}
+EXPORT_SYMBOL_GPL(virtio_device_can_resume);
+
+int virtio_device_resume(struct virtio_device *dev)
+{
+ return virtio_device_set_suspend_bit(dev, false);
+}
+EXPORT_SYMBOL_GPL(virtio_device_resume);
#endif
static int virtio_init(void)
diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index b655fccaf773..a6ca7718b5dc 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -495,31 +495,26 @@ static int virtio_pci_restore(struct device *dev)
return virtio_device_restore(&vp_dev->vdev);
}
-static bool vp_supports_pm_no_reset(struct device *dev)
+static int virtio_pci_suspend(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- u16 pmcsr;
-
- if (!pci_dev->pm_cap)
- return false;
-
- pci_read_config_word(pci_dev, pci_dev->pm_cap + PCI_PM_CTRL, &pmcsr);
- if (PCI_POSSIBLE_ERROR(pmcsr)) {
- dev_err(dev, "Unable to query pmcsr");
- return false;
- }
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
- return pmcsr & PCI_PM_CTRL_NO_SOFT_RESET;
-}
+ if (virtio_device_can_suspend(&vp_dev->vdev))
+ return virtio_device_suspend(&vp_dev->vdev);
-static int virtio_pci_suspend(struct device *dev)
-{
- return vp_supports_pm_no_reset(dev) ? 0 : virtio_pci_freeze(dev);
+ return virtio_pci_freeze(dev);
}
static int virtio_pci_resume(struct device *dev)
{
- return vp_supports_pm_no_reset(dev) ? 0 : virtio_pci_restore(dev);
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+
+ if (virtio_device_can_resume(&vp_dev->vdev))
+ return virtio_device_resume(&vp_dev->vdev);
+
+ return virtio_pci_restore(dev);
}
static const struct dev_pm_ops virtio_pci_pm_ops = {
@@ -623,9 +618,12 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
* Device is marked broken on surprise removal so that virtio upper
* layers can abort any ongoing operation.
*/
- if (!pci_device_is_present(pci_dev))
+ if (!pci_device_is_present(pci_dev)) {
virtio_break_device(&vp_dev->vdev);
+ virtio_device_mark_removed(&vp_dev->vdev);
+ }
+
pci_disable_sriov(pci_dev);
unregister_virtio_device(&vp_dev->vdev);
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index f62b530aa3b5..ac8734526b8d 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -209,6 +209,22 @@ static void vp_modern_avq_deactivate(struct virtio_device *vdev)
__virtqueue_break(admin_vq->info.vq);
}
+static bool vp_supports_pm_no_reset(struct pci_dev *pci_dev)
+{
+ u16 pmcsr;
+
+ if (!pci_dev->pm_cap)
+ return false;
+
+ pci_read_config_word(pci_dev, pci_dev->pm_cap + PCI_PM_CTRL, &pmcsr);
+ if (PCI_POSSIBLE_ERROR(pmcsr)) {
+ dev_err(&pci_dev->dev, "Unable to query pmcsr");
+ return false;
+ }
+
+ return pmcsr & PCI_PM_CTRL_NO_SOFT_RESET;
+}
+
static void vp_transport_features(struct virtio_device *vdev, u64 features)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -223,6 +239,9 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
if (features & BIT_ULL(VIRTIO_F_ADMIN_VQ))
__virtio_set_bit(vdev, VIRTIO_F_ADMIN_VQ);
+
+ if (features & BIT_ULL(VIRTIO_F_SUSPEND) && vp_supports_pm_no_reset(pci_dev))
+ __virtio_set_bit(vdev, VIRTIO_F_SUSPEND);
}
static int __vp_check_common_size_one_feature(struct virtio_device *vdev, u32 fbit,
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b0201747a263..2ca2ae290d4f 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -115,6 +115,7 @@ struct virtio_admin_cmd {
* struct virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
* @failed: saved value for VIRTIO_CONFIG_S_FAILED bit (for restore)
+ * @removed: whether or not the device was removed from under us
* @config_enabled: configuration change reporting enabled
* @config_change_pending: configuration change reported while disabled
* @config_lock: protects configuration change reporting
@@ -130,6 +131,7 @@ struct virtio_admin_cmd {
struct virtio_device {
int index;
bool failed;
+ bool removed;
bool config_enabled;
bool config_change_pending;
spinlock_t config_lock;
@@ -156,10 +158,16 @@ void __virtio_unbreak_device(struct virtio_device *dev);
void __virtqueue_break(struct virtqueue *_vq);
void __virtqueue_unbreak(struct virtqueue *_vq);
+void virtio_device_mark_removed(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);
+bool virtio_device_can_suspend(struct virtio_device *dev);
+bool virtio_device_can_resume(struct virtio_device *dev);
+int virtio_device_suspend(struct virtio_device *dev);
+int virtio_device_resume(struct virtio_device *dev);
#endif
void virtio_reset_device(struct virtio_device *dev);
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 2445f365bce7..4a6e2c28ea76 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -40,6 +40,8 @@
#define VIRTIO_CONFIG_S_DRIVER_OK 4
/* Driver has finished configuring features */
#define VIRTIO_CONFIG_S_FEATURES_OK 8
+/* Driver has suspended the device */
+#define VIRTIO_CONFIG_S_SUSPEND 0x10
/* Device entered invalid state, driver must reset it */
#define VIRTIO_CONFIG_S_NEEDS_RESET 0x40
/* We've given up on this device. */
@@ -52,7 +54,7 @@
* rest are per-device feature bits.
*/
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 42
+#define VIRTIO_TRANSPORT_F_END 43
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -120,4 +122,10 @@
*/
#define VIRTIO_F_ADMIN_VQ 41
+/*
+ * This feature indicates that the driver can suspend the device via the
+ * suspend bit in the device status byte.
+ */
+#define VIRTIO_F_SUSPEND 42
+
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
--
2.44.0.769.g3c40516874-goog