Re: [PATCH V2 05/12] virtio_config: introduce a new ready method

Michael S. Tsirkin
2021-10-13T09:57:36Z

On Tue, Oct 12, 2021 at 02:52:20PM +0800, Jason Wang wrote:
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> include/linux/virtio_config.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 8519b3ae5d52..f2891c6221a1 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -23,6 +23,8 @@ 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.
> + * @ready: make the device ready
> + * vdev: the virtio_device
> * @get: read the value of a configuration field
> * vdev: the virtio_device
> * offset: the offset of the configuration field

I think it's too vague. device_ready makes device ready to receive
kicks. What does this one do? I don't like calling things by where
they are called from, I prefer calling them by what they do.
It actually enables callbacks, right?
So enable_cbs?

> @@ -75,6 +77,7 @@ struct virtio_shm_region {
> */
> typedef void vq_callback_t(struct virtqueue *);
> struct virtio_config_ops {
> + void (*ready)(struct virtio_device *vdev);
> void (*get)(struct virtio_device *vdev, unsigned offset,
> void *buf, unsigned len);
> void (*set)(struct virtio_device *vdev, unsigned offset,
> @@ -229,6 +232,9 @@ void virtio_device_ready(struct virtio_device *dev)
> {
> unsigned status = dev->config->get_status(dev);
>
> + if (dev->config->ready)
> + dev->config->ready(dev);
> +
> BUG_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
> dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
> }
> --
> 2.25.1