All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] virtio: Add suspend support
@ 2024-04-17  8:54 David Stevens
  2024-04-17  8:54 ` [PATCH 1/1] virtio: Add support for the virtio suspend feature David Stevens
  0 siblings, 1 reply; 4+ messages in thread
From: David Stevens @ 2024-04-17  8:54 UTC (permalink / raw)
  To: Michael S . Tsirkin, Jason Wang, Cornelia Huck, Stefan Hajnoczi
  Cc: Eugenio Perez, Zhu Lingshan, virtio-dev, linux-kernel,
	virtualization, David Stevens

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/20230906081637.32185-3-lingshan.zhu@intel.com/
[2] https://lists.oasis-open.org/archives/virtio-comment/202402/msg00088.html

David Stevens (1):
  virtio: Add support for the virtio suspend feature

 drivers/virtio/virtio.c            | 32 ++++++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c | 29 +++++++++++----------------
 drivers/virtio/virtio_pci_modern.c | 19 ++++++++++++++++++
 include/linux/virtio.h             |  2 ++
 include/uapi/linux/virtio_config.h | 10 +++++++++-
 5 files changed, 74 insertions(+), 18 deletions(-)


base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
-- 
2.44.0.683.g7961c838ac-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH 1/1] virtio: Add support for the virtio suspend feature
  2024-04-17  8:54 [PATCH 0/1] virtio: Add suspend support David Stevens
@ 2024-04-17  8:54 ` David Stevens
  2024-04-18  7:14   ` Zhu, Lingshan
  0 siblings, 1 reply; 4+ messages in thread
From: David Stevens @ 2024-04-17  8:54 UTC (permalink / raw)
  To: Michael S . Tsirkin, Jason Wang, Cornelia Huck, Stefan Hajnoczi
  Cc: Eugenio Perez, Zhu Lingshan, virtio-dev, linux-kernel,
	virtualization, David Stevens

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 <stevensd@chromium.org>
---
 drivers/virtio/virtio.c            | 32 ++++++++++++++++++++++++++++++
 drivers/virtio/virtio_pci_common.c | 29 +++++++++++----------------
 drivers/virtio/virtio_pci_modern.c | 19 ++++++++++++++++++
 include/linux/virtio.h             |  2 ++
 include/uapi/linux/virtio_config.h | 10 +++++++++-
 5 files changed, 74 insertions(+), 18 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index f4080692b351..cd11495a5098 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>
@@ -580,6 +581,37 @@ 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;
+	dev->config->set_status(dev, target);
+
+	while ((status = dev->config->get_status(dev)) != target) {
+		if (status & VIRTIO_CONFIG_S_NEEDS_RESET)
+			return -EIO;
+		mdelay(10);
+	}
+	return 0;
+}
+
+int virtio_device_suspend(struct virtio_device *dev)
+{
+	return virtio_device_set_suspend_bit(dev, true);
+}
+EXPORT_SYMBOL_GPL(virtio_device_suspend);
+
+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..4d542de05970 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_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND))
+		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_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND))
+		return virtio_device_resume(&vp_dev->vdev);
+
+	return virtio_pci_restore(dev);
 }
 
 static const struct dev_pm_ops virtio_pci_pm_ops = {
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..8e456b04114e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -160,6 +160,8 @@ 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);
+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.683.g7961c838ac-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] virtio: Add support for the virtio suspend feature
  2024-04-17  8:54 ` [PATCH 1/1] virtio: Add support for the virtio suspend feature David Stevens
@ 2024-04-18  7:14   ` Zhu, Lingshan
  2024-04-18  7:34     ` Michael S. Tsirkin
  0 siblings, 1 reply; 4+ messages in thread
From: Zhu, Lingshan @ 2024-04-18  7:14 UTC (permalink / raw)
  To: David Stevens, Michael S . Tsirkin, Jason Wang, Cornelia Huck,
	Stefan Hajnoczi
  Cc: Eugenio Perez, virtio-dev, linux-kernel, virtualization



On 4/17/2024 4:54 PM, David Stevens wrote:
> 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 <stevensd@chromium.org>
> ---
>   drivers/virtio/virtio.c            | 32 ++++++++++++++++++++++++++++++
>   drivers/virtio/virtio_pci_common.c | 29 +++++++++++----------------
>   drivers/virtio/virtio_pci_modern.c | 19 ++++++++++++++++++
>   include/linux/virtio.h             |  2 ++
>   include/uapi/linux/virtio_config.h | 10 +++++++++-
>   5 files changed, 74 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index f4080692b351..cd11495a5098 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>
> @@ -580,6 +581,37 @@ 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;
> +	dev->config->set_status(dev, target);
I think it is better to verify whether the device SUSPEND bit is
already set or clear, we can just return if status == target.

Thanks
Zhu Lingshan
> +
> +	while ((status = dev->config->get_status(dev)) != target) {
> +		if (status & VIRTIO_CONFIG_S_NEEDS_RESET)
> +			return -EIO;
> +		mdelay(10);
> +	}
> +	return 0;
> +}
> +
> +int virtio_device_suspend(struct virtio_device *dev)
> +{
> +	return virtio_device_set_suspend_bit(dev, true);
> +}
> +EXPORT_SYMBOL_GPL(virtio_device_suspend);
> +
> +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..4d542de05970 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_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND))
> +		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_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND))
> +		return virtio_device_resume(&vp_dev->vdev);
> +
> +	return virtio_pci_restore(dev);
>   }
>   
>   static const struct dev_pm_ops virtio_pci_pm_ops = {
> 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..8e456b04114e 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -160,6 +160,8 @@ 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);
> +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 */


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] virtio: Add support for the virtio suspend feature
  2024-04-18  7:14   ` Zhu, Lingshan
@ 2024-04-18  7:34     ` Michael S. Tsirkin
  0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2024-04-18  7:34 UTC (permalink / raw)
  To: Zhu, Lingshan
  Cc: David Stevens, Jason Wang, Cornelia Huck, Stefan Hajnoczi,
	Eugenio Perez, virtio-dev, linux-kernel, virtualization

On Thu, Apr 18, 2024 at 03:14:37PM +0800, Zhu, Lingshan wrote:
> 
> 
> On 4/17/2024 4:54 PM, David Stevens wrote:
> > 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 <stevensd@chromium.org>
> > ---
> >   drivers/virtio/virtio.c            | 32 ++++++++++++++++++++++++++++++
> >   drivers/virtio/virtio_pci_common.c | 29 +++++++++++----------------
> >   drivers/virtio/virtio_pci_modern.c | 19 ++++++++++++++++++
> >   include/linux/virtio.h             |  2 ++
> >   include/uapi/linux/virtio_config.h | 10 +++++++++-
> >   5 files changed, 74 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> > index f4080692b351..cd11495a5098 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>
> > @@ -580,6 +581,37 @@ 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;
> > +	dev->config->set_status(dev, target);
> I think it is better to verify whether the device SUSPEND bit is
> already set or clear, we can just return if status == target.
> 
> Thanks
> Zhu Lingshan
> > +
> > +	while ((status = dev->config->get_status(dev)) != target) {
> > +		if (status & VIRTIO_CONFIG_S_NEEDS_RESET)
> > +			return -EIO;
> > +		mdelay(10);

Bad device state (set by surprise removal) should also
be handled here I think.


> > +	}
> > +	return 0;
> > +}
> > +
> > +int virtio_device_suspend(struct virtio_device *dev)
> > +{
> > +	return virtio_device_set_suspend_bit(dev, true);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_device_suspend);
> > +
> > +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..4d542de05970 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_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND))
> > +		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_has_feature(&vp_dev->vdev, VIRTIO_F_SUSPEND))
> > +		return virtio_device_resume(&vp_dev->vdev);
> > +
> > +	return virtio_pci_restore(dev);
> >   }
> >   static const struct dev_pm_ops virtio_pci_pm_ops = {
> > 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..8e456b04114e 100644
> > --- a/include/linux/virtio.h
> > +++ b/include/linux/virtio.h
> > @@ -160,6 +160,8 @@ 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);
> > +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 */


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-18  7:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17  8:54 [PATCH 0/1] virtio: Add suspend support David Stevens
2024-04-17  8:54 ` [PATCH 1/1] virtio: Add support for the virtio suspend feature David Stevens
2024-04-18  7:14   ` Zhu, Lingshan
2024-04-18  7:34     ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.