Suggested-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/virtio/virtio.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db1c0ddf6b..8d27fe1824 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -120,9 +120,29 @@ struct VirtioDeviceClass {
     /* This is what a VirtioDevice must implement */
     DeviceRealize realize;
     DeviceUnrealize unrealize;
+
+    /**
+     * get_features:
+     * @vdev: the VirtIODevice
+     * @requested_features: existing device feature bits from
+     *                      vdev->host_features
+     * @errp: pointer to error object
+     *
+     * Get the device feature bits.
+     *
+     * The ->get_features() function typically sets always-on device feature
+     * bits as well as conditional feature bits that require some logic to
+     * compute.
+     *
+     * Device feature bits can also be set in vdev->host_features before this
+     * function is called using DEFINE_PROP_BIT64() qdev properties.
+     *
+     * Returns: the final device feature bits to store in vdev->host_features.
+     */
     uint64_t (*get_features)(VirtIODevice *vdev,
                              uint64_t requested_features,
                              Error **errp);
+
     uint64_t (*bad_features)(VirtIODevice *vdev);
     void (*set_features)(VirtIODevice *vdev, uint64_t val);
     int (*validate_features)(VirtIODevice *vdev);
-- 
2.37.1

On Wed, Aug 03 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote:

> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  include/hw/virtio/virtio.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index db1c0ddf6b..8d27fe1824 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -120,9 +120,29 @@ struct VirtioDeviceClass {
>      /* This is what a VirtioDevice must implement */
>      DeviceRealize realize;
>      DeviceUnrealize unrealize;
> +
> +    /**
> +     * get_features:
> +     * @vdev: the VirtIODevice
> +     * @requested_features: existing device feature bits from
> +     *                      vdev->host_features
> +     * @errp: pointer to error object
> +     *
> +     * Get the device feature bits.
> +     *
> +     * The ->get_features() function typically sets always-on device feature
> +     * bits as well as conditional feature bits that require some logic to
> +     * compute.
> +     *
> +     * Device feature bits can also be set in vdev->host_features before this
> +     * function is called using DEFINE_PROP_BIT64() qdev properties.
> +     *
> +     * Returns: the final device feature bits to store in vdev->host_features.
> +     */

Not sure if we want to go full function doc for features, as none of the
other callbacks have it...

I thought about something like

"Called with vdev->host_features in requested_features. Returns device
feature bits to be stored in vdev->host_features after factoring in
device-specific feature bits."

The important part IMHO is that requested_features contains
vdev->host_features, so no need to merge them in.

>      uint64_t (*get_features)(VirtIODevice *vdev,
>                               uint64_t requested_features,
>                               Error **errp);
> +
>      uint64_t (*bad_features)(VirtIODevice *vdev);
>      void (*set_features)(VirtIODevice *vdev, uint64_t val);
>      int (*validate_features)(VirtIODevice *vdev);