linux-kernel - Re: [PATCH net-next v3 2/3] virtio: allow driver to disable the configure change notification

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1720512843.116356-3-xuanzhuo@linux.alibaba.com>
Date: Tue, 9 Jul 2024 16:14:03 +0800
From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To: Jason Wang <jasowang@...hat.com>
Cc: virtualization@...ts.linux.dev,
 linux-kernel@...r.kernel.org,
 davem@...emloft.net,
 edumazet@...gle.com,
 kuba@...nel.org,
 pabeni@...hat.com,
 netdev@...r.kernel.org,
 Venkat Venkatsubra <venkat.x.venkatsubra@...cle.com>,
 "Gia-Khanh Nguyen" <gia-khanh.nguyen@...cle.com>,
 mst@...hat.com,
 jasowang@...hat.com,
 eperezma@...hat.com
Subject: Re: [PATCH net-next v3 2/3] virtio: allow driver to disable the configure change notification

On Tue,  9 Jul 2024 16:02:13 +0800, Jason Wang <jasowang@...hat.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@...cle.com>
> Cc: Gia-Khanh Nguyen <gia-khanh.nguyen@...cle.com>
> Signed-off-by: Jason Wang <jasowang@...hat.com>

Reviewed-by: Xuan Zhuo <xuanzhuo@...ux.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
>