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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACGkMEuWRUANBCkeE5r+7+wob3nr-Mrnce_kLRHbpeF0OT_45Q@mail.gmail.com>
Date: Fri, 26 Sep 2025 12:55:11 +0800
From: Jason Wang <jasowang@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: Daniel Jurgens <danielj@...dia.com>, netdev@...r.kernel.org, alex.williamson@...hat.com, 
	pabeni@...hat.com, virtualization@...ts.linux.dev, parav@...dia.com, 
	shshitrit@...dia.com, yohadt@...dia.com, xuanzhuo@...ux.alibaba.com, 
	eperezma@...hat.com, shameerali.kolothum.thodi@...wei.com, jgg@...pe.ca, 
	kevin.tian@...el.com, kuba@...nel.org, andrew+netdev@...n.ch, 
	edumazet@...gle.com, Yishai Hadas <yishaih@...dia.com>
Subject: Re: [PATCH net-next v3 01/11] virtio-pci: Expose generic device
 capability operations

On Wed, Sep 24, 2025 at 2:22 PM Michael S. Tsirkin <mst@...hat.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 <danielj@...dia.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 <danielj@...dia.com>
> > > Reviewed-by: Parav Pandit <parav@...dia.com>
> > > Reviewed-by: Shahar Shitrit <shshitrit@...dia.com>
> > > Reviewed-by: Yishai Hadas <yishaih@...dia.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