Re: [PATCH v1 2/5] virtio-blk: move config space sizing code to virtio-blk-common

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Wed, Aug 24, 2022 at 12:18:34PM +0300, Daniil Tatianin wrote:
> +size_t virtio_blk_common_get_config_size(uint64_t host_features)
> +{
> +    size_t config_size = MAX(VIRTIO_BLK_CFG_SIZE,
> +        virtio_feature_get_config_size(feature_sizes, host_features));
> +
> +    assert(config_size <= sizeof(struct virtio_blk_config));
> +    return config_size;
> +}

This logic is common to all VIRTIO devices and I think it can be moved
to virtio_feature_get_config_size(). Then
virtio_blk_common_get_config_size() is no longer necessary and the
generic virtio_feature_get_config_size() can be called directly.

The only virtio-blk common part would be the
virtio_feature_get_config_size() parameter struct that describes the
minimum and maximum config space size, as well as how the feature bits
affect the size:

  size = virtio_feature_get_config_size(virtio_blk_config_size_params, host_features)

where virtio_blk_config_size_params is:

  const VirtIOConfigSizeParams virtio_blk_config_size_params = {
      .min_size = offsetof(struct virtio_blk_config, max_discard_sectors),
      .max_size = sizeof(struct virtio_blk_config),
      .features = {
          {.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
           .end = endof(struct virtio_blk_config, discard_sector_alignment)},
          ...,
      },
  };

Then virtio-blk-common.h just needs to define:

  extern const VirtIOConfigSizeParams virtio_blk_config_size_params;

Taking it one step further, maybe VirtioDeviceClass should include a
const VirtIOConfigSizeParams *config_size_params field so
vdev->config_size can be computed by common VIRTIO code and the devices
only need to describe the parameters.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]