diff mbox series

[LINUX,KERNEL,v5,1/2] virtio_pci: Add freeze_mode for virtio_pci_common_cfg

Message ID 20230919104607.2282248-2-Jiqian.Chen@amd.com (mailing list archive)
State New, archived
Headers show
Series add freeze_mode for virtio_pci and add S3 support for virtgpu | expand

Commit Message

Chen, Jiqian Sept. 19, 2023, 10:46 a.m. UTC 
When guest vm does S3, Qemu will reset and clear some things of virtio
devices, but guest can't aware that, so that may cause some problems.
For excample, Qemu calls virtio_reset->virtio_gpu_gl_reset, that will
destroy render resources of virtio-gpu. As a result, after guest resume,
the display can't come back and we only saw a black screen. Due to guest
can't re-create all the resources, so we need to let Qemu not to destroy
them when S3.

For above purpose, this patch add a new parameter named freeze_mode to
struct virtio_pci_common_cfg, and when guest suspends, it can set
freeze_mode to be FREEZE_S3, so that virtio devices can change their
reset behavior on Qemu side according to that mode.

Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
 drivers/virtio/virtio.c                | 13 +++++++++++++
 drivers/virtio/virtio_pci_modern.c     |  9 +++++++++
 drivers/virtio/virtio_pci_modern_dev.c | 16 ++++++++++++++++
 include/linux/virtio_config.h          |  1 +
 include/linux/virtio_pci_modern.h      |  2 ++
 include/uapi/linux/virtio_pci.h        | 16 ++++++++++++++--
 6 files changed, 55 insertions(+), 2 deletions(-)

Comments

Michael S. Tsirkin Oct. 3, 2023, 1:20 p.m. UTC | #1 
On Tue, Sep 19, 2023 at 06:46:06PM +0800, Jiqian Chen wrote:
> When guest vm does S3, Qemu will reset and clear some things of virtio
> devices, but guest can't aware that, so that may cause some problems.
> For excample, Qemu calls virtio_reset->virtio_gpu_gl_reset, that will
> destroy render resources of virtio-gpu. As a result, after guest resume,
> the display can't come back and we only saw a black screen. Due to guest
> can't re-create all the resources, so we need to let Qemu not to destroy
> them when S3.
> 
> For above purpose, this patch add a new parameter named freeze_mode to
> struct virtio_pci_common_cfg, and when guest suspends, it can set
> freeze_mode to be FREEZE_S3, so that virtio devices can change their
> reset behavior on Qemu side according to that mode.
> 
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> ---
>  drivers/virtio/virtio.c                | 13 +++++++++++++
>  drivers/virtio/virtio_pci_modern.c     |  9 +++++++++
>  drivers/virtio/virtio_pci_modern_dev.c | 16 ++++++++++++++++
>  include/linux/virtio_config.h          |  1 +
>  include/linux/virtio_pci_modern.h      |  2 ++
>  include/uapi/linux/virtio_pci.h        | 16 ++++++++++++++--
>  6 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 3893dc29eb26..b4eb8369d5a1 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -7,6 +7,7 @@
>  #include <linux/idr.h>
>  #include <linux/of.h>
>  #include <uapi/linux/virtio_ids.h>
> +#include <uapi/linux/virtio_pci.h>
>  
>  /* Unique numbering for virtio devices. */
>  static DEFINE_IDA(virtio_index_ida);
> @@ -486,10 +487,20 @@ void unregister_virtio_device(struct virtio_device *dev)
>  EXPORT_SYMBOL_GPL(unregister_virtio_device);
>  
>  #ifdef CONFIG_PM_SLEEP
> +static void virtio_set_freeze_mode(struct virtio_device *dev, u16 mode)
> +{
> +	if (!dev->config->set_freeze_mode)
> +		return;
> +	might_sleep();
> +	dev->config->set_freeze_mode(dev, mode);
> +}
> +
>  int virtio_device_freeze(struct virtio_device *dev)
>  {
>  	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
>  
> +	virtio_set_freeze_mode(dev, VIRTIO_PCI_FREEZE_MODE_FREEZE_S3);
> +
>  	virtio_config_disable(dev);
>  
>  	dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
> @@ -544,6 +555,8 @@ int virtio_device_restore(struct virtio_device *dev)
>  
>  	virtio_config_enable(dev);
>  
> +	virtio_set_freeze_mode(dev, VIRTIO_PCI_FREEZE_MODE_UNFREEZE);
> +
>  	return 0;
>  
>  err:
> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index d6bb68ba84e5..846b70919cbd 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -491,6 +491,13 @@ static bool vp_get_shm_region(struct virtio_device *vdev,
>  	return true;
>  }
>  
> +static void vp_set_freeze_mode(struct virtio_device *vdev, u16 mode)
> +{
> +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> +
> +	vp_modern_set_freeze_mode(&vp_dev->mdev, mode);
> +}
> +
>  static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
>  	.get		= NULL,
>  	.set		= NULL,
> @@ -509,6 +516,7 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
>  	.get_shm_region  = vp_get_shm_region,
>  	.disable_vq_and_reset = vp_modern_disable_vq_and_reset,
>  	.enable_vq_after_reset = vp_modern_enable_vq_after_reset,
> +	.set_freeze_mode = vp_set_freeze_mode,
>  };
>  
>  static const struct virtio_config_ops virtio_pci_config_ops = {
> @@ -529,6 +537,7 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
>  	.get_shm_region  = vp_get_shm_region,
>  	.disable_vq_and_reset = vp_modern_disable_vq_and_reset,
>  	.enable_vq_after_reset = vp_modern_enable_vq_after_reset,
> +	.set_freeze_mode = vp_set_freeze_mode,
>  };
>  
>  /* the PCI probing function */
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index aad7d9296e77..4a6f7d130b6e 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -203,6 +203,8 @@ static inline void check_offsets(void)
>  		     offsetof(struct virtio_pci_common_cfg, queue_used_lo));
>  	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_USEDHI !=
>  		     offsetof(struct virtio_pci_common_cfg, queue_used_hi));
> +	BUILD_BUG_ON(VIRTIO_PCI_COMMON_F_MODE !=
> +		     offsetof(struct virtio_pci_common_cfg, freeze_mode));
>  }
>  
>  /*
> @@ -714,6 +716,20 @@ void __iomem *vp_modern_map_vq_notify(struct virtio_pci_modern_device *mdev,
>  }
>  EXPORT_SYMBOL_GPL(vp_modern_map_vq_notify);
>  
> +/*
> + * vp_modern_set_freeze_mode - set freeze mode to device
> + * @mdev: the modern virtio-pci device
> + * @mode: the mode set to device
> + */
> +void vp_modern_set_freeze_mode(struct virtio_pci_modern_device *mdev,
> +				 u16 mode)
> +{
> +	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> +
> +	vp_iowrite16(mode, &cfg->freeze_mode);
> +}
> +EXPORT_SYMBOL_GPL(vp_modern_set_freeze_mode);
> +
>  MODULE_VERSION("0.1");
>  MODULE_DESCRIPTION("Modern Virtio PCI Device");
>  MODULE_AUTHOR("Jason Wang <jasowang@redhat.com>");
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 2b3438de2c4d..2a7443ff7f12 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -120,6 +120,7 @@ struct virtio_config_ops {
>  			       struct virtio_shm_region *region, u8 id);
>  	int (*disable_vq_and_reset)(struct virtqueue *vq);
>  	int (*enable_vq_after_reset)(struct virtqueue *vq);
> +	void (*set_freeze_mode)(struct virtio_device *vdev, u16 mode);
>  };
>  
>  /* If driver didn't advertise the feature, it will never appear. */
> diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
> index 067ac1d789bc..ba6eed216ded 100644
> --- a/include/linux/virtio_pci_modern.h
> +++ b/include/linux/virtio_pci_modern.h
> @@ -121,4 +121,6 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev);
>  void vp_modern_remove(struct virtio_pci_modern_device *mdev);
>  int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16 index);
>  void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 index);
> +void vp_modern_set_freeze_mode(struct virtio_pci_modern_device *mdev,
> +		   u16 mode);
>  #endif
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index f703afc7ad31..725ace458a1b 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -140,6 +140,15 @@ struct virtio_pci_notify_cap {
>  	__le32 notify_off_multiplier;	/* Multiplier for queue_notify_off. */
>  };
>  
> +typedef enum {
> +       VIRTIO_PCI_FREEZE_MODE_UNFREEZE = 0,
> +       VIRTIO_PCI_FREEZE_MODE_FREEZE_S3 = 3,
> +} virtio_pci_freeze_mode_t;

we don't normally do typedefs.

> +
> +#define VIRTIO_PCI_FREEZE_MODE_MASK \
> +	((1 << VIRTIO_PCI_FREEZE_MODE_UNFREEZE) | \
> +	(1 << VIRTIO_PCI_FREEZE_MODE_FREEZE_S3))
> +

not sure why is this useful generally.

>  /* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */
>  struct virtio_pci_common_cfg {
>  	/* About the whole device. */
> @@ -164,6 +173,8 @@ struct virtio_pci_common_cfg {
>  	__le32 queue_avail_hi;		/* read-write */
>  	__le32 queue_used_lo;		/* read-write */
>  	__le32 queue_used_hi;		/* read-write */
> +
> +	__le16 freeze_mode;		/* read-write */
>  };
>

Your patch will likely break uses of sizeof(struct virtio_pci_common_cfg)
on existing devices.
  
>  /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> @@ -202,8 +213,9 @@ struct virtio_pci_cfg_cap {
>  #define VIRTIO_PCI_COMMON_Q_AVAILHI	44
>  #define VIRTIO_PCI_COMMON_Q_USEDLO	48
>  #define VIRTIO_PCI_COMMON_Q_USEDHI	52
> -#define VIRTIO_PCI_COMMON_Q_NDATA	56
> -#define VIRTIO_PCI_COMMON_Q_RESET	58
> +#define VIRTIO_PCI_COMMON_F_MODE	56


F_ here stands for freeze? Please don't abbreviate.
Q for queue is a pun that works, F for freeze doesn't.


> +#define VIRTIO_PCI_COMMON_Q_NDATA	58
> +#define VIRTIO_PCI_COMMON_Q_RESET	60
>  
>  #endif /* VIRTIO_PCI_NO_MODERN */
>  
> -- 
> 2.34.1
diff mbox series

Patch

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 3893dc29eb26..b4eb8369d5a1 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -7,6 +7,7 @@ 
 #include <linux/idr.h>
 #include <linux/of.h>
 #include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_pci.h>
 
 /* Unique numbering for virtio devices. */
 static DEFINE_IDA(virtio_index_ida);
@@ -486,10 +487,20 @@  void unregister_virtio_device(struct virtio_device *dev)
 EXPORT_SYMBOL_GPL(unregister_virtio_device);
 
 #ifdef CONFIG_PM_SLEEP
+static void virtio_set_freeze_mode(struct virtio_device *dev, u16 mode)
+{
+	if (!dev->config->set_freeze_mode)
+		return;
+	might_sleep();
+	dev->config->set_freeze_mode(dev, mode);
+}
+
 int virtio_device_freeze(struct virtio_device *dev)
 {
 	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 
+	virtio_set_freeze_mode(dev, VIRTIO_PCI_FREEZE_MODE_FREEZE_S3);
+
 	virtio_config_disable(dev);
 
 	dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
@@ -544,6 +555,8 @@  int virtio_device_restore(struct virtio_device *dev)
 
 	virtio_config_enable(dev);
 
+	virtio_set_freeze_mode(dev, VIRTIO_PCI_FREEZE_MODE_UNFREEZE);
+
 	return 0;
 
 err:
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index d6bb68ba84e5..846b70919cbd 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -491,6 +491,13 @@  static bool vp_get_shm_region(struct virtio_device *vdev,
 	return true;
 }
 
+static void vp_set_freeze_mode(struct virtio_device *vdev, u16 mode)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+	vp_modern_set_freeze_mode(&vp_dev->mdev, mode);
+}
+
 static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
 	.get		= NULL,
 	.set		= NULL,
@@ -509,6 +516,7 @@  static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
 	.get_shm_region  = vp_get_shm_region,
 	.disable_vq_and_reset = vp_modern_disable_vq_and_reset,
 	.enable_vq_after_reset = vp_modern_enable_vq_after_reset,
+	.set_freeze_mode = vp_set_freeze_mode,
 };
 
 static const struct virtio_config_ops virtio_pci_config_ops = {
@@ -529,6 +537,7 @@  static const struct virtio_config_ops virtio_pci_config_ops = {
 	.get_shm_region  = vp_get_shm_region,
 	.disable_vq_and_reset = vp_modern_disable_vq_and_reset,
 	.enable_vq_after_reset = vp_modern_enable_vq_after_reset,
+	.set_freeze_mode = vp_set_freeze_mode,
 };
 
 /* the PCI probing function */
diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index aad7d9296e77..4a6f7d130b6e 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -203,6 +203,8 @@  static inline void check_offsets(void)
 		     offsetof(struct virtio_pci_common_cfg, queue_used_lo));
 	BUILD_BUG_ON(VIRTIO_PCI_COMMON_Q_USEDHI !=
 		     offsetof(struct virtio_pci_common_cfg, queue_used_hi));
+	BUILD_BUG_ON(VIRTIO_PCI_COMMON_F_MODE !=
+		     offsetof(struct virtio_pci_common_cfg, freeze_mode));
 }
 
 /*
@@ -714,6 +716,20 @@  void __iomem *vp_modern_map_vq_notify(struct virtio_pci_modern_device *mdev,
 }
 EXPORT_SYMBOL_GPL(vp_modern_map_vq_notify);
 
+/*
+ * vp_modern_set_freeze_mode - set freeze mode to device
+ * @mdev: the modern virtio-pci device
+ * @mode: the mode set to device
+ */
+void vp_modern_set_freeze_mode(struct virtio_pci_modern_device *mdev,
+				 u16 mode)
+{
+	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+
+	vp_iowrite16(mode, &cfg->freeze_mode);
+}
+EXPORT_SYMBOL_GPL(vp_modern_set_freeze_mode);
+
 MODULE_VERSION("0.1");
 MODULE_DESCRIPTION("Modern Virtio PCI Device");
 MODULE_AUTHOR("Jason Wang <jasowang@redhat.com>");
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 2b3438de2c4d..2a7443ff7f12 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -120,6 +120,7 @@  struct virtio_config_ops {
 			       struct virtio_shm_region *region, u8 id);
 	int (*disable_vq_and_reset)(struct virtqueue *vq);
 	int (*enable_vq_after_reset)(struct virtqueue *vq);
+	void (*set_freeze_mode)(struct virtio_device *vdev, u16 mode);
 };
 
 /* If driver didn't advertise the feature, it will never appear. */
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
index 067ac1d789bc..ba6eed216ded 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -121,4 +121,6 @@  int vp_modern_probe(struct virtio_pci_modern_device *mdev);
 void vp_modern_remove(struct virtio_pci_modern_device *mdev);
 int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16 index);
 void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 index);
+void vp_modern_set_freeze_mode(struct virtio_pci_modern_device *mdev,
+		   u16 mode);
 #endif
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index f703afc7ad31..725ace458a1b 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -140,6 +140,15 @@  struct virtio_pci_notify_cap {
 	__le32 notify_off_multiplier;	/* Multiplier for queue_notify_off. */
 };
 
+typedef enum {
+       VIRTIO_PCI_FREEZE_MODE_UNFREEZE = 0,
+       VIRTIO_PCI_FREEZE_MODE_FREEZE_S3 = 3,
+} virtio_pci_freeze_mode_t;
+
+#define VIRTIO_PCI_FREEZE_MODE_MASK \
+	((1 << VIRTIO_PCI_FREEZE_MODE_UNFREEZE) | \
+	(1 << VIRTIO_PCI_FREEZE_MODE_FREEZE_S3))
+
 /* Fields in VIRTIO_PCI_CAP_COMMON_CFG: */
 struct virtio_pci_common_cfg {
 	/* About the whole device. */
@@ -164,6 +173,8 @@  struct virtio_pci_common_cfg {
 	__le32 queue_avail_hi;		/* read-write */
 	__le32 queue_used_lo;		/* read-write */
 	__le32 queue_used_hi;		/* read-write */
+
+	__le16 freeze_mode;		/* read-write */
 };
 
 /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
@@ -202,8 +213,9 @@  struct virtio_pci_cfg_cap {
 #define VIRTIO_PCI_COMMON_Q_AVAILHI	44
 #define VIRTIO_PCI_COMMON_Q_USEDLO	48
 #define VIRTIO_PCI_COMMON_Q_USEDHI	52
-#define VIRTIO_PCI_COMMON_Q_NDATA	56
-#define VIRTIO_PCI_COMMON_Q_RESET	58
+#define VIRTIO_PCI_COMMON_F_MODE	56
+#define VIRTIO_PCI_COMMON_Q_NDATA	58
+#define VIRTIO_PCI_COMMON_Q_RESET	60
 
 #endif /* VIRTIO_PCI_NO_MODERN */