On 25-09-2025 11:46 am, Michael S. Tsirkin wrote:
On Wed, Sep 24, 2025 at 02:02:34PM -0500, Dan Jurgens wrote:
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 <[email protected]> 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 <[email protected]>
Reviewed-by: Parav Pandit <[email protected]>
Reviewed-by: Shahar Shitrit <[email protected]>
Reviewed-by: Yishai Hadas <[email protected]>
---
[...]
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.
It is very clear from the virtio_admin_ops definition that it is not
specific to admin vq. It is a admin command interface.
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.
Right, but most stuff is not transport specific. If you are going to
put in the work, what is transport specific is admin VQ access.
Commands themselves are transport agnostic, we just did not need
them in non-pci previously.
Should config_ops be extended to have admin cmd interface there?
No strong preference for putting function pointers in new admin_ops or
config_ops.
I just find it better to have admin_ops clearly defined as it makes the
code crystal clear of what the ops are about.
Today, one needs to be cautious when reading config_ops of below note.
"Note: Do not assume that a transport implements all of the operations".
Having well defined admin_ops appeared more clear. But config_ops
extension (or overloading) seems more simpler, no strong preference to me.
Regarding Dan's comment on "net driver directly accessing admin_ops
directly" seems a bad idea to me.
Function pointers are there for multiple transports to implement their
own implementation.
it is not meant to develop open coded drivers. In fact some day
config_ops struct definition can be restricted to only transport drivers.
Major part of the kernel does not follow open coding function pointer calls.
