diff mbox series

[net-next,v3,2/3] virtio: allow driver to disable the configure change notification

Message ID 20240709080214.9790-3-jasowang@redhat.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series virtio-net: synchronize op/admin state | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 818 this patch: 818
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 823 this patch: 823
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 843 this patch: 843
netdev/checkpatch warning CHECK: braces {} should be used on all arms of this statement
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-07-11--18-00 (tests: 696)

Commit Message

Jason Wang July 9, 2024, 8:02 a.m. UTC
Sometime, it would be useful to disable the configure change
notification from the driver. So this patch allows this by introducing
a variable config_change_driver_disabled and only allow the configure
change notification callback to be triggered when it is allowed by
both the virtio core and the driver. It is set to false by default to
hold the current semantic so we don't need to change any drivers.

The first user for this would be virtio-net.

Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Cc: Gia-Khanh Nguyen <gia-khanh.nguyen@oracle.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/virtio/virtio.c | 39 ++++++++++++++++++++++++++++++++++++---
 include/linux/virtio.h  |  7 +++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

Comments

Xuan Zhuo July 9, 2024, 8:13 a.m. UTC | #1
On Tue,  9 Jul 2024 16:02:13 +0800, Jason Wang <jasowang@redhat.com> wrote:
> Sometime, it would be useful to disable the configure change
> notification from the driver. So this patch allows this by introducing
> a variable config_change_driver_disabled and only allow the configure
> change notification callback to be triggered when it is allowed by
> both the virtio core and the driver. It is set to false by default to
> hold the current semantic so we don't need to change any drivers.
>
> The first user for this would be virtio-net.
>
> Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
> Cc: Gia-Khanh Nguyen <gia-khanh.nguyen@oracle.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>



> ---
>  drivers/virtio/virtio.c | 39 ++++++++++++++++++++++++++++++++++++---
>  include/linux/virtio.h  |  7 +++++++
>  2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 73bab89b5326..23a96fbc2810 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -127,10 +127,12 @@ static void __virtio_config_changed(struct virtio_device *dev)
>  {
>  	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
>
> -	if (!dev->config_core_enabled)
> +	if (!dev->config_core_enabled || dev->config_driver_disabled)
>  		dev->config_change_pending = true;
> -	else if (drv && drv->config_changed)
> +	else if (drv && drv->config_changed) {
>  		drv->config_changed(dev);
> +		dev->config_change_pending = false;
> +	}
>  }
>
>  void virtio_config_changed(struct virtio_device *dev)
> @@ -143,6 +145,38 @@ void virtio_config_changed(struct virtio_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(virtio_config_changed);
>
> +/**
> + * virtio_config_driver_disable - disable config change reporting by drivers
> + * @dev: the device to reset
> + *
> + * This is only allowed to be called by a driver and disalbing can't
> + * be nested.
> + */
> +void virtio_config_driver_disable(struct virtio_device *dev)
> +{
> +	spin_lock_irq(&dev->config_lock);
> +	dev->config_driver_disabled = true;
> +	spin_unlock_irq(&dev->config_lock);
> +}
> +EXPORT_SYMBOL_GPL(virtio_config_driver_disable);
> +
> +/**
> + * virtio_config_driver_enable - enable config change reporting by drivers
> + * @dev: the device to reset
> + *
> + * This is only allowed to be called by a driver and enabling can't
> + * be nested.
> + */
> +void virtio_config_driver_enable(struct virtio_device *dev)
> +{
> +	spin_lock_irq(&dev->config_lock);
> +	dev->config_driver_disabled = false;
> +	if (dev->config_change_pending)
> +		__virtio_config_changed(dev);
> +	spin_unlock_irq(&dev->config_lock);
> +}
> +EXPORT_SYMBOL_GPL(virtio_config_driver_enable);
> +
>  static void virtio_config_core_disable(struct virtio_device *dev)
>  {
>  	spin_lock_irq(&dev->config_lock);
> @@ -156,7 +190,6 @@ static void virtio_config_core_enable(struct virtio_device *dev)
>  	dev->config_core_enabled = true;
>  	if (dev->config_change_pending)
>  		__virtio_config_changed(dev);
> -	dev->config_change_pending = false;
>  	spin_unlock_irq(&dev->config_lock);
>  }
>
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index a6f6df72f01a..2be73de6f1f3 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -116,6 +116,8 @@ struct virtio_admin_cmd {
>   * @index: unique position on the virtio bus
>   * @failed: saved value for VIRTIO_CONFIG_S_FAILED bit (for restore)
>   * @config_core_enabled: configuration change reporting enabled by core
> + * @config_driver_disabled: configuration change reporting disabled by
> + *                          a driver
>   * @config_change_pending: configuration change reported while disabled
>   * @config_lock: protects configuration change reporting
>   * @vqs_list_lock: protects @vqs.
> @@ -133,6 +135,7 @@ struct virtio_device {
>  	int index;
>  	bool failed;
>  	bool config_core_enabled;
> +	bool config_driver_disabled;
>  	bool config_change_pending;
>  	spinlock_t config_lock;
>  	spinlock_t vqs_list_lock;
> @@ -163,6 +166,10 @@ void __virtqueue_break(struct virtqueue *_vq);
>  void __virtqueue_unbreak(struct virtqueue *_vq);
>
>  void virtio_config_changed(struct virtio_device *dev);
> +
> +void virtio_config_driver_disable(struct virtio_device *dev);
> +void virtio_config_driver_enable(struct virtio_device *dev);
> +
>  #ifdef CONFIG_PM_SLEEP
>  int virtio_device_freeze(struct virtio_device *dev);
>  int virtio_device_restore(struct virtio_device *dev);
> --
> 2.31.1
>
Xuan Zhuo July 9, 2024, 8:14 a.m. UTC | #2
On Tue,  9 Jul 2024 16:02:13 +0800, Jason Wang <jasowang@redhat.com> wrote:
> Sometime, it would be useful to disable the configure change
> notification from the driver. So this patch allows this by introducing
> a variable config_change_driver_disabled and only allow the configure
> change notification callback to be triggered when it is allowed by
> both the virtio core and the driver. It is set to false by default to
> hold the current semantic so we don't need to change any drivers.
>
> The first user for this would be virtio-net.
>
> Cc: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
> Cc: Gia-Khanh Nguyen <gia-khanh.nguyen@oracle.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>


> ---
>  drivers/virtio/virtio.c | 39 ++++++++++++++++++++++++++++++++++++---
>  include/linux/virtio.h  |  7 +++++++
>  2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 73bab89b5326..23a96fbc2810 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -127,10 +127,12 @@ static void __virtio_config_changed(struct virtio_device *dev)
>  {
>  	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
>
> -	if (!dev->config_core_enabled)
> +	if (!dev->config_core_enabled || dev->config_driver_disabled)
>  		dev->config_change_pending = true;
> -	else if (drv && drv->config_changed)
> +	else if (drv && drv->config_changed) {
>  		drv->config_changed(dev);
> +		dev->config_change_pending = false;
> +	}
>  }
>
>  void virtio_config_changed(struct virtio_device *dev)
> @@ -143,6 +145,38 @@ void virtio_config_changed(struct virtio_device *dev)
>  }
>  EXPORT_SYMBOL_GPL(virtio_config_changed);
>
> +/**
> + * virtio_config_driver_disable - disable config change reporting by drivers
> + * @dev: the device to reset
> + *
> + * This is only allowed to be called by a driver and disalbing can't
> + * be nested.
> + */
> +void virtio_config_driver_disable(struct virtio_device *dev)
> +{
> +	spin_lock_irq(&dev->config_lock);
> +	dev->config_driver_disabled = true;
> +	spin_unlock_irq(&dev->config_lock);
> +}
> +EXPORT_SYMBOL_GPL(virtio_config_driver_disable);
> +
> +/**
> + * virtio_config_driver_enable - enable config change reporting by drivers
> + * @dev: the device to reset
> + *
> + * This is only allowed to be called by a driver and enabling can't
> + * be nested.
> + */
> +void virtio_config_driver_enable(struct virtio_device *dev)
> +{
> +	spin_lock_irq(&dev->config_lock);
> +	dev->config_driver_disabled = false;
> +	if (dev->config_change_pending)
> +		__virtio_config_changed(dev);
> +	spin_unlock_irq(&dev->config_lock);
> +}
> +EXPORT_SYMBOL_GPL(virtio_config_driver_enable);
> +
>  static void virtio_config_core_disable(struct virtio_device *dev)
>  {
>  	spin_lock_irq(&dev->config_lock);
> @@ -156,7 +190,6 @@ static void virtio_config_core_enable(struct virtio_device *dev)
>  	dev->config_core_enabled = true;
>  	if (dev->config_change_pending)
>  		__virtio_config_changed(dev);
> -	dev->config_change_pending = false;
>  	spin_unlock_irq(&dev->config_lock);
>  }
>
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index a6f6df72f01a..2be73de6f1f3 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -116,6 +116,8 @@ struct virtio_admin_cmd {
>   * @index: unique position on the virtio bus
>   * @failed: saved value for VIRTIO_CONFIG_S_FAILED bit (for restore)
>   * @config_core_enabled: configuration change reporting enabled by core
> + * @config_driver_disabled: configuration change reporting disabled by
> + *                          a driver
>   * @config_change_pending: configuration change reported while disabled
>   * @config_lock: protects configuration change reporting
>   * @vqs_list_lock: protects @vqs.
> @@ -133,6 +135,7 @@ struct virtio_device {
>  	int index;
>  	bool failed;
>  	bool config_core_enabled;
> +	bool config_driver_disabled;
>  	bool config_change_pending;
>  	spinlock_t config_lock;
>  	spinlock_t vqs_list_lock;
> @@ -163,6 +166,10 @@ void __virtqueue_break(struct virtqueue *_vq);
>  void __virtqueue_unbreak(struct virtqueue *_vq);
>
>  void virtio_config_changed(struct virtio_device *dev);
> +
> +void virtio_config_driver_disable(struct virtio_device *dev);
> +void virtio_config_driver_enable(struct virtio_device *dev);
> +
>  #ifdef CONFIG_PM_SLEEP
>  int virtio_device_freeze(struct virtio_device *dev);
>  int virtio_device_restore(struct virtio_device *dev);
> --
> 2.31.1
>
diff mbox series

Patch

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 73bab89b5326..23a96fbc2810 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -127,10 +127,12 @@  static void __virtio_config_changed(struct virtio_device *dev)
 {
 	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 
-	if (!dev->config_core_enabled)
+	if (!dev->config_core_enabled || dev->config_driver_disabled)
 		dev->config_change_pending = true;
-	else if (drv && drv->config_changed)
+	else if (drv && drv->config_changed) {
 		drv->config_changed(dev);
+		dev->config_change_pending = false;
+	}
 }
 
 void virtio_config_changed(struct virtio_device *dev)
@@ -143,6 +145,38 @@  void virtio_config_changed(struct virtio_device *dev)
 }
 EXPORT_SYMBOL_GPL(virtio_config_changed);
 
+/**
+ * virtio_config_driver_disable - disable config change reporting by drivers
+ * @dev: the device to reset
+ *
+ * This is only allowed to be called by a driver and disalbing can't
+ * be nested.
+ */
+void virtio_config_driver_disable(struct virtio_device *dev)
+{
+	spin_lock_irq(&dev->config_lock);
+	dev->config_driver_disabled = true;
+	spin_unlock_irq(&dev->config_lock);
+}
+EXPORT_SYMBOL_GPL(virtio_config_driver_disable);
+
+/**
+ * virtio_config_driver_enable - enable config change reporting by drivers
+ * @dev: the device to reset
+ *
+ * This is only allowed to be called by a driver and enabling can't
+ * be nested.
+ */
+void virtio_config_driver_enable(struct virtio_device *dev)
+{
+	spin_lock_irq(&dev->config_lock);
+	dev->config_driver_disabled = false;
+	if (dev->config_change_pending)
+		__virtio_config_changed(dev);
+	spin_unlock_irq(&dev->config_lock);
+}
+EXPORT_SYMBOL_GPL(virtio_config_driver_enable);
+
 static void virtio_config_core_disable(struct virtio_device *dev)
 {
 	spin_lock_irq(&dev->config_lock);
@@ -156,7 +190,6 @@  static void virtio_config_core_enable(struct virtio_device *dev)
 	dev->config_core_enabled = true;
 	if (dev->config_change_pending)
 		__virtio_config_changed(dev);
-	dev->config_change_pending = false;
 	spin_unlock_irq(&dev->config_lock);
 }
 
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index a6f6df72f01a..2be73de6f1f3 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -116,6 +116,8 @@  struct virtio_admin_cmd {
  * @index: unique position on the virtio bus
  * @failed: saved value for VIRTIO_CONFIG_S_FAILED bit (for restore)
  * @config_core_enabled: configuration change reporting enabled by core
+ * @config_driver_disabled: configuration change reporting disabled by
+ *                          a driver
  * @config_change_pending: configuration change reported while disabled
  * @config_lock: protects configuration change reporting
  * @vqs_list_lock: protects @vqs.
@@ -133,6 +135,7 @@  struct virtio_device {
 	int index;
 	bool failed;
 	bool config_core_enabled;
+	bool config_driver_disabled;
 	bool config_change_pending;
 	spinlock_t config_lock;
 	spinlock_t vqs_list_lock;
@@ -163,6 +166,10 @@  void __virtqueue_break(struct virtqueue *_vq);
 void __virtqueue_unbreak(struct virtqueue *_vq);
 
 void virtio_config_changed(struct virtio_device *dev);
+
+void virtio_config_driver_disable(struct virtio_device *dev);
+void virtio_config_driver_enable(struct virtio_device *dev);
+
 #ifdef CONFIG_PM_SLEEP
 int virtio_device_freeze(struct virtio_device *dev);
 int virtio_device_restore(struct virtio_device *dev);