virtio-crypto: handle config changed by work queue

Message ID 20231007064309.844889-1-pizhenwei@bytedance.com
State New
Headers
Series virtio-crypto: handle config changed by work queue |

Commit Message

zhenwei pi Oct. 7, 2023, 6:43 a.m. UTC
  MST pointed out: config change callback is also handled incorrectly
in this driver, it takes a mutex from interrupt context.

Handle config changed by work queue instead.

Cc: Gonglei (Arei) <arei.gonglei@huawei.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 drivers/crypto/virtio/virtio_crypto_common.h |  3 +++
 drivers/crypto/virtio/virtio_crypto_core.c   | 14 +++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)
  

Comments

Herbert Xu Oct. 20, 2023, 5:50 a.m. UTC | #1
On Sat, Oct 07, 2023 at 02:43:09PM +0800, zhenwei pi wrote:
> MST pointed out: config change callback is also handled incorrectly
> in this driver, it takes a mutex from interrupt context.
> 
> Handle config changed by work queue instead.
> 
> Cc: Gonglei (Arei) <arei.gonglei@huawei.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> ---
>  drivers/crypto/virtio/virtio_crypto_common.h |  3 +++
>  drivers/crypto/virtio/virtio_crypto_core.c   | 14 +++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)

Patch applied.  Thanks.
  
Michael S. Tsirkin Oct. 20, 2023, 8:59 a.m. UTC | #2
On Fri, Oct 20, 2023 at 01:50:05PM +0800, Herbert Xu wrote:
> On Sat, Oct 07, 2023 at 02:43:09PM +0800, zhenwei pi wrote:
> > MST pointed out: config change callback is also handled incorrectly
> > in this driver, it takes a mutex from interrupt context.
> > 
> > Handle config changed by work queue instead.
> > 
> > Cc: Gonglei (Arei) <arei.gonglei@huawei.com>
> > Cc: Halil Pasic <pasic@linux.ibm.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> > ---
> >  drivers/crypto/virtio/virtio_crypto_common.h |  3 +++
> >  drivers/crypto/virtio/virtio_crypto_core.c   | 14 +++++++++++++-
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> Patch applied.  Thanks.


It's in my tree too, but git should be able to figure it out.

> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
  

Patch

diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
index 59a4c0259456..154590e1f764 100644
--- a/drivers/crypto/virtio/virtio_crypto_common.h
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -35,6 +35,9 @@  struct virtio_crypto {
 	struct virtqueue *ctrl_vq;
 	struct data_queue *data_vq;
 
+	/* Work struct for config space updates */
+	struct work_struct config_work;
+
 	/* To protect the vq operations for the controlq */
 	spinlock_t ctrl_lock;
 
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
index 94849fa3bd74..43a0838d31ff 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -335,6 +335,14 @@  static void virtcrypto_del_vqs(struct virtio_crypto *vcrypto)
 	virtcrypto_free_queues(vcrypto);
 }
 
+static void vcrypto_config_changed_work(struct work_struct *work)
+{
+	struct virtio_crypto *vcrypto =
+		container_of(work, struct virtio_crypto, config_work);
+
+	virtcrypto_update_status(vcrypto);
+}
+
 static int virtcrypto_probe(struct virtio_device *vdev)
 {
 	int err = -EFAULT;
@@ -454,6 +462,8 @@  static int virtcrypto_probe(struct virtio_device *vdev)
 	if (err)
 		goto free_engines;
 
+	INIT_WORK(&vcrypto->config_work, vcrypto_config_changed_work);
+
 	return 0;
 
 free_engines:
@@ -490,6 +500,7 @@  static void virtcrypto_remove(struct virtio_device *vdev)
 
 	dev_info(&vdev->dev, "Start virtcrypto_remove.\n");
 
+	flush_work(&vcrypto->config_work);
 	if (virtcrypto_dev_started(vcrypto))
 		virtcrypto_dev_stop(vcrypto);
 	virtio_reset_device(vdev);
@@ -504,7 +515,7 @@  static void virtcrypto_config_changed(struct virtio_device *vdev)
 {
 	struct virtio_crypto *vcrypto = vdev->priv;
 
-	virtcrypto_update_status(vcrypto);
+	schedule_work(&vcrypto->config_work);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -512,6 +523,7 @@  static int virtcrypto_freeze(struct virtio_device *vdev)
 {
 	struct virtio_crypto *vcrypto = vdev->priv;
 
+	flush_work(&vcrypto->config_work);
 	virtio_reset_device(vdev);
 	virtcrypto_free_unused_reqs(vcrypto);
 	if (virtcrypto_dev_started(vcrypto))