On 9/24/25 1:22 AM, Michael S. Tsirkin wrote:
> On Wed, Sep 24, 2025 at 09:16:32AM +0800, Jason Wang wrote:
>> On Tue, Sep 23, 2025 at 10:20 PM Daniel Jurgens <dani...@nvidia.com> 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 <dani...@nvidia.com>
>>> Reviewed-by: Parav Pandit <pa...@nvidia.com>
>>> Reviewed-by: Shahar Shitrit <shshit...@nvidia.com>
>>> Reviewed-by: Yishai Hadas <yish...@nvidia.com>
>>> ---
>>
>> [...]
>>
>>>
>>> size_t virtio_max_dma_size(const struct virtio_device *vdev);
>>>
>>> diff --git a/include/linux/virtio_admin.h b/include/linux/virtio_admin.h
>>> new file mode 100644
>>> index 000000000000..bbf543d20be4
>>> --- /dev/null
>>> +++ b/include/linux/virtio_admin.h
>>> @@ -0,0 +1,68 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only
>>> + *
>>> + * Header file for virtio admin operations
>>> + */
>>> +#include <uapi/linux/virtio_pci.h>
>>> +
>>> +#ifndef _LINUX_VIRTIO_ADMIN_H
>>> +#define _LINUX_VIRTIO_ADMIN_H
>>> +
>>> +struct virtio_device;
>>> +
>>> +/**
>>> + * VIRTIO_CAP_IN_LIST - Check if a capability is supported in the
>>> capability list
>>> + * @cap_list: Pointer to capability list structure containing
>>> supported_caps array
>>> + * @cap: Capability ID to check
>>> + *
>>> + * The cap_list contains a supported_caps array of little-endian 64-bit
>>> integers
>>> + * where each bit represents a capability. Bit 0 of the first element
>>> represents
>>> + * capability ID 0, bit 1 represents capability ID 1, and so on.
>>> + *
>>> + * Return: 1 if capability is supported, 0 otherwise
>>> + */
>>> +#define VIRTIO_CAP_IN_LIST(cap_list, cap) \
>>> + (!!(1 & (le64_to_cpu(cap_list->supported_caps[cap / 64]) >> cap %
>>> 64)))
>>> +
>>> +/**
>>> + * 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);
>>> +};
>>
>> Looking at this, it's nothing admin virtqueue specific, I wonder why
>> it is not part of virtio_config_ops.
>>
>> Thanks
>
> cap things are admin commands. But what I do not get is why they
> need to be callbacks.
>
> The only thing about admin commands that is pci specific is finding
> the admin vq.
>
> I'd expect an API for that in config then, and the rest of code can
> be completely transport independent.
>
>
The idea was that each transport would implement the callbacks, and we
have indirection at the virtio_device level. Similar to the config_ops.
So the drivers stay transport agnostic. I know these are PCI specific
now, but thought it should be implemented generically.
These could go in config ops. But I thought it was better to isolate
them in a new _ops structure.
An earlier implementation had the net driver accessing the admin_ops
directly. But Parav thought this was better.
