On Wed, Sep 24, 2025 at 2:22 PM Michael S. Tsirkin <m...@redhat.com> 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 think we had a discussion to decide to separate admin commands from
the admin vq.
Thanks
