diff mbox series

virtio: only reset device and restore status if needed in device resume

Message ID 20241031030847.3253873-1-qiang4.zhang@linux.intel.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series virtio: only reset device and restore status if needed in device resume | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Qiang Zhang Oct. 31, 2024, 3:08 a.m. UTC
From: Qiang Zhang <qiang4.zhang@intel.com>

Virtio core unconditionally reset and restore status for all virtio
devices before calling restore method. This breaks some virtio drivers
which don't need to do anything in suspend and resume because they
just want to keep device state retained.

Virtio GPIO is a typical example. GPIO states should be kept unchanged
after suspend and resume (e.g. output pins keep driving the output) and
Virtio GPIO driver does nothing in freeze and restore methods. But the
reset operation in virtio_device_restore breaks this.

Since some devices need reset in suspend and resume while some needn't,
create a new helper function for the original reset and status restore
logic so that virtio drivers can invoke it in their restore method
if necessary.

Signed-off-by: Qiang Zhang <qiang4.zhang@intel.com>
---
 drivers/block/virtio_blk.c                 |  4 +++
 drivers/char/hw_random/virtio-rng.c        |  4 +++
 drivers/char/virtio_console.c              |  4 +++
 drivers/crypto/virtio/virtio_crypto_core.c |  4 +++
 drivers/i2c/busses/i2c-virtio.c            |  6 ++++
 drivers/net/virtio_net.c                   |  4 +++
 drivers/scsi/virtio_scsi.c                 |  4 +++
 drivers/virtio/virtio.c                    | 35 ++++++++++++++--------
 drivers/virtio/virtio_balloon.c            |  4 +++
 drivers/virtio/virtio_input.c              |  4 +++
 drivers/virtio/virtio_mem.c                |  4 +++
 include/linux/virtio.h                     |  1 +
 sound/virtio/virtio_card.c                 |  4 +++
 13 files changed, 70 insertions(+), 12 deletions(-)

Comments

kernel test robot Oct. 31, 2024, 2:01 p.m. UTC | #1
Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus mst-vhost/linux-next axboe-block/for-next herbert-cryptodev-2.6/master andi-shyti/i2c/i2c-host mkp-scsi/for-next jejb-scsi/for-next tiwai-sound/for-next tiwai-sound/for-linus linus/master v6.12-rc5 next-20241031]
[cannot apply to herbert-crypto-2.6/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/qiang4-zhang-linux-intel-com/virtio-only-reset-device-and-restore-status-if-needed-in-device-resume/20241031-111315
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241031030847.3253873-1-qiang4.zhang%40linux.intel.com
patch subject: [PATCH] virtio: only reset device and restore status if needed in device resume
config: arc-randconfig-002-20241031 (https://download.01.org/0day-ci/archive/20241031/202410312148.w44ttwk7-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241031/202410312148.w44ttwk7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410312148.w44ttwk7-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/char/hw_random/virtio-rng.c: In function 'virtrng_restore':
>> drivers/char/hw_random/virtio-rng.c:221:15: error: implicit declaration of function 'virtio_device_reset_and_restore_status' [-Werror=implicit-function-declaration]
     221 |         err = virtio_device_reset_and_restore_status(vdev);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors
--
   drivers/i2c/busses/i2c-virtio.c: In function 'virtio_i2c_restore':
>> drivers/i2c/busses/i2c-virtio.c:256:15: error: implicit declaration of function 'virtio_device_reset_and_restore_status' [-Werror=implicit-function-declaration]
     256 |         ret = virtio_device_reset_and_restore_status(vdev);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [y]:
   - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]


vim +/virtio_device_reset_and_restore_status +221 drivers/char/hw_random/virtio-rng.c

   216	
   217	static int virtrng_restore(struct virtio_device *vdev)
   218	{
   219		int err;
   220	
 > 221		err = virtio_device_reset_and_restore_status(vdev);
   222		if (err)
   223			return err;
   224	
   225		err = probe_common(vdev);
   226		if (!err) {
   227			struct virtrng_info *vi = vdev->priv;
   228	
   229			/*
   230			 * Set hwrng_removed to ensure that virtio_read()
   231			 * does not block waiting for data before the
   232			 * registration is complete.
   233			 */
   234			vi->hwrng_removed = true;
   235			err = hwrng_register(&vi->hwrng);
   236			if (!err) {
   237				vi->hwrng_register_done = true;
   238				vi->hwrng_removed = false;
   239			}
   240		}
   241	
   242		return err;
   243	}
   244
kernel test robot Oct. 31, 2024, 2:12 p.m. UTC | #2
Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus mst-vhost/linux-next axboe-block/for-next herbert-cryptodev-2.6/master andi-shyti/i2c/i2c-host mkp-scsi/for-next jejb-scsi/for-next tiwai-sound/for-next tiwai-sound/for-linus linus/master v6.12-rc5 next-20241031]
[cannot apply to herbert-crypto-2.6/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/qiang4-zhang-linux-intel-com/virtio-only-reset-device-and-restore-status-if-needed-in-device-resume/20241031-111315
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20241031030847.3253873-1-qiang4.zhang%40linux.intel.com
patch subject: [PATCH] virtio: only reset device and restore status if needed in device resume
config: x86_64-buildonly-randconfig-003-20241031 (https://download.01.org/0day-ci/archive/20241031/202410312128.7ymyjL4j-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241031/202410312128.7ymyjL4j-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410312128.7ymyjL4j-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/i2c/busses/i2c-virtio.c:14:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/i2c/busses/i2c-virtio.c:256:8: error: call to undeclared function 'virtio_device_reset_and_restore_status'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     256 |         ret = virtio_device_reset_and_restore_status(vdev);
         |               ^
   1 warning and 1 error generated.


vim +/virtio_device_reset_and_restore_status +256 drivers/i2c/busses/i2c-virtio.c

   251	
   252	static int virtio_i2c_restore(struct virtio_device *vdev)
   253	{
   254		int ret;
   255	
 > 256		ret = virtio_device_reset_and_restore_status(vdev);
   257		if (ret)
   258			return ret;
   259	
   260		return virtio_i2c_setup_vqs(vdev->priv);
   261	}
   262
diff mbox series

Patch

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 194417abc105..bba7148bd219 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1612,6 +1612,10 @@  static int virtblk_restore(struct virtio_device *vdev)
 	struct virtio_blk *vblk = vdev->priv;
 	int ret;
 
+	ret = virtio_device_reset_and_restore_status(vdev);
+	if (ret)
+		return ret;
+
 	ret = init_vq(vdev->priv);
 	if (ret)
 		return ret;
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index dd998f4fe4f2..954280514f5a 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -218,6 +218,10 @@  static int virtrng_restore(struct virtio_device *vdev)
 {
 	int err;
 
+	err = virtio_device_reset_and_restore_status(vdev);
+	if (err)
+		return err;
+
 	err = probe_common(vdev);
 	if (!err) {
 		struct virtrng_info *vi = vdev->priv;
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c62b208b42f1..c431e4d5cd29 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2132,6 +2132,10 @@  static int virtcons_restore(struct virtio_device *vdev)
 
 	portdev = vdev->priv;
 
+	ret = virtio_device_reset_and_restore_status(vdev);
+	if (ret)
+		return ret;
+
 	ret = init_vqs(portdev);
 	if (ret)
 		return ret;
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index d0278eb568b9..b3363dd80448 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -536,6 +536,10 @@  static int virtcrypto_restore(struct virtio_device *vdev)
 	struct virtio_crypto *vcrypto = vdev->priv;
 	int err;
 
+	err = virtio_device_reset_and_restore_status(vdev);
+	if (err)
+		return err;
+
 	err = virtcrypto_init_vqs(vcrypto);
 	if (err)
 		return err;
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
index 2a351f961b89..ce6493d6fe8e 100644
--- a/drivers/i2c/busses/i2c-virtio.c
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -251,6 +251,12 @@  static int virtio_i2c_freeze(struct virtio_device *vdev)
 
 static int virtio_i2c_restore(struct virtio_device *vdev)
 {
+	int ret;
+
+	ret = virtio_device_reset_and_restore_status(vdev);
+	if (ret)
+		return ret;
+
 	return virtio_i2c_setup_vqs(vdev->priv);
 }
 
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 792e9eadbfc3..5be2a5f68f68 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -6693,6 +6693,10 @@  static __maybe_unused int virtnet_restore(struct virtio_device *vdev)
 	struct virtnet_info *vi = vdev->priv;
 	int err;
 
+	err = virtio_device_reset_and_restore_status(vdev);
+	if (err)
+		return err;
+
 	err = virtnet_restore_up(vdev);
 	if (err)
 		return err;
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 8471f38b730e..3aeddf6331d3 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -1016,6 +1016,10 @@  static int virtscsi_restore(struct virtio_device *vdev)
 	struct virtio_scsi *vscsi = shost_priv(sh);
 	int err;
 
+	err = virtio_device_reset_and_restore_status(vdev);
+	if (err)
+		return err;
+
 	err = virtscsi_init(vdev, vscsi);
 	if (err)
 		return err;
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index b9095751e43b..0446b30c83d6 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -549,7 +549,7 @@  int virtio_device_freeze(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(virtio_device_freeze);
 
-int virtio_device_restore(struct virtio_device *dev)
+int virtio_device_reset_and_restore_status(struct virtio_device *dev)
 {
 	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 	int ret;
@@ -574,23 +574,34 @@  int virtio_device_restore(struct virtio_device *dev)
 
 	ret = dev->config->finalize_features(dev);
 	if (ret)
-		goto err;
+		return ret;
 
 	ret = virtio_features_ok(dev);
 	if (ret)
-		goto err;
+		return ret;
 
-	if (drv->restore) {
-		ret = drv->restore(dev);
-		if (ret)
-			goto err;
-	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_device_reset_and_restore_status);
 
-	/* If restore didn't do it, mark device DRIVER_OK ourselves. */
-	if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
-		virtio_device_ready(dev);
+int virtio_device_restore(struct virtio_device *dev)
+{
+	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+	int ret;
 
-	virtio_config_core_enable(dev);
+	if (drv) {
+		if (drv->restore) {
+			ret = drv->restore(dev);
+			if (ret)
+				goto err;
+		}
+
+		/* If restore didn't do it, mark device DRIVER_OK ourselves. */
+		if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
+			virtio_device_ready(dev);
+
+		virtio_config_core_enable(dev);
+	}
 
 	return 0;
 
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index b36d2803674e..ba92b3e56391 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1163,6 +1163,10 @@  static int virtballoon_restore(struct virtio_device *vdev)
 	struct virtio_balloon *vb = vdev->priv;
 	int ret;
 
+	ret = virtio_device_reset_and_restore_status(vdev);
+	if (ret)
+		return ret;
+
 	ret = init_vqs(vdev->priv);
 	if (ret)
 		return ret;
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index a5d63269f20b..81c2ffd0e76e 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -374,6 +374,10 @@  static int virtinput_restore(struct virtio_device *vdev)
 	struct virtio_input *vi = vdev->priv;
 	int err;
 
+	err = virtio_device_reset_and_restore_status(vdev);
+	if (err)
+		return err;
+
 	err = virtinput_init_vqs(vi);
 	if (err)
 		return err;
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index b0b871441578..47c23aa43c20 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -3018,6 +3018,10 @@  static int virtio_mem_restore(struct virtio_device *vdev)
 	struct virtio_mem *vm = vdev->priv;
 	int ret;
 
+	ret = virtio_device_reset_and_restore_status(vdev);
+	if (ret)
+		return ret;
+
 	ret = virtio_mem_init_vq(vm);
 	if (ret)
 		return ret;
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 306137a15d07..ab850146f0a8 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -175,6 +175,7 @@  void virtio_config_driver_enable(struct virtio_device *dev);
 
 #ifdef CONFIG_PM_SLEEP
 int virtio_device_freeze(struct virtio_device *dev);
+int virtio_device_reset_and_restore_status(struct virtio_device *dev);
 int virtio_device_restore(struct virtio_device *dev);
 #endif
 void virtio_reset_device(struct virtio_device *dev);
diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
index 965209e1d872..3c7a6d76c46c 100644
--- a/sound/virtio/virtio_card.c
+++ b/sound/virtio/virtio_card.c
@@ -412,6 +412,10 @@  static int virtsnd_restore(struct virtio_device *vdev)
 	struct virtio_snd *snd = vdev->priv;
 	int rc;
 
+	rc = virtio_device_reset_and_restore_status(vdev);
+	if (rc)
+		return rc;
+
 	rc = virtsnd_find_vqs(snd);
 	if (rc)
 		return rc;