[PATCH 1/2] Revert "virtio: use virtio_device_ready() in virtio_device_restore()"

[PATCH 1/2] Revert "virtio: use virtio_device_ready() in virtio_device_restore()"

[Michael S. Tsirkin]

This reverts commit 8d65bc9a5be3f23c5e2ab36b6b8ef40095165b18.

We reverted the problematic changes, no more need for work
arounds on restore.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 75c8d560bbd3..22f15f444f75 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -526,9 +526,8 @@ int virtio_device_restore(struct virtio_device *dev)
 			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);
+	/* Finally, tell the device we're all set */
+	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
 
 	virtio_config_enable(dev);
 
-- 
MST

[PATCH 2/2] Revert "virtio_config: introduce a new .enable_cbs method"

[Michael S. Tsirkin]

This reverts commit d50497eb4e554e1f0351e1836ee7241c059592e6.

The new callback ended up not being used, and it's asymmetrical:
just enable, no disable.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/virtio_config.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index dafdc7f48c01..b341dd62aa4d 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -23,8 +23,6 @@ struct virtio_shm_region {
  *       any of @get/@set, @get_status/@set_status, or @get_features/
  *       @finalize_features are NOT safe to be called from an atomic
  *       context.
- * @enable_cbs: enable the callbacks
- *      vdev: the virtio_device
  * @get: read the value of a configuration field
  *	vdev: the virtio_device
  *	offset: the offset of the configuration field
@@ -78,7 +76,6 @@ struct virtio_shm_region {
  */
 typedef void vq_callback_t(struct virtqueue *);
 struct virtio_config_ops {
-	void (*enable_cbs)(struct virtio_device *vdev);
 	void (*get)(struct virtio_device *vdev, unsigned offset,
 		    void *buf, unsigned len);
 	void (*set)(struct virtio_device *vdev, unsigned offset,
@@ -233,9 +230,6 @@ void virtio_device_ready(struct virtio_device *dev)
 {
 	unsigned status = dev->config->get_status(dev);
 
-	if (dev->config->enable_cbs)
-                  dev->config->enable_cbs(dev);
-
 	BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
 	dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
 }
-- 
MST

Re: [PATCH 2/2] Revert "virtio_config: introduce a new .enable_cbs method"

[Jason Wang]

On Wed, Mar 30, 2022 at 2:23 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> This reverts commit d50497eb4e554e1f0351e1836ee7241c059592e6.
>
> The new callback ended up not being used, and it's asymmetrical:
> just enable, no disable.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  include/linux/virtio_config.h | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index dafdc7f48c01..b341dd62aa4d 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -23,8 +23,6 @@ struct virtio_shm_region {
>   *       any of @get/@set, @get_status/@set_status, or @get_features/
>   *       @finalize_features are NOT safe to be called from an atomic
>   *       context.
> - * @enable_cbs: enable the callbacks
> - *      vdev: the virtio_device
>   * @get: read the value of a configuration field
>   *     vdev: the virtio_device
>   *     offset: the offset of the configuration field
> @@ -78,7 +76,6 @@ struct virtio_shm_region {
>   */
>  typedef void vq_callback_t(struct virtqueue *);
>  struct virtio_config_ops {
> -       void (*enable_cbs)(struct virtio_device *vdev);
>         void (*get)(struct virtio_device *vdev, unsigned offset,
>                     void *buf, unsigned len);
>         void (*set)(struct virtio_device *vdev, unsigned offset,
> @@ -233,9 +230,6 @@ void virtio_device_ready(struct virtio_device *dev)
>  {
>         unsigned status = dev->config->get_status(dev);
>
> -       if (dev->config->enable_cbs)
> -                  dev->config->enable_cbs(dev);
> -
>         BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
>         dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
>  }
> --
> MST
>

Re: [PATCH 1/2] Revert "virtio: use virtio_device_ready() in virtio_device_restore()"

[Jason Wang]

On Wed, Mar 30, 2022 at 2:23 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> This reverts commit 8d65bc9a5be3f23c5e2ab36b6b8ef40095165b18.
>
> We reverted the problematic changes, no more need for work
> arounds on restore.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Acked-by: Jason Wang <jasowang@redhat.com>

> ---
>  drivers/virtio/virtio.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 75c8d560bbd3..22f15f444f75 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -526,9 +526,8 @@ int virtio_device_restore(struct virtio_device *dev)
>                         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);
> +       /* Finally, tell the device we're all set */
> +       virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
>
>         virtio_config_enable(dev);
>
> --
> MST
>