Re: [PATCH net-next v3 01/11] virtio-pci: Expose generic device capability operations

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Sep 23, 2025 at 09:19:10AM -0500, Daniel Jurgens wrote:
> Currently querying and setting capabilities is restricted to a single
> capability and contained within the virtio PCI driver. However, each
> device type has generic and device specific capabilities, that may be
> queried and set. In subsequent patches virtio_net will query and set
> flow filter capabilities.
> 
> Move the admin related definitions to a new header file. It needs to be
> abstracted away from the PCI specifics to be used by upper layer
> drivers.
> 
> Signed-off-by: Daniel Jurgens <danielj@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
> Reviewed-by: Yishai Hadas <yishaih@nvidia.com>


...


> +/**
> + * struct virtio_admin_ops - Operations for virtio admin functionality
> + *
> + * This structure contains function pointers for performing administrative
> + * operations on virtio devices. All data and caps pointers must be allocated
> + * on the heap by the caller.
> + */
> +struct virtio_admin_ops {
> +	/**
> +	 * @cap_id_list_query: Query the list of supported capability IDs
> +	 * @vdev: The virtio device to query
> +	 * @data: Pointer to result structure (must be heap allocated)
> +	 * Return: 0 on success, negative error code on failure
> +	 */
> +	int (*cap_id_list_query)(struct virtio_device *vdev,
> +				 struct virtio_admin_cmd_query_cap_id_result *data);
> +	/**
> +	 * @cap_get: Get capability data for a specific capability ID
> +	 * @vdev: The virtio device
> +	 * @id: Capability ID to retrieve
> +	 * @caps: Pointer to capability data structure (must be heap allocated)
> +	 * @cap_size: Size of the capability data structure
> +	 * Return: 0 on success, negative error code on failure
> +	 */
> +	int (*cap_get)(struct virtio_device *vdev,
> +		       u16 id,
> +		       void *caps,
> +		       size_t cap_size);
> +	/**
> +	 * @cap_set: Set capability data for a specific capability ID
> +	 * @vdev: The virtio device
> +	 * @id: Capability ID to set
> +	 * @caps: Pointer to capability data structure (must be heap allocated)
> +	 * @cap_size: Size of the capability data structure
> +	 * Return: 0 on success, negative error code on failure
> +	 */
> +	int (*cap_set)(struct virtio_device *vdev,
> +		       u16 id,
> +		       const void *caps,
> +		       size_t cap_size);
> +};
> +


I do not get why do we need this indirection. There is a single
implementation in the spec for now, and your patchset does not introduce
a new one.


-- 
MST